Skip to content

Commit 961a3d7

Browse files
rafaelgilnRafaelclaude
authored
ci(pr-validation): pin the PR lane to one provider's settled model (#1170)
getTestTargets() parametrizes ~17 agent specs over one model per ACTIVE provider, so every impacted LLM spec ran an openai AND an anthropic AND a google variant on this lane. Multi-provider coverage is daily-stable.yml's job; this lane answers "does this PR break the specs it touches", which one provider settles. The cost was not theoretical. Measured 2026-07-31: pr-validation ran 141 times in 3.5 days (37/49/49/6 per day), each model-needing run paying an anthropic variant on claude-sonnet-5 ($3/$15 per MTok) for assertions gpt-4o-mini ($0.15/$0.60) satisfies identically -- 20-25x per token, and with no prompt caching on the anthropic side, since Langflow sets no cache_control and every agent turn re-sends the system prompt and tool schemas at full price. Both the CI secret and the local .env key drained inside that window; Anthropic credit is account-scoped, so they share one balance and this lane was the volume driver. Why a script rather than two `env:` lines -- both failure modes read as success in the log: - MODEL_TEST_PROVIDER on its own does not narrow the run, it WIDENS it: the provider branch of getTestTargets() skips the first-per-provider dedup and runs every model that provider exposes (41 openai entries in the catalog collected 2026-07-30). The two variables are a pair, and the pair invariant is asserted end-to-end against the CLI. - The model has to be the one collect-models settled on. A hardcoded id fails silently the day the CI project loses access to it: getTestTargets() warns "not found in models.json", returns a target with no provider, the spec skips, and the PR still reads green -- the silent-skip failure #570 and #1012 exist to prevent. providers.json already records what probed successfully. When the provider is not active the script DECLINES to pin, emits a ::warning:: naming what collect-models reported, and leaves the lane on its existing per-provider parametrization -- deliberately falling back to the costlier path, because a check that runs nothing is worth less than one that costs more (#980's trade). A payload it cannot read at all is exit 2 rather than a quiet no-pin (#1035). Covered by npm run test:scripts: the pin, both decline paths, the unreadable-payload error, the CLI pair invariant, and a structural guard that the step still sits between the health gate and the specs run (both force-failed locally). The classifier verdict for this diff is `canary`, so this PR's own CI boots Langflow and walks pre-flight -> health gate -> pin -> Playwright for real. Co-authored-by: Rafael <rafael@oriontech.me> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2b3f1f9 commit 961a3d7

3 files changed

Lines changed: 480 additions & 0 deletions

File tree

.github/workflows/pr-validation.yml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,34 @@ jobs:
566566
# here is a HARD gate (not `continue-on-error`), so a failed sweep has
567567
# already reddened the job and there is no surviving run to warn about.
568568

569+
# Pin this lane to ONE provider's settled model (#1169). getTestTargets()
570+
# parametrizes over one model per active provider, so every impacted agent
571+
# spec runs an openai AND an anthropic AND a google variant here. That is
572+
# daily-stable.yml's job — it is the lane that owes multi-provider coverage.
573+
# This lane answers "does this PR break the specs it touches", which one
574+
# provider settles, and the difference is not cosmetic: measured 2026-07-31,
575+
# this workflow ran 141 times in 3.5 days, each model-needing run paying an
576+
# anthropic variant on claude-sonnet-5 ($3/$15 per MTok) for assertions
577+
# gpt-4o-mini ($0.15/$0.60) satisfies identically — 20-25x per token, with no
578+
# prompt caching on the anthropic side. Both the CI secret and the local key
579+
# drained inside that window (Anthropic credit is account-scoped, so they
580+
# share one balance).
581+
#
582+
# Why a script (see its header for the full argument): MODEL_TEST_PROVIDER
583+
# alone does not narrow the run, it runs the provider's ENTIRE catalog (the
584+
# dedup branch is skipped) — so the two variables must be emitted as a pair,
585+
# which this does. And the model must be the one collect-models settled on:
586+
# a hardcoded id skips silently the day the CI project loses access, leaving
587+
# a green PR that tested nothing (#570/#1012).
588+
#
589+
# Declines to pin — with a ::warning:: — when the provider is not active, so
590+
# a drained openai key costs a costlier multi-provider run rather than zero
591+
# coverage. Gated on needs_models for the same reason as the two steps above:
592+
# with no sweep there is no settled model to read.
593+
- name: Pin the lane to a single provider's settled model
594+
if: needs.detect-specs.outputs.needs_models == 'true'
595+
run: node scripts/select-pr-model-target.mjs --provider openai
596+
569597
- name: Run impacted specs
570598
env:
571599
CI: "true"

scripts/select-pr-model-target.mjs

Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
#!/usr/bin/env node
2+
/**
3+
* Picks the single model target the **PR lane** should run its LLM specs against,
4+
* out of what `collect-models` actually settled on (#1169).
5+
*
6+
* ## Why the PR lane pins one provider at all
7+
*
8+
* `getTestTargets()` (~17 agent specs) parametrizes over **one model per active
9+
* provider**, so every LLM spec selected by the impacted-specs job runs once per
10+
* provider whose key is in the repo secrets — openai *and* anthropic *and* google.
11+
* That is the right shape for `daily-stable.yml`, which is the lane that owes
12+
* multi-provider coverage. It is the wrong shape here, and the cost is not
13+
* theoretical: measured 2026-07-31, `pr-validation.yml` ran **141 times in
14+
* 3.5 days** (37/49/49/6 per day), each model-needing run paying an anthropic
15+
* variant of every impacted agent spec — on `claude-sonnet-5` at $3/$15 per MTok
16+
* against `gpt-4o-mini` at ~$0.15/$0.60, i.e. **20-25x the price per token** for
17+
* the same assertions, with no prompt caching on the anthropic side (Langflow
18+
* sets no `cache_control`, so every turn re-sends the agent prompt and tool
19+
* schemas at full price). Both the CI secret and the local `.env` key drained
20+
* inside that window — Anthropic credit is **account-scoped**, so the two share
21+
* one balance and the PR lane was the volume driver.
22+
*
23+
* So: the PR lane answers "does this PR break the specs it touches", which one
24+
* provider settles. The daily keeps answering "does it break on every provider".
25+
*
26+
* ## Why this is a script and not two `env:` lines
27+
*
28+
* Two reasons, both of which have already bitten this repo:
29+
*
30+
* 1. **`MODEL_TEST_PROVIDER` alone is a trap, not a filter.** In
31+
* `getTestTargets()` the `MODEL_TEST_PROVIDER` branch filters the catalog by
32+
* provider and **skips the first-per-provider dedup**, so setting it without
33+
* `MODEL_TEST_ID` runs *every* model that provider exposes — 41 openai entries
34+
* in the catalog collected 2026-07-30. That turns a cost fix into a 41x cost
35+
* regression. The two variables are a **pair**; this script never emits one
36+
* without the other.
37+
* 2. **The model has to be the settled one.** Hardcoding `gpt-4o-mini` in YAML
38+
* fails silently the day it retires or the CI project loses access:
39+
* `getTestTargets()` warns `MODEL_TEST_ID="…" not found in models.json` and
40+
* returns a target with no provider, so the spec skips and the PR still reads
41+
* green — the exact silent-skip failure #570 and #1012 exist to prevent.
42+
* `providers.json` already records what `collect-models` probed successfully;
43+
* read that.
44+
*
45+
* ## When it declines to pin
46+
*
47+
* If the chosen provider is not `active` (drained key, dead credential), pinning
48+
* to it would make every parametrized spec skip — trading spend for **zero**
49+
* coverage. The decision then is `ok: false`, the lane keeps its existing
50+
* multi-provider behaviour, and the reason is printed as a `::warning::`. That is
51+
* a deliberate fallback to the *more expensive* path, because a PR check that
52+
* runs nothing is worth less than one that costs more (#980's trade: a provider
53+
* outage must not silently erode coverage). A payload it cannot read at all is a
54+
* hard error (exit 2) rather than a quiet fallback — #1035's rule.
55+
*
56+
* Run:
57+
* node scripts/select-pr-model-target.mjs \
58+
* --providers-file tests/helpers/provider-setup/data/providers.json \
59+
* --provider openai
60+
*
61+
* Output (stdout, JSON): { ok, provider, model, reason, warnings }
62+
* Side effect: appends `MODEL_TEST_ID` / `MODEL_TEST_PROVIDER` to `$GITHUB_ENV`
63+
* when it pins and that variable is set.
64+
*/
65+
66+
import fs from "node:fs";
67+
68+
const HELP = `usage: select-pr-model-target.mjs [options]
69+
70+
--providers-file PATH providers.json written by collect-models
71+
(default: tests/helpers/provider-setup/data/providers.json)
72+
--provider NAME provider to pin the lane to (default: openai)
73+
`;
74+
75+
const DEFAULT_PROVIDERS_FILE =
76+
"tests/helpers/provider-setup/data/providers.json";
77+
78+
/**
79+
* @param {unknown} providers parsed `providers.json` payload
80+
* @param {{ provider?: string }} [options]
81+
* @returns {{ ok: boolean, provider: string, model: string|null, reason: string|null, warnings: string[] }}
82+
* @throws {Error} when the payload is not a readable provider record list
83+
*/
84+
export function selectPrModelTarget(providers, options = {}) {
85+
const provider = options.provider ?? "openai";
86+
87+
if (!Array.isArray(providers)) {
88+
throw new Error(
89+
`providers.json must be an array of provider records, got ${
90+
providers === null ? "null" : typeof providers
91+
}`,
92+
);
93+
}
94+
95+
for (const [i, record] of providers.entries()) {
96+
if (record === null || typeof record !== "object") {
97+
throw new Error(
98+
`providers.json[${i}] must be an object, got ${
99+
record === null ? "null" : typeof record
100+
}`,
101+
);
102+
}
103+
if (typeof record.provider !== "string" || record.provider === "") {
104+
throw new Error(`providers.json[${i}] has no "provider" name`);
105+
}
106+
if (typeof record.status !== "string" || record.status === "") {
107+
throw new Error(
108+
`providers.json[${i}] ("${record.provider}") has no "status"`,
109+
);
110+
}
111+
}
112+
113+
const record = providers.find((r) => r.provider === provider);
114+
if (!record) {
115+
return {
116+
ok: false,
117+
provider,
118+
model: null,
119+
reason:
120+
`provider "${provider}" is absent from providers.json (present: ` +
121+
`${providers.map((r) => r.provider).join(", ") || "none"}) — ` +
122+
`leaving the lane on its default per-provider parametrization`,
123+
warnings: [],
124+
};
125+
}
126+
127+
if (record.status !== "active") {
128+
return {
129+
ok: false,
130+
provider,
131+
model: null,
132+
reason:
133+
`provider "${provider}" probed "${record.status}" — pinning the lane to ` +
134+
`it would skip every parametrized spec, so the lane keeps its default ` +
135+
`per-provider parametrization (costlier, but it covers something). ` +
136+
`collect-models reported: ${record.error ?? "no error message"}`,
137+
warnings: [],
138+
};
139+
}
140+
141+
if (typeof record.model !== "string" || record.model === "") {
142+
throw new Error(
143+
`provider "${provider}" is active but carries no settled "model" — ` +
144+
`providers.json is inconsistent`,
145+
);
146+
}
147+
148+
return {
149+
ok: true,
150+
provider,
151+
model: record.model,
152+
reason: null,
153+
warnings: [],
154+
};
155+
}
156+
157+
/**
158+
* Reads the providers file. A missing file is a legitimate state (the sweep was
159+
* skipped, or ran `continue-on-error` on a canary), not a crash.
160+
* @returns {{ providers: unknown, missing: boolean }}
161+
*/
162+
export function readProvidersFile(providersFile, { readFile, exists } = {}) {
163+
const fileExists = exists ?? ((p) => fs.existsSync(p));
164+
const read = readFile ?? ((p) => fs.readFileSync(p, "utf-8"));
165+
166+
if (!fileExists(providersFile)) return { providers: null, missing: true };
167+
return { providers: JSON.parse(read(providersFile)), missing: false };
168+
}
169+
170+
function parseArgs(argv) {
171+
const args = { providersFile: DEFAULT_PROVIDERS_FILE, provider: "openai" };
172+
for (let i = 0; i < argv.length; i++) {
173+
const flag = argv[i];
174+
if (flag === "--help" || flag === "-h") {
175+
args.help = true;
176+
continue;
177+
}
178+
const value = argv[i + 1];
179+
if (value === undefined) throw new Error(`missing value for ${flag}`);
180+
i++;
181+
if (flag === "--providers-file") args.providersFile = value;
182+
else if (flag === "--provider") args.provider = value;
183+
else throw new Error(`unknown flag: ${flag}`);
184+
}
185+
return args;
186+
}
187+
188+
if (import.meta.url === `file://${process.argv[1]}`) {
189+
let args;
190+
try {
191+
args = parseArgs(process.argv.slice(2));
192+
} catch (error) {
193+
process.stderr.write(`::error::select-pr-model-target: ${error.message}\n`);
194+
process.exit(2);
195+
}
196+
if (args.help) {
197+
process.stdout.write(HELP);
198+
process.exit(0);
199+
}
200+
201+
let result;
202+
try {
203+
const { providers, missing } = readProvidersFile(args.providersFile);
204+
result = missing
205+
? {
206+
ok: false,
207+
provider: args.provider,
208+
model: null,
209+
reason:
210+
`${args.providersFile} does not exist — collect-models did not write ` +
211+
`it, so there is no settled model to pin to`,
212+
warnings: [],
213+
}
214+
: selectPrModelTarget(providers, { provider: args.provider });
215+
} catch (error) {
216+
// Fail loud: a payload this cannot read must not read as "nothing to pin"
217+
// (#1035). The lane is expected to fail here rather than quietly pay for a
218+
// multi-provider run it did not choose.
219+
process.stderr.write(`::error::select-pr-model-target: ${error.message}\n`);
220+
process.exit(2);
221+
}
222+
223+
if (result.ok) {
224+
// Both variables, always together — MODEL_TEST_PROVIDER on its own makes
225+
// getTestTargets skip the per-provider dedup and run the whole catalog.
226+
const lines = [
227+
`MODEL_TEST_ID=${result.model}`,
228+
`MODEL_TEST_PROVIDER=${result.provider}`,
229+
];
230+
if (process.env.GITHUB_ENV) {
231+
fs.appendFileSync(process.env.GITHUB_ENV, `${lines.join("\n")}\n`);
232+
}
233+
process.stderr.write(
234+
`PR lane pinned to ${result.provider} / ${result.model} ` +
235+
`(settled by collect-models); other providers' variants will not run here — ` +
236+
`daily-stable.yml keeps the multi-provider coverage.\n`,
237+
);
238+
} else {
239+
process.stderr.write(`::warning::select-pr-model-target: ${result.reason}\n`);
240+
}
241+
242+
process.stdout.write(`${JSON.stringify(result)}\n`);
243+
process.exit(0);
244+
}

0 commit comments

Comments
 (0)