Skip to content

fix(memory): move the citation contract to where instructions are read - #198

Merged
kevintseng merged 6 commits into
mainfrom
fix/citation-contract-and-signal-census
Aug 23, 2026
Merged

fix(memory): move the citation contract to where instructions are read#198
kevintseng merged 6 commits into
mainfrom
fix/citation-contract-and-signal-census

Conversation

@kevintseng

Copy link
Copy Markdown
Contributor

Summary

The signal memesh uses to justify injecting memories at all has never produced a number. Read off a real database, not a fixture:

citation_sessions_total = 4          ← 4 sessions received injected memories
citation_sessions_cited = (absent)   ← the branch that writes it never ran

None of those four sessions cited a memory. R1 spent +70 tokens per session building that accounting: write side shipped, read side (analytics.ts) shipped, tests on both, review passed. The live values were read for the first time eight days later.

Why it produced nothing — and why it is not Claude Code's fault

The instruction asking for [mem:id] markers was not missing. It is appended after the fenced block deliberately, so it reads as an instruction rather than as data. The problem is one layer further out: Claude Code wraps every hook's additionalContext in a system-reminder ending

IMPORTANT: this context may or may not be relevant to your tasks. You should not respond to this context unless it is highly relevant.

The instruction lands inside that container along with the memories and is read the same way — as data.

That wrapper is correct and must stay. Memory content is attacker-influenced in the general case (anything the agent was ever told can end up in an observation), so a hook's injected text must not be able to drive the agent. What it protects is memesh's own users. memesh was simply using the wrong channel: it put a contract into the data pipe.

Verified against the running binary (2.1.241), not documentation — the current docs describe tool_result and compaction_trigger, which appear zero times in that binary. It sends tool_response and trigger, which is what memesh already reads.

What changed

src/core/citation-rule.ts writes .claude/rules/memesh-citations.md — the layer Claude Code loads as instructions. Only the contract moves; memory content stays fenced and treated as data. Leaf module (node builtins only) so generate-hook-core can mirror it for the hooks.
install-hooks.ts writes it on the npm path, including when a plugin runtime is detected — hooks must not double-fire, but nothing in the plugin runtime writes a rules file
session-start.js self-heals it for plugin installs, which never run install-hooks — the majority of users
session-start.js removes the injected instruction line. A per-session copy of an instruction that is read as data is a per-session cost with no effect.
session-summary.js citation_sessions_cited initialised unconditionally — "zero cited" and "never ran" were the same absent key
doctor.ts new citation_compliance row. The numbers were already correct and readable; reaching anyone required going to look.
scripts/audit/measure-signals.mjs read-only census of what every measuring mechanism actually produces on a live database
dashboard/src/lib/i18n.ts doctor.msg.citation.none.* in all 11 locales

Safety

The rule file sits inside the user's own config tree, so both write and remove refuse to touch a file without the <!-- managed-by: memesh --> marker, and report foreign-file rather than failing silently. Pinned by tests in both directions.

Verification

Real values, not fixtures:

node scripts/audit/measure-signals.mjs     19 signals · 0 producing nothing · 1 unmeasurable
                                           citation.sessions = "? cited / 4 injected"  (live DB)
SessionStart hook, throwaway HOME          exit=0; wrote memesh-citations.md (599 bytes)
                                           grep -c "cite it once inline" on injected block = 0
npm run typecheck                          exit=0
npm run build                              exit=0
node scripts/run-tests-isolated.mjs        160 files, 2354 tests passed, exit=0
node scripts/audit/verification-audit.mjs  exit=0  (C8: denominator=25 hits=0)
  • npx tsc --noEmit clean
  • npm run build clean — dist/ and dashboard/dist/ regenerated and committed (plugin-marketplace installs run committed dist/)
  • npm test -- --run passing — 160 files, 2354 tests
  • Hooks touched → exercised end-to-end against a real payload shape taken from the 2.1.241 binary
  • LLM flows untouched

What the guards caught, and what they missed

