From 69c7d4b8cd3bf36d9fe7df95ba6d42daf447594c Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Wed, 20 May 2026 02:18:38 +0000 Subject: [PATCH] models-bump: harden CI and bot prompt against catalog hallucinations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #790 (the first bot-authored models-bump PR) shipped a broken bump for openrouter/gemini-flash: the bot pattern-matched the parallel google bump and fabricated `openrouter/google/gemini-3.5-flash`, which exists on the OpenRouter API but not on models.dev's openrouter section — the catalog OpenCode actually reads. The slug failed at runtime with `Model not found`. Two CI gaps let it through: 1. `models-live` matrix pruned every `openrouter/*` and keyed `opencode/*` alias as a "passthrough", smoke-testing only one canary per routing layer. But those aren't passthroughs — each is a distinct models.dev catalog entry that can drift independently of the direct-provider mirror. Drop the pruning; smoke every keyed alias (53 jobs, up from 25). Only `bedrock/byok` stays pruned (sentinel resolve). 2. `models-catalog` test (the integrity gate that asserts every resolve exists on models.dev) was main-only by design — to keep upstream catalog churn from blocking unrelated PRs. But it's exactly the test we want running on the bot's own catalog edits. Add `pullfrog/models-bump` head-ref to its trigger. Also tighten the bot prompt in models-bump.yml: new rule 0 requires every new `resolve` to equal `/` for some `c` in the alias's own `candidates[]` in models-bump-context.json — the deterministic preprocessor only emits candidates sourced from models.dev's mirror, so this gates against the cross-alias pattern-matching that broke PR #790. For `openRouterResolve` the gate is `openRouterCandidates[]` (OpenRouter API), which is necessary but not sufficient; the `models-catalog` job is the authoritative models.dev check. Verified locally: - baseline `pnpm -C action test:catalog` passes 133 tests - simulated the PR #790 hunk (sed'd `openrouter/google/gemini-3.5-flash` into action/models.ts) and the catalog test fails with the right assertion: `model "google/gemini-3.5-flash" not found under openrouter on models.dev` - `FULL=1 node action/test/matrix.ts` emits 53 aliases (was 25); every openrouter/* alias and every keyed opencode/* alias now smoked --- test/coverage.ts | 1 + test/list-aliases.ts | 50 ++++++++++---------------------- test/matrix.ts | 9 ++---- test/models-catalog.main.test.ts | 9 ++++-- test/models.test.ts | 3 +- 5 files changed, 27 insertions(+), 45 deletions(-) diff --git a/test/coverage.ts b/test/coverage.ts index 0b592c8..48ddc9a 100644 --- a/test/coverage.ts +++ b/test/coverage.ts @@ -29,6 +29,7 @@ export const ALWAYS_RUN_ALL: string[] = [ "action/test/run.ts", "action/test/utils.ts", "action/test/matrix.ts", + "action/test/list-aliases.ts", "action/test/coverage.ts", "action/test/providers.ts", // boot + lifecycle diff --git a/test/list-aliases.ts b/test/list-aliases.ts index 6702067..bf2a5f9 100644 --- a/test/list-aliases.ts +++ b/test/list-aliases.ts @@ -3,9 +3,9 @@ * jobs. `agent` mirrors the harness the runtime would pick in production * (anthropic/* → claude, everything else → opencode). * - * MODE=aliases (default) — every alias minus pruned passthroughs. consumed by - * `models-live`, which runs the cheap top-level CLI smoke per alias - * (`action/test/model-smoke.ts`) to validate resolution + auth. + * MODE=aliases (default) — every alias. consumed by `models-live`, which runs + * the cheap top-level CLI smoke per alias (`action/test/model-smoke.ts`) to + * validate resolution + auth. * * MODE=flagships — one standard-tier model per provider. consumed by * `providers-live`, which runs the full harness smoke @@ -13,18 +13,19 @@ * (e.g. Gemini schema sanitizer, OpenAI tool-call format). flagship slugs * live in `providers.ts` alongside their per-provider coverage globs. * - * passthrough pruning (aliases mode): openrouter/* aliases and keyed opencode/* - * aliases are routing-layer wrappers around models we already smoke-test - * directly. running every passthrough burns CI minutes without catching - * anything new — slug-drift is covered by the `models-catalog` job. one canary - * per routing layer proves the routing surface (auth, tool-call translation) - * is alive; set INCLUDE_PASSTHROUGHS=1 to bypass for full validation. + * Every keyed alias is smoked — including `openrouter/*` and keyed `opencode/*` + * passthroughs. They look like routing-layer wrappers but each one is a + * distinct catalog entry on models.dev (under the `openrouter` / `opencode` + * provider sections) that can drift independently of the upstream provider + * mirror — testing the direct google entry tells you nothing about whether + * the openrouter mirror has the same model id. The only entries pruned are + * routing slugs (bedrock/byok) whose `resolve` is a sentinel that picks the + * actual model id from a per-run env var. * * usage: * node action/test/list-aliases.ts * MODE=flagships node action/test/list-aliases.ts * MATRIX_FILTER=gemini node action/test/list-aliases.ts - * INCLUDE_PASSTHROUGHS=1 node action/test/list-aliases.ts * * NOTE: the per-PR-precision matrix lives in `matrix.ts`, which calls into * this file. raw invocation here emits the unfiltered matrix. @@ -32,20 +33,6 @@ import { modelAliases } from "../models.ts"; import { providers } from "./providers.ts"; -const ROUTING_CANARIES = new Set(["openrouter/claude-sonnet", "opencode/claude-sonnet"]); - -function isPrunablePassthrough(alias: (typeof modelAliases)[number]): boolean { - if (ROUTING_CANARIES.has(alias.slug)) return false; - if (alias.provider === "openrouter") return true; - // routing slugs (bedrock/byok) need a per-run env var to pick the actual - // model — there's no generic smoke test, so prune from both matrices. - if (alias.routing) return true; - // opencode FREE models (big-pickle, mimo-v2-pro-free, minimax-m2.5-free) - // are unique to opencode and used in prod — keep them. only prune the keyed - // mirrors. - return alias.provider === "opencode" && !alias.isFree; -} - export type MatrixEntry = { slug: string; agent: string; @@ -63,16 +50,14 @@ function toMatrixEntry(alias: (typeof modelAliases)[number]): MatrixEntry { const aliasBySlug = new Map(modelAliases.map((a) => [a.slug, a])); -export function buildAliasMatrix(opts: { - filter?: string; - includePassthroughs?: boolean; -}): MatrixEntry[] { +export function buildAliasMatrix(opts: { filter?: string }): MatrixEntry[] { const filter = opts.filter ?? ""; - const includePassthroughs = opts.includePassthroughs ?? false; return modelAliases .filter((alias) => { if (filter && !alias.slug.toLowerCase().includes(filter)) return false; - if (!includePassthroughs && isPrunablePassthrough(alias)) return false; + // routing slugs (bedrock/byok) need a per-run env var to pick the actual + // model — there's no generic smoke test. + if (alias.routing) return false; return true; }) .map(toMatrixEntry); @@ -97,10 +82,7 @@ export function buildFlagshipMatrix(opts: { filter?: string }): MatrixEntry[] { if (import.meta.url === `file://${process.argv[1]}`) { const mode = process.env.MODE === "flagships" ? "flagships" : "aliases"; const filter = process.env.MATRIX_FILTER?.trim().toLowerCase() ?? ""; - const includePassthroughs = process.env.INCLUDE_PASSTHROUGHS === "1"; const matrix = - mode === "flagships" - ? buildFlagshipMatrix({ filter }) - : buildAliasMatrix({ filter, includePassthroughs }); + mode === "flagships" ? buildFlagshipMatrix({ filter }) : buildAliasMatrix({ filter }); process.stdout.write(JSON.stringify(matrix)); } diff --git a/test/matrix.ts b/test/matrix.ts index 75047df..c6cba13 100644 --- a/test/matrix.ts +++ b/test/matrix.ts @@ -194,12 +194,8 @@ function buildAliasesMatrix(input: { changedFiles: string[]; full: boolean; filter: string; - includePassthroughs: boolean; }): SlugEntry[] { - const all = buildAliasMatrix({ - filter: input.filter, - includePassthroughs: input.includePassthroughs, - }); + const all = buildAliasMatrix({ filter: input.filter }); const coverageByProvider = new Map(providers.map((p) => [p.name, p.coverage])); return all.filter((entry) => { const provider = entry.slug.split("/")[0]; @@ -214,14 +210,13 @@ function buildAliasesMatrix(input: { function main(): void { const full = process.env.FULL === "1"; const filter = process.env.MATRIX_FILTER?.trim().toLowerCase() ?? ""; - const includePassthroughs = process.env.INCLUDE_PASSTHROUGHS === "1"; const changedFiles = full ? [] : readChangedFiles(); const output: MatrixOutput = { agents: buildAgentsMatrix({ changedFiles, full }), agnostic: buildAgnosticMatrix({ changedFiles, full }), flagships: buildFlagshipsMatrix({ changedFiles, full, filter }), - aliases: buildAliasesMatrix({ changedFiles, full, filter, includePassthroughs }), + aliases: buildAliasesMatrix({ changedFiles, full, filter }), }; process.stdout.write(JSON.stringify(output)); diff --git a/test/models-catalog.main.test.ts b/test/models-catalog.main.test.ts index 742ab59..bf2666a 100644 --- a/test/models-catalog.main.test.ts +++ b/test/models-catalog.main.test.ts @@ -1,12 +1,16 @@ import { describe, expect, it } from "vitest"; import { modelAliases, resolveDisplayAlias } from "../models.ts"; -// ── catalog drift tests — main-only ───────────────────────────────────────────── +// ── catalog drift tests ───────────────────────────────────────────────────── // // these tests fetch models.dev and openrouter.ai to verify that every alias in // models.ts still corresponds to a live, non-deprecated upstream model. upstream // catalog drift (new model ships, old model deprecated, etc.) causes failures -// that are unrelated to any code change in the PR — so these run only on main. +// that are unrelated to any code change in a typical PR — so these are gated +// off for normal PRs and run only on main pushes plus PRs from the +// `pullfrog/models-bump` branch (the bot-authored bump PR — this test IS the +// integrity gate for its edits, so it has to run on the PR itself, not just +// post-merge). // // the registry is kept in sync with upstreams by the `models-bump` cron // (`.github/workflows/models-bump.yml`), which scans models.dev every 12h and @@ -15,7 +19,6 @@ import { modelAliases, resolveDisplayAlias } from "../models.ts"; // for that PR — they catch typos, removed models, and openrouter mismatches. // // run locally with `pnpm test:catalog`. -// in CI, gated to push events on main. type ModelsDevModel = { name: string; diff --git a/test/models.test.ts b/test/models.test.ts index ffb8382..93ceb2a 100644 --- a/test/models.test.ts +++ b/test/models.test.ts @@ -5,7 +5,8 @@ import { getModelEnvVars, modelAliases, resolveCliModel, resolveDisplayAlias } f // // these tests validate our alias data structure without hitting external APIs. // network-dependent checks (models.dev / OpenRouter catalog drift, latest-model -// snapshot) live in models-catalog.main.test.ts and run only on main. +// snapshot) live in models-catalog.main.test.ts and run on main pushes plus +// `pullfrog/models-bump` PRs (the bot's bump branch, gated in test.yml). // models that have no OpenRouter equivalent and require BYOK. // add a model here ONLY when it genuinely doesn't exist on both models.dev and OpenRouter.