Skip to content

fix(server): background PR sync reads summaries in batches - #13198

Merged
juliusmarminge merged 2 commits into
audit-github-rate-limit-usagefrom
batch-pr-sync-summaries
Sep 23, 2026
Merged

juliusmarminge merged 2 commits into
audit-github-rate-limit-usagefrom
batch-pr-sync-summaries

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Stacked on #13189.

Problem

Every minute, PullRequestSyncReactor runs one gh pr view for each open linked pull request, and each run is a GraphQL call against the shared 5000/h budget. The traces put this loop at most of the fleet's spend: about 1900 calls/h on cups and about 1345/h on nucbox-1. While this PR was being opened, the budget ran out entirely ("API rate limit already exceeded").

Fix

  • GitHubPullRequestCli.getPullRequestSummary now goes through an Effect RequestResolver.
    • Concurrent reads on the same host and credential (pinned credential fingerprint and rate-limit scope) are combined into one aliased GraphQL query of up to 25 pull requests.
    • The query reads the fields the thread snapshot needs, with checks as the head commit's rollup enum.
    • A 10 ms batch window lets the sweep's reads gather after each one's cache check.
  • Fallbacks:
    • A pull request the batch answered nothing for, or a selector GraphQL cannot address, is read on its own with gh pr view, as before.
    • If the whole batch fails, each pull request is read on its own, except when the failure is a rate-limit pause.
  • The reactor sweep's concurrency goes from 8 to 25, so one sweep's reads on a host land in the same batch.
  • Other callers of summary (thread overview and so on) go through the same resolver. A single read costs 10 ms of latency and is otherwise unchanged.

Estimated savings

These come from the same trace-derived counts as #13189. They assume the linked PRs are spread over a few hosts and credentials, so a sweep of N PRs costs about ⌈N/25⌉ calls.

Machine Sync gh pr view calls/h now After batching
cups ~1900 ~80–100
nucbox-1 ~1345 ~60–80
mbp / macmini small small

On top of #13189, the fleet should drop from about 3200 calls/h to roughly 400–500 calls/h. Each batched query should still cost about 1 GraphQL point, since its nested connections are small.

Verification

  • vp test run on GitHubPullRequestCli.test.ts, gitHubPullRequestJson.test.ts, PullRequestSyncReactor.test.ts, GitHubPullRequestProvider.test.ts and PullRequestService.test.ts: all pass.
  • New tests cover:
    • two concurrent reads sharing one aliased request, filed back by position;
    • a PR the batch skipped falling back to gh pr view;
    • the decoder skipping null or undecodable aliases, and reading merged state, bot authors and the checks rollup.
  • The server typecheck has no errors.

Done by Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code


Devin Review

Summary by CodeRabbit

  • Performance
    • Pull-request summaries are retrieved more efficiently, especially when several summaries come from the same host. Background sync can also process more groups at once, helping updates complete sooner.
  • Reliability
    • If a batched summary is unavailable, the system falls back to retrieving that pull request individually, helping preserve summary availability.

Concurrent summary reads on one host and credential now share aliased
GraphQL requests of 25 through a RequestResolver, falling back to
`gh pr view` for anything the batch could not answer. The sync sweep runs
25 groups at once so a sweep's reads land in the same batch.

Co-Authored-By: Claude Opus 5.5 <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 Sep 23, 2026
@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 13.5 KiB — 15.1 KiB ✅
Codex Thread snapshot wire — 7.1 KiB — 7.3 KiB ✅
Codex Live turn WebSocket wire — 6.4 KiB — 7.8 KiB ✅
Codex Live turn WebSocket decoded — 56.2 KiB — 66.4 KiB ✅
Codex Live turn messages — 9 — 21 ✅
Claude Total thread wire — 13.5 KiB — 15.1 KiB ✅
Claude Thread snapshot wire — 7.1 KiB — 7.3 KiB ✅
Claude Live turn WebSocket wire — 6.4 KiB — 7.8 KiB ✅
Claude Live turn WebSocket decoded — 57.0 KiB — 66.4 KiB ✅
Claude Live turn messages — 9 — 21 ✅

Baseline: unavailable · PR result: ec1440f · 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.6 KiB

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

