Skip to content

Commit 76a396f

Browse files
committed
fix(model): make the model-data drift check unable to pass without checking
The drift mechanism had three ways to report success without having verified anything, and the function holding every hard contract had no test coverage. - Remove the fail-open CLI guard. `isInvokedDirectly()` compared `process.argv[1]` against `import.meta.url`; on any invocation where they differ, `main()` never ran and the process exited 0 having done nothing. The symlink fix addressed one instance, not the class. The generator module is now import-pure and `scripts/sync-model-limits-cli.mjs` calls `main()` unconditionally. Verified through a symlink, an absolute path from another cwd, and `npm exec`. - The CI step now captures stdout and requires the run summary line, so "exited 0 having done nothing" fails instead of passing for free. - An empty price cell no longer becomes $0. `parseDocsTable` leaves an absent cell unset (distinct from present-but-empty) and throws when a requested column is missing from a row; `parsePrice("")` throws. `"-"` still means $0, which is what Cursor documents it as. - `generate()` takes injectable `modelIds`/`overrides`, so strict overrides, ambiguity on both tables, docs-over-override precedence and the deterministic sort are exercised against the fixtures rather than resting on one-time manual probes. `normalizeForComparison` is covered too: it is what keeps `--check` from failing daily. - Matching consults the `Provider` column. `claude`/`gpt` are dropped from both sides to survive word-order differences, which also discarded vendor identity; an id naming a dropped vendor now matches only a row whose Provider agrees. Counts unchanged: 29 exact / 0 ambiguous on context, 27 exact / 6 overridden on pricing. - Fixtures gain the verbatim `Claude Opus 4.7 (fast mode)` row ($30/$150, 6x Opus's rate) and assert `claude-opus-4-7` refuses it. - Write-mode I/O failure exits 2, not 1: a failed write does not establish that the committed file is stale. - `Synced:` relabelled `Data last changed:`. Write mode short-circuits when only the date would move, so the committed date was never a verification date and read as though the file were a year stale. - `schedule`/`workflow_dispatch` are workflow-wide triggers, so `build` and `integration` are now scoped to push/pull_request. - `SOURCES.context` points at the URL that currently serves markdown. The `.md` form began returning HTTP 404, which made every run exit 2 — the scheduled job's exit-2-is-a-warning branch would have swallowed that indefinitely. No emitted value changes. Zero numeric values change in the regenerated `src/model-limits.ts`: key sets, key order, `DEFAULT_*`, `MODEL_OUTPUT_LIMITS` and all three `resolve*` functions are byte-identical. The only diff is the header. Tests 416 -> 442. Each new test was mutation-checked against the pre-change behaviour to confirm it can fail.
1 parent e75308e commit 76a396f

9 files changed

Lines changed: 561 additions & 58 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
name: CI
22

3+
# `schedule` and `workflow_dispatch` exist for the model-data-drift job only.
4+
# Triggers are workflow-wide in Actions, so every job below carries an `if:`
5+
# that scopes it to the events it is actually for.
36
on:
47
push:
58
branches: ["**"]
@@ -14,6 +17,9 @@ permissions:
1417
jobs:
1518
build:
1619
name: typecheck · test · build (node ${{ matrix.node-version }})
20+
# Code CI only. The weekly cron and manual dispatch exist for
21+
# model-data-drift; re-running the matrix on them buys nothing.
22+
if: github.event_name == 'push' || github.event_name == 'pull_request'
1723
runs-on: ubuntu-latest
1824
strategy:
1925
fail-fast: false
@@ -68,9 +74,10 @@ jobs:
6874

6975
- name: Check committed model limits against Cursor docs
7076
run: |
77+
log="$RUNNER_TEMP/drift.log"
7178
set +e
72-
npm run sync:model-limits -- --check
73-
code=$?
79+
npm run sync:model-limits -- --check 2>&1 | tee "$log"
80+
code=${PIPESTATUS[0]}
7481
set -e
7582
if [ "$code" = "1" ]; then
7683
echo "::error::src/model-limits.ts is stale. Run 'npm run sync:model-limits' and commit."
@@ -80,10 +87,23 @@ jobs:
8087
echo "::warning::Could not verify model data (docs unreachable or unparseable)."
8188
exit 0
8289
fi
83-
exit "$code"
90+
if [ "$code" != "0" ]; then
91+
echo "::error::Unexpected exit code $code from the drift check."
92+
exit "$code"
93+
fi
94+
# Exit 0 is only trustworthy if the run also reported its summary. A
95+
# bare 0 is what "did nothing at all" looks like, and that has
96+
# happened: an entry-point guard once skipped main() entirely and the
97+
# job passed for free. Demand the evidence, not just the code.
98+
if ! grep -qE 'sync-model-limits: src/model-limits\.ts is up to date \(context: [0-9]+ from docs, [0-9]+ overridden \| cost: [0-9]+ from docs, [0-9]+ overridden \| [0-9]+ model ids\)' "$log"; then
99+
echo "::error::Drift check exited 0 without printing a run summary — it did not actually verify anything."
100+
exit 1
101+
fi
84102
85103
integration:
86104
name: e2e · opencode loads plugin & lists models
105+
# Code CI only, same reason as `build`.
106+
if: github.event_name == 'push' || github.event_name == 'pull_request'
87107
runs-on: ubuntu-latest
88108
steps:
89109
- uses: actions/checkout@v7

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
"test": "vitest run",
5454
"test:watch": "vitest",
5555
"test:e2e": "vitest run --config vitest.e2e.config.ts --passWithNoTests",
56-
"sync:model-limits": "node scripts/sync-model-limits.mjs",
56+
"sync:model-limits": "node scripts/sync-model-limits-cli.mjs",
5757
"prepublishOnly": "npm run typecheck && npm test && npm run build"
5858
},
5959
"dependencies": {

‎scripts/sync-model-limits-cli.mjs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/usr/bin/env node
2+
/**
3+
* CLI entry point for `scripts/sync-model-limits.mjs`.
4+
*
5+
* This file exists so the generator module stays import-pure (the tests import
6+
* it) without needing an "am I the entry point?" guard inside it. Such a guard
7+
* — comparing `process.argv[1]` against `import.meta.url` — is fail-open: on
8+
* any invocation where the two differ (a symlinked path, a wrapper, an exec
9+
* shim) `main()` never runs and the process exits 0 having done nothing, which
10+
* makes the scheduled drift check permanently green and permanently useless.
11+
* That already happened once, via a symlinked `/tmp` path on macOS.
12+
*
13+
* There is no guard here. Running this file always runs `main()`.
14+
*/
15+
import { main } from "./sync-model-limits.mjs";
16+
17+
process.exitCode = await main(process.argv.slice(2));

‎scripts/sync-model-limits.d.mts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,17 @@
22
* Types for `sync-model-limits.mjs`. The script is plain ESM JavaScript (it
33
* runs via `node` with no build step), but `tsconfig.json` includes `test`, so
44
* `test/sync-model-limits.test.ts` needs a declaration to import it.
5+
*
6+
* This mirror is hand-maintained. `allowJs: true` would remove the need for it,
7+
* but it does not work in this repo: it pulls `src/sidecar/agent-host.mjs` into
8+
* the program, and that file assigns to `console.log`, which strips `log`,
9+
* `debug`, `info`, `warn`, and `error` off the global `Console` type and breaks
10+
* 30 checks in existing `.ts` files. Measured, not assumed — see
11+
* `.superpowers/sdd/task-6-report.md`. Two things keep this file honest in the
12+
* meantime: `test/sync-model-limits.test.ts` asserts the module's runtime
13+
* export names match the list declared here, and the tests call every declared
14+
* signature, so a parameter that is declared but missing (or vice versa) fails
15+
* `npm run typecheck`.
516
*/
617

718
export type DocsRow = Record<string, string>;
@@ -29,10 +40,17 @@ export declare function parseTokens(text: string): number | undefined;
2940
export declare function parsePrice(text: string): number;
3041
export declare function matchModelId(id: string, docRows: DocsRow[]): MatchResult | undefined;
3142
export declare function normalizeForComparison(text: string): string;
32-
export declare function generate(input: { contextMd: string; pricingMd: string; date?: string }): {
43+
export declare function generate(input: {
44+
contextMd: string;
45+
pricingMd: string;
46+
modelIds?: readonly string[];
47+
overrides?: Record<string, { context?: number; cost?: ModelCost; why?: string }>;
48+
date?: string;
49+
}): {
3350
text: string;
3451
stats: {
3552
context: { matched: number; overridden: number };
3653
cost: { matched: number; overridden: number };
3754
};
3855
};
56+
export declare function main(argv: string[]): Promise<number>;

0 commit comments

Comments
 (0)