Count unreadable records in three, not one - #6
Merged
Conversation
`unknown_record` counted three unrelated things: a record `type` engram does not recognize, a line that is not valid JSON, and a conversation record with no `uuid`. Only the first is what the counter is documented to signal — a format change in a file engram does not own. The merge made torn lines indistinguishable from format changes. A session that read a transcript while its harness was still appending reported 56 unknown records; every one was a partial line that was complete minutes later, and nothing about the format had moved. Acting on that report meant looking for a change that did not exist. A signal wrong two times in three stops being read, which costs exactly the early warning the counter exists to give. Split into `unknown_record` (unrecognized `type`, worth acting on), `torn_line` (interrupted write, nothing to fix here, often transient), and `missing_uuid` (a real turn dropped for want of a stable id). Both readers report torn lines to the new counter; only the Claude Code reader can produce `missing_uuid`, since the Codex reader keys on record index rather than uuid. The slash command told a reader that any non-zero count meant the format had changed, which is how the misreport reached a user in the first place; it now names all three and what each implies. CLAUDE.md carried the merge as a deliberate decision, so it is amended rather than left contradicting the code. Callers reading `filtered.unknown_record` see a smaller number for the same transcript; the three counters sum to the old value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FCJbPm4FWS99y9utbUe13z
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
unknown_recordcounted three unrelated things: an unrecognized recordtype, a line that is not valid JSON, and a conversation record with nouuid. Only the first is what the counter is documented to signal.Why this matters
The merge made torn lines indistinguishable from format changes. A session that read a transcript while its harness was still appending reported 56 unknown records — every one a partial line that was complete minutes later, with nothing about the format having moved. That report reached a user three times as "the transcript format may have changed."
Same transcript, after this change:
{"unknown_record": 0, "torn_line": 1, "missing_uuid": 0}A signal that is wrong two times in three stops being read, which costs precisely the early warning the counter exists to give. It has earned its keep before — it surfaced three miscategorized Codex tool types — and that is worth protecting.
The split
unknown_recordtypetorn_linemissing_uuiduuidBoth readers report torn lines to the new counter. Only the Claude Code reader can produce
missing_uuid— the Codex reader keys on record index, not uuid.Docs
plugins/engram/commands/save-chat.mdtold the reader that any non-zero count meant the format had changed. That instruction is how the misreport reached a user; it now names all three counters and what each implies.CLAUDE.mddocumented the merge as a deliberate decision ("it correctly flags genuinely torn lines"), so it is amended rather than left contradicting the code.Compatibility
Breaking for envelope consumers.
filtered.unknown_recordreturns a smaller number for the same transcript; the three counters sum to the old value. Noted inCHANGELOG.mdunder Unreleased.Verification
cargo fmt --check,cargo clippy --all-targets -D warnings, 245 tests (181 unit + 64 integration) — all pass. Three tests cover the new behaviour, including one per reader asserting a torn line does not register as a format change.🤖 Generated with Claude Code
https://claude.ai/code/session_01FCJbPm4FWS99y9utbUe13z