From 28671d75fcf378360e916b0096c70fdf0bf267c0 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:14:40 -0500 Subject: [PATCH] review: a fallback stays inside the reviewer's model family A review whose model the endpoint parked or could not route continued on REDLINE_FALLBACK_MODEL whatever that model was. A host that reads on one vendor's models because another vendor's coding agent writes its pull requests could have a review finished by the author's own family, with nobody choosing that. review.mjs now uses the fallback only when it is of the stopped model's family (modelFamily, sameModelFamily). Otherwise it logs why and fails the run, as it does with no fallback. The default fallback, claude-opus-5-5, now serves only a Claude reviewer: a host that reads on another family names a fallback of that family. The README, the hosted quickstart and the changelog say so, and the quickstart says how to choose the reviewer's family. --- CHANGELOG.md | 8 +++++ README.md | 14 ++++++--- docs/hosted-quickstart.md | 5 +++ scripts/redline/review.mjs | 39 ++++++++++++++++++++--- scripts/redline/review.test.mjs | 56 +++++++++++++++++++++++++++++++-- 5 files changed, 109 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e13f3f0..6b90dd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,14 @@ names the commit's date and the pull request it came from. ## November 2026 +- A fallback stays inside the reviewer's model family. A review whose model was parked or + unroutable continued on `REDLINE_FALLBACK_MODEL` whatever that model was, so a host that read on + one vendor's models could have a review finished by another vendor's without anyone choosing + that. `review.mjs` now uses the fallback only when it is of the stopped model's family + (`modelFamily`: `claude`, `gpt`, or an unlisted id's leading name); otherwise the run logs why + and fails, as it does with no fallback. A host that reads on a family other than Claude sets + `REDLINE_FALLBACK_MODEL` to a model of that family: the default, `claude-opus-5-5`, no longer + serves it. The hosted quickstart says how to choose the reviewer's family. - A starter rubric for hosted mode is public at [askalf/redline-rubric](https://github.com/askalf/redline-rubric), linked from the README and the hosted quickstart. This repository still ships no prompt. diff --git a/README.md b/README.md index 5136383..609f2d1 100644 --- a/README.md +++ b/README.md @@ -15,11 +15,15 @@ On our hosts, routine and deep reviews run on OpenAI models through a ChatGPT su `REDLINE_MODEL=gpt-5.6-terra` reads routine pull requests and `REDLINE_DEEP_MODEL=gpt-6-astra` reads large or sensitive ones. Two paths have their own settings: a pull request that only moves Redline pins is read on `REDLINE_PIN_MODEL`, and a review whose model the endpoint has parked or cannot -route moves to `REDLINE_FALLBACK_MODEL` (any other model error fails the review). When a setting is -unset, the default in -[`scripts/redline/review.mjs`](scripts/redline/review.mjs) applies: `claude-fable-5-1` for -`REDLINE_MODEL`, and `claude-opus-5-5` for the pin and fallback models, so those two paths run on -Claude unless a host sets them. +route moves to `REDLINE_FALLBACK_MODEL` when that is a model of the same family (any other model +error fails the review). A fallback of another family is never used. Which family reads a repository +is a choice: where a coding agent writes the pull requests, another vendor's model reads them, so +the reviewer does not share the author's blind spots. A parked model does not undo that choice: the +review fails, and a new run reads the pull request once its model is back. When a setting is unset, +the default in [`scripts/redline/review.mjs`](scripts/redline/review.mjs) applies: `claude-fable-5-1` for +`REDLINE_MODEL`, and `claude-opus-5-5` for the pin and fallback models. So a pull request that only +moves pins is read on Claude unless a host sets another model, and a host that reads on another +family has no fallback until it names one of that family. **Who this is for.** The askalf repositories call it today. Another repository can call the review on a GitHub-hosted runner with a model API key and a rubric of its own, and no runner host diff --git a/docs/hosted-quickstart.md b/docs/hosted-quickstart.md index 50dc2a0..5ae5f15 100644 --- a/docs/hosted-quickstart.md +++ b/docs/hosted-quickstart.md @@ -95,6 +95,11 @@ jobs: The endpoint defaults to `https://api.anthropic.com`; pass `model-url` for another one that answers the Anthropic Messages API. +Choose the model by who writes your pull requests. If a coding agent writes them, a model from +another vendor is the stronger reviewer, since one of the author's own family tends to share its +blind spots; `model-url` takes any endpoint that serves such a model over the Anthropic Messages +API. Hosted mode sets no fallback model, so a review is never read on a model you did not name. + ## 4. `.github/workflows/redline-post.yml` The same ``, the same `reviewer-login` and the same `dispatch-actor` as above. diff --git a/scripts/redline/review.mjs b/scripts/redline/review.mjs index 3decf13..53f33a5 100644 --- a/scripts/redline/review.mjs +++ b/scripts/redline/review.mjs @@ -64,6 +64,11 @@ // - A model dario refuses as unroutable (a 400 marked model_unroutable: no provider lists it) goes // to REDLINE_FALLBACK_MODEL the same way: the same turn is sent again, with the history kept. // A 400 without that marker still fails the run. +// - A fallback stays inside the reviewer's model family (modelFamily). Which family reads a +// repository is the host's choice: where one vendor's coding agent writes the pull requests, +// another vendor's model reads them, so the reviewer does not share the author's blind spots. +// A parked or unroutable model must not undo that choice, so a REDLINE_FALLBACK_MODEL of +// another family is not used: the run says why and fails as it does with no fallback. // // CLI (the workflow's review step): // REPO=owner/name PR= HEAD_SHA= CHECKOUT= GH_READ_TOKEN=... [REDLINE_REREAD=1] \ @@ -71,8 +76,9 @@ // The env file holds DARIO_API_KEY, or DARIO_SOCKET (a dario key socket: the socket is the key, and // no key is stored or sent), and optionally REDLINE_GITHUB_TOKEN (or GITHUB_PAT_REVIEWER), // DARIO_URL (default http://127.0.0.1:3456), REDLINE_MODEL and REDLINE_FALLBACK_MODEL (default -// claude-opus-5-5; set it empty for no fallback), and REDLINE_PIN_MODEL (default claude-opus-5-5, -// the model for a PR that only moves Redline pins; set it empty to read those on REDLINE_MODEL), and +// claude-opus-5-5, used only for a model of its own family; set it empty for no fallback), and +// REDLINE_PIN_MODEL (default claude-opus-5-5, the model for a PR that only moves Redline pins; set +// it empty to read those on REDLINE_MODEL), and // REDLINE_DEEP_MODEL (default empty, which turns it off: the model for a PR that deepReview() picks, // one touching workflows, auth, crypto, secrets or the like, or a large one), and REDLINE_DEEP_API_KEY // (default empty: the deep model's reads go on DARIO_API_KEY; set, they go on this dario key, so a @@ -134,6 +140,23 @@ export function metaPhrase(text) { } export const DEFAULT_MODEL = 'claude-fable-5-1'; export const DEFAULT_FALLBACK_MODEL = 'claude-opus-5-5'; +// The ids that name a vendor family outright. `o3` and `o4-mini` are OpenAI ids with no `gpt-`. +const MODEL_FAMILIES = [['claude', /^claude-/], ['gpt', /^(?:gpt-|chatgpt-|codex-|o[0-9]+(?![a-z0-9]))/]]; +/** + * The family of a model id, lowercase: `claude` or `gpt` for an id MODEL_FAMILIES lists, else the + * id's leading run of letters and digits (`gemini-3-pro` is `gemini`). A provider path before the + * id (`vendor/claude-opus-5-5`) is not part of it. Empty when nothing is left to name one. + */ +export function modelFamily(model) { + const id = String(model ?? '').trim().toLowerCase().split('/').pop(); + for (const [family, re] of MODEL_FAMILIES) if (re.test(id)) return family; + return /^[a-z0-9]+/.exec(id)?.[0] ?? ''; +} +/** Whether two model ids are of one family. An id with no family is of none, not even its own. */ +export function sameModelFamily(a, b) { + const family = modelFamily(a); + return family !== '' && family === modelFamily(b); +} // A PR that only moves Redline pins (redline-pin-bump.yml opens one in every caller) is read on // this model instead of REDLINE_MODEL: it changes one commit sha per caller file, and every bump // wave is a review in each caller, so it does not spend the reviewer model's seat. @@ -1297,9 +1320,15 @@ export async function reviewChange(ctx, { pr, files, commits, reviews = [], prot }); } catch (e) { if (!(e instanceof ModelParked || e instanceof ModelUnroutable) || !ctx.fallbackModel || ctx.fallbackModel === ctx.model) throw e; - ctx.log?.(e instanceof ModelParked - ? `${ctx.model} is parked in dario for ${Math.ceil(e.retryAfterMs / 1000)}s; the review continues on ${ctx.fallbackModel}` - : `no provider in dario lists ${ctx.model}; the review continues on ${ctx.fallbackModel}`); + const why = e instanceof ModelParked + ? `${ctx.model} is parked in dario for ${Math.ceil(e.retryAfterMs / 1000)}s` + : `no provider in dario lists ${ctx.model}`; + // The reviewer's family is the host's choice; a fallback does not change it. + if (!sameModelFamily(ctx.fallbackModel, ctx.model)) { + ctx.log?.(`${why}; the fallback ${ctx.fallbackModel} is of another model family, so it is not used and the review stops`); + throw e; + } + ctx.log?.(`${why}; the review continues on ${ctx.fallbackModel}`); ctx.model = ctx.fallbackModel; turn--; continue; diff --git a/scripts/redline/review.test.mjs b/scripts/redline/review.test.mjs index 380f72c..9efc3cc 100644 --- a/scripts/redline/review.test.mjs +++ b/scripts/redline/review.test.mjs @@ -636,6 +636,19 @@ console.log('\n captured data (redline-protected)'); const r = await runReview(ctx); check('429 and 5xx from the model are retried with backoff', r.verdict === 'APPROVE' && calls.sleeps === 2); } +{ + const f = reviewModule.modelFamily; + const same = reviewModule.sameModelFamily; + check('modelFamily names the vendor family of a listed id, whatever its case, date or effort suffix', + f('claude-fable-5-1') === 'claude' && f(' Claude-Opus-5-5-20261001 ') === 'claude' && f('gpt-5.6-terra') === 'gpt' && f('gpt-6-astra:high') === 'gpt' + && f('chatgpt-5') === 'gpt' && f('codex-2') === 'gpt' && f('o3') === 'gpt' && f('o4-mini') === 'gpt' && f('o3:high') === 'gpt'); + check('an unlisted id is its leading run of letters and digits, and a provider path is not part of the family', + f('gemini-3-pro') === 'gemini' && f('opus') === 'opus' && f('o3x-1') === 'o3x' && f('m') === 'm' && f('vendor/claude-opus-5-5') === 'claude' && f('a/b/gpt-6-astra') === 'gpt'); + check('an id that names nothing has no family', f('') === '' && f(null) === '' && f(undefined) === '' && f('vendor/') === '' && f('-x') === ''); + check('sameModelFamily: one family is the same, two are not, and no family is never the same', + same('claude-fable-5-1', 'claude-opus-4-6') && same('gpt-5.6-terra', 'o3') && same('gemini-3-pro', 'gemini-3-flash') + && !same('gpt-6-astra', 'claude-opus-5-5') && !same('claude-opus-5-5', 'gpt-6-astra') && !same('', '') && !same('claude-opus-5-5', '') && !same(null, 'claude-opus-5-5')); +} { const parked = (retryAfter) => [429, { 'x-dario-upstream-rejection': 'pool_parked', 'retry-after': String(retryAfter) }]; { @@ -669,18 +682,55 @@ console.log('\n captured data (redline-protected)'); const e = await throws(() => runReview(ctx)); check('with no fallback a park fails the run at once', e instanceof reviewModule.ModelParked && calls.model.length === 1 && calls.sleeps === 0); } + { + const { ctx, calls } = world({ modelStatus: [parked(351838)], turns: [submit(APPROVE)] }); + ctx.model = 'gpt-5.6-terra'; ctx.fallbackModel = 'claude-opus-5-5'; + const lines = []; + ctx.log = (l) => lines.push(l); + const e = await throws(() => runReview(ctx)); + check('a parked model does not hand the review to a fallback of another family: the run fails on the one call', + e instanceof reviewModule.ModelParked && calls.model.map((b) => b.model).join() === 'gpt-5.6-terra' && calls.sleeps === 0 && calls.posted.length === 0); + check('the refusal is logged with the park', + lines.some((l) => l.includes('gpt-5.6-terra is parked in dario for 351838s; the fallback claude-opus-5-5 is of another model family, so it is not used and the review stops')) + && !lines.some((l) => l.includes('the review continues on'))); + } + { + const { ctx, calls } = world({ modelStatus: [parked(351838)], turns: [submit(APPROVE)] }); + ctx.model = 'claude-fable-5-1'; ctx.fallbackModel = 'gpt-6-astra'; + const e = await throws(() => runReview(ctx)); + check('the rule holds the other way round: a parked Claude model does not fall back to a GPT one', + e instanceof reviewModule.ModelParked && calls.model.map((b) => b.model).join() === 'claude-fable-5-1' && calls.posted.length === 0); + } + { + const { ctx, calls } = world({ modelStatus: [parked(351838)], turns: [submit(APPROVE)] }); + ctx.model = 'gpt-5.6-terra'; ctx.fallbackModel = 'gpt-6-astra'; + const r = await runReview(ctx); + check('a fallback of the parked model\'s own family is used, whatever the family', + r.verdict === 'APPROVE' && calls.sleeps === 0 && calls.model.map((b) => b.model).join() === 'gpt-5.6-terra,gpt-6-astra'); + } { const unroutable = [400, { 'x-dario-upstream-rejection': 'model_unroutable' }]; { const { ctx, calls } = world({ modelStatus: [200, unroutable], turns: [use('redline_list', {}), submit(APPROVE)] }); - ctx.model = 'gpt-6-astra'; ctx.fallbackModel = 'claude-opus-5-5'; + ctx.model = 'gpt-6-astra'; ctx.fallbackModel = 'gpt-5.6-terra'; const lines = []; ctx.log = (l) => lines.push(l); const r = await runReview(ctx); check('a model dario stops listing mid-review is not retried: the turn goes again on the fallback', - r.verdict === 'APPROVE' && calls.sleeps === 0 && calls.model.map((b) => b.model).join() === 'gpt-6-astra,gpt-6-astra,claude-opus-5-5' + r.verdict === 'APPROVE' && calls.sleeps === 0 && calls.model.map((b) => b.model).join() === 'gpt-6-astra,gpt-6-astra,gpt-5.6-terra' && JSON.stringify(calls.model[2].messages) === JSON.stringify(calls.model[1].messages)); - check('the switch is logged with the reason', lines.some((l) => l.includes('no provider in dario lists gpt-6-astra; the review continues on claude-opus-5-5'))); + check('the switch is logged with the reason', lines.some((l) => l.includes('no provider in dario lists gpt-6-astra; the review continues on gpt-5.6-terra'))); + } + { + const { ctx, calls } = world({ modelStatus: [200, unroutable], turns: [use('redline_list', {}), submit(APPROVE)] }); + ctx.model = 'gpt-6-astra'; ctx.fallbackModel = 'claude-opus-5-5'; + const lines = []; + ctx.log = (l) => lines.push(l); + const e = await throws(() => runReview(ctx)); + check('an unroutable model does not hand the review to a fallback of another family either', + e instanceof reviewModule.ModelUnroutable && calls.model.map((b) => b.model).join() === 'gpt-6-astra,gpt-6-astra' && calls.sleeps === 0 && calls.posted.length === 0); + check('the refusal is logged with the reason', + lines.some((l) => l.includes('no provider in dario lists gpt-6-astra; the fallback claude-opus-5-5 is of another model family, so it is not used and the review stops'))); } { const { ctx, calls } = world({ modelStatus: [unroutable], turns: [submit(APPROVE)] });