"number title url state isDraft mergeable reviewDecision additions deletions changedFiles " +
"updatedAt mergedAt closedAt headRefName baseRefName " +
"author { __typename login avatarUrl ... on User { name } } " +
"latestReviews(first: 20) { nodes { state author { login } } } " +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium pullRequest/gitHubPullRequestJson.ts:1859

latestReviews(first: 20) drops reviews beyond the first 20, so when reviewDecision is null, toReviewDecisionWithReviews computes the status from an incomplete set and can persist the wrong approving or changes-requested status for pull requests with more than 20 reviewers. Fetch the complete review connection (including pagination) before using it as the fallback.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/pullRequest/gitHubPullRequestJson.ts around line 1859:

`latestReviews(first: 20)` drops reviews beyond the first 20, so when `reviewDecision` is null, `toReviewDecisionWithReviews` computes the status from an incomplete set and can persist the wrong approving or changes-requested status for pull requests with more than 20 reviewers. Fetch the complete review connection (including pagination) before using it as the fallback.

@juliusmarminge
juliusmarminge added this pull request to stack #13199 September 23, 2026 03:13
@macroscopeapp

macroscopeapp Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR materially changes production GitHub-read scheduling by introducing grouped GraphQL batching, fallback reads, and higher sync concurrency rather than making a small isolated fix. It also adds a line-level static-analysis suppression, while an unresolved Medium finding flags incomplete review pagination that can affect persisted review status.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juliusmarminge
juliusmarminge merged commit 18de6bb into main Sep 23, 2026
23 of 24 checks passed
@juliusmarminge
juliusmarminge deleted the batch-pr-sync-summaries branch September 23, 2026 03:50
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 23, 2026
Merges `pingdotgg/t3code` at `aca3c87cd` into the fork — 62 commits from
base
`5a61f50cc`. Landed 339 files (8865+/6131-) against 335 in the upstream
range;
fork delta unchanged at 786 files. The gap of 4 is the three `docs/fork`
files
below plus `ThreadStatusIndicators.test.tsx`, which auto-merged clean
while
still passing the `variant` prop upstream deleted.

`verify.mjs`: all 10 checks pass, tests included.

## Conflicts

Six, each resolved with the verdict `preflight.mjs` printed.

| Path | Verdict | Resolution |
| --- | --- | --- |
| `DiffPanel.tsx` | converged — diff-panel-gates | Upstream rewrote the
scope dropdown as `DropdownMenuRadioGroup` / `DropdownMenuRadioItem`
with the per-turn list in a submenu. Took it whole; re-applied the two
`FEATURES.turnDiffs` gates at their new anchors. |
| `LegacySidebar.tsx` | converged — mobile-touch-upstream-files |
Upstream deleted the add-project button's icon-color className. Took
upstream's button inside the fork's `FEATURES.projectManagement`
wrapper. |
| `ThreadStatusIndicators.tsx` | converged — thread-status-indicators |
Upstream swapped `variant` for a `render` prop and extracted
`PullRequestBadge`. The fork's several-links popover is now a sibling
`PullRequestLinksBadge`; both render a shared `PullRequestBadgeFace`,
extracted so `duplicate-adds.mjs` does not read the shared icon-and-text
span as a merge artifact. |
| `settings/ProjectActionsList.tsx` | unlisted → decide-then-add-entry |
pingdotgg#13029 restyled the Edit button to `variant="ghost-muted"`. Took
upstream's button, kept the `editable` wrapper, added a
`project-actions-list` inventory entry. |
| `settings/ProjectDefaultsSettings.tsx` | converged —
project-defaults-settings | Took upstream whole, re-applied the five
`workspaceOwnsProjectDefaults` gates. Upstream now renders the model and
workspace rows from both a `category === "project"` and a `category ===
"general"` branch, so each gate exists twice. |
| `settings/ProjectSettingsPanel.tsx` | converged —
project-settings-panel | Upstream's new info Alert is unconditional and
first; its new `<ProjectDefaultsSettings category="project" />` section
is gated on `workspaceSettings` as a sibling rather than a fragment,
because folding it in would have re-indented upstream's JSX. |

`pnpm-lock.yaml` auto-merged and was re-derived with `install.mjs`,
which moved
`type-fest` 5.7.0 → 5.10.0 in two msw snapshot blocks and nothing else.
The
fork's `moatless-api` and `mermaid` edges survive.

