fix(memory-ingest): stop dropping capture_origin at ingest (issue #368) - #410
Open
cdeust wants to merge 5 commits into
Open
fix(memory-ingest): stop dropping capture_origin at ingest (issue #368)#410cdeust wants to merge 5 commits into
cdeust wants to merge 5 commits into
Conversation
mcp_server/core/memory_ingest.py::ingest_memory accepted a caller-supplied
capture_origin field but never forwarded it to store.insert_memory, unlike
every other metadata field (source, tags, heat, ...). Every ingest_memory
caller therefore fell through to insert_memory's "unknown" default
regardless of what it intended — a silently lost field, not a harness gap.
This is why the trust-factor calibration's gated arm
(docs/provenance/trust-factor-calibration.md) could not discriminate W: the
LME/LoCoMo/BEAM benchmarks all landed on capture_origin='unknown', which
core.capture_origin.trust_factor demotes uniformly, and a uniform multiplier
cannot change WRRF order — hence LME identical to four decimals across
W in {1.0, 0.7, 0.6, 0.5}.
Fixes:
- memory_ingest.py: pass capture_origin through, default unchanged
("unknown" for callers that omit it — no behavior change elsewhere).
- benchmarks/lib/capture_origin_mix.py (new): the realistic production
mixture of capture_origin values (local_action .966 / network .025 /
deliberate .009), sourced from a measurement over this machine's own
886 Claude Code session transcripts (~/.claude/projects/**/*.jsonl) —
the only representative "existing store" available; the local memory.db
predates the capture_origin column. Deterministic sampler.
- benchmarks/lib/bench_db.py: wires the mixture into load_memories() via
_apply_capture_origin_mix(), so LME/LoCoMo/BEAM now insert a realistic
blend of trusted/untrusted origins instead of an all-unknown corpus.
16 new tests (tests_py/core/test_memory_ingest_capture_origin.py,
tests_py/benchmarks/test_capture_origin_mix.py,
tests_py/benchmarks/test_bench_db_capture_origin.py). Full suite:
7291 passed, 142 skipped, 0 failed.
What this does NOT do yet: re-run the 5-cell W sweep
(benchmarks/trust_factor_sweep.sh) under the now-mixed origins. W=0.7
remains the production value, still resting on the invalidated gated arm
documented in docs/provenance/trust-factor-calibration.md. That re-measure
is next, separately, once its cost (Docker, ~7h across 5 sequential cells)
is authorized — or a cheaper discriminating measurement is found.
Co-Authored-By: Claude <noreply@anthropic.com>
benchmarks/lib/bench_db.py hard-imports PgMemoryStore (-> psycopg) at
module level (established convention, tests_py/benchmarks/
test_lib_init_no_psycopg.py). This test file imported it unconditionally,
which broke collection on the SQLite-only CI lane
("ModuleNotFoundError: No module named 'psycopg'").
Fix: pytest.importorskip("psycopg", ...) before the benchmarks.lib.bench_db
import, matching the pattern already used by
tests_py/integration/test_recall_trust_ranking.py. Verified locally by
poisoning sys.modules for psycopg/psycopg_pool/pgvector — the suite now
skips cleanly instead of erroring, and still passes normally with psycopg
present.
Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-10 incident: a 5-cell trust-factor sweep ran while three other agents' full pytest suites were active on the same 10-core machine (load average ~11-14). Cell W=1.0 crashed on a native fatal error (rc=120, truncated CPython fatal-error signature) -- a visible failure. But a cell that merely FINISHED under the same contention (saturated connection pool, cold cache, GC pressure) would have produced a plausible-looking degraded number with nothing in the artifact to distinguish it from a clean run. The whole grid was discarded and re-run rather than salvaged, per this project's own rule: a measurement from a harness carrying a known defect is invalid and gets redone, not patched after the fact -- and machine contention during a benchmark run is exactly such a defect. Fix: benchmarks/lib/write_manifest.py::machine_load_snapshot() records load average (1/5/15 min, os.getloadavg()), cpu_count, concurrent pytest process count, and concurrent Docker container count -- wired into build_manifest() as `machine_load`, placed alongside `git_sha` in every MANIFEST.json this harness writes. Best-effort like every other field in this module (embedding_model_revision, reranker_state): a failed probe records None rather than aborting manifest generation. 6 new tests (tests_py/benchmarks/test_write_manifest_machine_load.py). Verified importable and passing with psycopg poisoned out of sys.modules (matches this module's existing psycopg-independence). Co-Authored-By: Claude <noreply@anthropic.com>
Follow-up to aa093d2's machine_load_snapshot: that fix captured load only at the END of a reproduce.sh run (inside write_manifest). A single end-of-run reading cannot distinguish "this cell ran under contention the whole time" from "load spiked right at the end" -- exactly the ambiguity that forced discarding a whole 5-cell sweep rather than salvaging the cells that merely finished (vs. the one that visibly crashed). Fix: benchmarks/lib/write_manifest.py::write_start_snapshot() captures the same machine_load_snapshot() and writes it to RESULTS_DIR/START_SNAPSHOT.json. benchmarks/reproduce.sh calls it first thing in main(), before start_db, so it predates even the container/DB overhead. build_manifest() reads that file back at cell end and folds it into MANIFEST.json as machine_load_at_start, alongside the renamed machine_load_at_end (was machine_load). START_SNAPSHOT.json is excluded from the results_files list. 10 tests (was 6), covering both snapshot points and the results_files exclusion. ruff clean, bash -n clean on reproduce.sh, verified end-to-end against a throwaway results dir. Co-Authored-By: Claude <noreply@anthropic.com>
…(CI) Test (SQLite backend) failed on GitHub's Linux CI runner: TestMachineLoadSnapshot::test_pytest_process_count_sees_the_process_running_this_test -- assert 0 >= 1. Not a flaky test: machine_load_snapshot()'s own pytest process was genuinely undercounted. Root cause: GNU procps `ps aux` truncates the COMMAND column to $COLUMNS (default 80) when stdout is not a terminal and COLUMNS is unset -- unlike BSD ps (macOS), which does not truncate piped output by default, so this never reproduced locally. `ps aux`'s fixed-width USER/PID/%CPU/... prefix alone is close to 80 columns, and pytest's actual invocation path (interpreter + script, e.g. ".../bin/python3.12 .../bin/pytest ...") pushes the "pytest" substring well past that -- so it was silently cut off on every Linux CI run, not just this one. A real undercount in the manifest field this whole fix exists to make trustworthy, not merely a test bug. Fix: pass an explicit wide `COLUMNS` override in the `ps aux` subprocess environment -- both GNU procps and BSD ps honor it, so this is portable and harmless where it wasn't the problem (macOS). Co-Authored-By: Claude <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
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
mcp_server/core/memory_ingest.py::ingest_memoryaccepted a caller-suppliedcapture_originfield but never forwarded it tostore.insert_memory,unlike every other metadata field (
source,tags,heat, ...). Everyingest_memorycaller therefore silently fell through toinsert_memory's"unknown"default regardless of what it intended. This is a productioncode defect, not a harness gap — a field accepted then silently dropped.
This is why
docs/provenance/trust-factor-calibration.md's gated arm couldnot discriminate the trust-factor weight W: LME/LoCoMo/BEAM all landed on
capture_origin='unknown', whichcore.capture_origin.trust_factordemotesuniformly, and a uniform multiplier cannot change WRRF order — hence LME
identical to four decimals across W in {1.0, 0.7, 0.6, 0.5} in that document's
existing results table.
What's fixed
memory_ingest.py—capture_originnow passes through like every otherfield. Default unchanged (
"unknown"for callers that omit it) — nobehavior change for any existing caller that doesn't set it.
benchmarks/lib/capture_origin_mix.py(new) — the realistic productionmixture of
capture_originvalues (local_action.966 /network.025 /deliberate.009), sourced from a measurement over this machine's own 886Claude Code session transcripts (
~/.claude/projects/**/*.jsonl) — theonly representative "existing store" available (the local
memory.dbpredates the
capture_origincolumn, no live production DB touched).Deterministic sampler, documented derivation and limitation in the module
docstring.
benchmarks/lib/bench_db.py— wires the mixture intoload_memories()via_apply_capture_origin_mix(), so LME/LoCoMo/BEAM now insert a realisticblend of trusted/untrusted origins instead of an all-
unknowncorpus. Neveroverwrites an explicit origin already set by a caller (e.g. the adversarial
corpus).
What's NOT fixed yet (next, separately)
The production value W = 0.7 still rests on the invalidated gated arm
documented in
docs/provenance/trust-factor-calibration.md— that documentis unchanged by this PR. Re-running the 5-cell W sweep
(
benchmarks/trust_factor_sweep.sh) under the now-mixed origins is thedirect continuation of this same contract, not a separate task, but it costs
~7h across 5 sequential Docker-isolated
reproduce.shcells and that cost isbeing reported to the repo owner for a go/no-go before running it — plus a
check for whether the adversarial arm (
benchmarks/lib/trust_factor_sweep.py,seconds on in-memory SQLite) is now sufficient on its own, since the origins
it discriminates against are the same ones the gated arm was blind to before
this fix.
Test plan
uv run --no-sync pytest -q -p no:randomly— 7291 passed, 142 skipped,0 failed
tests_py/core/test_memory_ingest_capture_origin.py,tests_py/benchmarks/test_capture_origin_mix.py,tests_py/benchmarks/test_bench_db_capture_origin.pyruff check/ruff format --checkclean on touched filesCo-Authored-By: Claude noreply@anthropic.com