fix(cli): score a throwing generator as unscorable, so os lint --eval's meanScore stops reading 100 when nothing was generated

.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.

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,

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`],

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);
+  });
+});