Skip to content

perf(server): fork threads load faster when local turns fill the first page - #20104

Open
juliusmarminge wants to merge 7 commits into
mainfrom
t3code/takeover-18071
Open

juliusmarminge wants to merge 7 commits into
mainfrom
t3code/takeover-18071

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

When a fork's own user turns fill the first page, its initial bounded snapshot no longer reads and decodes the parent's retained turn items. The contributor measured filled-fork snapshot preparation going from 5.51 ms to 3.70 ms, with 31% less CPU. Every other thread shape is unchanged.

Problem

After #17387, an initial bounded snapshot of a forked thread still read and decoded the parent's retained turn items, including large tool outputs. It did this even when ten of the fork's own user turns filled the page. Those inherited payloads are discarded during bounding. Only their count matters, because it sets item positions and the history cursor.

Change

  • The window CTE in ProjectionStore.ts moves into one helper, retainedWindowRows(threadId, window). The payload read and a new count read both use it, so they select the same retained rows by construction.
  • The count runs only when all of these hold:
    • the fork's local user turns fill the first page (>= max(userTurnLimit, 10));
    • the read is top-level and has no history anchor.
  • In that case, buildVisibleTurnItems offsets local positions by the parent's retained-row count instead of loading the inherited rows.
  • The count query returns NULL when the parent is itself a run fork. That takes the existing full ancestry read, because the grandparent's rows come through the parent. If the source run is missing, it returns 0, which matches the full read.
  • Partial pages, nested forks, subagent forks, anchored history reads and the compatibility snapshot path all keep the existing read.

Scope and approval

Closes #18915. A focused follow-up to #17387. Replaces #18071 (and its earlier copy #17406). The idea and query selection are the contributor's. This version shares one CTE helper instead of duplicating it, and adds a nested-fork guard and a comparison test.

Verification

Original work by @bompus in #18071, taken over and finished by maintainers.

Claude Opus 5.5 and Claude Fable 5.1 in Claude Code, with review by GPT-6.1 Sol in Codex.

🤖 Generated with Claude Code


Devin Review

bompus and others added 7 commits October 8, 2026 23:22
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the PR's fork cursor test with one that compares the bounded
snapshot of a fork against the full projection. With nine local turns the
page still includes inherited parent rows and reads their bodies; with ten
it skips them, and positions, the fork marker, cursor and hasMoreHistory
still match the full read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t page

After #17387 an initial bounded snapshot of a forked thread still read and
decoded its parent's retained turn items, including large tool outputs,
even when the fork's own user turns filled the returned page. Those
inherited rows are dropped during bounding; only their count sets the
positions and the history cursor.

Share the window selection between the payload read and a count, and when
local turns fill the page count the parent's retained rows in SQL instead
of reading their bodies. The count query returns NULL for a parent that is
itself a run fork, which keeps the full ancestry read for nested forks.

Closes #18915

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A fork of a fork inherits the grandparent's rows through its parent, so
counting only the parent's own rows would misplace the page. Add the
nested case to the filled fork page test; it fails when the count query
stops excluding run-forked parents.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@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 Oct 16, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 16, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This production optimization changes initial fork snapshot preparation by automatically replacing inherited payload reads with a count when local turns fill the first page. Although focused tests verify output and cursor parity, the automatic decision to skip substantial request-path processing warrants human review.

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

@github-actions

github-actions Bot commented Oct 16, 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 5.0 KiB 0 B (0.0%) 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 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.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: bb4e4e7 · PR result: e2ebea4 · 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.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown

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: 9d248e74-f5bf-4cef-825a-1c533a131c9c
📥 Commits

Reviewing files that changed from the base of the PR and between bb4e4e7 and e2ebea4.

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

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Bounded fork snapshots now count retained parent rows instead of decoding parent payloads when local turns fill the page and the source is not itself a run fork. The count offsets local item positions. Tests cover pagination results, cursors, and nested forks.

Changes: Fork snapshot paging

Layer / File(s) Summary
Share retained-window selection
apps/server/src/orchestration-v2/ProjectionStore.ts
A shared query selects retained rows for bounded projection reads.
Count inherited rows and position local items
apps/server/src/orchestration-v2/ProjectionStore.ts, apps/server/src/orchestration-v2/ProjectionStore.test.ts
Eligible fork reads count retained source rows and use the count to position local items. Tests compare first-page results and cursors for partial pages, full pages, and nested forks.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to e2ebe

When a fork's own turns fill the first snapshot page, the fork snapshot now counts the inherited parent rows instead of loading and decoding them. Positions and the history cursor match the full read. All other cases keep the existing path. No outstanding merge risk was found.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly describes the main performance change: faster fork-thread loading when local turns fill the first page.
Description check Passed The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the behavior, implementation, scope limits, linked issue, tests, and unverified benchm…
Linked Issues check Passed Direct issue [#18915] requires skipping inherited payload hydration when a fork's local user turns fill the first page, while preserving inherited row positions and the history cursor. `ProjectionStor…
Out of Scope Changes check Passed The changes are limited to the projection window query, inherited-position handling, and focused coverage in ProjectionStore.test.ts. The test changes directly verify the optimization and its requir…
✨ 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.

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

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

A fork whose own turns fill the first page still reads and decodes its parent's tool outputs

2 participants