Skip to content

fix(server): preserve usage in oversized transcript records - #13650

Open
shivamhwp wants to merge 2 commits into
mainfrom
fix/usage-stream-large-records
Open

shivamhwp wants to merge 2 commits into
mainfrom
fix/usage-stream-large-records

Conversation

@shivamhwp

@shivamhwp shivamhwp commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Large tool-output records can make Usage lose an entire transcript's totals. This streams oversized records and keeps their usage, without loading the full tool output into a string. Ordinary records keep native parsing.

Closes #7258.

UI

Same history, different accuracy. These recordings rescan the same 647 MiB fixture: a large Codex tool output, a large Claude usage record, and a duplicate Claude file. The expected total is 10,730 tokens. Main loses all 530 Codex tokens; this PR retains them without double-counting the duplicate.

Before — 530 tokens missing After — all usage counted
Before recording: stopwatch runs, then main reports incomplete usage After recording: stopwatch runs, then PR reports complete usage
10,200 / 10,730 tokens · 1.778 s 10,730 / 10,730 tokens · 2.359 s
Full-size video Full-size video

The previews above are animated recordings at original speed, with a test-only stopwatch from the refresh click to the received result. Initial navigation and the sidebar are omitted for readability. Each refresh forces a complete reread; filesystem caches were not cleared.

This case shows the accuracy fix, not a speedup. Main ends sooner with a failed Codex read. The PR takes about 0.58 s longer to return the complete total. Neither app crashes in these recordings.

Other test results

  • Normal histories: 29 real files (309 MiB) returned identical 9,680 usage records. Summed per-file median read times fell from 680 to 575 ms across five alternating runs—about 106 ms saved, not a page-load measurement.
  • Large-record memory: in a separate 517 MiB reader test, main failed at 1,628 MiB peak RSS; the PR succeeded at 168 MiB. Three fresh processes per version; maximum RSS reported. Ordinary-record RSS rose slightly, from 97 to 108 MiB.
  • Regression coverage: 347 focused tests and CI passed. Browser checks covered date ranges, duplicates, appends, reload and restart; reader tests cover partial/malformed records and shared-parser limits.

Tested 099777b8 against main e5a46d6c on Linux web. Native clients and remote/tunnel execution were not exercised locally; selected metadata remains uncapped. Full test details and limitations.

Implemented and tested with GPT-6-Astra through the Codex harness in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 25, 2026
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −58 B (−0.4%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +1 B (+0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.4 KiB −59 B (−0.9%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.2 KiB −44 B (−0.1%) 66.4 KiB ✅
Codex Live turn messages 10 9 −1 (−10.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +32 B (+0.2%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −4 B (−0.1%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.5 KiB +36 B (+0.5%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.1 KiB +44 B (+0.1%) 66.4 KiB ✅
Claude Live turn messages 9 10 +1 (+11.1%) 21 ✅

Baseline: e5a46d6 · PR result: 099777b · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes production usage and cost metering for oversized transcripts and introduces a non-trivial selective JSON streaming path plus shared parser changes. Its tests are broad, but the runtime and accounting impact warrant human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 36753476-9a08-4650-9b6b-f9b14431e9ab

📥 Commits

Reviewing files that changed from the base of the PR and between 79634e8 and 099777b.

📒 Files selected for processing (3)
  • apps/server/src/usage/UsageService.test.ts
  • apps/server/src/usage/usageTranscriptReader.ts
  • apps/server/src/usage/usageTranscriptStreaming.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The transcript JSON reader now selects tokens directly and supports a configurable depth limit. Usage scanning parses small lines normally and projects selected fields from large lines. It tracks byte offsets and incomplete tails for resumed scans.

Changes

Transcript usage scanning

Layer / File(s) Summary
JSON token selection and validation
apps/server/src/project/AgentSessionJson.ts, apps/server/src/project/AgentSessionJson.test.ts
The reader tracks selected paths and array indexes, preserves repeated keys according to their encountered order, and accepts an optional depth limit. Tests cover chunk sizes, path selection, malformed input, and limits.
Provider usage projection and parsing
apps/server/src/usage/usageTranscriptReader.ts, apps/server/src/usage/usageTranscripts.ts, apps/server/src/usage/usageTranscriptStreaming.test.ts
The usage reader defines provider-specific fields for large-record projection. Provider line parsers delegate parsed values to exported record parsers. Tests check retained usage fields, metadata, and parsing edge cases.
Incremental scanning and resume behavior
apps/server/src/usage/usageTranscriptReader.ts, apps/server/src/usage/usageTranscriptStreaming.test.ts, apps/server/src/usage/UsageService.test.ts
The reader processes lines incrementally, tracks byte offsets separately from decoding, and leaves incomplete tails uncommitted for replay. Tests cover resume cases, Codex state and duplicate handling, large-record usage persistence, and parser edge cases.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 09977

Malformed oversized records do not prevent later usage records from being scanned. No merge-blocking issue was established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 09977

Large transcripts can now contribute usage without loading the entire record as a string. The review found no demonstrated new route to transcript files or material weakening of the existing cache and resume behavior. The remaining uncertainty is who can write the configured transcript directories in deployment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The identified production exposure is usage scanning of configured provider transcript directories, not a new request-facing file path. The effective attackable scope depends on who can write those directories.

Trust Boundaries and Controls

  • inferred — Transcript contents cross a filesystem-to-parser boundary. The new projection reduces materialization of unrelated large tool content, but the selected usage subtree has no explicit depth or allocation budget in this caller; the prior whole-line usage parser likewise lacked the shared reader's depth control.

Resilience and Maintainability Implications

  • observed — A failed transcript read returns without replacing cached usage, while successful resumed reads deduplicate the cached base, new complete records, and replayable tail before updating the cache.

Hardening Proposals

  • proposed — If transcript directories can be written across a trust boundary, consider a usage-specific bound on selected-subtree depth and allocation that preserves the intended large-record support. This is a hardening proposal, not an established PR regression.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue [#7258] requires Usage scans to survive oversized or malformed JSONL records. readTranscriptRecords uses byte-level line scanning and switches records above the threshold to selected-field str…
Out of Scope Changes check ✅ Passed The changes stay within issue [#7258]. The shared JSON reader and extracted record parsers support the required streaming usage projection. The tests cover selector semantics, allocation and depth lim…
Title check ✅ Passed The title clearly and concisely describes the main change: preserving usage data in oversized transcript records.
Description check ✅ Passed The description explains what changed, why it changed, test results, performance impact, limitations, and UI evidence. It does not use the exact template headings and omits the checklist, but it is ot…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usage scan crashes server on oversized Codex JSONL record

1 participant