From f5ca81f28c854707c6b93c071b25a48f0a369426 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Fri, 1 May 2026 18:20:48 +0200 Subject: [PATCH] perf: parallelise rivet head oracle and base validate runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Wave-1 Performance engineer flagged this as Bug #20 in `docs/agent-fleet/bugs.md`. The rivet oracle path in `reviewPullRequest` ran two independent `withTempRepoCheckout` calls serially: 1. head oracle (validate + impact at PR head) 2. base validate (for `subtractFindings` delta filter) Each tarball is ~50 MB on the rivet repo, extract takes seconds, `rivet validate` is another 1-3 s. Sequential wall-time was conservatively 15-40 s, fully blocking the webhook handler. The two operations have zero data dependency: separate tarballs, separate tempdirs, separate rivet invocations. Trivially parallelisable. ## What `Promise.all` over the two `withTempRepoCheckout` calls. Each side keeps its own error handler — base-ref failures (e.g. force-push lost the base commit, or schema drift between base and head) gracefully degrade to head-only oracle findings, same behaviour as before. ## Source Bug #20, wave-1 Performance engineer (`docs/agent-fleet/bugs.md`). ## Test plan - [x] 834 tests pass — existing oracle tests cover both head and base paths via the injected `runner` mock; concurrency doesn't change the assertions - [x] eslint clean - [ ] After deploy: wall-time of an AI review on a rivet repo drops by ~50% of the rivet-oracle component. Visible in the bot's "Rivet oracle complete" log line timing. ## Risk & rollout - Risk: low. Same calls, same args, same downstream consumers, just scheduled concurrently. Each side's failure is still locally caught. - 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 ea12e4f..16fbf41 100644 --- a/src/ai-review.js +++ b/src/ai-review.js @@ -422,7 +422,13 @@ async function reviewPullRequest(octokit, owner, repo, prNumber) { : path.resolve(__dirname, '..', rivetCfg.binary_path); try { if (headSha && (await hasRivetYaml(octokit, owner, repo, headSha))) { - oracleSummary = await withTempRepoCheckout( + // Run head oracle and base validate in parallel — they are + // independent (different tarballs, different cwds, different + // commands). Sequential they cost ~2× tarball-fetch + 2× extract + + // 2× rivet runtime; parallel halves wall-time. Each side has its + // own try/catch so a base-ref failure (e.g. base SHA gone, schema + // drift) gracefully degrades to head-only findings. + const headPromise = withTempRepoCheckout( octokit, owner, repo, @@ -432,15 +438,9 @@ async function reviewPullRequest(octokit, owner, repo, prNumber) { timeout: rivetCfg.timeout_ms || 60000 }) ); - let headOracleFindings = oracleSummary?.findings || []; - // Delta filter: subtract findings already present at the PR base. - // Without this, the review surfaces the entire repo backlog every - // time (see spar#175 — 91 pre-existing warnings drowning the diff). - let baseValidate = null; - if (baseSha && baseSha !== headSha) { - try { - baseValidate = await withTempRepoCheckout( + const basePromise = (baseSha && baseSha !== headSha) + ? withTempRepoCheckout( octokit, owner, repo, @@ -448,14 +448,21 @@ async function reviewPullRequest(octokit, owner, repo, prNumber) { (repoPath) => runRivetValidate(resolvedBinary, repoPath, { timeout: rivetCfg.timeout_ms || 60000 }) - ); - } catch (err) { - getLogger().warn( - { pr: prNumber, err: err.message }, - 'base-ref validate failed — falling back to head-only oracle findings' - ); - } - } + ).catch((err) => { + getLogger().warn( + { pr: prNumber, err: err.message }, + 'base-ref validate failed — falling back to head-only oracle findings' + ); + return null; + }) + : Promise.resolve(null); + + const [headResult, baseValidate] = await Promise.all([ + headPromise, + basePromise + ]); + oracleSummary = headResult; + let headOracleFindings = oracleSummary?.findings || []; const droppedCount = headOracleFindings.length; headOracleFindings = subtractFindings( headOracleFindings,