From 4a0b9f39ffe6db5c9d4f78239881fd1f0a0a0260 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 01:05:06 +0000 Subject: [PATCH 1/2] fix(cli): score a throwing generator as unscorable, not as an empty stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `os lint --eval`'s throwing-generator path substituted `stack = {}` and then scored it. The empty stack is 100 / A / `valid: true`, so a live eval in which every generation threw reported `meanScore: 100` beside `ok: false, passed: 0`. Both failure paths now take the same `unscorableScore()` verdict — 0 / F / `valid: false` — so a case with no stack contributes 0 to the mean instead of a perfect score it never earned. `passed` is untouched; it was already correct. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- packages/cli/src/lint/metadata-eval.ts | 59 ++++++++++++++++++++------ 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/lint/metadata-eval.ts b/packages/cli/src/lint/metadata-eval.ts index 7986fe93ee..f7b4ac2a1b 100644 --- a/packages/cli/src/lint/metadata-eval.ts +++ b/packages/cli/src/lint/metadata-eval.ts @@ -43,6 +43,12 @@ export interface MetadataEvalCaseResult { * the string names the cause. */ generationError?: string; + /** + * The rubric's verdict on the stack. When `generationError` is set there was + * no stack to judge, and this carries the unscorable sentinel — 0 / F / + * `valid: false` — for BOTH causes alike. ⛔ Never a score borrowed from the + * empty stack, which reads 100 / A / `valid: true`. + */ score: MetadataScore; minScore: number; passed: boolean; @@ -55,7 +61,17 @@ export interface MetadataEvalReport { total: number; passed: number; failed: number; - /** Mean score across all cases (0–100, rounded). */ + /** + * Mean score across all cases (0–100, rounded). + * + * The denominator is every case ATTEMPTED, ⛔ not the subset that could be + * scored: a case whose generation failed contributes its 0, and is counted. + * Stated because the alternative is a real metric with a different meaning — + * a mean over scorable cases would report the quality of the generations + * that arrived while staying silent about how many never did, and a + * `meanScore` that silently switched denominators would be a worse defect + * than a wrong one. `total` / `passed` / `failed` carry the counts. + */ meanScore: number; /** True when every case passed. */ ok: boolean; @@ -77,14 +93,21 @@ export interface RunMetadataEvalOptions { const DEFAULT_MIN_SCORE = 75; /** - * The score attached to a case whose stack could not be scored AT ALL. + * The score attached to a case whose stack could not be scored AT ALL — the + * generator threw, or what it returned could not be walked. ONE verdict for + * the whole class, because the class is one: there is no stack to judge. * - * ⛔ Deliberately NOT `scoreMetadata({})`, even though the throwing-generator - * path above substitutes an empty stack: the empty stack scores 100 / A / - * `valid: true` (pinned in `score.test.ts`), and stamping that on a stack - * nobody could parse would put a benign-looking verdict next to a failure. - * A stack that cannot be walked is not an empty stack, and `valid: true` for - * one that was never parsed is simply false. + * ⛔ Deliberately NOT `scoreMetadata({})`. The empty stack scores 100 / A / + * `valid: true` (pinned in `score.test.ts`, re-driven on this tree), so + * substituting it stamps a benign-looking verdict on a failure. A stack that + * cannot be walked is not an empty stack, and `valid: true` for one that was + * never parsed is simply false. + * + * That substitution is exactly what the throwing-generator path below used to + * do, which is how a live eval whose every generation threw could report + * `meanScore: 100` beside `ok: false`, `passed: 0` — a clean number nothing + * earned, and the first number a human reads. `passed` was never wrong; the + * `score` under it was. * * ⛔ This is not a measurement and must never be read as one — the case is * already failed by its `generationError`. It exists so `MetadataScore` stays @@ -138,7 +161,9 @@ export async function runMetadataEval( stack = await options.generate!(c.prompt, c.id); } catch (err: any) { generationError = err?.message || String(err); - stack = {}; + // ⛔ No empty-stack substitution here. There is no stack: `stack` + // keeps the fixture and is never read below, because a case that + // reached this line is scored by `unscorableScore()` instead. } } @@ -155,11 +180,19 @@ export async function runMetadataEval( // `ok: false`. Swallowing it into a passing case would be worse than the // crash it replaces. let score: MetadataScore; - try { - score = scoreMetadata(stack); - } catch (err: any) { - generationError = `Failed to score the ${source} stack: ${err?.message || String(err)}`; + if (generationError) { + // The generator threw — nothing was produced, so there is nothing to + // score. Same outcome class as a stack nobody can walk, therefore the + // same verdict: an unscorable case contributes 0 to `meanScore`, never + // the 100 an empty stack would have borrowed. score = unscorableScore(); + } else { + try { + score = scoreMetadata(stack); + } catch (err: any) { + generationError = `Failed to score the ${source} stack: ${err?.message || String(err)}`; + score = unscorableScore(); + } } results.push({ id: c.id, From dac629d195b8f66ae2eeb4e5b3a083b9b353a160 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 01:06:41 +0000 Subject: [PATCH 2/2] test(cli): pin the throwing-generator verdict and the meanScore denominator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit + e2e legs for the repair: a generator that throws answers 0 / F / `valid: false` on the published `--json` face, `meanScore` reads 0 for a run where every generation threw, and the two failure paths are asserted equal. The denominator is pinned deliberately — `meanScore` is a mean over cases ATTEMPTED, so the failed case is a 0 in the numerator AND a 1 in the denominator. A later switch to a scorable-only mean goes red rather than silently changing what the metric means. `passed` / `ok` / `failed` are asserted UNCHANGED in both legs: they were already correct, and a "repair" to them should be red. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- ...lint-eval-throwing-generator-unscorable.md | 32 +++++ ...int-eval-json-unscorable-stack.e2e.test.ts | 41 ++++++ packages/cli/test/metadata-eval.test.ts | 123 ++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 .changeset/lint-eval-throwing-generator-unscorable.md diff --git a/.changeset/lint-eval-throwing-generator-unscorable.md b/.changeset/lint-eval-throwing-generator-unscorable.md new file mode 100644 index 0000000000..83a6bdfb3a --- /dev/null +++ b/.changeset/lint-eval-throwing-generator-unscorable.md @@ -0,0 +1,32 @@ +--- +"@objectstack/cli": patch +--- + +`os lint --eval` no longer scores a failed generation as a perfect one: a generator that throws now counts 0 toward `meanScore` instead of 100. + +The harness has always handled a throwing `--generator` by substituting an empty stack and scoring that. An empty stack is **100 / grade `A` / `valid: true`** — it has nothing wrong with it because it has nothing in it. So a live eval in which every single generation failed reported the best possible headline number: + +``` +os lint --eval --json --generator ./throws.mjs +exit 1 · ok: false · passed: 0 · failed: 5 · meanScore: 100 +every case: score 100 · grade A · valid true · generationError "model unavailable" +``` + +`meanScore` is the first number a human scanning that report reads, and it read perfect precisely when the model under test produced nothing. + +**What was NOT wrong: `passed`.** It carries its own guard (`!generationError && …`), so the failed cases were reported as failed and `ok` was `false` throughout. A reader who cross-read `ok`/`passed` was safe; a reader who checked the mean and moved on got exactly the wrong impression. That is the whole defect, and nothing about `passed`, `ok`, `total`, `failed` or the exit code changes here. + +The repair is the verdict the sibling failure path already used. A generator that *returns* a value nobody can walk was already scored `0 / F / valid: false`, with the reason written into the module: a stack that cannot be walked is not an empty stack, and `valid: true` for one that was never parsed is simply false. A stack that was never produced is not an empty stack either — so both now answer the same: + +```json +{ "id": "invoice_with_line_items", + "generationError": "model unavailable", + "passed": false, + "score": { "score": 0, "grade": "F", "valid": false } } +``` + +and the run above now reports `meanScore: 0`. + +`meanScore`'s denominator is unchanged and is now stated in the payload's own documentation: the mean is over every case **attempted**, so a failed case contributes its 0 and is counted. The alternative — averaging only over cases that could be scored — is a different metric that would report the quality of the generations that arrived while staying silent about how many never did; a `meanScore` that switched denominators without saying so would be a worse defect than the one being fixed. + +No key is added to or removed from the `--json` payload, and nothing a generator can return is newly accepted or rejected: an off-shape stack is still a **scored** case whose schema errors are why it fails, never a generation error. diff --git a/packages/cli/test/lint-eval-json-unscorable-stack.e2e.test.ts b/packages/cli/test/lint-eval-json-unscorable-stack.e2e.test.ts index a19a91be1e..54512eba16 100644 --- a/packages/cli/test/lint-eval-json-unscorable-stack.e2e.test.ts +++ b/packages/cli/test/lint-eval-json-unscorable-stack.e2e.test.ts @@ -39,6 +39,12 @@ * directly, against the specific benign shape a swallow would produce * (`scoreMetadata({})` is 100 / A / `valid: true`). * + * That benign shape was not hypothetical on the OTHER failure path: a + * generator that THREW had the empty stack substituted for it and scored, so + * `meanScore` read 100 on a run where nothing was generated. Both paths now + * answer 0 / F / `valid: false`, and `every generation threw ⇒ meanScore 0` + * pins it on the same published `--json` face. + * * ## Why the negative controls are here * * The reachable class is narrow, and that narrowness is a MEASUREMENT: every @@ -217,6 +223,41 @@ describe('os lint --eval --json — the negative controls still answer the same' expect(payload.results[0].generationError).toBe('model unavailable'); }, 120_000); + /** + * ⭐ The machine face of the defect this file's sibling card names, driven + * here rather than reasoned about. Measured on this entry BEFORE the repair, + * with a generator that throws for every prompt: + * + * exit 1 · ok: false · passed: 0 · failed: 5 · meanScore: 100 + * every case: score 100 · grade A · valid true · generationError set + * + * ⇒ the published `--json` payload's headline number read PERFECT exactly + * when the model under test produced nothing. The throwing path substituted + * an empty stack and scored it, and the empty stack is 100 / A / `valid`. + * + * ⛔ `passed` was never part of it and is asserted here unchanged — the + * report always said `ok: false`, which is what made the 100 survivable + * enough to sit on `main`. + */ + it('⭐ every generation threw ⇒ meanScore 0 on the --json face, never 100', async () => { + const run = await runEval( + generator('throws-all', `export default function () { throw new Error('model unavailable'); }\n`), + ); + const payload = payloadOf(run, 'throwing generator — mean'); + + expect(run.code).toBe(1); + expect(payload.meanScore).toBe(0); + expect(payload.results.every((r) => r.score.score === 0)).toBe(true); + expect(payload.results.every((r) => r.score.grade === 'F')).toBe(true); + expect(payload.results.every((r) => r.score.valid === false)).toBe(true); + + // The half that was already correct, pinned so a repair to it goes red. + expect(payload.ok).toBe(false); + expect(payload.passed).toBe(0); + expect(payload.failed).toBe(payload.total); + expect(payload.results.every((r) => r.passed === false)).toBe(true); + }, 120_000); + it.each([ ['manifest-as-string', `export default () => ({ manifest: 'not-an-object' });\n`], ['objects-as-string', `export default () => ({ objects: 'not-an-array' });\n`], diff --git a/packages/cli/test/metadata-eval.test.ts b/packages/cli/test/metadata-eval.test.ts index b4598b9342..c044c15c54 100644 --- a/packages/cli/test/metadata-eval.test.ts +++ b/packages/cli/test/metadata-eval.test.ts @@ -211,3 +211,126 @@ describe('runMetadataEval — a stack that cannot be scored is a FAILED case, no expect(report.ok).toBe(false); }); }); + +/** + * ⛔ A generator that THREW must not be scored as an empty stack. + * + * The measured before-shape, driven on this tree through the CLI's source + * entry with a generator that throws for every prompt: + * + * os lint --eval --json --generator ./throws.mjs + * exit 1 · ok: false · passed: 0 · failed: 5 · meanScore: 100 + * every case: score 100, grade A, valid true, generationError 'model unavailable' + * + * ⇒ the eval's headline number read PERFECT precisely when the model under + * test produced nothing, and `meanScore` is the first number a human reads. + * + * ## What was wrong, and what was NOT + * + * ⛔ Not `passed`. `passed: !generationError && …` already excluded the case, + * and `ok: passed === results.length` followed it, so the report DID say + * `ok: false`. The wrong value was the `score` stamped on the failed case — + * the throwing path substituted `stack = {}` and scored that, and the empty + * stack is 100 / A / `valid: true`. `meanScore` then summed it. + * + * ## Why one verdict for both failure paths + * + * The sibling path — a generator that RETURNS a value nobody can walk — + * already answered `unscorableScore()` (0 / F / `valid: false`), with the + * reason written into the module: a stack that cannot be walked is not an + * empty stack. A stack that was never produced is not an empty stack either. + * Same outcome class, so the same verdict; one rule in the file, not two that + * disagree. + * + * ## The denominator is pinned here on purpose + * + * The alternative repair — drop failed cases from `meanScore`'s denominator — + * gives DIFFERENT numbers on a run where only some generations threw, and it + * silently changes what the metric means (a mean over scored cases, not over + * attempted ones). `the failed case is counted in the denominator` fails if + * anyone later makes that switch without saying so. + */ +describe('runMetadataEval — a generator that THREW scores 0, not 100', () => { + const oneCase: MetadataEvalCase[] = [ + { id: 'c1', prompt: 'invoice with lines', fixture: { manifest: { id: 'a', namespace: 'aa', version: '1.0.0', name: 'A', type: 'app' } } }, + ]; + const throwingGen = () => { + throw new Error('model unavailable'); + }; + + it('⛔ the failed case is NOT scored as an empty stack', async () => { + // The control that makes the assertion below mean something: this is the + // exact verdict the old `stack = {}` substitution produced. + expect(scoreMetadata({}).score).toBe(100); + expect(scoreMetadata({}).grade).toBe('A'); + expect(scoreMetadata({}).valid).toBe(true); + + const report = await runMetadataEval(oneCase, { generate: throwingGen }); + const only = report.results[0]; + + expect(only.generationError).toBe('model unavailable'); + expect(only.passed).toBe(false); + expect(only.score.score).toBe(0); + expect(only.score.grade).toBe('F'); + expect(only.score.valid).toBe(false); + }); + + it('⭐ an eval whose every generation threw reports meanScore 0, not 100', async () => { + const cases: MetadataEvalCase[] = [ + { ...oneCase[0], id: 'a' }, + { ...oneCase[0], id: 'b' }, + { ...oneCase[0], id: 'c' }, + ]; + const report = await runMetadataEval(cases, { generate: throwingGen }); + + expect(report.meanScore).toBe(0); + // The half that was always right, asserted so a future "fix" to it is red. + expect(report.ok).toBe(false); + expect(report.passed).toBe(0); + expect(report.failed).toBe(3); + }); + + it('both failure paths now agree — thrown and unwalkable score the same', async () => { + const poison = () => ({ + name: 'poison', + get objects(): never { + throw new Error('poison getter'); + }, + }); + + const thrown = (await runMetadataEval(oneCase, { generate: throwingGen })).results[0].score; + const unwalkable = (await runMetadataEval(oneCase, { generate: poison })).results[0].score; + + expect(thrown.score).toBe(unwalkable.score); + expect(thrown.grade).toBe(unwalkable.grade); + expect(thrown.valid).toBe(unwalkable.valid); + }); + + it('the failed case is COUNTED in the denominator, not dropped from it', async () => { + const cases: MetadataEvalCase[] = [ + { ...oneCase[0], id: 'threw' }, + { ...oneCase[0], id: 'clean' }, + ]; + const cleanStack = { + objects: [ + { name: 'invoice', label: 'Invoice', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name', required: true } } }, + ], + }; + const generate = (_prompt: string, id: string) => { + if (id === 'threw') throw new Error('model unavailable'); + return cleanStack; + }; + + const report = await runMetadataEval(cases, { generate }); + const clean = report.results[1].score.score; + + expect(report.results[0].score.score).toBe(0); + expect(clean).toBeGreaterThan(0); + // Mean over cases ATTEMPTED: the failure is a 0 in the numerator and a 1 + // in the denominator. + expect(report.meanScore).toBe(Math.round(clean / 2)); + // ⛔ …and NOT a mean over the scorable subset, which would report the + // clean case's own score and say nothing about the one that never ran. + expect(report.meanScore).not.toBe(clean); + }); +});