Skip to content

Commit 54e2369

Browse files
os-litantclaude
andauthored
fix(cli): score a throwing generator as unscorable, so os lint --eval's meanScore stops reading 100 when nothing was generated (#15659)
* fix(cli): score a throwing generator as unscorable, not as an empty stack `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(cli): pin the throwing-generator verdict and the meanScore denominator 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 95d5cbb commit 54e2369

4 files changed

Lines changed: 242 additions & 13 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`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.
6+
7+
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:
8+
9+
```
10+
os lint --eval --json --generator ./throws.mjs
11+
exit 1 · ok: false · passed: 0 · failed: 5 · meanScore: 100
12+
every case: score 100 · grade A · valid true · generationError "model unavailable"
13+
```
14+
15+
`meanScore` is the first number a human scanning that report reads, and it read perfect precisely when the model under test produced nothing.
16+
17+
**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.
18+
19+
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:
20+
21+
```json
22+
{ "id": "invoice_with_line_items",
23+
"generationError": "model unavailable",
24+
"passed": false,
25+
"score": { "score": 0, "grade": "F", "valid": false } }
26+
```
27+
28+
and the run above now reports `meanScore: 0`.
29+
30+
`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.
31+
32+
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.

packages/cli/src/lint/metadata-eval.ts

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ export interface MetadataEvalCaseResult {
4343
* the string names the cause.
4444
*/
4545
generationError?: string;
46+
/**
47+
* The rubric's verdict on the stack. When `generationError` is set there was
48+
* no stack to judge, and this carries the unscorable sentinel — 0 / F /
49+
* `valid: false` — for BOTH causes alike. ⛔ Never a score borrowed from the
50+
* empty stack, which reads 100 / A / `valid: true`.
51+
*/
4652
score: MetadataScore;
4753
minScore: number;
4854
passed: boolean;
@@ -55,7 +61,17 @@ export interface MetadataEvalReport {
5561
total: number;
5662
passed: number;
5763
failed: number;
58-
/** Mean score across all cases (0–100, rounded). */
64+
/**
65+
* Mean score across all cases (0–100, rounded).
66+
*
67+
* The denominator is every case ATTEMPTED, ⛔ not the subset that could be
68+
* scored: a case whose generation failed contributes its 0, and is counted.
69+
* Stated because the alternative is a real metric with a different meaning —
70+
* a mean over scorable cases would report the quality of the generations
71+
* that arrived while staying silent about how many never did, and a
72+
* `meanScore` that silently switched denominators would be a worse defect
73+
* than a wrong one. `total` / `passed` / `failed` carry the counts.
74+
*/
5975
meanScore: number;
6076
/** True when every case passed. */
6177
ok: boolean;
@@ -77,14 +93,21 @@ export interface RunMetadataEvalOptions {
7793
const DEFAULT_MIN_SCORE = 75;
7894

7995
/**
80-
* The score attached to a case whose stack could not be scored AT ALL.
96+
* The score attached to a case whose stack could not be scored AT ALL — the
97+
* generator threw, or what it returned could not be walked. ONE verdict for
98+
* the whole class, because the class is one: there is no stack to judge.
8199
*
82-
* ⛔ Deliberately NOT `scoreMetadata({})`, even though the throwing-generator
83-
* path above substitutes an empty stack: the empty stack scores 100 / A /
84-
* `valid: true` (pinned in `score.test.ts`), and stamping that on a stack
85-
* nobody could parse would put a benign-looking verdict next to a failure.
86-
* A stack that cannot be walked is not an empty stack, and `valid: true` for
87-
* one that was never parsed is simply false.
100+
* ⛔ Deliberately NOT `scoreMetadata({})`. The empty stack scores 100 / A /
101+
* `valid: true` (pinned in `score.test.ts`, re-driven on this tree), so
102+
* substituting it stamps a benign-looking verdict on a failure. A stack that
103+
* cannot be walked is not an empty stack, and `valid: true` for one that was
104+
* never parsed is simply false.
105+
*
106+
* That substitution is exactly what the throwing-generator path below used to
107+
* do, which is how a live eval whose every generation threw could report
108+
* `meanScore: 100` beside `ok: false`, `passed: 0` — a clean number nothing
109+
* earned, and the first number a human reads. `passed` was never wrong; the
110+
* `score` under it was.
88111
*
89112
* ⛔ This is not a measurement and must never be read as one — the case is
90113
* already failed by its `generationError`. It exists so `MetadataScore` stays
@@ -138,7 +161,9 @@ export async function runMetadataEval(
138161
stack = await options.generate!(c.prompt, c.id);
139162
} catch (err: any) {
140163
generationError = err?.message || String(err);
141-
stack = {};
164+
// ⛔ No empty-stack substitution here. There is no stack: `stack`
165+
// keeps the fixture and is never read below, because a case that
166+
// reached this line is scored by `unscorableScore()` instead.
142167
}
143168
}
144169

@@ -155,11 +180,19 @@ export async function runMetadataEval(
155180
// `ok: false`. Swallowing it into a passing case would be worse than the
156181
// crash it replaces.
157182
let score: MetadataScore;
158-
try {
159-
score = scoreMetadata(stack);
160-
} catch (err: any) {
161-
generationError = `Failed to score the ${source} stack: ${err?.message || String(err)}`;
183+
if (generationError) {
184+
// The generator threw — nothing was produced, so there is nothing to
185+
// score. Same outcome class as a stack nobody can walk, therefore the
186+
// same verdict: an unscorable case contributes 0 to `meanScore`, never
187+
// the 100 an empty stack would have borrowed.
162188
score = unscorableScore();
189+
} else {
190+
try {
191+
score = scoreMetadata(stack);
192+
} catch (err: any) {
193+
generationError = `Failed to score the ${source} stack: ${err?.message || String(err)}`;
194+
score = unscorableScore();
195+
}
163196
}
164197
results.push({
165198
id: c.id,

packages/cli/test/lint-eval-json-unscorable-stack.e2e.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@
3939
* directly, against the specific benign shape a swallow would produce
4040
* (`scoreMetadata({})` is 100 / A / `valid: true`).
4141
*
42+
* That benign shape was not hypothetical on the OTHER failure path: a
43+
* generator that THREW had the empty stack substituted for it and scored, so
44+
* `meanScore` read 100 on a run where nothing was generated. Both paths now
45+
* answer 0 / F / `valid: false`, and `every generation threw ⇒ meanScore 0`
46+
* pins it on the same published `--json` face.
47+
*
4248
* ## Why the negative controls are here
4349
*
4450
* 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'
217223
expect(payload.results[0].generationError).toBe('model unavailable');
218224
}, 120_000);
219225

226+
/**
227+
* ⭐ The machine face of the defect this file's sibling card names, driven
228+
* here rather than reasoned about. Measured on this entry BEFORE the repair,
229+
* with a generator that throws for every prompt:
230+
*
231+
* exit 1 · ok: false · passed: 0 · failed: 5 · meanScore: 100
232+
* every case: score 100 · grade A · valid true · generationError set
233+
*
234+
* ⇒ the published `--json` payload's headline number read PERFECT exactly
235+
* when the model under test produced nothing. The throwing path substituted
236+
* an empty stack and scored it, and the empty stack is 100 / A / `valid`.
237+
*
238+
* ⛔ `passed` was never part of it and is asserted here unchanged — the
239+
* report always said `ok: false`, which is what made the 100 survivable
240+
* enough to sit on `main`.
241+
*/
242+
it('⭐ every generation threw ⇒ meanScore 0 on the --json face, never 100', async () => {
243+
const run = await runEval(
244+
generator('throws-all', `export default function () { throw new Error('model unavailable'); }\n`),
245+
);
246+
const payload = payloadOf(run, 'throwing generator — mean');
247+
248+
expect(run.code).toBe(1);
249+
expect(payload.meanScore).toBe(0);
250+
expect(payload.results.every((r) => r.score.score === 0)).toBe(true);
251+
expect(payload.results.every((r) => r.score.grade === 'F')).toBe(true);
252+
expect(payload.results.every((r) => r.score.valid === false)).toBe(true);
253+
254+
// The half that was already correct, pinned so a repair to it goes red.
255+
expect(payload.ok).toBe(false);
256+
expect(payload.passed).toBe(0);
257+
expect(payload.failed).toBe(payload.total);
258+
expect(payload.results.every((r) => r.passed === false)).toBe(true);
259+
}, 120_000);
260+
220261
it.each([
221262
['manifest-as-string', `export default () => ({ manifest: 'not-an-object' });\n`],
222263
['objects-as-string', `export default () => ({ objects: 'not-an-array' });\n`],

packages/cli/test/metadata-eval.test.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,126 @@ describe('runMetadataEval — a stack that cannot be scored is a FAILED case, no
211211
expect(report.ok).toBe(false);
212212
});
213213
});
214+
215+
/**
216+
* ⛔ A generator that THREW must not be scored as an empty stack.
217+
*
218+
* The measured before-shape, driven on this tree through the CLI's source
219+
* entry with a generator that throws for every prompt:
220+
*
221+
* os lint --eval --json --generator ./throws.mjs
222+
* exit 1 · ok: false · passed: 0 · failed: 5 · meanScore: 100
223+
* every case: score 100, grade A, valid true, generationError 'model unavailable'
224+
*
225+
* ⇒ the eval's headline number read PERFECT precisely when the model under
226+
* test produced nothing, and `meanScore` is the first number a human reads.
227+
*
228+
* ## What was wrong, and what was NOT
229+
*
230+
* ⛔ Not `passed`. `passed: !generationError && …` already excluded the case,
231+
* and `ok: passed === results.length` followed it, so the report DID say
232+
* `ok: false`. The wrong value was the `score` stamped on the failed case —
233+
* the throwing path substituted `stack = {}` and scored that, and the empty
234+
* stack is 100 / A / `valid: true`. `meanScore` then summed it.
235+
*
236+
* ## Why one verdict for both failure paths
237+
*
238+
* The sibling path — a generator that RETURNS a value nobody can walk —
239+
* already answered `unscorableScore()` (0 / F / `valid: false`), with the
240+
* reason written into the module: a stack that cannot be walked is not an
241+
* empty stack. A stack that was never produced is not an empty stack either.
242+
* Same outcome class, so the same verdict; one rule in the file, not two that
243+
* disagree.
244+
*
245+
* ## The denominator is pinned here on purpose
246+
*
247+
* The alternative repair — drop failed cases from `meanScore`'s denominator —
248+
* gives DIFFERENT numbers on a run where only some generations threw, and it
249+
* silently changes what the metric means (a mean over scored cases, not over
250+
* attempted ones). `the failed case is counted in the denominator` fails if
251+
* anyone later makes that switch without saying so.
252+
*/
253+
describe('runMetadataEval — a generator that THREW scores 0, not 100', () => {
254+
const oneCase: MetadataEvalCase[] = [
255+
{ id: 'c1', prompt: 'invoice with lines', fixture: { manifest: { id: 'a', namespace: 'aa', version: '1.0.0', name: 'A', type: 'app' } } },
256+
];
257+
const throwingGen = () => {
258+
throw new Error('model unavailable');
259+
};
260+
261+
it('⛔ the failed case is NOT scored as an empty stack', async () => {
262+
// The control that makes the assertion below mean something: this is the
263+
// exact verdict the old `stack = {}` substitution produced.
264+
expect(scoreMetadata({}).score).toBe(100);
265+
expect(scoreMetadata({}).grade).toBe('A');
266+
expect(scoreMetadata({}).valid).toBe(true);
267+
268+
const report = await runMetadataEval(oneCase, { generate: throwingGen });
269+
const only = report.results[0];
270+
271+
expect(only.generationError).toBe('model unavailable');
272+
expect(only.passed).toBe(false);
273+
expect(only.score.score).toBe(0);
274+
expect(only.score.grade).toBe('F');
275+
expect(only.score.valid).toBe(false);
276+
});
277+
278+
it('⭐ an eval whose every generation threw reports meanScore 0, not 100', async () => {
279+
const cases: MetadataEvalCase[] = [
280+
{ ...oneCase[0], id: 'a' },
281+
{ ...oneCase[0], id: 'b' },
282+
{ ...oneCase[0], id: 'c' },
283+
];
284+
const report = await runMetadataEval(cases, { generate: throwingGen });
285+
286+
expect(report.meanScore).toBe(0);
287+
// The half that was always right, asserted so a future "fix" to it is red.
288+
expect(report.ok).toBe(false);
289+
expect(report.passed).toBe(0);
290+
expect(report.failed).toBe(3);
291+
});
292+
293+
it('both failure paths now agree — thrown and unwalkable score the same', async () => {
294+
const poison = () => ({
295+
name: 'poison',
296+
get objects(): never {
297+
throw new Error('poison getter');
298+
},
299+
});
300+
301+
const thrown = (await runMetadataEval(oneCase, { generate: throwingGen })).results[0].score;
302+
const unwalkable = (await runMetadataEval(oneCase, { generate: poison })).results[0].score;
303+
304+
expect(thrown.score).toBe(unwalkable.score);
305+
expect(thrown.grade).toBe(unwalkable.grade);
306+
expect(thrown.valid).toBe(unwalkable.valid);
307+
});
308+
309+
it('the failed case is COUNTED in the denominator, not dropped from it', async () => {
310+
const cases: MetadataEvalCase[] = [
311+
{ ...oneCase[0], id: 'threw' },
312+
{ ...oneCase[0], id: 'clean' },
313+
];
314+
const cleanStack = {
315+
objects: [
316+
{ name: 'invoice', label: 'Invoice', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name', required: true } } },
317+
],
318+
};
319+
const generate = (_prompt: string, id: string) => {
320+
if (id === 'threw') throw new Error('model unavailable');
321+
return cleanStack;
322+
};
323+
324+
const report = await runMetadataEval(cases, { generate });
325+
const clean = report.results[1].score.score;
326+
327+
expect(report.results[0].score.score).toBe(0);
328+
expect(clean).toBeGreaterThan(0);
329+
// Mean over cases ATTEMPTED: the failure is a 0 in the numerator and a 1
330+
// in the denominator.
331+
expect(report.meanScore).toBe(Math.round(clean / 2));
332+
// ⛔ …and NOT a mean over the scorable subset, which would report the
333+
// clean case's own score and say nothing about the one that never ran.
334+
expect(report.meanScore).not.toBe(clean);
335+
});
336+
});

0 commit comments

Comments
 (0)