Skip to content

perf(server): bounded snapshots of threads with large messages skip a full encode - #20064

Merged
juliusmarminge merged 9 commits into
mainfrom
t3code/takeover-18072
Oct 20, 2026
Merged

juliusmarminge merged 9 commits into
mainfrom
t3code/takeover-18072

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Preparing a bounded snapshot with oversized message text now skips the final full JSON encode when the retained text alone proves it exceeds the payload budget. Local items count once per encoded array; inherited items count once. When that lower bound fits, the existing exact encoded-size check still decides overflow. Projection contents, paging and the response flag are unchanged.

Merged current main into the original branch and kept its paging and checkpoint-budget changes. This PR remains limited to the final overflow check.

Original work by @bompus, carried forward in this maintainer PR. The earlier takeover already reduced coverage to two focused regressions; both remain in the existing threadHistoryPaging.test.ts.

Verification

  • Prior PR head: 25 paging tests; after integrating main: 27/27 pass, including the same two takeover tests.
  • Replacing production code with main makes the shortcut regression fail at its assertion that the full projection is not serialized. The exact-size fallback is covered at the byte boundary for local and inherited rows.
  • Server typecheck, targeted lint, format and diff checks pass.
  • One fresh parallel adversarial round on the complete integrated diff: Claude Opus 5.5 and GPT-6 Sol; no blocking findings.
  • Nonvisual server optimization: focused server execution proves the encode shortcut and unchanged fallback. No browser screenshot is applicable. A separate running-server or end-to-end performance benchmark was not performed; earlier timing figures are omitted because they were measured on older revisions.
  • macOS, Windows, iOS, Android and Electron desktop were not exercised. Transport contracts and client rendering are unchanged.

Current-head CI passes for all executed validation jobs. Skipped jobs remain skipped; a canceled duplicate native-fingerprint run is superseded by a successful run. No merge is authorized.

Models: GPT-6.1 Sol (integration), Claude Opus 5.5 and GPT-6 Sol (review), through T3 Code delegation and the Codex/Claude harnesses.

bompus and others added 7 commits October 9, 2026 01:10
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…and its exact fallback

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ss over both arrays

The overflow shortcut walked visibleTurnItems and turnItems with duplicated
loops and a duck-typed `"text" in item && typeof item.text === "string"`
guard. The contract union already narrows `text` to a string for the three
item types that carry it, so `"text" in item` is enough, and a shared
predicate over `.some()` on both arrays stops at the same point as before.
Behavior is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 15, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 15, 2026
@github-actions github-actions Bot added the size:S 10-29 changed lines (additions + deletions). label Oct 15, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 2d58626

Macroscope's review found this PR approvable — This is a small, self-contained server performance optimization that avoids redundant JSON serialization for provably oversized snapshots while preserving the existing output and budget flag behavior. Added tests cover the shortcut and exact-size fallback without changing defaults, APIs, or production workflows.

No code changes detected at 262aa2e. Prior analysis still applies.

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

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 458aca49-9201-44bf-9b74-4b6f5b7c1aa1
📥 Commits

Reviewing files that changed from the base of the PR and between 6a7a461 and 262aa2e.

📒 Files selected for processing: 2
  • apps/server/src/orchestration-v2/threadHistoryPaging.test.ts
  • apps/server/src/orchestration-v2/threadHistoryPaging.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

Thread history projection preparation now checks accumulated timeline text lengths against the payload budget. The existing control-state and serialized-projection size checks remain. Tests cover overflow reporting and size boundaries for local and inherited assistant rows.

Changes: Thread history payload budget

Layer / File(s) Summary
Pre-serialization budget check
apps/server/src/orchestration-v2/threadHistoryPaging.ts, apps/server/src/orchestration-v2/threadHistoryPaging.test.ts
The projection checks accumulated text lengths across visible rows and turn items against the policy budget. Tests verify overflow reporting without serializing the projection and check exact size boundaries for local and inherited assistant rows.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 262aa

No merge-blocking issue is identified; complete the pending CI and configured reviews before merging.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the problem, implementation, verification results, limitations, and agent usage. It does not provide the required Scope and approval information, such as a linked issue or exp… Add a Scope and approval section. Link the triaged issue or approval discussion and include the explicit maintainer approval comment. If this is an obvious focused fix that does not need prior approval, explain why it qualifies.
Passed Passed checks: 3 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the server performance change: bounded thread snapshots skip a full encode for large messages.
Full details: Description check

Explanation

The description explains the problem, implementation, verification results, limitations, and agent usage. It does not provide the required Scope and approval information, such as a linked issue or explicit maintainer approval with supporting context.

Resolution

Add a Scope and approval section. Link the triaged issue or approval discussion and include the explicit maintainer approval comment. If this is an obvious focused fix that does not need prior approval, explain why it qualifies.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-actions

github-actions Bot commented Oct 15, 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 5.0 KiB 4.9 KiB −23 B (−0.5%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB −23 B (−1.9%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.8 KiB −41 B (−0.2%) 29.3 KiB ✅
Codex Live turn messages 2 1 −1 (−50.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 9077e81 · PR result: 262aa2e · 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: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

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

@juliusmarminge

Copy link
Copy Markdown
Member Author

Taking over this one as maintainer for batch composer-chat-07. I will preserve this bounded-snapshot text-length shortcut and its exact fallback, merge current main, and verify it independently. The long-thread snapshot work in the other batch remains separate.

Keep main's current history paging and checkpoint budget behavior while retaining the text-length overflow shortcut and exact JSON-size fallback. Preserve both takeover regression tests alongside main's tests.
@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge
juliusmarminge merged commit b437ec1 into main Oct 20, 2026
30 of 31 checks passed
@juliusmarminge
juliusmarminge deleted the t3code/takeover-18072 branch October 20, 2026 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:S 10-29 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.

2 participants