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)] });