fix(knowledge): report lexical/ANN score provenance instead of fixed score bands - #2381
fix(knowledge): report lexical/ANN score provenance instead of fixed score bands#2381ohdearquant wants to merge 5 commits into
Conversation
…score bands knowledge.search scores are hybrid, request-relative ranking values, not calibrated probabilities, so the documented fixed score bands were routinely wrong. A genuine full-text miss also used to fall back to a scan of the newest corpus rows, which rank fusion could dress up as topically relevant. Each result now carries score_provenance (lexical/ANN sources, whether embedding rerank ran, and the normalization used), and the response carries a top-level candidate_provenance describing the lexical candidate stage (matched, no match, filtered by status/kind, or timed out) and whether the returned set is ANN-only. A true lexical miss now returns no candidates instead of the newest rows.
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.
Verdict on head 80da848: REQUEST-CHANGES, 1 blocking finding. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
… recency fallback Removing the recency fallback from fetch_fts_candidates made two candidate classes the scorer still promises unreachable, because fts_knowledge cannot surface them regardless of corpus content: a query shorter than the trigram tokenizer's minimum span (e.g. "RAG", "ML") never forms a trigram, so exact_name_bonus scoring never got a candidate; and tags is not one of the FTS-indexed columns, so a tag-only query never got one either. A genuine FTS miss now runs one further bounded, direct-predicate SQL lookup before reporting empty: atoms whose name contains the raw query (case-insensitive) or whose tags JSON array contains a query token, capped at the existing candidate-pool limit and still scoped by namespace/status/kind eligibility inside the SQL. This is not a reintroduction of the removed recency scan — it never orders by recency and only returns rows that overlap the query by name substring or literal tag. candidate_provenance.lexical reports this outcome as a new exact_match state, kept distinct from matched (a real FTS/bm25 hit) so a caller can still tell which lexical mechanism produced the candidates. ADR-047 and the API reference are updated to document the new state. Also fixes the rerank-provenance test fixture: the embedding stub's uniform-scaling vec![v / norm; DIM] shape collapsed to the same vector for every positive v (norm is v * sqrt(DIM), so v / norm cancels v out entirely), so it never exercised a genuine cosine difference between texts. Replaced with one unit basis vector per text position.
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.
Verdict on head ecd4c2a: REQUEST-CHANGES, 1 blocking finding. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
The bounded name/tag recovery lookup in fetch_fts_candidates only ran after a genuine FTS miss (every extracted term matched nothing eligible). Two earlier return points skipped it entirely: a non-empty FTS match returned Matched immediately, and an FTS match whose rows were all ineligible returned Filtered immediately. Either return meant a name- or tag-only atom was silently dropped whenever the query also happened to match something else in the trigram index, or matched only an ineligible row. The recovery lookup now runs whenever the query yields a name needle or tag terms, independent of what FTS found, and its rows are unioned (deduplicated by id, FTS rows ordered first) with whatever FTS produced. Provenance stays Matched whenever FTS contributed at least one eligible row; a Filtered or genuine-miss outcome that recovery turns non-empty now reports ExactMatch, the state already used for recovery-sourced rows, so no new state variant was needed. Filtered and NoMatch are unchanged when recovery also finds nothing. Adds three tests: a non-empty FTS match with a tag-only sibling, an ineligible-only FTS match with an eligible tag-only sibling, and a dedup check for an atom reachable both ways. The first two fail against the prior control flow. Updates the knowledge-pack API reference and ADR-047 to describe the union.
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.
Verdict on head 8bde674: REQUEST-CHANGES, 2 blocking findings. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
1 participant
Replaces the fixed score bands (0.46 / 0.42) with per-hit score provenance and a request-level candidate provenance on
knowledge.search.ScoreProvenanceon every scored hit (lexical, ann, embedding_rerank), merged on RRF fusion overlap.LexicalCandidateState(matched, no_match, filtered, partial_timeout, timed_out) replaces the bounded full-scan fallback: a genuine FTS miss returns nothing instead of the newest corpus rows.candidate_provenanceon the response reports the lexical state and whether a fallback ran.First of three stacked changes; the next two build on the
LexicalCandidateStatevocabulary introduced here.