fix(recall): deterministic rescue ranking + rerank scale-mixing fix (flag-gated) - #324
Open
heybeaux wants to merge 8 commits into
Open
fix(recall): deterministic rescue ranking + rerank scale-mixing fix (flag-gated)#324heybeaux wants to merge 8 commits into
heybeaux wants to merge 8 commits into
Conversation
Read-only Phase 1 deliverables for the Engram memory-formation + query-transformation initiative: - 01 current-state map: ingestion→recall data flow, score semantics, data model, intervention points, existing LLM usage - 02 research memo: literature review + blind-spot pass with cited primary sources; separates published evidence from inference - 03 experiment spec: A–E ablation matrix, graduated corpora, retrieval + downstream metrics, saturation-aware reproducibility controls - README: exec summary, sequencing, open questions No code, data, or migrations touched. Prototypes remain unbuilt pending review. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Three Phase 1 assumptions were falsified by running against live Engram: - Local Engram serves on 47291, not 3001 (3001 is whalehawk provider-server) - `content` is a legacy alias for `raw`, not a second field. The documented "store formed text in content, keep raw for provenance" plan would have been a silent no-op and invalidated intervention B. Union is now realized inside the embedded string via contextual prefix + verbatim observation with recorded prefixLength. - Local embedder 500s under parallel recall, which silently degraded arm C into the baseline — a bias against the intervention under test Also documents a pre-existing pgvector dimension mismatch (1536 vs 384) on the write path, recommended for its own ticket. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Verified independently twice against the committed v0.2 noisy report: 18/20 queries end in a 5-or-more-way tie at the top score, and only 2/20 top-1 results are decided by score. The reported "13/20 rank 1" is 11-of-13 decided by array order within a tied block; under fair tie-breaking the same system scores ~0.21, not 0.65. Context false-positive injection rate is 0.87. Root cause is the score semantics already documented in the current-state map: keyword-rescue paths assign hard-coded constants (FTS 1.25, ILIKE 1.1) rather than a similarity, so every rescued candidate lands on an identical score. This blocks the A-E ablation: interventions work by supplying better candidates, but a better candidate cannot change top-1 when the tie-break decides it. Sequencing revised so scoring/tie-breaking is fixed before the matrix runs. Downstream lift figures in RESULTS.md are unaffected; the retrieval narrative attached to them is not. Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Engram's top-1 recall result was decided by Postgres row order, not by score. On the 20-task noisy benchmark corpus 18/20 queries ended in a 5-or-more-way tie at the top score and only 2/20 were uniquely decided by score, so the published "13/20 rank 1" collapses to ~0.21 under fair tie-breaking. See docs/research/memory-formation-query-transform/04-finding-tie-domination.md. Two independent causes: 1. Both keyword-rescue paths assigned constants. The FTS/BM25 path computed an RRF value and threw it away, calling scoreMap.set(id, 1.25) for every rescued candidate; the ILIKE path did the same with 1.1. The SQL already ordered by ts_rank but never SELECTed it. 2. Every score sort was a bare (b.score ?? 0) - (a.score ?? 0). Because Array.prototype.sort is stable, equal scores kept whatever order the DB returned. Fix, in new src/memory/memory-ranking.util.ts: - ftsRescueScore() maps ts_rank (now SELECTed in all three FTS query variants) into (1.10, 1.25]: 0.9 * (ts_rank / max ts_rank) + 0.1 * normalised RRF. Band top is exactly 1.25 for the best hit and values decrease strictly with rank, so near-duplicates with identical ts_rank no longer collapse into a tie. - ilikeRescueScore() maps lexical coverage (matched terms / extracted terms, computed in SQL) into (1.00, 1.10] with the same RRF tie-break. - compareByRankKeys() is the single comparator now used by every score sort in memory-query.service.ts and memory-query-ranking.service.ts: score desc, vectorScore (cosine) desc, importanceScore desc, createdAt desc, id asc. The final key makes the ordering a total order, so the same candidate set sorts identically regardless of DB row order. - Sticky keyword re-add after reranking preserves the banded score instead of clamping to 1.1. Inter-band priority is unchanged on purpose: FTS rescue still outranks ILIKE rescue, which still outranks every cosine hit (ceiling 1.0). The graph-recall x1.2 boost and cross-encoder reranker behaviour are untouched so the re-baseline measures one variable. Tests: new memory-ranking.util.spec.ts (17) plus a regression block in memory-query.service.spec.ts asserting distinct scores for distinct ts_rank, band ordering vs a 0.99 cosine hit, and identical output for the same tied set fed in three different input orders. src/memory: 66 suites / 1143 tests pass.
…UE (default off) The rescue score bands added in c905438 do not merely break ties in favour of lexical hits — they replace the ranking. Measured on the 20-task noisy prefix corpus with a new retrieval-only probe (no LLM, no generation): - gold is in the candidate pool for 20/20 tasks, so this is a ranking failure, not a recall failure; - inside an FTS-rescued block the candidate with the *highest* cosine is ranked *last*, because ftsRescueScore() overwrites scoreMap with a ts_rank-derived value and the cosine only survives as an unused tie-break key; - the sticky keyword re-add re-injects rescued memories with their raw pre-rerank band scores (1.25/1.15/1.05) after applyReranking has rescaled every other candidate to <= ~1.0, so band values are sorted against rescaled values and win categorically. This also makes results non-monotonic in `limit`: the same query returns 5 band hits at limit=5 and zero band values at limit=50. Prototype fix, off by default (RECALL_RELATIVE_RESCUE=true to enable): relativeRescueScore() turns lexical agreement into a bounded boost on the candidate's own cosine (score = cosine * (1 + maxBoost * quality); 0.2 FTS, 0.1 ILIKE, 0.15 identity) instead of an absolute band above the cosine ceiling. Lexical-only candidates with no vector hit are anchored at 0.9 * bestCosine so the fresh-exact-write guarantee survives without letting them displace the best semantic hit. The sticky re-add appends below the rescaled floor in relative mode instead of above it. Also under the flag: the flat 1.15 identity rescue constant (same flat-constant defect class c905438 fixed) becomes continuous. The flat 0.75 forcedFts constant is annotated, not changed — topIds and memoryMap are both built from `sorted`, so that branch is unreachable and forcedFts is always empty. Measured, both arms against usage counters reset to the same cold snapshot (recall increments retrievalCount and applyUsageWeighting feeds it back into the score, so the benchmark perturbs itself): gold in top-5 6/20 -> 7/20 gold in top-10 6/20 -> 8/20 MRR@10 0.150 -> 0.174 gold in top-1 0/20 -> 0/20 Small, and deliberately reported as such: after the fix the ranking is the semantic ranking, and on 11 of 20 tasks the gold memory sits behind all ten of its distractors in that ordering. The distractors restate the query verbatim; under bge-base they are the closer match. The band fix is worth having for determinism, scale hygiene and limit-monotonicity, not for benchmark score. Default behaviour unchanged: src/memory 66 suites / 1150 tests pass. Analysis in docs/research/memory-formation-query-transform/05-finding-band-inversion.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…_SCALE_FIX The sticky keyword re-add reinjected rescued candidates at their raw pre-rerank band scores (1.25 / 1.15 / 1.05) into an ordering that applyReranking had already rescaled to <= ~1.0. Raw beat rescaled categorically, so the limit=10 page was whatever the lexical rescue produced and the reranker's judgement was discarded — and ranking was non-monotonic in `limit` (the same memory scored 1.24804 at limit=10 and 0.42623 at limit=250). RECALL_RERANK_SCALE_FIX=true (default off) ranks the whole candidate pool instead of truncating at `limit`, so rescued candidates stay sticky *in the rescaled scale* and the re-add becomes a no-op; `limit` is applied once, at the end, to a total order. Adds RECALL_NO_RESCUE=true as the vector-only control arm. Measured on the noisy mnemon corpus (20 tasks, limit=10, usage counters reset per arm): arm gold@1 gold@5 gold@10 MRR@10 default 0 6 6 0.1500 scalefix 0 14 14 0.3500 relative 0 7 8 0.1738 both 0 7 8 0.1738 norescue 6 7 8 0.3312 Limit-monotonicity verified live: default violates the prefix property on 20/20 queries, scalefix holds on 20/20 (both 5-vs-10 same-pool and 10-vs-250). Default arm output is unchanged with all flags off. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…e fix
The 5-arm ablation on the 20-task noisy prefix corpus (limit=10, usage
counters reset per arm) killed the relative-rescue prototype:
arm gold@1 gold@5 gold@10 MRR@10
default 0 6 6 0.150
scalefix 0 14 14 0.350
relative 0 7 8 0.174
both 0 7 8 0.174
vector-only 6 7 8 0.331
The kill signal is that `both` is numerically IDENTICAL to `relative`, not
that `relative` merely underperformed. relativeRescueScore() rewrites
candidate scores into the rescaled scale upstream, at the FTS/ILIKE/identity
injection sites. By the time RECALL_RERANK_SCALE_FIX runs there is no scale
mismatch left for it to correct, so the scale fix degrades to a no-op. The
two flags do not compose: enabling relative rescue *replaces* the scale fix
and drags 14/20 gold@5 back down to 7/20. They are mutually exclusive by
construction, and the scale fix is strictly better on every metric.
Leaving the prototype in the tree would have left a config that silently
halves recall quality one env var away, so it goes now rather than rotting.
Removed:
- relativeRescueScore() and the RELATIVE_* boost/anchor constants
- the RECALL_RELATIVE_RESCUE flag read and all three relative-mode
branches (FTS, ILIKE, identity), plus the relative-mode variant of the
sticky re-add's below-the-floor append
- the 7 relativeRescueScore unit tests
- scripts/research/run-rescue-ab.sh (existed only to A/B this flag);
run-scale-fix-arms.sh drops the now-impossible relative/both arms
Kept deliberately:
- RECALL_RERANK_SCALE_FIX and RECALL_NO_RESCUE and everything they gate
- everything from c905438 (ftsRescueScore, ilikeRescueScore,
compareByRankKeys, the ts_rank SELECTs)
- the unreachable-forcedFts annotation and the flat-1.15 identity note —
findings, not prototype
- all docs under docs/research/memory-formation-query-transform/. The
analysis in 05-finding-band-inversion.md is still valid and is *why* we
know to kill this; it now carries a note at the top recording the
removal and the both==relative collapse so nobody re-derives it.
memory-ranking.util.ts and memory-ranking.util.spec.ts are now byte-identical
to c905438; the only remaining delta in memory-query.service.ts vs c905438 is
the scale-fix work. With all flags off recall output is unchanged.
src/memory: 67 suites / 1152 tests pass (1159 before; the 7 fewer are exactly
the removed relativeRescueScore tests).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ Recall Benchmark ResultsFull outputCommit: 835cdc2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three recall-ranking defects found during the memory-formation R&D spike. All behavior changes remain behind explicit flags that default off, so the existing default path is preserved.
1. Deterministic, continuous rescue ranking
Keyword rescue previously assigned flat constants, then relied on database row order to break ties. This PR now:
ts_rankand ILIKE lexical coverage into continuous rescue scores;id ASCtie-breaking behindRECALL_RESCUE_SQL_TIEBREAK=true.The SQL tiebreak does not move the benchmark metric, but it fixes real restart-to-restart nondeterminism and is retained on correctness grounds.
2. Rerank/re-add scale mixing
RECALL_RERANK_SCALE_FIX=trueranks the full reranked candidate pool and applieslimitonce at the end. This prevents raw rescue-band scores from being reinserted above already-rescaled reranker scores and restores limit monotonicity.RECALL_NO_RESCUE=trueremains available as the vector-only control arm.3. Lexical coverage floor
RECALL_LEXICAL_COVERAGE_FLOOR=truerequires multi-term evidence before the OR-joined ILIKE rescue can promote a candidate. This removes single-token false positives such as the corpus prefix artifact matching every query only because both containedMNEMON.The discarded importance-ramp prototype is not included. It added no measurable value over the coverage floor.
Measured results
20-task noisy corpus,
limit=10, usage counters reset per arm:Independent repeats and seed variants were bit-identical. The remaining 6 misses sit at pool rank 11, one slot beyond the page, so the next research target is candidate-pool depth and near-duplicate cluster handling rather than more rescue-band tuning.
Safety and rollout
RECALL_RERANK_SCALE_FIX=true,RECALL_LEXICAL_COVERAGE_FLOOR=true, andRECALL_RESCUE_SQL_TIEBREAK=true.Verification
8af07f6