production readiness: attribution, the hook that never ran, and a release path - #41
Merged
Merged
Conversation
Three defects, one root cause between them: a fact can reach the graph with no agent_id, and nothing notices. A Claude Code MCP server is a long-lived stdio process that imports this package once, when the client spawns it. An editable install does not change that. One server (pid 53784, started 2026-08-22 17:47, before agent_id shipped on 2026-08-23) was still writing on 2026-09-03; 30 facts in the author's store have no agent_id because of it. Apache AGE drops a null-valued property at CREATE, so the key is absent rather than null, which is exactly why migration 0007's `WHERE e.agent_id = 'unknown'` never matched them. - `_author_of` returned None both for a fact that isn't there and for a fact with no author, so `record_recall_save` answered a real fact with "no fact <id> in this scope - pass the fact_id from a query_memory result, not a remembered one". The id did come from query_memory, so that advice cannot be followed: re-query, same id, same error. It now separates the two and names the migration that fixes it. - `write_episode` refuses a falsy agent_id, at the entry point and again at the CREATE that loses it. A fact nobody can attribute cannot evidence a cross-tool recall save, so failing the write beats keeping it. - Migration 0011 backfills the absent key to 'unknown', not 'claude-code': 0007 could name a client because it reasoned about one store, and by 2026-08-29 `adopt` was handing out per-client ids. 'unknown' is also the only value record_recall_save already refuses to count. Separately, the PreCompact hook returned hookSpecificOutput.additionalContext, copied from the SessionStart hook where that shape is correct. PreCompact does not define that field, so every compaction since the hook landed answered "Hook JSON output validation failed - (root): Invalid input" while the hook exited 0. The reminder never reached a model. It now prints plain text, which Claude Code passes as newCustomInstructions - the instructions steering the summary itself. No test had ever executed a hook script, which is how that survived. One now runs each of them and rejects a hookSpecificOutput naming an event whose schema has no such field. Both new suites were confirmed to fail against the code they describe before being kept.
…e that cannot be taken `unattributed_facts` matched only `agent_id = 'unknown'`, the literal string migration 0003 backfilled. A fact written with no agent_id at all is a different shape - Apache AGE drops a null-valued property at CREATE, so the key is absent, and an equality test never matches it. Thirty such facts sat in the author's store while `echo-memory health` printed "every fact has a recorded author" and criterion 6's `unattributed == 0` passed. A blind spot in the thing built to do the measuring reads as a clean bill of health, which is worse than no measurement at all. `IS NULL` matches an absent key as well as a null one, so a single clause covers both. The same absent value reached the user as a tool named "None", via str(None), in both `status` and `health`. It now folds into the 'unknown' placeholder, so one name means one thing. Three messages then told the user to run `alembic upgrade head`: - A pip install ships the migrations but not alembic.ini, which lives at the repo root and is not package data. `alembic upgrade head` finds no config, which is the whole reason `echo-memory init-db` exists (cli/initdb.py). Most users were being pointed at a command that cannot start. - For a fact already at 'unknown' the advice is also empty. Migration 0011 is what produces that state, so re-running migrations changes nothing, and who wrote those facts is only knowable from a session that has exited. `health` now names what is actually actionable - a long-running MCP client keeps the code it imported at spawn, so it goes on writing the old shape until restarted - and the two record_recall_save errors say to cite a different fact. Migration 0011 applied to the author's store: 30 facts backfilled, health score 65 -> 30 as the previously hidden count came into view.
Draining the queue on 2026-09-07 found 4 files listed and 3 of them already fully recorded, one under a session id that did not match the file's own originSessionId. Following the instruction as written would have written 9 duplicate facts, and duplicate nodes are one of the three things criterion 6 counts against the store. The queue tracks a content digest, so a file is listed when it CHANGES. Nothing about that implies its facts are absent: a past session can write the episode and never run `pending --done`, which is exactly what had happened to both dugout files and the eigen one. Every surface that surfaced the queue told the agent to write, and none told it to look first. All four now say the same thing: query memory for the file's subject, mark it done if the content is already there, write only what is genuinely absent. Covers the MCP query_memory response, `echo-memory pending`, the Stop gate reason, and the skill. Checking cannot be automated here for the reason the queue exists at all - the server never calls a model, so it cannot compare prose to facts. Session id matching was considered and rejected: the eigen file was ingested under a different session, so it produces false negatives, and a false "already done" loses a memory permanently.
Two test fixtures carried chat-module-api.dugoutlive.com, a real registered domain belonging to a client, in an Apache-2.0 repository. The project codename appears throughout the tests as a realistic fixture and is harmless on its own; a resolvable production hostname is not the same thing. Replaced with a .internal suffix, which keeps the fixture's shape - a hostname whose environment is not obvious from its name is the whole point of the fact being recalled - without publishing anything that resolves.
write_episode holds back any fact touching an ambiguous mention and returns ambiguous_entities so the caller can say which candidate it meant. It was still creating every new entity first, including entities that only the held-back facts referenced. Those have nothing to connect to, and nothing obliges the caller to make the follow-up call, so they stay in the graph unreachable. Writing five dugout facts on 2026-09-07 hit it: the response said edges_created: [], which reads as "nothing was written", and left "ECS secret resolution at task startup" behind as an orphan. Orphans are one of the things `echo-memory health` counts against the store. A new entity is now created only when a fact being written this call uses it, or when no fact mentions it at all, since the caller asked for that one directly. Deferring costs nothing: the follow-up call carries the same entities, so anything genuinely needed is created then, next to the fact that gives it an edge. The first version of the test passed against the unfixed code. unit_vector_at_angle only moves within e1 and e2, so the "far apart" bystander vector was still 0.83 similar to the seeds, which made it ambiguous too and quietly voided the test. It uses a third dimension now, and was confirmed to fail without the fix.
Answering "is the pip package updated when we push to git": no, and the gap had already opened wide. echo-mem 0.1.0 went to PyPI by hand on 2026-08-25 and was never republished. By 2026-09-07 the published package was 30 commits and five migrations behind main, 0007 through 0011, so anyone doing `pip install echo-mem` got a build that could not reach the current schema and still carried the PreCompact hook that never ran. pyproject said 0.1.0 the whole time, so one version string named two materially different codebases and nothing in the artifact revealed which. - version bumped to 0.2.0 - .github/workflows/release.yml publishes on a v-prefixed tag, refuses a tag that disagrees with pyproject, and checks the wheel actually carries every migration script in src/echo_memory/migrations/versions - docs/DEVELOPMENT.md gains a Releasing section The migration check earns its place: migrations are package data and alembic.ini deliberately is not shipped, so init-db drives Alembic through its Python API against the scripts inside the installed package. A wheel missing them installs cleanly and then cannot create its own database, which is the exact failure init-db exists to prevent. Verified by installing the built wheel into a clean venv outside the repo: init-db reached 0011 and the MCP server started. Auth is PyPI Trusted Publishing, so no token is stored. It needs a one-time publisher setup on PyPI before the first tagged release will succeed; nothing here publishes on its own. Also corrected DEVELOPMENT.md's record_recall_save documentation, which still showed `record_recall_save(scope, note, written_by="cursor")` and called both arguments required. PR #39 removed them; the real signature is (scope, fact_id, note), and following the docs would have failed outright.
ayushcodes10
changed the title
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
An audit of what was outstanding turned up defects that share one shape:
something was silently not working, and the thing that should have reported it
said everything was fine.
The root cause behind most of them
A Claude Code MCP server is a long-lived stdio process. It imports the package
once, when the client spawns it, and keeps that code for its whole life — an
editable install does not change that. One server on this machine (pid 53784,
started 2026-08-22 17:47, before
agent_idshipped on 2026-08-23) was stillwriting facts on 2026-09-03. Thirty facts in the author's store have no author
because of it.
Apache AGE drops a property whose value is null at CREATE, so the key is
absent rather than null. That is precisely why migration 0007's
WHERE e.agent_id = 'unknown'never matched them, and why nothing downstreamnoticed.
This reproduced live while writing the PR: a
write_episodecall made throughthis session's own server came back with
agent_idunset, because that serveralso predates the fix.
What was broken
record_recall_savesent agents into an unfollowable loop._author_ofreturned
Noneboth for a fact that is not there and for a fact with noauthor, so a real fact was answered with "no fact
<id>in this scope — passthe fact_id from a query_memory result, not a remembered one". The id did
come from query_memory. Re-query, same id, same error, forever.
The measurement had the same blind spot as the migration.
unattributed_factscounted only the literal'unknown', sohealthprinted"every fact has a recorded author" over a store where thirty had none, and
criterion 6's
unattributed == 0passed. A blind spot in the tool built to dothe measuring reads as a clean bill of health. The same absent value reached
the user as a tool literally named
"None", viastr(None).The PreCompact hook never ran, once. It returned
hookSpecificOutput.additionalContext, copied from the SessionStart hook wherethat shape is correct. PreCompact does not define that field, so every
compaction since the hook landed answered
Hook JSON output validation failed — (root): Invalid inputwhile the hook exited 0. Plain stdout is the rightmechanism and a better one: Claude Code passes it as
newCustomInstructions,which steers the summary itself.
Three messages recommended a command most users cannot run. A pip install
ships the migrations but not
alembic.ini, which is whyecho-memory init-dbexists (
cli/initdb.py).The capture queue invited duplicate writes. Draining it found 4 files
listed and 3 already fully recorded; following the instruction as written would
have written 9 duplicate facts. The queue tracks a digest, so it lists files
that changed — which does not mean their facts are missing.
An ambiguity deferral left orphan nodes.
write_episodeholds back factstouching an ambiguous mention but was still creating every new entity first,
including entities only those held-back facts referenced. The response said
edges_created: []while having quietly made a node with nothing to connect to.Nothing published.
pip install echo-memserved 0.1.0 from 2026-08-25 — 30commits and five migrations behind main, so a pip user could not reach the
current schema.
pyproject.tomlstill said0.1.0, so one version string namedtwo different codebases.
A real client domain was in a public repo. Two test fixtures carried
chat-module-api.dugoutlive.com.Changes
_author_ofdistinguishes absent from unattributed; both errors say what to dowrite_episoderefuses a falsyagent_id, at the entry point and at the CREATE0011backfills the absent key to'unknown', not'claude-code'unattributed_factsmatchesIS NULLas well as'unknown'health/statusfold a missing author into the'unknown'placeholder0.2.0plusrelease.yml, which publishes on a tag, refuses a tagdisagreeing with pyproject, and checks the wheel carries every migration
DEVELOPMENT.md'srecord_recall_savedocs corrected — they showed asignature PR stop letting one tool claim to be another #39 removed, so following them failed outright
Testing
444 passing, ruff clean. Three new suites, each confirmed to fail against the
code it describes before being kept — including one that first passed against
unfixed code because
unit_vector_at_angleonly moves within two dimensions, sothe "far apart" fixture was still 0.83 similar and silently ambiguous.
tests/unit/test_hook_output_contract.pyruns every hook script and rejects ahookSpecificOutputnaming an event whose schema has no such field. No test hadever executed a hook script, which is how that survived.
Release path verified by installing the built wheel into a clean venv outside
the repo:
init-dbreached 0011 and the MCP server started.Migration 0011 applied to the author's store: 30 facts backfilled, 0 remaining.
Health score moved 65 → 30 as the hidden count came into view — it went down
because it was wrong before, not because anything got worse.
Not done, deliberately
on PyPI, and releasing is yours to trigger with a tag.
ruff formatwould reformat 61 files. CI only runsruff check, and thatdiff would bury everything above.
'unknown'forever. Who wrote them is onlyknowable from sessions that have exited.