C8 caught the new doctor row the moment it was added (hits=1 new=1) — that detector was added yesterday for exactly this, and it worked on its first real opportunity.

Two things it structurally could not see, both caught by the suite:

  • the doctor stub answered expect(sql).toMatch(/COUNT\(/) for any unknown query, so the new memesh_metadata read made four tests fail
  • the new message code had no entry in the 11 locale catalogues — dashboard-i18n.test.ts failed on it

Both are the same shape as the bug this PR fixes: a piece added on one side, and the other side never told.

Known limitations / follow-ups

  • The fix is unproven. Moving the contract to the instruction layer is supported by this session's own evidence (every CLAUDE.md rule obeyed; zero citations from the injected one) and by outside reports that additionalContext loses to system-level instructions — but whether the citation rate actually moves off 0% can only be answered by running it for a week and reading citation_compliance again. That number, not this PR, is the verdict.
  • Instruction-layer rules are soft: sources describe CLAUDE.md as context Claude weighs, not a policy engine. The only hard enforcement available is a hook exit 2, which the Stop hook could use to refuse to end a session that cited nothing. Deliberately NOT in this PR — it changes session behaviour and needs a decision.
  • access_count still only moves on the kg.search() path; hook injection uses its own SQL. The census reports this rather than hiding it.

Comment thread scripts/hooks/_generated/citation-rule.js Fixed
Comment thread scripts/hooks/_generated/citation-rule.js Fixed
Comment thread src/core/citation-rule.ts Fixed
Comment thread src/core/citation-rule.ts Fixed
The ROI signal memesh uses to justify injecting memories has never produced
a number. Measured on a real database, not a fixture:

    citation_sessions_total = 4
    citation_sessions_cited = (key absent — the branch never ran)

Four sessions received injected memories. None cited one. R1 spent +70
tokens per session building that accounting and shipped the write side, the
read side (analytics.ts), tests on both, and passed review. Nobody read the
live values until eight days later.

WHY IT PRODUCED NOTHING

Not a missing instruction. The line asking for [mem:id] markers is appended
AFTER the fenced block on purpose, so it reads as an instruction rather than
as data. The problem is one layer further out: Claude Code wraps every
hook's additionalContext in a system-reminder ending "this context may or
may not be relevant to your tasks. You should not respond to this context
unless it is highly relevant". The instruction lands inside that container
with the memories and is read the same way — as data.

That wrapper is correct and stays: memory content is attacker-influenced in
the general case, so a hook's text must not be able to drive the agent.
Verified against the running binary (2.1.241), not against documentation —
the docs describe fields (tool_result, compaction_trigger) that appear zero
times in it.

WHAT CHANGES

- src/core/citation-rule.ts writes .claude/rules/memesh-citations.md, the
  layer Claude Code loads as instructions rather than context. Only the
  CONTRACT moves; memory content stays fenced and treated as data. A leaf
  module (node builtins only) so generate-hook-core can mirror it.
- Both install paths write it. install-hooks covers npm; the SessionStart
  hook self-heals for plugin installs, which never run that command — the
  majority, and the path that would otherwise never get the contract.
- The injected instruction line is REMOVED rather than duplicated. A
  per-session copy of an instruction that is read as data is a per-session
  cost with no effect.
- citation_sessions_cited is initialised unconditionally. Writing it only on
  a citation made "zero cited" and "this never ran" the same absent key.
- doctor gets a citation_compliance row. The numbers were already correct
  and already readable; reaching anyone required going to look for them.
- scripts/audit/measure-signals.mjs: a read-only census of what every
  measuring mechanism actually produces on a live database. The other
  detectors here are static and cannot see this class — the source is right,
  only the live number is wrong.
- Refuses to overwrite or delete a file at that path without the
  managed-by marker. It sits inside the user's own config tree.

C8 caught the new doctor row as unpinned the moment it was added, which is
what it was written for. Two regressions it could not see were caught by the
suite: the doctor stub could not answer the new metadata query, and the new
message code had no entry in the 11 locale catalogues.

Verified-By: node scripts/run-tests-isolated.mjs → "Test Files 160 passed (160) / Tests 2354 passed (2354)", exit=0
Verified-By: npm run typecheck → exit=0, no TS errors
Verified-By: npm run build → exit=0, "Build is ready for use"
Verified-By: node scripts/audit/verification-audit.mjs → exit=0, "Every hit is triaged; every detector saw a non-empty candidate set" (C8: denominator=25 hits=0)
Verified-By: node scripts/audit/measure-signals.mjs → "19 signals · 0 producing nothing · 1 unmeasurable"; citation.sessions = "? cited / 4 injected" on the live database, not a fixture
Verified-By: SessionStart hook against a throwaway HOME → exit=0, wrote .claude/rules/memesh-citations.md (599 bytes); grep -c "cite it once inline" on the injected block = 0
@kevintseng
kevintseng force-pushed the fix/citation-contract-and-signal-census branch from 0a08214 to df2522f Compare August 23, 2026 17:04
…lacks

C3 caught `measure-signals.mjs` with no automated caller, which is accurate:
every other `measure-*.mjs` in this directory carries a NOT-A-GATE baseline
entry saying "a manual measurement tool a human runs".

That triage is also how a tool stops being run. Nothing calls it, nothing
notices when it breaks, and the number it was built to surface goes back to
being invisible — which is the exact defect THIS tool exists to catch. So it
gets tests instead of a baseline entry, and becomes the one measurement
harness here with an automated caller.

Pinned: the three-state classification, because that is the whole product.
A counter that is ABSENT is not a counter that is ZERO — collapsing those
two is what let `citation_sessions_cited` read as 0% compliance when the
truth was "this branch has never run", and the opposite error would tell a
healthy install it is broken. Also pinned: a missing database exits 2 rather
than reporting zero healthy signals, and the census leaves the file it reads
byte-identical (it is pointed at the user's real graph by default).

Verified-By: npx vitest run tests/audit/measure-signals.test.ts → "Test Files 1 passed (1) / Tests 9 passed (9)", exit=0
Verified-By: node scripts/audit/verification-audit.mjs → exit=0, "C3: denominator=36 hits=6 new=0", "Every hit is triaged"
Verified-By: npm run typecheck → exit=0
Verified-By: node scripts/run-tests-isolated.mjs → "Test Files 161 passed (161) / Tests 2363 passed (2363)", exit=0
None of these were found by this repository's own suite, its detectors, or the
author. They came from `codex review` — a different model, no shared context.

[P1] The self-heal ignored the installed scope.
`session-start.js` passed `'user'` as a literal, so a `--scope project`
install wrote `~/.claude/rules/memesh-citations.md` on every session: a
project-only install leaking into every other project on the machine, and
surviving `uninstall-hooks --scope project`, which only knows the project
path. Scope now comes from the install marker's `scope` field. A missing
marker still means user — that is exactly what a plugin install looks like,
and it is the majority case.

[P2] `foreign-file` was swallowed by both install paths.
When a file already sits at the rule path without memesh's marker,
`writeCitationRule` correctly refuses and returns `foreign-file` — and the
CLI ignored `result.citationRule` and printed success. The contract was never
installed and the user was never told. That is the silent-failure shape this
whole change exists to remove, reintroduced inside the fix for it. Both exits
now report it, and the warning says what it costs: memesh cannot tell whether
the memories it injects are ever used.

[P2] An unreadable rule file was diagnosed as a broken database.
`citationRuleState` throws on a permissions error or a directory at that path,
and the call sat inside doctor's database try/catch — so a filesystem problem
surfaced to the user as `database.broken`, sending them to debug a database
that was fine. It now has its own try and its own `unreadable` state.

Pinned by `tests/hooks/citation-rule-self-heal.test.ts`, which spawns the real
hook: the P1 defect was a literal argument at the call site, so a unit test of
`writeCitationRule` (which has always honoured its scope parameter) could not
have caught it. Break-tested — restoring the hardcoded `'user'` turns the
project-scope case red.

The C1 detector then caught the new test file for asserting existence without
asserting content, which was a real hole: an empty file satisfies `existsSync`
and teaches an agent nothing. The contract's length and its `[mem:N]` example
are now asserted too.

Verified-By: npx vitest run tests/hooks/citation-rule-self-heal.test.ts → "Tests 6 passed (6)", exit=0
Verified-By: break-test in an isolated copy, scope reverted to hardcoded 'user' → exit=1, "× writes into the PROJECT and never touches the home directory on a project install"
Verified-By: npm run typecheck → exit=0
Verified-By: npm run build → exit=0
Verified-By: node scripts/run-tests-isolated.mjs → "Test Files 162 passed (162) / Tests 2369 passed (2369)", exit=0
Verified-By: node scripts/audit/verification-audit.mjs → exit=0, "Every hit is triaged; every detector saw a non-empty candidate set"
…er ran

Two CI failures, two different kinds of miss.

CODEQL — 4 high-severity js/file-system-race

`citation-rule.ts` checked `existsSync` and then called `readFileSync`. Between
those two syscalls the path can be replaced, and this path lives inside the
user's own `~/.claude/` tree. Reported high on both the source and its
generated hook mirror.

Not suppressed with a comment. The pair is replaced by a single read that
classifies ENOENT as absent, which has no window at all and is also more
correct: "the file vanished between the check and the read" stops being a
crash and becomes the `absent` state the callers already handle. All three
entry points (write / remove / state) go through it, and `existsSync` is gone
from this module — the injected `fsImpl` types shrank accordingly, so a test
double can no longer supply a seam the code does not use.

LINT — a warning I never saw because I never ran it

`tests/audit/measure-signals.test.ts:71` rethrew without `cause`, and the repo
lints with `--max-warnings 0`. I ran typecheck, the full suite, the audit and
the build before pushing, and not `npm run lint`. The gate did its job; my
verification was incomplete.

Verified-By: npm run lint → exit=0
Verified-By: npm run typecheck → exit=0
Verified-By: npm run build → exit=0
Verified-By: node scripts/run-tests-isolated.mjs → "Test Files 162 passed (162) / Tests 2369 passed (2369)", exit=0
Verified-By: node scripts/audit/verification-audit.mjs → exit=0
The Windows leg failed on an unrelated file — `memory-tool.test.ts`'s
`beforeEach` timed out at 30s creating a temp dir and opening a database. That
file is untouched on this branch, and the same hook takes ~15ms locally (43
tests, 665ms total). So the variable is not that code: it is how much I/O the
suite is putting on that runner by the time it gets there.

This branch adds four test files that between them launch about 27 child
processes. Two of them have to: `citation-rule-self-heal` spawns the real
SessionStart hook because the defect it pins was a literal argument at a call
site, and `measure-signals` spawns the census script because that script IS
the thing under test. Replacing either with a direct call stops testing the
thing.

The five in this file bought nothing. `seedDatabase()` shelled out to
`memesh remember` purely to get a database with the real schema — which
`openDatabase` + `KnowledgeGraph` produce directly, through the same
migrations and the same write path. Same coverage, five fewer process
launches on a runner where each one costs seconds.

Not a fix for the timeout — I cannot show that these five spawns caused it,
and I am not going to claim they did. It is removing cost that was never
buying anything, which is the part I can actually stand behind.

Verified-By: npx vitest run tests/cli/doctor-citation-compliance.test.ts → "Tests 5 passed (5)", exit=0
Verified-By: npm run lint → exit=0
Verified-By: npm run typecheck → exit=0
Verified-By: npm run build → exit=0
Verified-By: node scripts/run-tests-isolated.mjs → "Test Files 162 passed (162) / Tests 2369 passed (2369)", exit=0, Duration 84.49s
Verified-By: node scripts/audit/verification-audit.mjs → exit=0
…-and-signal-census

# Conflicts:
#	scripts/audit/baseline.json
@kevintseng
kevintseng merged commit 5ff1078 into main Aug 23, 2026
13 checks passed
@kevintseng
kevintseng deleted the fix/citation-contract-and-signal-census branch August 23, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants