From 73321dd034d9ce02c2d22e23a86cb1ee832900a0 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Thu, 30 Apr 2026 07:25:50 +0200 Subject: [PATCH] perf: parallelise the three PR-data fetches in reviewPullRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why The hot path of `reviewPullRequest` (`src/ai-review.js:382-398`) issued three strictly-sequential GitHub REST calls before any work began: PR metadata, the diff via `mediaType: { format: 'diff' }`, and the files list. Each call costs ~200-500 ms of egress time on the netcup VPS; serially that's ~600-1500 ms of wall-time waste on every review. The three calls are independent — none depends on the other two's response — so they parallelise trivially. ## What `Promise.all([request, request, request])` wrapper in `reviewPullRequest`. The downstream consumers (rivet oracle path, `buildReviewPrompt`, `recordReview`) keep their `prData` / `diffResponse` / `filesResponse` contract unchanged. octokit.request invocations are still dispatched in source order (JS is single-threaded), so the existing `mockResolvedValueOnce`-style test rigs work unchanged. ## Source Wave-1 Performance engineer flagged this as Bug #19 in `docs/agent-fleet/bugs.md`. Estimated savings: 0.5-1 s per review. ## Test plan - [x] All 806 tests pass - [x] eslint clean - [ ] After deploy: PR-review wall-time drops by the GitHub-egress component; visible in `pr-review-timing` logs (when added per Performance agent's "one profile I'd run" recommendation). ## Risk & rollout - Risk: low. Same data, same downstream consumers, just dispatched concurrently. - Rollout: self-update on merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai-review.js | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/ai-review.js b/src/ai-review.js index 3307261..ea12e4f 100644 --- a/src/ai-review.js +++ b/src/ai-review.js @@ -379,23 +379,30 @@ async function reviewPullRequest(octokit, owner, repo, prNumber) { } try { - const prData = await octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', { - owner, - repo, - pull_number: prNumber - }); - - const diffResponse = await octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', { - owner, - repo, - pull_number: prNumber, - mediaType: { format: 'diff' } - }); - - const filesResponse = await octokit.request( - 'GET /repos/{owner}/{repo}/pulls/{pull_number}/files', - { owner, repo, pull_number: prNumber } - ); + // Three independent reads — fan them out instead of waiting serially. + // ~600-1500 ms before any work was wall-time waste; Promise.all halves + // it on the typical Probot path. octokit.request invocations are still + // dispatched in source order (JS is single-threaded), so the existing + // mockResolvedValueOnce-style test rigs and downstream consumers + // (rivet oracle path, buildReviewPrompt, recordReview) keep their + // prData / diffResponse / filesResponse contract. + const [prData, diffResponse, filesResponse] = await Promise.all([ + octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', { + owner, + repo, + pull_number: prNumber + }), + octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', { + owner, + repo, + pull_number: prNumber, + mediaType: { format: 'diff' } + }), + octokit.request( + 'GET /repos/{owner}/{repo}/pulls/{pull_number}/files', + { owner, repo, pull_number: prNumber } + ) + ]); // Mechanical oracle: only runs for repos that ship rivet.yaml AND have // the binary configured. Failures are non-fatal — the model path still