Skip to content

perf(client): streamed updates no longer rescan history for Stop state - #29104

Open
Yuruzuu wants to merge 1 commit into
pingdotgg:mainfrom
Yuruzuu:perf/stop-derivation-replacements
Open

Yuruzuu wants to merge 1 commit into
pingdotgg:mainfrom
Yuruzuu:perf/stop-derivation-replacements

Conversation

@Yuruzuu

@Yuruzuu Yuruzuu commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

While a thread is open, every text or tool update in a running thread rescans its turn items to work out Stop state. Those updates almost never affect Stop, so the scan repeats and returns the same value.

Change

runStopAtom reran presentRunStop whenever the turnItems array changed. presentRunStop only reads runs and interrupt items. The atom now skips it when runs are unchanged and the reducer's replacement hint shows one replaced item, with neither the old nor the new item an interrupt type. A skip still advances the stored input. Inserts, skipped or batched updates, snapshots, and run changes carry no usable hint, so they derive in full. The change is in client-runtime, so web and mobile share it. Providers are untouched. I added no interrupt index and no timers, and run selection is unchanged.

Scope and approval

This qualifies for the small, focused fix exception in CONTRIBUTING.md. There is no feature, product default, or contract change.

Verification

vp test run on threadDetail.test.ts and threadExecution.test.ts: 43 passed after rebasing onto upstream/main 61c3794c32. vp run check:changed passed.

does not rescan history for consecutive replacements unrelated to Stop fails without the fix. With 99 history items and two text updates, runId reads went from 198 to 0.

A second test compares the atom with a full derivation across 15 transitions. Removing either the old-type or new-type check makes a test fail.

Benchmark freshness: These timing numbers predate the rebase onto upstream/main 61c3794c32 (including #29105); the original benchmark harness was not available to rerun. The count test was rerun on the rebased code: temporarily replacing threadDetail.ts with upstream/main makes it fail with 198 runId reads instead of 0.

Benchmark: Node, mounted runStopAtom, fed by real reducer output. It times registry.set plus registry.get per update. Runs alternated in separate processes with identical input. Each row is 21 runs per side, on a machine running other work. Each value is the median with p25–p75 in parentheses, in µs per update.

Input Before After
Text update, 10 items 0.60 (0.54–0.66) 0.56 (0.52–0.80)
Text update, 100 items 1.07 (0.99–1.23) 0.68 (0.61–1.20)
Text update, 1,000 items 4.37 (4.07–6.47) 0.76 (0.64–1.30)
Text update, 5,000 items 24.18 (21.83–28.01) 0.64 (0.59–0.72)
Text update, 1,000 items, Stop pending 5.13 (4.31–5.71) 0.62 (0.60–0.78)
New item, 1,000 items (no skip) 7.81 (7.07–11.31) 7.09 (6.49–8.47)
Two updates in one read, 1,000 items (no skip) 4.74 (4.14–5.87) 4.72 (4.20–5.61)

Ten items and both no-skip cases tie. This measures the atom only, not app latency. I did not run this in a real client.

Related

#29105 is merged and its question-history reuse is preserved. #28991 remains open as of this rebase; it changes this atom's input in threadDetail.ts and conflicts; whichever lands second needs a small rebase. It stops streamed assistant text reaching this atom, so this change would then mainly help tool and other replacements.

Adversarial review by GPT-6.1 Terra via Codex in T3 Code.
Made with Claude Opus 5.5 via Claude Code in T3 Code.
Rebased and verified with GPT-6.1 Sol via Codex in T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 93bc8a5

Macroscope's review found this PR approvable — Streamed updates avoid rescanning thread history for Stop state when only a non-interrupt item changes. Relevant changes still trigger full derivation, keeping this a bounded client-side performance optimization.

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

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The run-stop atom now reuses its presentation for qualifying turn-item replacements when the runs are unchanged. Tests cover reuse and compare atom output with presentRunStop across state transitions.

Changes: Run-stop presentation reuse

Layer / File(s) Summary
Compare inputs and validate presentation reuse
packages/client-runtime/src/state/threadDetail.ts, packages/client-runtime/src/state/threadDetail.test.ts
The atom checks whether changed turn items contain a single replacement that is not a run-interrupt request or result. For qualifying inputs, it updates saved references without recalculating the presentation. Tests check that unrelated history items are not read and compare atom output with presentRunStop across transitions.

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

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to f6292

No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: streamed updates no longer trigger a rescan of history to compute Stop state. The perf(client) prefix follows conventional commit style. It matches the changeset, whic…
Description check Passed The description follows the template sections (Problem, Change, Scope and approval, Verification). The Problem section states the expected behavior and cause. The Change section explains the skip rule…
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.
Full details: Title check

Explanation

The title names the main change: streamed updates no longer trigger a rescan of history to compute Stop state. The perf(client) prefix follows conventional commit style. It matches the changeset, which adds a skip in runStopAtom for single-item replacements where runs are unchanged.

Full details: Description check

Explanation

The description follows the template sections (Problem, Change, Scope and approval, Verification). The Problem section states the expected behavior and cause. The Change section explains the skip rule and why inserts, batched updates, snapshots, and run changes still do a full derivation. The Scope section cites the small focused fix exception in CONTRIBUTING.md and says there is no feature or contract change. The Verification section gives focused test commands, results, a failing-without-fix test, and benchmark data with its freshness caveat. The description does not include screenshots, which is acceptable because the change is not a UI change. It also does not link a triaged issue. Under the template, a small obvious bug fix can qualify without a prior issue if the author explains why, and the author did explain why. The description is complete enough to pass.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ast-grep (0.51.1)

ast-grep could not parse rule config: /ast-grep-rules/ast-grep-essentials/rules/go/security/path-traversal-os-open-request-go.yml

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 93bc8a5 and f629240.

Files selected for processing: 2
  • packages/client-runtime/src/state/threadDetail.test.ts
  • packages/client-runtime/src/state/threadDetail.ts

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

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

Made with GPT-6.1 Sol via Codex in T3 Code.
@Yuruzuu
Yuruzuu force-pushed the perf/stop-derivation-replacements branch from 93bc8a5 to f629240 Compare November 3, 2026 03:46

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:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant