Skip to content

perf(server): restarts no longer replay PR discovery for every old thread - #13757

Open
t3dotgg wants to merge 1 commit into
mainfrom
t3code/pr-backfill-recent
Open

t3dotgg wants to merge 1 commit into
mainfrom
t3code/pr-backfill-recent

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Every server restart ran pull request discovery again for every settled branch thread with no saved PR. A "no PR" result is not saved, so each boot repeated the full pass: several git spawns and a gh call per thread, and up to 5 lookups per thread when gh fails. On a 5,000-thread scale bench with gh missing, that was about 3,850 git spawns/min for about 5 minutes after each boot.

Fix

The startup backfill from #10101 now only covers threads settled in the last 7 days (BACKFILL_WINDOW).

The check uses settledAt, not updatedAt. Auto-settle sets settledAt to the last activity, but sets updatedAt to the settle time. With updatedAt, one mass auto-settle would put every old thread back in the window for a week of restarts.

Decision: 7-day window or delete the backfill

The backfill was a one-time migration for threads from before #10101 (merged 2026-09-06, shipped in v0.0.39). Since v0.0.39, the 1-minute pass checks every unsettled branch thread, so each thread is checked until it settles. The backfill only matters for a settled thread with no saved PR.

Who Today 7-day window (this PR) Delete
Users upgrading from 0.0.38 or older All settled threads get a lookup Only threads settled in the last 7 days No settled thread gets a lookup
Users who fix gh later, or open a PR outside T3 Code after the thread settled All settled threads found on next restart Only threads settled in the last 7 days Not found
Cost on every restart All settled threads with no PR Threads settled in the last 7 days with no PR None

In both lost cases, unsettling the thread runs discovery again. Deleting also removes the retry state in the reactor (about 50 lines) and 2 of its tests.

I kept the window because it is the smaller behavior change. Deleting it is an easy follow-up if we prefer that.

Verification

  • vp test run apps/server/src/orchestration/ThreadPullRequestReactor.test.ts: 16 passed, including the unsettled-vs-full-read parity test from #13765.
  • The new test seeds a recent settled thread, an old one, and an auto-settled one (old settledAt, recent updatedAt). Only the recent one gets a sync. Keying the check on updatedAt makes it fail.
  • vp lint and vp fmt on the changed files, and vp run --filter t3 typecheck.
  • I did not re-run the scale bench on this build.

Made by Claude Opus 5.5 (1M context) in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

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

macroscopeapp Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes production startup behavior by suppressing pull-request discovery for older settled threads, rather than merely optimizing an unchanged path. Although the implementation is small and tested, the cutoff can affect whether significant Git/GitHub processing and PR links are created after restarts.

No code changes detected at 80bd4c8. Prior analysis still applies.

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

@github-actions

github-actions Bot commented Sep 26, 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 +15 B (+0.1%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +4 B (+0.1%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.4 KiB 6.5 KiB +11 B (+0.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.2 KiB 56.3 KiB +44 B (+0.1%) 66.4 KiB ✅
Codex Live turn messages 9 10 +1 (+11.1%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +4 B (+0.0%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −9 B (−0.1%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.5 KiB +13 B (+0.2%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB ✅
Claude Live turn messages 9 9 0 (0.0%) 21 ✅

Baseline: 8872666 · PR result: d1273a3 · 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.

@coderabbitai

coderabbitai Bot commented Sep 26, 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: Team

Run ID: c330807d-ce27-4e63-9ffc-46f8a962bcec

📥 Commits

Reviewing files that changed from the base of the PR and between 80bd4c8 and d1273a3.

📒 Files selected for processing (2)
  • apps/server/src/orchestration/ThreadPullRequestReactor.test.ts
  • apps/server/src/orchestration/ThreadPullRequestReactor.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

Walkthrough

Startup backfill now queues settled threads without a branch pull request only when their settlement timestamp is within the previous seven days. If settledAt is absent, the filter uses updatedAt. A test checks the cutoff and timestamp behavior.

Changes

Startup backfill

Layer / File(s) Summary
Filter startup backfill by settlement time
apps/server/src/orchestration/ThreadPullRequestReactor.ts, apps/server/src/orchestration/ThreadPullRequestReactor.test.ts
The reactor applies a seven-day cutoff using settledAt, or updatedAt when settledAt is absent. The test checks which threads are synchronized.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to d1273

The seven-day startup backfill limit has no identified issue requiring a fix before merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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 and concisely describes the main change: preventing repeated pull-request discovery for old threads after restarts.
Description check ✅ Passed The description explains the problem, the seven-day settledAt-based fix, the rationale, trade-offs, and verification results. It does not use the template headings or include the checklist, but it is …
✨ Finishing Touches
📝 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.

@t3dotgg
t3dotgg force-pushed the t3code/pr-backfill-recent branch from d548f23 to 80bd4c8 Compare September 26, 2026 07:23
@t3dotgg t3dotgg closed this Sep 26, 2026
@t3dotgg
t3dotgg force-pushed the t3code/pr-backfill-recent branch from 80bd4c8 to 8872666 Compare September 26, 2026 08:36
…read

Each boot marked every settled branch thread without a saved PR for
discovery. A "no PR" answer is not saved, so every restart repeated
the whole pass: 4-6 git spawns per thread plus a gh call, retried up
to 5 times. With gh missing, the scale bench measured about 3,850 git
spawns/min, 17k spans/min and 24 MB/min of trace for about 5 min after
each boot.

The backfill came from #10101 as a one-time migration. Limit it to
threads settled in the last 7 days (BACKFILL_WINDOW). The check uses
settledAt, not updatedAt: auto-settle stamps updatedAt with the settle
time, so a mass auto-settle would put every old thread back in the
window. A branch or link change on a settled thread still gets its own
single-thread pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size:XS 0-9 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Sep 26, 2026
@t3dotgg t3dotgg reopened this Sep 26, 2026
@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Sep 26, 2026

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: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.

1 participant