fix(cursor-review): fail loud when a panel model id is delisted#22
Open
mattmillerai wants to merge 1 commit into
Open
fix(cursor-review): fail loud when a panel model id is delisted#22mattmillerai wants to merge 1 commit into
mattmillerai wants to merge 1 commit into
Conversation
Cursor delisting a pinned catalog model id used to degrade the panel silently: the per-cell `cursor-agent --model <id>` call exits non-zero with 0 bytes of stdout, which the extractor tagged `empty` — indistinguishable from "the model ran and found nothing" — so the panel ran a lab short for weeks, undetected. Two changes make catalog drift fail loud: - New preflight job runs `cursor-agent models` once before the 8-cell fan-out and hard-fails the run if any pinned id is absent from the live catalog, naming the missing id(s) and the closest same-lab hint. Fork PRs (no CURSOR_API_KEY) skip it, matching the gate's existing fork handling. The job is also the single source of truth for the matrix model list, so the validated list and the reviewed list cannot drift apart. - Defense-in-depth in extract-findings.py: the run step now forwards the cursor-agent exit code + stderr, and a cell whose call exited non-zero or whose stderr matches `Cannot use this model:` is classified `error` (which post-review.py reports as `(error)` and the existing degraded-panel path fires) instead of a silent `empty`.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
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
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.
ELI-5
The review panel pins one exact model name per AI lab. When a lab quietly
retires (delists) one of those names, the panel used to just… skip that model
and say nothing — so we'd run a review with 3 labs instead of 4 for weeks and
never notice. This PR makes that mistake loud: the run now checks every
pinned model against the live catalog before it starts, and fails the whole
run (with a clear "this model id is gone, here's the closest one" message) if
any pin is dead. As a backup, even if that check is bypassed, a dead model now
shows up as
(error)in the panel summary instead of a silent "found nothing".What changed
1. Preflight catalog check (fail-loud, primary). A new
preflightjob runscursor-agent modelsonce, before the 8-cell fan-out, and hard-fails the run ifany pinned panel model id is absent from the live catalog — naming the missing
id(s) plus the closest same-lab hint. This job is also the single source of
truth for the matrix model list (the
reviewmatrix now consumes it viafromJSON), so the list we validate and the list we review can never driftapart. Fork PRs / runs without
CURSOR_API_KEYskip the check cleanly, matchingthe gate's existing fork handling (forks already yield
should_run=false).2. Defense-in-depth in
extract-findings.py(backup). The per-cell run stepnow forwards the cursor-agent exit code + stderr into
extract-findings.py. Acell whose call exited non-zero or whose stderr matches
Cannot use this model:is now classifiedstatus=error(whichpost-review.pyalready renders as
(error)and counts as a failed cell, firing the existingdegraded-panel path) instead of a silent
empty. A non-zero exit that stillproduced valid findings is left untouched, so real findings are never discarded.
Testing
python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py'—27 tests, all green. New coverage:
ClassifyRunErrorTest,ParseExitCodeTest,and end-to-end
test_delisted_model_is_error_not_empty(the core regression).reviewmatrix expands from the preflightoutput).
Notes / judgment calls
matt/bump-kimi-k2.7-code). That PR carries thelive
kimi-k2.7-codepin; this PR is the durability half. GitHub retargets tomainonce fix(cursor-review): bump dead kimi-k2.5 model id to kimi-k2.7-code #21 merges. Review sees only this net diff.model list a second time in the preflight (a new drift vector — the whole bug
this fixes), the preflight owns the list and the matrix derives from it.
the separate
judge_modelinput. Its default equals a panel pin (so it'scovered transitively), and a delisted judge already degrades via consolidate's
judge-failed banner rather than silently. A custom, delisted
judge_modeloverride would not be caught here — noted as a possible follow-up.
bump ticket) is intentionally not included — fail-loud on the next PR run is
enough for now.