fix: one torn-line-safe JSONL home for both lane prototypes - #899
Conversation
Both lane prototypes carried a BYTE-IDENTICAL append_jsonl and a reader with the same body under two names. That is one defect with two addresses, not a class with two instances, so it gets one fix rather than two: a shared jsonl_store module both consume. Fixing them separately would have written the same fix and the same witnesses twice and left the copies free to diverge again, which is how the situation arose. The defect, in both copies: the writer appended with no terminator repair and no fsync, so a writer killed mid-write left a remnant and the NEXT record glued onto it - a record that had itself completed became part of one unparseable line and was lost. The reader parsed with a bare json.loads over read_text().splitlines(), so the remnant raised, and the whole-file decode meant one invalid byte anywhere destroyed every record in the file rather than its own line. WHERE THE COUNT LIVES, which was the open design question: these are module-level functions with no instance to hang health on, and a module-level accumulator would be hidden state and wrong under concurrent readers. So the count is a RETURN VALUE - read_jsonl returns rows AND the lines it could not read. The question dissolves rather than getting answered. And the count reaches a CONSUMER: both CLI callers report it on stderr, so a --json caller's stdout stays machine-readable. A count nobody surfaces is the same silence as no count at all, which is the defect this fix exists to close. The reporter lives in the shared module too. The first version of this change put a helper in one consumer and an inline copy of the same three lines in the other - byte-adjacent duplication of a reporting rule across two files, which is the exact shape being eliminated, committed inside the fix. Caught before the gate. Structure is validated; schema is not. A generic reader has no fields to check, so a line that is well-formed JSON and a well-formed object IS a row even when its values are odd. That boundary is pinned by its own witness, added after a witness failed against a wrong expectation of mine rather than against the code - the hostile-content table had been copied from a surface that does have a schema. 21 witnesses, 7 mutation rows, each mutation killing witnesses whose failure TYPE matches it: removing the decode guard kills seven by real UnicodeDecodeError; removing the parse guard kills six by real JSONDecodeError, RecursionError and the integer-literal ValueError; silencing the count kills fifteen; silencing the reporter kills exactly one, which is what proves the count reaches a consumer. The fsync witness is deliberately weaker than it sounds and says so: it pins that fsync is CALLED, not that durability holds, because a real crash is unwitnessable in-process. It exists because without it, deleting the fsync killed nothing at all - and a guard nothing checks is not a guard. Unbounded line length is a resource limit on the READ layer and is deliberately not defended, named in the module docstring rather than left to be discovered. Ref #897 Co-Authored-By: Claude <noreply@anthropic.com>
|
Review record for this PR, bound to head Two reviewers, two distinct display names, both APPROVE at this exact head. No commit has been pushed since either verdict, so both bind the bytes below. r1 — Stromus — APPROVE. RAN: r2 — Sentinel — APPROVE. RAN: 21 passed; parse-guard mutation red on exactly six witnesses; restore hash matched. Non-blocking residual recorded: this module validates line STRUCTURE (is it a JSON object?) and never record SCHEMA, so a syntactically valid object carrying a hostile value is accepted as a row. That is the intended boundary and is pinned by its own witness. Author's declared residuals, named rather than presented as clean:
Premium boundary: grip is OSS; this is local file mechanics over opaque paths and carries no identity, org, or policy content. |
What
Both lane prototypes carried a byte-identical
append_jsonland a reader with the same body under two names (iter_lane_events,load_jsonl). That is one defect with two addresses, not a class with two instances — so it gets one fix: a sharedgr2/prototypes/jsonl_store.pythat both consume.This merges what was queued as two separate fixes. Fixing them separately would have written the same fix and the same witnesses twice and left the copies free to diverge again, which is how the situation arose in the first place.
The defect, present in both copies:
fsync, so a writer killed mid-write left a remnant and the next record glued onto it — a record that had itself completed became part of one unparseable line and was lost;json.loadsoverread_text().splitlines(), so the remnant raised, and the whole-file decode meant one invalid byte anywhere destroyed every record in the file rather than its own line.Where the count lives — the open design question, dissolved
The sibling fix on
AppendSurfacehung its malformed count on an instance attribute. That does not port: these are module-level functions with no object to hang health on, and a module-level accumulator would be hidden state and wrong under concurrent readers.So the count is a return value.
read_jsonlreturns rows and the lines it could not read. The question does not get answered so much as removed — no instance, no hidden state, no side channel, and a caller that ignoresmalformedstill gets correct rows.And the count reaches a consumer. Both CLI callers report it on stderr, so a
--jsoncaller's stdout stays machine-readable. A count nobody surfaces is the same silence as no count at all, which is precisely the defect this fix exists to close; the CLI is the layer that already looks.The reporter lives in the shared module too. The first version of this change put a helper in one consumer and an inline copy of the same three lines in the other — byte-adjacent duplication of a reporting rule across two files, which is the exact shape being eliminated, committed inside the fix. Caught before the gate rather than by it.
Structure is validated; schema is not
A generic reader has no fields to check, so a line that is well-formed JSON and a well-formed object is a row, even when its values are odd (
1e999→inf,NaN). Nothing coerces, so nothing raises. The siblingAppendSurfacedoes have a record schema and rejects these; the difference is real and is pinned by its own witness rather than assumed.That witness exists because one of mine failed against a wrong expectation rather than against the code — the hostile-content table had been copied from a surface that has a schema. The fixture was wrong; the code was right.
Evidence (RAN)
UnicodeDecodeError; removing the parse guard kills six by realJSONDecodeError,RecursionError, and the integer-literalValueError; removing the terminator repair kills two; silencing the count kills fifteen; silencing the reporter kills exactly one — the discrimination that proves the count actually reaches a consumer rather than sitting in a field nobody reads. Each row asserted its anchor applied (occurrence count == 1 and the file hash changed) and restored from a hash-verified copy; the failure-type extractor reads onlyE-prefixed traceback lines and is proven against a green control that must yield zero.fsyncwitness is deliberately weaker than it sounds, and says so in its own docstring. It pins thatfsyncis called, not that durability holds — a real crash is unwitnessable in-process. It exists because without it, deleting thefsynckilled nothing at all, and a guard nothing checks is not a guard. Durability against power loss remains asserted, not demonstrated.origin/devcontent and the new module moved aside. Delta is exactly the 21 new witnesses; the one failure is the same pre-existingtest_gr2_overlay_alias_resolvesin both runs.E501,E731) and out of scope. The new module is clean.Not in scope
AppendSurfacekeeps its own copy of these primitives for now. Folding it into this module would churn just-gated code for no behavioural change; it is named as a deliberate residual rather than presented as clean.json.loads(path.read_text())document reads inlane_workspace_prototype.py— those are documents rather than append logs, so a torn write there fails loudly instead of silently. Different defect, not widened into this one.Premium boundary: grip is OSS; this is local file mechanics over opaque identifiers and carries no identity, org, or policy content.
Ref #897