Sweep: one hit — `apps/server/src/provider/ProviderAuthFlow.ts` and its
test,
new from pingdotgg#12983 on the keyword `auth`. A false positive for the
auth-session
concern (a provider CLI's own sign-in, not the user session), taken
as-is, and
recorded under _Provider setup_ in the gaps because it adds
`provider.auth.respond`. Tripwires steady; no new upstream workflows; no
stale
inventory entries.

Unsupported methods: ADD and DROP both empty, KEEP unchanged at two. No
`rpc.ts` union edits — `provider.auth.respond` took
`ProviderSetupRpcError`,
which already carries `UnsupportedMethodError`.

## Feature classification

### Usable as-is

- **The web component-library pass** — roughly 24 commits
(pingdotgg#12984–pingdotgg#13043) over
`apps/web/src/components/ui/*`: button variants and sizes, dropdown
radio
groups and submenus, popover and tooltip `render` props. Nothing here
touches
the wire; it is the source of four of the six conflicts and all of them
were
  restyles.
- **Settings scope sentence and scope pickers in breadcrumbs** —
`242816af8`
  and `db9a0671b`, including the new `SettingsScopeSentence.tsx`. Reads
  settings the fork already serves.
- **Small web fixes** — `219c1d265`, `6975efd3d`, `68607c5a9`,
`b954af60c`,
  `7c2702d68` + `da6a85b13`, `aff9318bf`, `438bf466f`.
- **Every mobile-only commit**, and the non-targets: `e4422eec7`
(desktop),
  `d7819c188` (device-hub bump, moot while `FEATURES.deviceHub` is off),
  `ca864a25b` / `f25a8e4b7` / `17e34773b` (model manifest).

### Unsupported in Moatless / needs implementation

- **`7e65b226e` feat(auth): share provider sign-in flows and credential
  bindings (pingdotgg#12983).** Adds `provider.auth.respond`
(`WS_METHODS.providerAuthRespond`) and `ProviderAuthRespondInput`,
reshapes
  `provider.auth.start`'s payload from `ProviderSetupInput` to
  `ProviderAuthStartInput`, and adds `methods`, `interaction` and
`credentialOwner` to `ProviderAuthState`. Server-only implementation in
  `apps/server/src/provider/ProviderAuthFlow.ts` and
  `ProviderCredentialStore.ts`.

It makes a provider sign-in interactive — the server asks, the client
answers
through the new method — which is the same shape as the nine
`provider.auth.*`
/ `provider.install.*` methods the backend already refuses. It falls
under the
existing _Provider setup_ gap, now listing ten methods; it needed no
union
edit because it took the same error type as its siblings. Closes if
Moatless
  ever manages provider credentials on the client's behalf.

### Backend behavior to consider reproducing in Moatless

All six are now bullets under _Runtime fixes upstream made to its own
server_
in `docs/fork/gaps.md`, with the closing condition on each. Four are
GitHub
quota work.

- **`96c4bfa0a` provider compatibility advisory** (pingdotgg#13130,
`apps/server/src/provider/providerCompatibility.ts`) — a per-driver
policy on
the model manifest yields a `supported` / `unsupported` / `broken`
advisory on
the published `ServerProvider`, rendered above the composer. Moatless
installs
  the provider CLIs, so it is the side that knows the version.
- **`f193a6863` bypass owned caches on an explicit provider refresh**
(pingdotgg#13109) —
splits `server.refreshProviders` by its existing `refreshModels` flag: a
user
refresh force-refreshes the model manifest and version cache, background
polls
  keep their timers. The flag is already on the contract.
- **`eafb4a934` GitHub PR lookups stop probing owner-qualified heads**
(pingdotgg#13200)
— `gh pr list --head` answers an `owner:branch` selector with nothing
while
still spending a GraphQL call; upstream drops those selectors and widens
the
  remaining probe to 100, which GitHub prices like `first:1`.
- **`18de6bb32` batched PR summary reads** (pingdotgg#13198) — summaries batch
behind a
10ms request window and a per-batch GraphQL query, with the sync
reactor's
  concurrency raised 8 → 25 so a sweep's reads land in the same window.
- **`5975ec78b` `wouldSettle` before an uncached PR re-query** (pingdotgg#13189)
— a
settlement sweep pays for the reused-branch-race lookup only when some
thread
  in the group would actually settle.
- **`f22331240` three-dot PR diffs** (pingdotgg#13170) — `readRangeContext` moved
its
diff stat and patch to `base...HEAD` while leaving the commit log
two-dot, so
a PR description written after the base advanced describes the branch's
own
  changes. One character per command.

## Docs

- `docs/fork/inventory.json` — new `project-actions-list` entry;
  `provider-settings-gates` gained `ProviderModelsSection.tsx` and
  `ProviderSettingsPanel.environment.test.tsx`.
- `docs/fork/gaps.md` — _Provider setup_ names `provider.auth.respond`
as its
tenth method; six new bullets under _Runtime fixes upstream made to its
own
  server_.
- `docs/fork/upstream-merge-log.md` — dated entry.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/5258e642-4bee-4f38-878b-747496b2d2ea
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 23, 2026
## What's Changed
* chore(mobile): drop dead nitro-markdown tgz override and @expo/metro-runtime by @juliusmarminge in pingdotgg/t3code#13148
* feat(web): show settings scope as a sentence at the top of the page by @juliusmarminge in pingdotgg/t3code#13139
* refactor(web): move settings scope pickers into breadcrumbs by @Yash-Singh1 in pingdotgg/t3code#13165
* feat(auth): share provider sign-in flows and credential bindings by @juliusmarminge in pingdotgg/t3code#12983
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13161
* chore(mobile): name the two project favicon caches by their job by @juliusmarminge in pingdotgg/t3code#13160
* revert(mobile): git sheets back to Platform.OS ternaries (un-guarded uniwind variants broke both platforms) by @juliusmarminge in pingdotgg/t3code#13169
* docs(mobile): document the two mobile routes that intentionally skip deep links by @juliusmarminge in pingdotgg/t3code#13164
* refactor(mobile): break module cycles with focused extractions by @juliusmarminge in pingdotgg/t3code#13151
* fix(server): generate PR diffs from branch changes by @Yash-Singh1 in pingdotgg/t3code#13170
* fix(web): preserve nested scroll behavior in chat timeline by @Yash-Singh1 in pingdotgg/t3code#13167
* test(web): cover usage model ordering without static markup by @flamboh in pingdotgg/t3code#13104
* fix(desktop): find linuxbrew node for the WSL backend by @CodyRay in pingdotgg/t3code#7827
* chore(models): use GPT-6 Luna for text generation by @extoci in pingdotgg/t3code#13115
* fix(mobile): keep ordinary offline outbox failures out of console.warn by @juliusmarminge in pingdotgg/t3code#13144
* feat(providers): check remote compatibility ranges by @juliusmarminge in pingdotgg/t3code#13130
* chore(lint): keep mobile theme escape-hatch allowlist honest by @juliusmarminge in pingdotgg/t3code#13146
* fix(web): the pull request badge reads at the meta size again by @juliusmarminge in pingdotgg/t3code#13175
* fix(mobile): uniwind platform variants stay guarded on both platforms by @juliusmarminge in pingdotgg/t3code#13172
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13185
* refactor(mobile): remaining className platform ternaries become class variants by @juliusmarminge in pingdotgg/t3code#13188
* fix(web): align provider emails without clipping by @Derpedyea in pingdotgg/t3code#13174
* perf(mobile): recycle the default v2 home list and scope the snooze minute tick by @juliusmarminge in pingdotgg/t3code#13149
* refactor(mobile): retire the legacy grouped thread list by @juliusmarminge in pingdotgg/t3code#13183
* fix(server): background PR checks spend less GitHub quota by @juliusmarminge in pingdotgg/t3code#13189
* fix(server): background PR sync reads summaries in batches by @juliusmarminge in pingdotgg/t3code#13198
* fix(server): GitHub PR lookups stop probing owner-qualified heads by @juliusmarminge in pingdotgg/t3code#13200
* chore(mobile): clear the legacy-list deletion fallout by @juliusmarminge in pingdotgg/t3code#13203

## New Contributors
* @CodyRay made their first contribution in pingdotgg/t3code#7827

**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260922.2123...v0.0.43-nightly.20260923.2135

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260923.2135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

1 participant