docs(benchmarks): ENGRAM-typed retrieval — third null architectural lever - #64
Merged
Conversation
📝 WalkthroughWalkthroughA new ENGRAM-style typed retrieval experiment section is added to docs/benchmarks.md describing a per-turn classifier into {episodic, semantic, procedural}, typed metadata storage, routed multi-store retrieval (including oracle routing), a three-cell experiment (leader, engram_typed, oracle_routed), results, and reproduction pointers. ChangesBenchmark Documentation: ENGRAM Typed Retrieval Experiment
Estimated Code Review Effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (2 files)
Reviewed by grok-code-fast-1:optimized:free · 170,807 tokens |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/benchmarks.md`:
- Line 302: Replace ephemeral /tmp paths in the benchmarks provenance note with
a stable repository artifact path or documented bench-output directory: update
the sentence in docs/benchmarks.md that mentions
`/tmp/engram_classifications.json` and `/tmp/engram_routed_3cell_summary.tsv` to
point to a reproducible location (e.g.,
`benchmarks/artifacts/engram_classifications.json` and
`benchmarks/artifacts/engram_routed_3cell_summary.tsv` or a CI-attached artifact
URL) and, if needed, add a brief note in the engram_routed_3cell.sh bench script
to write outputs to that same repo artifact path or to upload them as CI
artifacts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| The fourth lever — generator size — is the next experiment. `qwen3.6:35b-a3b` (Q4_K_M, ~23 GB, MoE: 35 B total / 3 B active) at the same leader recipe runs as Phase 1 on Fedora the night of May 6. | ||
|
|
||
| Measured on Fedora 12 GB 3060 host, May 6 2026. Branch `feat/engram-typed-retrieval` carries the EngramRouter classifier, the `engram_typed` / `oracle_routed` retrieval modes, and the bench script `engram_routed_3cell.sh` for reproduction. Cached classifications at `/tmp/engram_classifications.json`, full sweep summary at `/tmp/engram_routed_3cell_summary.tsv` on the bench host. |
There was a problem hiding this comment.
Avoid /tmp host paths in benchmark provenance notes
/tmp/engram_classifications.json and /tmp/engram_routed_3cell_summary.tsv are ephemeral and not reproducible for other readers. Prefer a stable repo path (or attached artifact location) for referenced outputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/benchmarks.md` at line 302, Replace ephemeral /tmp paths in the
benchmarks provenance note with a stable repository artifact path or documented
bench-output directory: update the sentence in docs/benchmarks.md that mentions
`/tmp/engram_classifications.json` and `/tmp/engram_routed_3cell_summary.tsv` to
point to a reproducible location (e.g.,
`benchmarks/artifacts/engram_classifications.json` and
`benchmarks/artifacts/engram_routed_3cell_summary.tsv` or a CI-attached artifact
URL) and, if needed, add a brief note in the engram_routed_3cell.sh bench script
to write outputs to that same repo artifact path or to upload them as CI
artifacts.
…ever May 6 three-cell experiment at the leader recipe replicating the ENGRAM paper (arXiv:2511.12960). Overall is flat: leader_baseline_repro 0.54 engram_typed 0.53 oracle_routed 0.53 Per-category, typed retrieval REGRESSES Multi-hop hard (-0.23): the 3-way fanout dilutes cross-encoder ranking on the set-merged candidates. Single-hop unchanged across all three cells, matching the prompt and embedder nulls — the Single-hop ceiling at our 9B generator tier is not in retrieval architecture at all. Oracle-routed (perfect category routing — upper bound on what a query classifier could do) is also flat at 0.53, so we're not building Phase 2b. Includes a why-it-doesn't-replicate analysis (ENGRAM's GPT-4o-mini generator + text-embedding-3-small retrieval saturate different failure modes than our local stack; typed routing recovers recall a frontier generator can use, but our 9B is bottlenecked on generator extraction quality — not retrieval recall). Phase 1 (qwen3.6-MoE) is the generator-size lever, queued the night of May 6. Methodology note flags the wiring bug found mid-experiment: the first engram_typed run reported +0.16 Multi-hop, which turned out to be the runner's vector-only path silently ignoring --retrieval-mode. Fixed in 9306bb2 on feat/engram-typed-retrieval; pre-fix numbers retracted internally; table is post-fix.
jaylfc
force-pushed
the
docs/engram-negative-may6
branch
from
May 10, 2026 17:56
e206265 to
2587d2d
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/benchmarks.md (1)
438-438:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEphemeral
/tmppaths reduce reproducibility.Same issue as previously flagged:
/tmp/engram_classifications.jsonand/tmp/engram_routed_3cell_summary.tsvare ephemeral host paths that won't be available to other readers. Consider moving these to a stable repository location (e.g.,benchmarks/artifacts/) or documenting them as CI artifacts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/benchmarks.md` at line 438, The docs currently reference ephemeral host paths `/tmp/engram_classifications.json` and `/tmp/engram_routed_3cell_summary.tsv` which harms reproducibility; update the `docs/benchmarks.md` entry (the paragraph mentioning branch `feat/engram-typed-retrieval` and script `engram_routed_3cell.sh`) to point to stable repository artifact locations (e.g., `benchmarks/artifacts/engram_classifications.json` and `benchmarks/artifacts/engram_routed_3cell_summary.tsv`) or explicitly note they are produced as CI artifacts with a link/path where CI stores them, and adjust any mention of the cached classifications and full sweep summary to the new repo-relative paths or CI artifact instructions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/benchmarks.md`:
- Line 438: The docs currently reference ephemeral host paths
`/tmp/engram_classifications.json` and `/tmp/engram_routed_3cell_summary.tsv`
which harms reproducibility; update the `docs/benchmarks.md` entry (the
paragraph mentioning branch `feat/engram-typed-retrieval` and script
`engram_routed_3cell.sh`) to point to stable repository artifact locations
(e.g., `benchmarks/artifacts/engram_classifications.json` and
`benchmarks/artifacts/engram_routed_3cell_summary.tsv`) or explicitly note they
are produced as CI artifacts with a link/path where CI stores them, and adjust
any mention of the cached classifications and full sweep summary to the new
repo-relative paths or CI artifact instructions.
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
1 participant
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
Third architectural negative result on top of the prompt and embedder sweeps: typed memory routing per the ENGRAM paper (arXiv:2511.12960) does not transfer to our local 9B generator + qwen3:4b judge stack.
Three-cell experiment at the leader recipe (`k=20 + adj=2 + llm-exp + RRF`), 200 QAs subset:
Headlines:
Why it doesn't replicate the paper's +31 pp: ENGRAM uses GPT-4o-mini + text-embedding-3-small. At that tier, typed routing recovers recall a frontier generator extracts well. Our 9B's bottleneck is extraction, not retrieval recall — typed routing doesn't address it.
Methodology note
The first engram_typed run reported +0.16 Multi-hop. Wiring bug: `--retrieval-mode` was silently ignored when `--strategy=vector-only` (bench default) because the runner's `_retrieve` short-circuited to a direct `vmem.search` call. Fix landed in commit `9306bb2` on `feat/engram-typed-retrieval`; pre-fix numbers retracted internally; the table here is post-fix and matches `oracle_routed`'s independently-computed per-category numbers.
Test plan
Summary by CodeRabbit