Repository navigation
fix(recall-gate): score sufficiency on vector relevance, not fused rank - #1918
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Terminal weak-score handling and stale relevance during expansion can produce incorrect gate decisions.
3 open findings
What changed in this PR
Fixes recall sufficiency scoring by using vector cosine relevance and preserving committed expansion results.
Changes:
- Adds vector relevance to Memory and Topic Memory fusion.
- Updates gate scoring, scope handling, and expansion commit behavior.
- Revises tests and RFC documentation.
| File | Description |
|---|---|
src/powercontext/builtin/artifacts/search.py |
Adds shared cosine conversion. |
src/powercontext/builtin/artifacts/memory/models.py |
Adds in-process relevance. |
src/powercontext/builtin/artifacts/memory/fusion.py |
Computes Memory relevance. |
src/powercontext/builtin/artifacts/topic_memory/models.py |
Adds Topic Memory relevance. |
src/powercontext/builtin/artifacts/topic_memory/fusion.py |
Computes Topic Memory relevance. |
src/powercontext/builtin/runtime/recall_sufficiency.py |
Scores gate signals using relevance. |
src/powercontext/builtin/runtime/application.py |
Stages and commits expansion rounds. |
tests/builtin/artifacts/memory/test_fusion.py |
Tests Memory relevance. |
tests/builtin/artifacts/topic_memory/test_fusion.py |
Tests Topic Memory relevance. |
tests/builtin/runtime/test_recall_sufficiency.py |
Tests revised gate semantics. |
tests/e2e/test_recall_sufficiency_gate.py |
Tests runtime expansion behavior. |
docs/en/rfcs/1560_recall_sufficiency_gate.md |
Updates English RFC. |
docs/zh/rfcs/1560_recall_sufficiency_gate.md |
Updates Chinese RFC. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| text=candidates[identity].text, | ||
| score=scores[identity], | ||
| matched_by=tuple(channel for channel in ("fts", "vector") if channel in channels[identity]), | ||
| relevance=relevance.get(identity), |
There was a problem hiding this comment.
I walked through this and think the current behavior is the defensible one, not a loss: a candidate that gains an admitted vector channel only under the lowered floor always has cosine below the round-zero baseline (0.3), and _score_signals uses only at-or-above-baseline scores for the gap and can only lower top_score with below-baseline values. Promoting such a scope to scored would risk exactly the wrong outcome - converting a recoverable thin-candidates case into a terminal weak-top-1 on a 0.25 cosine. The RFC states this rule explicitly ("lower-ranked vector hits cannot turn an FTS-only first hit into a weak score"), and the accumulated candidate order keeps round 0 first, so the first-hit rule stays invariant to expansion order (there is an e2e test pinning exactly that). If the admitted-pool top cosine is ever surfaced to the gate (the separate fusion-limit gap), that would be the point to revisit this too.
There was a problem hiding this comment.
Agreed — this matches the intended invariant: a vector row that only survives the lowered floor is by construction below the 0.3 baseline, so promoting it to scored would invert a recoverable thin-candidates round into weak-top-1 on a below-threshold cosine. No change needed; the per-Scope first-hit rule and the ordering e2e test pin this.
| while ( | ||
| not assessment.sufficient | ||
| and assessment.reason != REASON_WEAK_TOP_ONE | ||
| and families_recoverable > 0 | ||
| and len(expansions) < policy.max_rounds | ||
| ): |
There was a problem hiding this comment.
[P2] Confirmed, with the concrete mechanism: the gate's relevance signals are computed over the fusion-exposed hits only, not the admitted pool. MemoryService.search calls fuse_rankings(..., limit=coarse_limit) with coarse_limit = memory_limit (default 16, prepared_context.py:156), and _gated_recall_effort feeds exactly those fused hits into build_recall_candidates. Under RRF (k=60) any dual-channel hit (1/(60+r1)+1/(60+r2) ~ 0.032) outranks a vector-only hit (1/61 ~ 0.016) regardless of cosine, so once 16 dual-channel weak hits fill the fused top, a vector-admitted candidate with a much higher cosine stays invisible. Round 0 then reports top_score ~ 0.31-0.35, the verdict is weak-top-1, and the loop terminates - although the expansion round (lowered lexical floor admits the strong candidate into FTS, RRF lifts it into the top 16) could expose a 0.90 cosine.
So the RFC's justification - "Lowering the admission floor over the same retrieved pool cannot raise a top cosine that is already admitted" - holds for the admitted pool but not for the fusion-limited set the gate actually scores. The same applies to Topic Memory (topic_memory_candidate_limit = 8). Possible directions: (a) report the admitted pool's top cosine alongside the fused hits and score against that, so the terminal claim matches the gate's basis; (b) keep the terminal behavior but scope the RFC claim to the exposed set and document this crowding configuration as a known limitation; (c) treat weak-top-1 as terminal only when the exposed top equals the admitted pool's top cosine. Even without a code change, the RFC sentence needs the correction, since it is the stated rationale for ending expansion early.
There was a problem hiding this comment.
Addressed in d508f280: weak-top-1 is no longer loop-terminating — an insufficient score now takes the bounded expansion path like every other non-sufficient reason, and when the budget is spent with the top still weak the reason is kept rather than rewritten to at-max-rounds.
Among (a)/(b)/(c) this is the minimal variant: the gate keeps scoring the exposed window (not the admitted pool, which would score evidence that cannot be delivered), and the termination is removed rather than conditioned, because conditioning on "exposed top == pool top" (c) would need a new signal plumbed through fusion → search → gate for a bounded two-round saving. New e2e test_bounded_expansion_surfaces_a_strong_hit_hidden_by_the_fusion_limit reproduces exactly your construction (16 dual-channel weak hits + vector-rank-1 hit at 0.90 sharing one query term) through the real SQLite path and asserts the surfaced top_score≈0.90 and sufficient.
Residual: a surfaced hit informs the verdict but may still be dropped from delivery by _prefix_preserving_counts when the candidate cap is full — noted in the PR thread; whether delivery should re-rank by the new fused order is candidate selection (#1643).
| An assessment whose final reason is `weak-top-1` stops without expansion. Lowering the admission floor over the same | ||
| pool cannot improve an already admitted top cosine, and below-base-floor candidates cannot improve the baseline gap. | ||
| Candidate-count, family-coverage and lexical insufficiency may still trigger expansion when admission counters show | ||
| recoverable candidates: a round that is both thin and weak still expands once, because re-admitting floor-dropped | ||
| evidence can add coverage even though it cannot raise the top cosine. A new budget probe assesses each expansion's |
There was a problem hiding this comment.
Confirmed with the check-order evidence: RecallSufficiencyGate.assess returns weak-top-1 before evaluating lexical_overlap, and _gated_recall_effort's loop condition treats weak-top-1 as terminal, so a candidate set that is both score-weak and lexically insufficient gets reason weak-top-1 and does not expand. Lexical insufficiency triggers expansion only when it is the deciding reason, i.e. the top passes both score checks. Suggest narrowing both the en and zh sentences to what the check order actually guarantees: candidate-count and family-coverage insufficiency expand (both are checked before weak-top-1); lexical insufficiency expands only when the top passes the score checks. The existing "a round that is both thin and weak still expands once" sentence is correct as written; it is the inclusion of lexical insufficiency in the same list that overstates.
There was a problem hiding this comment.
Fixed in d508f280 — the paragraph was rewritten in both locales: weak-top-1 is now described as a recorded insufficient-score reason, not a terminal verdict, with the fusion-window cap spelled out as the reason a weak top is still expandable. Your check-order point is resolved by the same change: since weak-top-1 no longer stops the loop, a score-weak + lexically-weak set still expands (its final reason remains weak-top-1), so the "may still trigger expansion" enumeration is accurate again.
ae1edc4 to
d508f28
Compare
|
Thanks @hidb4ai for the review, and @Copilot for the findings — one of them identified a real defect this PR introduced, fixed in the latest push. Requesting re-review. Disposition of the three Copilot findingsFinding 2 (weak top score can improve after bounded expansion) — confirmed, fixed. The fused window is capped ( Finding 1 (duplicate hits lose newly admitted relevance) — intentional, verified. Vector rows are admitted per row before fusion, so a round-0 FTS-only hit whose vector row first survives at the lowered floor necessarily has cosine < 0.3 — below Finding 3 (lexical insufficiency vs weak-top-1 order) — wording already scoped, clarified further. Residual noteWhen a hidden strong hit is surfaced mid-expansion it counts toward the gate's score, but the per-scope delivery cap ( Verified on the pushed head: |
c8a7fc3 to
4b90803
Compare
The recall sufficiency gate read the fused/RRF ranking score for its min_top_score and min_top_gap thresholds, so saturated ranking values could mask thin semantic evidence. Propagate per-hit vector cosine relevance through Memory and Topic Memory fusion, score the gate on that signal, and keep expanding on a weak top-1 while recoverable candidates remain: the capped fusion window can hide a stronger hit that a lowered floor later surfaces. Failed expansion or assessment now preserves the last committed candidate set instead of reverting to round zero, committed sets keep the latest round's fused order inside each family's cap, and the policy is versioned as powercontext.recall-gate.v2. The PreparedContext HTTP contract is unchanged; RFC 1560 (en/zh) is updated to match. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
4b90803 to
67a024c
Compare
|
Heads-up for reviewers: this fix is still being iterated on. Recent pushes are addressing review findings on |
|
Follow-up on the weak-top fix ( Problem. When a hidden hit enters the fused window under a lowered floor, it was appended to the accumulated pool behind the round-zero prefix. The gate then scored it ( Fix. Committed candidates now keep the latest round's fused order inside each family's cap ( Verification. 76 gate tests pass (52 unit + 24 e2e); |
|
Update: the iteration is complete and the change is stable. All review findings have been addressed on
Ready for review. CI is pending maintainer approval to run; locally the full suite passes (1534 tests) with |
Recall-gate relevance (oceanbase#1918) reads Atomic Memory hits' vector distance as cosine relevance. Its e2e expectations follow Atomic Memory search, which embeds kind and body together, limits each channel before fusion and reports recoverability as an existence probe rather than admission counts. Co-Authored-By: Claude Code <noreply@anthropic.com>


Which issue or RFC does this PR close?
Closes #1916.
Updates RFC 1560 (#1560), implemented in #1596.
Rationale for this change
The recall sufficiency gate applies
min_top_scoreandmin_top_gapto normalized RRF values. For Memory these valuesdepend only on rank:
1.0, somin_top_scorecannot fire;top_gapis a function of the admitted count, so the score check fails for 1–3 candidates and passes for 4 or more,whatever the evidence is.
Separately, when a later expansion round fails, prepare returns the round-zero candidates. This drops an expansion round
the gate has already assessed, even though
RecallEffortstill reports that round. The issue has a reproduction of bothdefects.
The fix scores on the vector cosine relevance of the admitted vector channel. That is the scale the existing
0.3admission baseline uses, and the scale
min_top_scoreandmin_top_gapdescribe. The fix also records an expansionround only after the gate has assessed it.
What changes are included in this PR?
Summary
relevanceto each fused hit: the best cosinesimilarity among its admitted vector channels,
clamp(1 - d² / 2, -1, 1)for unit L2 distance. FTS-only hits keeprelevance=None. The field is declaredexclude=True, like the existingretrieval_scoreandchannel_scores,so it never enters
model_dump()or any serialized projection. The fusedscoreand ranking are unchanged.top_scoreandtop_gapcome from relevance:per Scope, so the verdict does not depend on the order in which Scopes are prepared.
0.3) and needs at least two of them.>= 0.70) skips the gap check.weak-top-1as a recorded reason, not a verdict. The score signals are read off the fused candidate window,which each family's search caps before the gate sees hits, so a strong vector-only hit can sit outside it until a
lower lexical floor admits its second channel. A weak top therefore still expands while a family reports
recoverable candidates; the reason is kept when the rounds are spent.
the gate returns a verdict. A failure in a later round returns the last committed set.
RecallEffort.signalsrecordsthat set's aggregate signals. Committed candidates keep the latest round's fused order inside each family's cap, so
a hit that only entered the window under a lowered floor is delivered to the Builder rather than dropped behind the
round-zero prefix.
powercontext.recall-gate.v1topowercontext.recall-gate.v2.the failure semantics.
Change map
artifacts/search.py)unit_l2_cosine_similarityis sharedartifacts/memory)MemoryHitcarried a fused rankscoreMemoryHit.relevancecarries the admitted vector channel's cosineartifacts/topic_memory)TopicMemorySearchHitcarried a fused rankscoreTopicMemorySearchHit.relevancecarries the best admittedtopic_vector/detail_vectorcosineruntime/recall_sufficiency.py)v1RecallCandidate.scope_id;RecallEffort.signals; policyv2min_top_scoreandmin_top_gapmeasure relevanceruntime/application.py)Architecture and behavior
Scenarios:
weak-top-1records the insufficient score signal; expansion still runs while evidence is recoverable,and the reason survives
at-max-roundsrewriting. The>= 0.70strong-match bound exempts the gap check.Before
sequenceDiagram participant L as Expansion loop participant S as Family searches participant G as RecallSufficiencyGate L->>G: assess(round 0): top_score = normalized RRF (about 1.0) G-->>L: weak-top-1 iff 1-3 candidates loop insufficient and recoverable and rounds left L->>S: search with lowered admission floor S-->>L: hits L->>L: merge and record round L->>G: assess(merged) G-->>L: verdict end alt search or assess raises L-->>L: return round-zero candidates else L-->>L: return merged candidates endAfter
sequenceDiagram participant L as Expansion loop participant S as Family searches participant G as RecallSufficiencyGate L->>G: assess(round 0): top_score = vector cosine, per-Scope first hit G-->>L: verdict loop insufficient and recoverable and rounds left L->>S: search with lowered admission floor S-->>L: hits L->>L: stage merged candidates (capped) L->>G: assess(staged) G-->>L: verdict L->>L: commit staged set, record round and signals end alt search or assess raises L-->>L: return last committed set (expansion-failed) else L-->>L: return last committed set endThe changed edges:
assessreads (S1, S2).assess(S4).When the gate is disabled,
_gated_recall_effortis not entered, so S5 is unchanged.Are there any user-facing changes?
PreparedContextkeeps its four fields, andopenapi/powercontext.yamland the generated codeare unchanged.
search_responseandtopic_memory_search_responsebuild their HTTP models field by field, sorelevanceis not serialized.MemoryHitandTopicMemorySearchHitgain an optionalrelevance: float | Noneattribute (default
None,exclude=True). It is readable on the in-process objects but absent frommodel_dump()and serialized output, matching the existing
retrieval_score/channel_scoresconvention.weak-top-1; expansion still runs while a family reports recoverablecandidates, because the fused window can hide a strong hit that a lower floor surfaces.
RecallEffort.policyreportspowercontext.recall-gate.v2.Boundaries and risks
test_default_off_matches_a_sufficient_round_zero_byte_for_bytetest_fts_only_candidates_skip_relevance_thresholds,test_expansion_fts_only_hit_preserves_round_zero_scored_familyRecallCandidate.scope_id; pooled max, mean and gap do not depend on ordertest_scoped_first_hit_rule_applies_per_scope_through_the_runtime0.3test_lower_floor_candidates_cannot_manufacture_a_top_gapweak-top-1would mask thinnessthin-candidatesandthin-familiesare checked beforeweak-top-1test_gate_reports_thin_candidates_even_when_the_top_is_weakweak-top-1stops expansion while a strong hit sits outside the capped fusion windowtest_bounded_expansion_surfaces_a_strong_hit_hidden_by_the_fusion_limit(verdict plus committed-set membership)assessreturnstest_later_round_failure_preserves_committed_expansion(search and assessment failure)relevanceleaks into a responsemake contract-test0.35,0.02and the0.70strong-match bound are not calibrated for a production embedding model0.70is a module constantNot verifiedwith a real embedding model. RFC 1560 open question 1 remains openadded_embeddings/added_generation_callsHow was this change tested?
All commands were run at
67a024cd, rebased onto968fbddc(the upstreamTopicMemorySearchHitrefactor from RFC#1858 was merged:
relevanceis computed inside the newfuse_rrfpipeline and shares itsexclude=Trueconvention):
The one failure is
tests/builtin/runtime/test_model_usage_recorder.py::test_commit_lock_rolls_back_usage_and_restores_connection(
IndexError: tuple index out of range). That file passes in isolation (29 tests) and is untouched by this PR; thesame
IndexErrorflake has been observed on the unmodified base in full-suite runs, so it is a pre-existingorder-dependent issue.
New and updated tests:
thin-candidatesand expands;build_recall_candidates(memory_scope_ids=...)wiring, which makes the test fail if the call sites drop the Scope ids.0.31(weak-top-1, with one expansion recoveringthe floor-dropped hit);
asserted to reach the Builder's committed candidate set (the per-section render limit is a separate
request-level display cap);
The issue's reproduction script, run at both revisions:
86537e4f(base)67a024cd(this PR)0.90 / 0.50, one recoverable candidatetop_score=0.992,weak-top-1,at-max-roundsafter 3 roundstop_score=0.900,sufficient, 1 round0.31 / 0.31, one recoverable candidatetop_score=1.000,weak-top-1,at-max-roundsafter 3 roundstop_score=0.310,weak-top-1, 3 bounded rounds recovering floor-dropped evidenceNot verified:
SQLite;
toxacross Python 3.12–3.14, and the fullmake testsuite.AI usage statement
Implemented with Devin (SWE-2). Reviewed with Cursor (Grok 4.7 and Claude Opus 5.5), Codex (GPT-6 Astra) and Devin
(SWE-2-Max).