fix(cursor-review): give the judge the same timeout as the panel it adjudicates - #219
Merged
Merged
Conversation
…djudicates The `consolidate` job was capped at 15 minutes while `review` — one panel cell — gets 30. The judge is the strictly harder call: it reads all 8 cell artifacts with the same top reasoning tier a single cell uses. That asymmetry made the failure mode the expensive one. A diff slow enough to stretch the panel stretches the judge with it, so all 8 cells would run green for ~25 minutes and then the judge would be killed at the last step, discarding every cell's work with nothing posted to the PR. It also hid itself: GitHub reports a `timeout-minutes` kill as `cancelled` rather than `failed`, so the run page reads as though a human stopped it. Observed twice in one day on the cloud repo, both at 15m0Xs — about 3% of recent runs burning a full panel and throwing the result away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughThe workflow documents the relationship between review and consolidation timeouts. It increases the consolidate job timeout to 40 minutes and gives the judge step a 30-minute timeout with error continuation so fallback findings can still be posted. ChangesReview resilience
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change gives the judge a matching step timeout and preserves the existing degraded review fallback; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 4 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 1 |
| 🟢 Low | 1 |
| ⚪ Nit | 1 |
Panel: 8/8 reviewers contributed findings.
…er repo Review follow-ups on the judge timeout fix. Raising the `consolidate` JOB cap alone only moved the cliff: a `timeout-minutes` kill at the job level takes `Build consolidated findings file` and `Post review` with it, so a hung judge still burns the whole budget and posts nothing — the one failure mode that can never reach the degraded panel-union fallback the workflow already has. Time-box `Run judge` itself instead, `continue-on-error` so the timeout is absorbed, and leave the job cap above it — the same step-under-job idiom the `ledger` job uses. A hung judge now costs the adjudication, not the panel's 8 cells. The step cap is the `review` panel cap (30m), and the job cap sits 10m over it to cover this job's own overhead (fetch-depth: 0 checkout, CLI install, three artifact downloads, the post), so the judge's effective MODEL budget is at least a single cell's rather than smaller than one. Also drops the consumer repo name from the job comment. This repo is public and deliberately not self-enrolled in public-repo-hygiene, so nothing would have caught it; the observation is kept, anonymized. And cross-references the coupling at the `review` cap, so a future bump there does not silently recreate the skew.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
The Cursor review runs 8 model calls in parallel (the panel), then one more call (the judge) reads all 8 and writes the single review that lands on your PR. The panel was allowed 30 minutes; the judge only 15 — even though the judge has to read everything the panel produced.
So on a big PR the panel would finish fine after ~25 minutes, and then the judge would get killed right at the end. All that work, no review posted.
What was happening
consolidatehadtimeout-minutes: 15;review(one panel cell) has 30. The judge callscursor-agentonce more withprompt-judge.mdagainst all 8 cell artifacts, using the same top reasoning tier a single cell uses — the strictly harder call, on half the budget.A diff slow enough to stretch the panel stretches the judge with it, so the two go long together and only the judge has a tight cap.
It also hid itself. GitHub reports a
timeout-minuteskill ascancelled, notfailed, so the run page reads as though someone stopped it by hand rather than a cap firing.Evidence
Two occurrences on one consumer repo on 2026-08-24, both landing exactly on the cap:
In the first, all 8 panel cells succeeded between 19:24 and 19:49 — the slowest (
adversarial (claude-opus-5-thinking-max)) took 25 minutes ofreview's 30. Then the judge burned its full 15 and was killed. Nothing posted.Across the last 60 Cursor Review runs on that repo, 5 were
cancelled: 2 were this judge timeout, the other 3 were ordinary concurrency cancels (every job killed within seconds of start when a new push superseded the run). So ~3% of reviews were running a full panel and discarding the result.notify-completealready branches onneeds.consolidate.result, so the DM correctly degraded to a warning — the loss was the review itself, not a false success signal.The change
Raising the
consolidatejob cap alone would only move the cliff. A job-leveltimeout-minuteskill takesBuild consolidated findings fileandPost reviewwith it, and that is the one failure mode that can never reach the degraded panel-union fallback the workflow already has — so a hung judge would still burn the budget and post nothing, just 30 minutes later.So the cap goes on the step, in the shape the
ledgerjob already uses:Run judgegetstimeout-minutes: 30— thereviewpanel cap, so the judge is never given less model time than one of the cells it reads — pluscontinue-on-error: true, which is what makes the timeout absorbable. On a hung judge the steps below then read the--initseed (status=error), take the existing panel-union path and post what the 8 cells found. A hung judge now costs the adjudication, not the panel.consolidatejob cap goes15→40, deliberately above the step so the step trips first. The 10m of slack covers this job's own overhead —fetch-depth: 0checkout, CLI install, three artifact downloads, the post — which a panel cell does not all pay, so the judge's effective model budget is the step cap, not the job cap.reviewcap records the coupling, so a future bump there does not silently recreate the same skew.continue-on-errorhides no real signal:cursor-agent's exit code is already captured in-script, and a judge that did not submit is reported to the PR asjudge_statusbyPost review.Verification
python3 -m unittest discover -s .github/workflow-pins/tests— 216 passedpython3 .github/workflow-pins/check_workflow_pins.py— OK, 11 workflows, none with a default refpython3 -m unittest discover -s .github/cursor-review/tests— 194 passedreview= 30,consolidatejob = 40,Run judgestep = 30 withcontinue-on-error: truecheck_public_repo_hygiene.pyover the edited workflow: 13 findings, all pre-existing(BE-####)commit-convention refs, zero repo-name refsNote for the merger
This touches
.github/workflows/cursor-review.yml, so merging tripsbump-cursor-review-callers.yml's path filter and rolls the cursor-review fleet. That is expected for any change to this file.Provenance
Authored by: agent-work loop
Verified:
workflow-pins(216) andcursor-review(194) unittest suites pass;check_workflow_pins.pyclean; the workflow YAML parses and the three caps resolve as stated;check_public_repo_hygiene.pyover the edited file reports no repo-name reference. Not verified end-to-end: the step-timeout → degraded-fallback path has not been exercised against a genuinely hung judge in CI — it is reasoned from the--initseed'sstatus=errorand the existingdegradedbranch inBuild consolidated findings file.Deviations: The job cap is 40, not the 30 the first commit used. The review panel asked for headroom above the panel cap, and the step-under-job idiom requires the job to outlive the step; 40 satisfies both. The panel also suggested asserting the review/consolidate coupling mechanically in
check_workflow_pins.py— not done: that checker is scoped toworkflows_refpinning, and a timeout-coupling rule is unrelated machinery. A comment at thereviewcap records the coupling instead.🤖 Generated with Claude Code