Skip to content

Add TinyFish Search backend - #74

Merged
1broseidon merged 2 commits into
1broseidon:mainfrom
alexkarpandrus:agent/tinyfish-search
Oct 9, 2026
Merged

1broseidon merged 2 commits into
1broseidon:mainfrom
alexkarpandrus:agent/tinyfish-search

Conversation

@alexkarpandrus

Copy link
Copy Markdown
Contributor

What

Add TinyFish Search as a keyed web-search backend with configuration, a health probe, offline tests, and documentation.

Why

Make TinyFish results available through Ketch's existing search paths. Refs #73. This draft seeks guidance on provider fit: the documented endpoint is unversioned, and evidence of established use remains under discussion. Affiliation: None with TinyFish. TinyFish Fetch is out of scope.

Test plan

  • make build and make lint passed.
  • GOFLAGS=-p=1 make test, GOMAXPROCS=2 make test, and go test -race ./search -run TinyFish -count=1 passed.
  • A live TinyFish Search request returned three results with snippets, including https://go.dev/doc/.
  • Plain make test failed in the unchanged code/TestGitHubBuildRunsGHOnce test at its two-second mocked-gh timeout.

Security & data impact

Security impact: The API key is sent in a header, not the URL; search queries are sent to TinyFish when selected.
Data classification affected: User-supplied search queries; no new storage.
Audit log updated: n/a

@alexkarpandrus
alexkarpandrus marked this pull request as ready for review September 27, 2026 19:46

Copy link
Copy Markdown
Owner

Triage summary

What it is: Adds TinyFish as a new keyed search backend (search/tinyfish.go + tests), wired into the provider registry, auto chain (AutoRank 45), doctor probes, config golden fixtures, and docs (README/AGENTS.md/MANUAL.md/CHANGELOG.md/skills reference). 13 files changed, well-tested (offline unit tests cover success, error mapping, key rotation, cancellation, registry wiring).

Why this needs human review rather than auto-fix:

  • Adds a brand-new provider — expands the --backend CLI surface (-b tinyfish) and the config schema (tinyfish_api_key/tinyfish_api_keys), both explicitly out of AUTO-FIX scope regardless of code quality.
  • The PR is itself a draft admission request: the author flags that TinyFish's documented endpoint is unversioned and asks maintainers for a ruling on provider fit (see companion issue Propose TinyFish Search as a web search backend #73 and CONTRIBUTING.md's provider criteria — established use, competitive results, a versioned public API). That's a product/policy call, not something CI or triage can settle.
  • From a first-time external contributor to a security/config-sensitive area (new outbound API integration with a key).

CI status: No check runs reported yet on this head SHA at triage time (get_check_runs returned 0). PR body reports make build/make lint clean and targeted tests passing; author also notes an unrelated pre-existing flake in code/TestGitHubBuildRunsGHOnce under plain make test (a 2s mocked-gh timeout), unrelated to this diff.

Proposed disposition: Maintainer resolves the admission question on #73 first (unversioned endpoint, evidence of established use). If accepted, this implementation looks mechanically sound (key-pool support, error taxonomy, health probe, regenerated goldens) and can be reviewed on its merits; if declined, close both together.

No code changed, nothing merged.


Generated by Claude Code

@1broseidon 1broseidon added triaged Processed by the daily triage loop needs-review Triage loop flagged this for human approval labels Sep 28, 2026 — with Claude

@1broseidon 1broseidon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @alexkarpandrus, I ran some benchmarks against the other backends. Result quality is solid, so I'm happy to bring TinyFish in.

One blocker: on some queries the API returns Google redirect stubs instead of real URLs, e.g. /url?opi=…&q=https://www.postgresql.org/docs/…&sa=U&ved=…. It's consistent per query: postgres JSON_TABLE function and latest Go release notes come back with all 10 results like that. Since the URL passes straight through, these would break --scrape, tags and dedup.

Could you:

  1. Unwrap /url?q= redirects and drop any result that still isn't an absolute http(s) URL, with a test using a real stubbed response
  2. Wrap the response decode in an io.LimitReader like the other backends

I'll approve the CI run so you get checks on the next push. Thanks for the clean implementation!

@alexkarpandrus

Copy link
Copy Markdown
Contributor Author

Thanks for checking the result quality. Both requests are addressed in d7d3bea:

  • Unwrap relative /url?q=... redirects and discard results that are not absolute HTTP(S) URLs. Added a stubbed-response regression test for the reported redirect format, malformed/non-HTTP URLs, and destination query encoding.
  • Decode through io.LimitReader with a 1 MiB cap, with an oversized-response regression test.

Live TinyFish checks for postgres JSON_TABLE function and latest Go release notes each returned 10 valid HTTP(S) URLs with no relative redirect stubs. The top PostgreSQL and Go documentation URLs also scraped successfully with caching disabled.

Local verification passed: make build, make lint, GOFLAGS=-p=1 make test, and go test -race ./search -run TinyFish -count=1. The normal parallel full-suite run still hit the same unchanged TestGitHubBuildRunsGHOnce failure noted in the PR; the serialized full suite passed.

@alexkarpandrus

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and resolved the README/CHANGELOG conflicts while preserving both sets of additions. The redirect/response-limit fix is now 545fd66; the TinyFish code and tests are unchanged by the rebase.

Rechecked the rebased revision: build, lint, the full suite with GOFLAGS=-p=1, and TinyFish race tests pass. Both live review queries again returned 10 valid HTTP(S) URLs with no relative redirect stubs.

Copy link
Copy Markdown
Owner

Triage update (re-triaged: new activity since last pass)

What changed since the prior triage comment:

  • The admission question on Propose TinyFish Search as a web search backend #73 is resolved — the maintainer reviewed live result quality, was satisfied, and closed Propose TinyFish Search as a web search backend #73 as accepted.
  • The maintainer's PR review (CHANGES_REQUESTED) asked for two fixes: unwrap /url?q= redirect stubs and drop non-absolute-HTTP(S) results, and bound the response decode with an io.LimitReader. The author addressed both in d7d3bea, added regression tests for the redirect format and the oversized-response cap, and reported live re-checks with clean results and all local verification (make build, make lint, full suite, race tests) passing.
  • The branch was then rebased onto current main (conflicts in README/CHANGELOG resolved), landing at head 545fd66. CI (approved by the maintainer for this external contributor) ran on that head and is green (success).

Why this still needs your call rather than an auto-merge or auto-fix:

  • It still adds a brand-new keyed search backend — new --backend tinyfish CLI surface and new config keys (tinyfish_api_key/tinyfish_api_keys). That's explicitly out of AUTO-FIX scope regardless of code quality or test coverage, and only you can merge.
  • Your CHANGES_REQUESTED review is now stale relative to the fix — worth a quick look to confirm the redirect-unwrapping and size-cap changes satisfy your ask before merging.

Current state: #73 closed (accepted). CI green on 545fd66. No new blockers found. Proposed disposition: re-review the fix commit and merge if satisfied — no further action needed from triage.

No code changed, nothing merged, by this pass.


Generated by Claude Code

@1broseidon 1broseidon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alexkarpandrus, nice fix, and good call covering the javascript: case in the test. CI is green across the board. Merging!

@1broseidon
1broseidon merged commit 443d00a into 1broseidon:main Oct 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review Triage loop flagged this for human approval triaged Processed by the daily triage loop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants