fix(embedding): frequent-token subsampling matches its cited CBM source (#184)#185
Merged
Conversation
…ource (#184) The algorithmic fallback embedder cited CBM_SEM_MAX_OCCUR (semantic.c:874 cooccur_sparse_one_target) but implemented different semantics: it strided the WHOLE per-position loop whenever a *document* exceeded 512 tokens, dropping first-order terms too. CBM strides only a *single token's* co-occurrence pass when that *token* occurs >512 times, never the first-order term. A §8 source-fidelity defect. On LongMemEval-S, 86.2% of memories exceed 512 tokens (median 1622), so the document-striding fired on most stored vectors and measurably HURT retrieval (-2.0 pp Recall@10 vs no striding). The CBM-faithful per-token trigger fires on only 2/2556 memories — inert on prose, exactly as semantic.h:52 claims. Fix (option (a)): first-order terms always complete; co-occurrence strided only for a token whose within-document occurrence count exceeds _MAX_OCCUR. The code now matches both the cited source and the module docstring (which already described the CBM semantics). Extracted _first_order_pass / _cooccurrence_pass / _accumulate_window seams. Evidence (benchmarks/results/subsample-fidelity-184/, n=50): off MRR 0.3786 R@10 68.0% current MRR 0.3777 R@10 66.0% (shipped — hurts) faithful_a MRR 0.3786 R@10 68.0% (this fix) Committed #169 sweep re-run on the fixed embedder: fallback 0.378/66% -> 0.379/68%. Tests: 9 new (stride trigger, boundary at exactly _MAX_OCCUR, negative stride==1 assertion, first-order-never-subsampled, direct window/first-order contract). Scoped mutation (mutmut): 0 survivors in the stride/first-order/ window path; remaining survivors are documented-equivalent (internal dtype, norm guard) or pre-existing in unchanged functions. Closes #184 Co-Authored-By: Claude <noreply@anthropic.com>
…ree) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Closes #184.
What actually diverged
mcp_server/shared/algorithmic_embedding.py::embed_textcited CBM'sCBM_SEM_MAX_OCCUR = 512(semantic.h:52, applied insemantic.c:874 cooccur_sparse_one_target) for frequent-token subsampling, but implementeddifferent semantics on both axes the issue names:
Mikolov et al. 2013's
P(discard) = 1 - sqrt(t/f(w))was never the mechanism inplay: it needs a corpus-relative
f(w)this stateless per-text seam does nothave (the same reason IDF is excluded). CBM's mechanism is stride-sampling of a
frequent token's occurrence positions — inspired by word2vec subsampling, not
implementing its formula. The shipped code borrowed the constant and the
citation but applied a document-length cost bound the source does not describe.
Which fix path the evidence chose: (a) match the source
Measured on LongMemEval-S (n=50 haystack, 2556 memories):
majority of stored vectors (the committed feat: zero-download semantic fallback — algorithmic embeddings + code-aware FTS tokenizer #169 numbers were produced with it).
memories would trigger the CBM per-token stride → inert on prose, exactly as
semantic.h:52claims.Three-way retrieval comparison (
benchmarks/results/subsample-fidelity-184/):The shipped document-striding does not merely fail to help — it hurts (−2.0
pp Recall@10), because it drops the first-order term at half the positions of the
~86% of memories over the cap. Chose (a): CBM-faithful. It restores §8 source
fidelity (code now matches the citation and the module docstring, which already
described the CBM semantics), scores best, and keeps a source-faithful cost guard
that genuinely fires (2/2556), which the acceptance criteria expect tested rather
than removed. Removal would score identically but delete a documented, firing
guard for no §8 benefit.
Benchmark deltas
Committed #169 sweep re-run on the fixed embedder (§8 — a changed embedder
invalidates recorded numbers;
benchmarks/results/semantic-fallback-169/updated, only the
fallbackrow moves —no-vector/sentence-transformersdo not use this embedder):
Elapsed rose (38.6 → 62.9 s) because the co-occurrence pass is no longer strided
on long prose — an accepted trade for a non-hot-path encode whose vectors upgrade
to neural on the first consolidate.
Completion Ledger
# source:on_MAX_OCCURstates per-token trigger + co-occurrence scope_MAX_OCCUR)test_frequent_token_triggers_subsampling…,test_boundary_at_exactly_max_occur_does_not_stride, direction-preserved teststride == 1test_normal_memory_takes_no_subsampling,test_first_order_terms_are_never_subsampled_cooccurrence_pass/_first_order_pass/_accumulate_window; remaining survivors documented-equivalent or pre-existing in unchanged functionssemantic-fallback-169/MANIFEST + JSON updatedGates
ruff check+ruff format --check: pass.pytest: 5364 passed, 9 skipped, 0 failed (43m) — test: test_rebuild_profiles depends on live ~/.claude data — 3 failures reproduce on main #174 zero-failure baseline holds. The 4 warnings are pre-existing asyncio-teardownResourceWarnings, unrelated.§15 No-Deviation
Scope held to the one module, its test, and the two benchmark result dirs. No
unrelated files touched;
pyproject.tomlmutmut scope restored to its committedvalue after the per-change run.
🤖 Generated with Claude Code