Skip to content

fix(client-runtime): cache running threads when their view closes - #25213

Open
maria-rcks wants to merge 1 commit into
mainfrom
maria/takeover-pr-24210
Open

maria-rcks wants to merge 1 commit into
mainfrom
maria/takeover-pr-24210

Conversation

@maria-rcks

@maria-rcks maria-rcks commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Closing a running thread skips its disk-cache save, so reopening it after a client relaunch can show "Loading messages..." until the server responds.

Change

Save the latest committed snapshot once when the view closes, including active runs. Keep the streaming-write guard, expanded-history guard, queued cache repairs, and persistence deduplication unchanged. The shared client runtime serves web, desktop, and mobile.

Verification

  • The regression test fails on main (zero saved snapshots), then passes with the fix. All 39 existing-file thread-sync tests pass.
  • Blacksmith vp run check:changed passes format, lint, knip, affected-package typechecks, and tests. Lint reports two pre-existing polling warnings outside the added test.
  • Real web client, system Chrome, Codex GPT-6-Astra: open a running turn, navigate away, close the browser, relaunch, reopen. Separate threads use the same prompt and sleep 120 command. Snapshot HTTP responses are held only in the browser for both reopens. Main shows a blank timeline until release; the fix displays the saved messages and running command without requesting a snapshot. Clips are functional evidence, not latency measurements.
  • CodeRabbit reported its review limit and posted no findings; its check is green.
  • Unverified: native iOS/Android and Electron runtime, remote/relay/tunnel, and very large cache-write cost. No provider adapter or wire contract changes.

Before, main 3ca62a564457:

Before: reopening the running thread waits for server messages

After, e86f36682a17:

After: the matching prompt and running command appear from cache

Reopen recording, main followed by the fix. Conversation crops match; startup splash frames are trimmed.

Before and after: reopening a running thread after browser relaunch

Takeover

Replaces #24210, with thanks to @shivamhwp for the original implementation.

Built with GPT-6.1-Sol in the Codex harness on T3 Code.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The thread finalizer now saves the current projection when history is not expanded, including projections with preparing, starting, or running runs. A test verifies the saved snapshot after the view scope closes.

Changes: Running thread persistence

Layer / File(s) Summary
Persist running thread snapshots
packages/client-runtime/src/state/threads.ts, packages/client-runtime/src/state/threads-sync.test.ts
The finalizer saves the current projection when history is not expanded. The test verifies that a running thread is not saved during a 10-second wait and that scope closure saves one snapshot with the latest sequence, updated title, and running status.

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

Change: Bug fix

Possibly related issues

Possibly related PRs

  • pingdotgg/t3code#24586: This PR also concerns persistence guards for running thread snapshots when a view closes.
  • pingdotgg/t3code#24210: This PR also saves the latest running thread snapshot when its view closes.
  • pingdotgg/t3code#28404: This PR concerns resuming server replay from cached sequence cursors, which relates to reopening a thread with saved sequence progress.

Suggested labels: bugfix, takeover

Suggested reviewers: shivamhwp

Merge Risk: Minimal · up to 759f6

Closing a running thread view should retain its latest snapshot. No concrete issue requiring a fix before merge was established.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly describes the main change: caching running threads when their views close.
Description check Passed The description is mostly complete. It explains the problem, change, and verification. It omits the required Scope and approval section. The visual evidence is not hosted on GitHub as the template req…
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: Description check

Explanation

The description is mostly complete. It explains the problem, change, and verification. It omits the required Scope and approval section. The visual evidence is not hosted on GitHub as the template requires. Add the approval link or explain why the fix qualifies without approval, and upload the visual evidence to GitHub.

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

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between cd890ba and 759f67a.

Files selected for processing: 2

  • packages/client-runtime/src/state/threads-sync.test.ts
  • packages/client-runtime/src/state/threads.ts

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 26, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 759f67a

Macroscope's review found this PR approvable — Running threads now save their latest committed snapshot when their view closes, allowing cached messages to appear on reopening. Existing streaming-write, expanded-history, and persistence safeguards remain unchanged.

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

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 26, 2026
@torztomasz

Copy link
Copy Markdown

🤖 Generated by an LLM on behalf of Tomasz Tórz.

Native iOS data for the "not verified" note. We landed on the identical change independently.

Scenario: a thread is running when the reader opens it and leaves; the app is relaunched; the reader reopens it. iOS simulator, T3 server on a copy of a real 6 GB database, 5G profile (120 ms RTT, 10/5 Mbps) through a shaping proxy, server CPU limited to 10 % to match a loaded host. The run was frozen mid-turn (dev replay agent paused), so the thread was confirmed running, Stop button visible, when left.

Touch-down → first committed feed after the relaunch:

  • main: 1.45 s median (1.32–1.58, n=4), loaded from the host
  • this change: 0.69 s median (0.67–0.69, n=4), from the device cache

Happy to share the harness (proxy, CPU throttle, timing marks) if useful.

Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
@maria-rcks
maria-rcks force-pushed the maria/takeover-pr-24210 branch from e86f366 to 759f67a Compare November 6, 2026 04:37
@macroscopeapp
macroscopeapp Bot dismissed their stale review November 6, 2026 04:37

Dismissing prior approval to re-evaluate 759f67a

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:XS 0-9 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