Skip to content

Search answers a service with each hit's score, ranks and source, and pages - #1273

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/search-returns-its-scores
Oct 25, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/search-returns-its-scores

Conversation

@gedaiu

@gedaiu gedaiu commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

PR 1 of #1271. Filters follow in a separate PR.

POST /kbs/{id}/search now accepts a read token on the same terms as the other reads in ADR 0014 decision 7: Viewer is the ceiling, and a base outside kb_ids returns 404.

Each hit includes its RRF score, its 1-based BM25 and vector ranks (null when the hit did not come from that channel) and its source. A hit from a pushed source also carries its external_id. The response reports vector_used, which is false when the vector channel was skipped. offset pages through one fused list. Every page asks each channel for the same 100 candidates, so pages never overlap or skip hits. offset + top_k above 100 returns 422, and the page that reaches 100 answers has_more: false. A score ranks hits within one query only. Until filters land, an unknown request field returns 422 instead of being ignored.

Two things change for callers. The search page now fuses 100 candidates per channel where it used to fuse 24, as chat still does, so the same query can rank differently than before and differently from chat. And paging has limits: pages join up only while the base does not change between requests, and each channel counts its ranks before the final as_of filter, so a historical page can skip rank numbers.

two_pages_of_fifteen_join_into_the_first_thirty_hits checks that two pages of 15 equal the first 30 hits; it fails on the old code. a_page_ending_at_depth_100_of_101_hits_has_no_more covers the cap. The token-read tests now cover search, including both 422s. ADR 0014, access-and-audit and chat-and-mcp describe the route, and web/src/docs/ingest.md has a section for integrators next to the read-back by external_id.

@gedaiu

gedaiu commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Moving this to draft: CONTRIBUTING asks for a change to the search contract to be agreed in its issue first, and #1271 has no agreed design yet. I'll ask there, and rework this to whatever we settle on before marking it ready. Sorry for jumping ahead.

@gedaiu
gedaiu force-pushed the feat/search-returns-its-scores branch from 3611670 to f0b73cb Compare October 23, 2026 13:26
@gedaiu
gedaiu marked this pull request as ready for review October 23, 2026 13:26
@gedaiu

gedaiu commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @WaylandYang for agreeing the design on #1271. I've rebased this onto dev, after #1266, and it covers each point:

  • Token access: search takes a read token under decision 7, with Viewer as the ceiling and 404 outside kb_ids.
  • Every hit has its RRF score, its BM25 and vector ranks, and its source.
  • vector_used is false when the vector channel was skipped.
  • offset pages over one fused list, and each channel always returns 101 candidates, so pages stay consistent up to a depth of 100.
  • two_pages_of_fifteen_join_into_the_first_thirty_hits fails on the old code and passes here.

Filters will come in the next PR.

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this matches what we agreed on #1271, and the paging test is exactly right. A few small things before merge: (1) on the page that ends at depth 100, has_more is true but the next page is 422; please return false at the cap (or document it) with a test. (2) Web search now fuses 101 per channel, so its ranking can differ from before and from chat; please say so in the PR body and chat-and-mcp.md. (3) Note that offset pages are consistent only while the base doesn't change, and that ranks are counted before the as_of filter. (4) A short section for integrators in web/src/docs/ingest.md, next to the read-back by external_id. (5) In 0014, date the search entry for when it lands and add #1271 to the status line. Optional: keep external_id derivation next to pushed_key, and reject unknown request fields until filters land.

@gedaiu
gedaiu force-pushed the feat/search-returns-its-scores branch from f0b73cb to 08c810c Compare October 24, 2026 12:57
@gedaiu

gedaiu commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review, all fixed in the amended commit:

  1. The page that reaches depth 100 now answers has_more: false. a_page_ending_at_depth_100_of_101_hits_has_no_more seeds 101 chunks and fails on the old code. Since nothing past 100 is needed now, each channel asks for 100 candidates instead of 101.
  2. The PR body and chat-and-mcp.md now say the search page fuses 100 per channel against chat's 24, so its ranking can differ from before and from chat.
  3. Both places also say pages join up only while the base doesn't change, and that ranks are counted before the final as_of filter.
  4. web/src/docs/ingest.md has a "Searching what you pushed" section after the read-back: the request, the response fields, has_more, vector_used, the 422 at depth and the token.
  5. ADR 0014 dates search 2026-10-24 and lists Search cannot serve a backend: no token, no scores, no paging, no filter #1271 on the status line.

Of the optional ones: unknown request fields are now a 422 (deny_unknown_fields), tested next to the depth 422. I left the external_id derivation in the SQL for now to keep the diff small. Happy to move it next to pushed_key here or in the filters PR, whichever you prefer.

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, all five points check out, and the cap test is exactly what I wanted. One last nit: ingest.md says any field other than q/top_k/offset is a 422, but as_of is accepted. Please list it there with the note that ranks are counted before as_of. Moving the external_id derivation next to pushed_key can go in the filters PR.

@gedaiu
gedaiu force-pushed the feat/search-returns-its-scores branch from 08c810c to fee7b51 Compare October 24, 2026 19:59
@gedaiu

gedaiu commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! ingest.md now lists as_of as an accepted field, with the note that ranks are counted before it filters the hits. I'll move the external_id derivation next to pushed_key in the filters PR.

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, ingest.md now lists as_of with the note that ranks are counted before it filters, and nothing else changed. Approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants