fix(history): skip oversized JSONL records, isolate source failures - #654
Open
sudomaggie wants to merge 4 commits into
Open
fix(history): skip oversized JSONL records, isolate source failures#654sudomaggie wants to merge 4 commits into
sudomaggie wants to merge 4 commits into
Conversation
The 1 MiB per-record cap added in c94db52 raised a hard error. Real Claude and Codex transcripts routinely exceed it — a base64 image in a tool_result, a long command's custom_tool_call_output — so the cap fired on ordinary data and the error propagated all the way out, emptying the sidebar of every external provider and making the Update button in Runtime > Scanning fail. The failure was permanent, not transient: the raising sync never reached sync_source_cache_from_conn, so nothing was written, the record kept its old signature, and every later scan re-read the same file and failed identically. Three layers, each bounding a different blast radius: - next_line skips an oversized record rather than raising, draining it into the boundary tail so the watermark stays byte-accurate and peak memory stays fixed regardless of record size. The cap rises to 16 MiB; it now bounds the reader's buffer rather than declaring what a valid record is. - Per-record isolation in the five parse loops: one unparsable session no longer costs its source every other session, and the record stays eligible for a later retry on its old cached row. - Per-source isolation in load_imported_history_sessions: one provider's store no longer decides whether the other seventeen are visible. Claude Code is first in the loader list, so its failure dropped all of them. Verified against the real stores behind the report: a full scan of 174 Claude and 331 Codex sessions completes, and re-running it with the cap forced back to 1 MiB exercises the skip path over the actual oversized records with the same 505-session result and byte-identical token totals. Pre-commit hook ran. Total eslint: 0, total circular: 0
Skipping an over-cap record kept the parse alive but still lost the record, and it lost it silently. The record was salvageable: the parsers keep at most 50_000 chars of any single value (MAX_TOOL_OUTPUT_CHARS, MAX_TEXT_CHARS_PER_CHUNK), so the reader was materializing megabytes to hand downstream a few kilobytes. next_line now streams each record through JsonStringTruncator, which drops the interior of an oversized string value and appends a marker. Structure outside strings passes through untouched, so a truncated record keeps its fields, nesting and timestamps and still parses — only the payload shortens. Peak memory now tracks a record's structure rather than its payload, so size alone no longer decides whether a record survives. Correctness rests on where a cut may land. A cut is taken only in ScanState::InString and only on a UTF-8 leading byte, so it can never split a \uXXXX sequence, leave a dangling backslash, or halve a character — each of which would invalidate the whole record rather than one value. The tests sweep every alignment within a repeating escape unit and every byte of a 4-byte character. The per-value budget is bounded by what the record has already emitted. A per-value budget alone does not bound a record: a real Claude tool_result carrying several images is 1.28 MB across values of ~640 KB, every one of them legal. Tightening the allowance as the record fills means string content can never push a record past the cap — only structure can, and that is the one shape truncation cannot rescue, so it remains skipped. Verified against the same real stores: 505 sessions over 174 Claude and 331 Codex, stable across repeated runs. The Codex records that triggered the original report carry single 2.1-2.4 MB values and now truncate and parse; both their sessions index with full names and token counts. Pre-commit hook ran. Total eslint: 0, total circular: 0
The outage could leave a source at zero rows: Clear + rescan wipes cache rows, round usage and watermarks via prune_missing_records_from_conn before the resync that then raised. Recovery from that state was reasoned about but never demonstrated, and it decides the release note — whether users need another Clear + rescan or just a normal update. Wipes a synced source the same way that path does, then runs a plain incremental scan and asserts both sessions return with their token counts intact. A record with no stored signature is always offered as changed, so no clear is required to recover. Pre-commit hook ran. Total eslint: 0, total circular: 0
cline, trae, workbuddy and warp still propagated a per-record parse error out of their sync loop — the same shape this branch fixed for claude_code, codex, kimi, qwen_code and anthropic_jsonl. sync_source_cache_from_conn is never reached, so nothing is written and the record keeps its old signature, and every later scan re-reads the same record and fails the same way. Per-source isolation already caps the damage at one provider rather than all eighteen, so this is no longer catastrophic. It is still permanent for whoever uses that provider: one bad record empties Cline, or Trae, or WorkBuddy, or Warp, and no amount of rescanning brings it back. cursor_cli already had exactly this guard, with a comment making the same argument, so all nine per-record loops now behave alike. No new tests: these four discover from fixed home paths with no injectable root, which is why their existing tests cover only pure helpers and never the sync loop. Adding integration coverage means adding root injection to four loaders, which is a refactor rather than part of this fix. Pre-commit hook ran. Total eslint: 0, total circular: 0
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.
Problem
All external sessions disappeared from the sidebar, and Runtime → Scanning → Update failed with an error badge on Claude Code / codex (externally loaded sessions).
c94db5281("perf(history): resume JSONL from bounded seams", #619) added a 1 MiB per-record cap toWatermarkedTranscriptReader::next_lineas a hardErr, replacing an uncappedread_until. Real transcripts exceed it routinely — these are ordinary tool results, not corruption:~/.claude/projects/…/bb897a82-….jsonlline 830 = 1,280,979 bytes — atool_resultcarrying a base64 image~/.codex/sessions/2026/07/31/rollout-…019fb47e….jsonlline 19 = 2,779,472 bytes — acustom_tool_call_outputThe error then propagated through three
?s with nothing catching it:claude_code/history.rs:1142— aborts the file parseclaude_code/history.rs:614— aborts the whole Claude sync, sosync_source_cache_from_connis never reached and nothing is written. The record keeps its old signature, so every later scan re-reads the same file and fails identically — permanently stuck, not transient.aggregation.rs:497— sits insidefor loader in EXTERNAL_HISTORY_SOURCE_LOADERS. Claude Code is first in that list, so its failure broke the loop before the other 17 providers were reached. That's why all external sessions vanished, not just Claude's.Log evidence — zero occurrences through 2026-07-31, first appearance 2026-08-01, the day after the merge:
Solution
Three layers, each bounding a different blast radius:
next_lineskips an oversized record instead of raising. It drains the record into the boundary tail rather than buffering it, so the watermark stays byte-accurate and peak memory stays fixed regardless of record size. An unterminated oversized tail (live writer mid-append) still leaves the watermark untouched. The cap rises to 16 MiB and is now documented as a bound on the reader's buffer, not a claim about what a valid record is.imported_history::skip_unparsable_record, applied to the five parse loops (claude_code, codex, kimi, qwen_code, anthropic_jsonl). One unparsable session no longer costs its source every other session; the record stays on its last-known cached row and eligible for retry.load_imported_history_sessions. One provider's on-disk store no longer decides whether the other seventeen are visible.Adds
tracingtoorgtrack-corefor the skip diagnostics (already a transitive dep viaapp_paths).Potential risks
tracing::warn!with source, path and byte count — nothing in the UI. Session identity, counts and usage are unaffected (verified: token totals byte-identical). At 16 MiB nothing in real data reaches this path; a >16 MiB record can only be a tool result, since a user/assistant message that large exceeds any model's context window.load_imported_history_sessionsreturnsOkwith a source missing rather thanErr. Each skipped source warns, but if every source failed the caller would see an empty list rather than an error.Validation
cargo test -p orgtrack_core— 492 passed, 0 failedcargo check -p org2 --lib— clean;cargo clippy(org2, orgtrack_core) clean via pre-commitpi,qwen_code) to assert skip-and-continue, and to assert the watermark advances past the skipped bytes so later scans don't re-read themorgtrack scan --db <temp>(throwaway DB, read-only on user data): 174 Claude + 331 Codex sessions indexed, 505 total. Re-run with the cap forced back to 1 MiB to exercise the skip path over the actual oversized records — same 505-session result, byte-identical token totals.