[PR #367] [MERGED] ci(cursor-review): drive the panel list from a variable so a tier A/B needs no PR #366

New issue

Closed

opened 2026-10-26 22:39:05 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:39:05 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/367
Author: @mattmillerai
Created: 10/7/2026
Status: ✅ Merged
Merged: 10/7/2026
Merged by: @mattmillerai

Base: main ← Head: matt/cursor-panel-models-var

📝 Commits (4)

3a1ca3f ci(cursor-review): drive the panel list from a variable so a tier A/B needs no PR
cea0ebd ci(cursor-review): log which panel list is in effect when none is overridden
be87a4a fix(cursor-review): address the panel findings on the panel_models variable
69fcf34 fix(cursor-review): hold a panel_models override to exactly one JSON document

📊 Changes

3 files changed (+148 additions, -5 deletions)

View changed files

📝 .github/workflows/ci-cursor-review.yml (+44 -0)
📝 .github/workflows/cursor-review.yml (+29 -4)
📝 docs/callers/cursor-review.md (+75 -1)

📄 Description

ELI5

panel_models has always been the documented way to A/B reasoning tiers — its own description says "e.g. an A/B of -xhigh vs -max reasoning tiers" — but no caller has ever passed it. So trying one meant editing the shared default and bumping the whole fleet, and reverting meant doing it again.

This wires this repo's own caller to a variable:

panel_models: ${{ vars.CURSOR_PANEL_MODELS }}

Set it, watch a few rounds, unset it to revert.

Inert until used

The reusable reads an empty panel_models as "use the built-in list", so an unset variable is exactly today's behaviour. Nothing changes on merge for this repo's caller, which stays pinned to 0eac47e until the next bump.

A repo variable overrides an org variable of the same name, so one repo can trial a list the rest of the org isn't on, and an org value can be set later without touching any caller.

The first experiment

The panel's -max → -xhigh tier drop. The judge already made that move in #243 on spend grounds, with the rationale recorded: it had run at xhigh with no observed quality gap while max cost ~10% of every run.

gh variable set CURSOR_PANEL_MODELS --repo Comfy-Org/github-workflows \
  --body '["gpt-5.6-sol-max","claude-opus-5-thinking-xhigh","kimi-k3-high"]'

Two traps, documented

Cursor catalog ids are not API slugs. This is deliberately not pointed at the org's CLAUDE_MODEL_* variables. Those hold Anthropic API slugs (claude-opus-5); the panel needs catalog ids with a tier suffix (claude-opus-5-thinking-xhigh). Preflight's live-catalog check would reject the former. The two look interchangeable and are not, so the docs say so explicitly.

Change one axis at a time. Version and tier are separate variables; moving both leaves the result unattributable. Worth knowing for the obvious follow-up: Opus 5.5 is in the catalog and ZDR-clean, but ships no -thinking- tier — so claude-opus-5-5-xhigh is not a like-for-like swap from claude-opus-5-thinking-max.

Reusable-side hardening (`cursor-review.yml`)

Wiring the input to a variable makes it settable with no PR, so the reusable's override handling is tightened too — this part reaches every caller on its next bump:

the override must be exactly one JSON array of 1–12 catalog-shaped ids (12 keeps models × 2 review types well under GitHub's 256-job matrix cap; a stream like ["a"] ["b"] no longer passes on its last document);
a rejected value is echoed with newlines stripped and truncated, so it cannot inject ::workflow commands:: into a public run log;
the run now logs which list is in effect in both branches, so deleting the variable mid-experiment is visible.

Why `judge_model` is left alone

Its input carries a real default that catalog-drift.py parses out of cursor-review.yml, and a caller passing an explicitly-empty value gets '' rather than that default — so a variable-wired caller would have to restate the default and then drift from it. That change belongs in the reusable, not a caller.

Tests

python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py' — 1144 pass
python3 .github/workflow-pins/check_workflow_pins.py — OK
python3 .github/agents-md-integrity/check_agents_md.py --root . — passed
panel_models confirmed present at this caller's pinned ref (0eac47e), so the input resolves at startup today.

Provenance

Authored by: agent-work loop
Verified:

cursor-review, workflow-pins unit suites pass at the current head; workflow-pin, AGENTS.md-integrity and org-repo-literal lints pass
the panel_models step script, extracted and run under bash -e -o pipefail: empty → built-in list; a single array → deduped models=; multi-document, null-prefixed, non-string and malformed values → exit 1 with no models output
panel_models exists at the caller's pinned ref 0eac47e

Deviations:

also hardens the reusable's override validation (bounds, single-document, safe echo) in response to review, beyond the caller-only wiring the title describes

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/367 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/7/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/cursor-panel-models-var` --- ### 📝 Commits (4) - [`3a1ca3f`](https://github.com/Comfy-Org/github-workflows/commit/3a1ca3f944ab97ca86a3f49cd51bbb29629a1189) ci(cursor-review): drive the panel list from a variable so a tier A/B needs no PR - [`cea0ebd`](https://github.com/Comfy-Org/github-workflows/commit/cea0ebd5e2286c12a6fa49f8e43cc94a1328c9b2) ci(cursor-review): log which panel list is in effect when none is overridden - [`be87a4a`](https://github.com/Comfy-Org/github-workflows/commit/be87a4a5aa4a453fe78c0f91a80c93415e7bf3e7) fix(cursor-review): address the panel findings on the panel_models variable - [`69fcf34`](https://github.com/Comfy-Org/github-workflows/commit/69fcf34c4a88fb34afc8b141c422f0556937e0fc) fix(cursor-review): hold a panel_models override to exactly one JSON document ### 📊 Changes **3 files changed** (+148 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci-cursor-review.yml` (+44 -0) 📝 `.github/workflows/cursor-review.yml` (+29 -4) 📝 `docs/callers/cursor-review.md` (+75 -1) </details> ### 📄 Description ## ELI5 `panel_models` has always been the documented way to A/B reasoning tiers — its own description says *"e.g. an A/B of `-xhigh` vs `-max` reasoning tiers"* — but no caller has ever passed it. So trying one meant editing the shared default and bumping the whole fleet, and reverting meant doing it again. This wires **this repo's own caller** to a variable: ```yaml panel_models: ${{ vars.CURSOR_PANEL_MODELS }} ``` Set it, watch a few rounds, unset it to revert. ## Inert until used The reusable reads an empty `panel_models` as "use the built-in list", so an unset variable is **exactly** today's behaviour. Nothing changes on merge for this repo's caller, which stays pinned to `0eac47e` until the next bump. A repo variable overrides an org variable of the same name, so one repo can trial a list the rest of the org isn't on, and an org value can be set later without touching any caller. ## The first experiment The panel's `-max` → `-xhigh` tier drop. The judge already made that move in #243 on spend grounds, with the rationale recorded: it had run at xhigh with no observed quality gap while `max` cost ~10% of every run. ```bash gh variable set CURSOR_PANEL_MODELS --repo Comfy-Org/github-workflows \ --body '["gpt-5.6-sol-max","claude-opus-5-thinking-xhigh","kimi-k3-high"]' ``` ## Two traps, documented **Cursor catalog ids are not API slugs.** This is deliberately *not* pointed at the org's `CLAUDE_MODEL_*` variables. Those hold Anthropic API slugs (`claude-opus-5`); the panel needs catalog ids with a tier suffix (`claude-opus-5-thinking-xhigh`). Preflight's live-catalog check would reject the former. The two look interchangeable and are not, so the docs say so explicitly. **Change one axis at a time.** Version and tier are separate variables; moving both leaves the result unattributable. Worth knowing for the obvious follow-up: Opus 5.5 is in the catalog and ZDR-clean, but ships **no `-thinking-` tier** — so `claude-opus-5-5-xhigh` is not a like-for-like swap from `claude-opus-5-thinking-max`. ## Reusable-side hardening (`cursor-review.yml`) Wiring the input to a variable makes it settable with no PR, so the reusable's override handling is tightened too — this part reaches every caller on its next bump: - the override must be **exactly one** JSON array of **1–12** catalog-shaped ids (12 keeps models × 2 review types well under GitHub's 256-job matrix cap; a stream like `["a"] ["b"]` no longer passes on its last document); - a rejected value is echoed with newlines stripped and truncated, so it cannot inject `::workflow commands::` into a public run log; - the run now logs which list is in effect in **both** branches, so deleting the variable mid-experiment is visible. ## Why `judge_model` is left alone Its input carries a real default that `catalog-drift.py` parses out of `cursor-review.yml`, and a caller passing an explicitly-empty value gets `''` rather than that default — so a variable-wired caller would have to restate the default and then drift from it. That change belongs in the reusable, not a caller. ## Tests `python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py'` — 1144 pass `python3 .github/workflow-pins/check_workflow_pins.py` — OK `python3 .github/agents-md-integrity/check_agents_md.py --root .` — passed `panel_models` confirmed present at this caller's pinned ref (`0eac47e`), so the input resolves at startup today. ## Provenance **Authored by:** agent-work loop **Verified:** - cursor-review, workflow-pins unit suites pass at the current head; workflow-pin, AGENTS.md-integrity and org-repo-literal lints pass - the `panel_models` step script, extracted and run under `bash -e -o pipefail`: empty → built-in list; a single array → deduped `models=`; multi-document, `null`-prefixed, non-string and malformed values → exit 1 with no `models` output - `panel_models` exists at the caller's pinned ref `0eac47e` **Deviations:** - also hardens the reusable's override validation (bounds, single-document, safe echo) in response to review, beyond the caller-only wiring the title describes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:39:05 +00:00