Skip to content

perf(agent-manager): batch PR status lookups into one GraphQL request per sync - #14116

Merged
marius-kilocode merged 3 commits into
mainfrom
research-github-rate-limit-subscription-solution
Sep 14, 2026
Merged

marius-kilocode merged 3 commits into
mainfrom
research-github-rate-limit-subscription-solution

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

What Problem This Solves

A full Agent Manager sync resolved each worktree with 2 to 3 gh calls:

  • worktree with a PR: gh pr view plus a reviewThreads GraphQL query
  • worktree without a PR: gh pr view, gh pr view <branch>, gh pr list --search <sha>

With N worktrees that scales linearly in GraphQL points and process spawns against a 5,000 point/hour budget shared with every agent session that also runs gh. GraphQL requests are POST, so ETag and 304 revalidation do not apply, which is why caching alone cannot reduce this cost.

Why This Change Was Made

Batching is the available lever while staying on gh. One aliased GraphQL document resolves every worktree by headRefName, with a HEAD-SHA alias for same-repo PRs whose local branch was renamed. The response is reshaped into the gh pr view --json shape so parsePRResult, checks, and parseReviewers are reused unchanged, and so full-sync and active-tick results hash identically, which avoids badge flicker.

Selection mirrors gh's own finder because the naive query produces wrong badges:

  • states [OPEN, CLOSED, MERGED], newest first, open preferred, so a merged PR keeps its badge instead of disappearing
  • a fork PR is only attributed when the local HEAD matches, since headRefName: "main" on this repository alone matches 76 fork PRs
  • a SHA-associated PR only counts when open with an exact head match, because a fresh branch off main sits on the last merged PR's squash commit
  • a default-branch worktree does not show its latest merged PR, matching gh

Anything the batch cannot decide stays unresolved so the existing per-worktree path runs: ambiguous candidates, refs/pull/N/head tracking refs, GraphQL errors, and unknown fields. A failed batch is never worse than the previous behavior.

The seed orchestration lives in pr/am-pr-seed.ts behind a small callback host. PRStatusPoller.ts grows only by the call site, a host adapter, and one optional fetchOne parameter.

User Impact

PR badges stay correct with fewer GitHub API calls and fewer gh process spawns. The selected worktree keeps its 15s refresh, the all-worktree sweep stays at 2 minutes, and multi-project is unaffected because each project already has its own poller batching only its own repository.

Evidence

Measured against Kilo-Org/kilocode with GH_DEBUG=api and rateLimit { cost }:

  • one rich gh pr view costs 1 GraphQL point; one reviewThreads query costs 1 point
  • 10 branches batched: 1 request, cost: 2 to 3, 0 errors, all 10 resolved including 6 fork PRs and an ambiguous fork pair

Isolated VS Code on a disposable fixture with two open PRs, one merged PR whose remote branch was deleted, and one fresh branch off main, counting only children of the extension host during one full sync:

  • 1 batched gh api graphql, 3 reviewThreads follow-ups, 0 gh pr view or gh pr list
  • Batched PR lookup cost: 1 logged each 2 minutes
  • with gh unauthenticated: one batch failure logged, then the legacy path reported the same auth state as before

Checks: tests/unit/ 5,620 pass, 0 fail; typecheck, lint, knip, check-kilocode-change, and format clean.

Limitations: the per-PR reviewThreads query and the 15s active tick are unchanged, so at 10 worktrees the hourly reduction is roughly a quarter and grows with worktree count.

… per sync

A full sync resolved each worktree with 2 to 3 `gh` calls, and GraphQL is
POST so ETag revalidation cannot reduce that cost. Resolve every worktree in
one `gh api graphql` request per 10 worktrees instead, then hand the results
to the per-worktree fetch so it skips its own lookups.

Merged PRs keep their badge, forks that only share a branch name are not
attributed to a worktree, and anything the batch cannot decide falls back to
the existing per-worktree path unchanged.
Comment thread packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts
  • packages/kilo-vscode/tests/unit/am-pr-seed.test.ts

The previous SUGGESTION (wasted git rev-parse spawns after a poll generation was superseded) is resolved in 0d7fa2cc: collect() now breaks out of the loop when host.stale() is true, and the new regression test asserts only one branch lookup runs once a generation is superseded.

No memory leaks found: the added guard only shortens the loop and retains nothing new across poll cycles.

Previous Review Summaries (2 snapshots, latest commit f99f82a)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit f99f82a)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts 62 Refactor dropped the early return on host.stale(), so collect() keeps calling host.branch(wt) for every remaining worktree after a poll generation is superseded (wasted git child processes; result discarded by the later stale guard).
Files Reviewed (3 files)
  • packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts - no issues
  • packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts - 1 issue
  • packages/kilo-vscode/tests/unit/am-pr-seed.test.ts - no issues

The previously reported WARNING (unhandled host.branch(wt) rejection aborting the full sync) is fixed in f99f82a: one() now catches the branch rejection and the fetchAll call site catches any remaining seed() rejection. The new regression test covers the rejected-branch fallback.

No memory leaks found: the seed map, host adapter closures, and thunks are all short-lived and nothing new is retained across poll cycles.

Fix these issues in Kilo Cloud

Previous review (commit 81b9c21)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts 63 host.branch(wt) is awaited without error handling, so a rejecting branch callback rejects seed() and fetchAll() (unhandled rejection) and skips the whole full sync; previously a getBranch failure only affected one worktree.
Files Reviewed (7 files)
  • .changeset/batched-pr-poller.md - no issues
  • packages/kilo-vscode/src/agent-manager/PRStatusPoller.ts - no issues
  • packages/kilo-vscode/src/agent-manager/pr/am-pr-batch.ts - no issues
  • packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts - 1 issue
  • packages/kilo-vscode/tests/unit/am-pr-batch.test.ts - no issues
  • packages/kilo-vscode/tests/unit/am-pr-seed.test.ts - no issues
  • packages/kilo-vscode/tests/unit/am-pr-status-bridge.test.ts - no issues

No memory leaks found: the seed map, host adapter closures, and thunks are all short-lived and nothing new is retained across poll cycles.

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

…l sync

A branch callback that rejects during batched seeding propagated out of
`seed()` and skipped the whole full sync, where the previous per-worktree
lookup only failed that one worktree. Resolve each worktree's branch
independently and guard the seed call so those worktrees fall back to the
legacy path.
Comment thread packages/kilo-vscode/src/agent-manager/pr/am-pr-seed.ts
…s stale

Batched seeding kept spawning branch lookups for every remaining worktree
after a stop or restart superseded the poll generation, and then discarded
the result. Break out of the loop as soon as the generation is stale.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants