Repository navigation
A search may name a source and an external_id prefix, and both channels recall only those documents before fusion - #1427
Conversation
WaylandYang
left a comment
There was a problem hiding this comment.
Thanks, this is the shape we agreed on #1271. Both channels filter before fusion: the Tantivy term set and the ANY($5) inside the nearest-neighbour CTE. The prefix goes through starts_with on the pushed_key, a source from another base answers like a missing one, and the external_id derivation now sits next to pushed_key. Three small things before merge:
- In
a_source_filter_pages_through_its_six_hits_under_a_hundred_better_ones, please also assert that every hit has both abm25_rankand avector_rank. As it stands, BM25 alone satisfies the test, so moving the vector filter after theLIMITwould still pass. - Please add a test that
_and%inexternal_id_prefixare plain characters: pushingdocs_a.mdanddocsXa.mdwith the prefixdocs_should find only the first. - In ADR 0014, add the filters to the chunk-search paragraph of decision 7 (
source_id, 404 outside the base;external_id_prefixfor push sources, 422prefix_needs_push_source; applied inside both channels), plus a dated revision line.
Optional: retrieval.rs now imports from api::sources_routes. Moving pushed_key/pushed_identity into a small shared module would keep sources_routes private. MCP search_chunks can follow in its own PR.
…ls recall only those documents before fusion Signed-off-by: Bogdan Szabo <bogdan@re-cinq.com>
bb72c58 to
3f461fc
Compare
|
Thanks for the careful read, @WaylandYang. You were right that BM25 alone carried that test. All three are in, plus the optional one:
Optional: |
WaylandYang
left a comment
There was a problem hiding this comment.
Thanks @gedaiu, all three are in. The six-hit test now needs both ranks and fails with the vector filter after the LIMIT, _/% are taken literally, and 0014 records the filters. Approving; the stale sources_routes::pushed_identity reference in models.rs:483 can go in the MCP search_chunks PR.
Part of #1271, the filters PR we agreed on. Thanks again for the careful review on #1273. The search request now takes
source_idandexternal_id_prefix. A source from another base is a 404, like the other source routes. The prefix needs thesource_idof an API or Statements source; otherwise it is a 422 (prefix_needs_push_source). I went with a prefix as the issue proposed, since an exactexternal_idalready reads back through #1280. Both filters go into both channels before fusion: Tantivy gets adocument_idterm set, and the vector CTE getsdocument_id = ANY(...)before itsLIMIT. So the base's other hits can't use up the 100 candidates, and paging works the same as without a filter. As you suggested, theexternal_idderivation moved out of the SQL intopushed_identity, next topushed_key. The prefix is matched against the keypushed_keybuilds, withstarts_with, so_and%are plain characters. Unknown fields are still a 422.ingest.mdandchat-and-mcp.mddescribe the filters.New tests:
a_source_filter_pages_through_its_six_hits_under_a_hundred_better_onesputs six weak hits of one source under a hundred better ones, and checks that pages of three join into the whole list. It fails if the filter runs after fusion.a_search_by_source_and_external_id_prefix_docs_finds_only_docs_a_mdanda_search_by_another_bases_source_gets_404_and_a_prefix_without_a_push_source_gets_422fail on dev. I rancargo testfor utopia-server (762 passed), utopia-store (474), utopia-search (7) and utopia-core (25) against a migrated Postgres withUTOPIA_TEST_REQUIRE_DB=1, pluscargo fmt --checkandcargo clippy --workspace --all-targets -D warnings. MCPsearch_chunkscomes next.