Skip to content

Commit 984f1da

Browse files
os-litantclaude
andauthored
fix(cli): record a linter crash in the metadata score instead of scoring it clean (#15881)
`scoreMetadata` caught a `lintConfig` throw and continued with `issues = []`, so the penalty was 0 and a stack half of whose rubric never ran scored 100 / A / `valid: true` with every count zero — byte-for-byte the verdict a genuinely clean stack gets, on both faces (`os lint --score` and the eval harness's `passed`). The throw is reachable on a schema-valid stack: a localized `label` on an app or on a view's `list` parses clean and makes the label-case rule throw. That rule's crash is a separate defect; this change stops the scorer publishing a clean verdict it did not earn. A crashed run is now carried by `lintError` (new optional field), by a synthetic `error` issue (`rubric/lint-crashed`) so `counts.errors` and `valid` report it, and by `score` 0 / grade `F`. The schema verdict is untouched. Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N Co-authored-by: Claude <noreply@anthropic.com>
1 parent da1cffb commit 984f1da

3 files changed

Lines changed: 260 additions & 4 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
`scoreMetadata` no longer scores a stack whose linter crashed as a perfect one.
6+
7+
The metadata rubric is two halves: a schema parse and the lint sweep. When `lintConfig` threw, the scorer caught the throw and continued with `issues = []` — so the penalty was 0 and a stack half of whose rubric never ran came back as **100 / grade `A` / `valid: true`, every count zero, `issues: []`** — byte-for-byte the verdict a genuinely clean stack gets. "The linter found nothing" and "the linter never ran" collapsed into the better-looking one.
8+
9+
The crash is reachable on a schema-valid stack: a localized `label` (`{ en: 'Todos', 'zh-CN': '待办' }`) on an app, or on a view's `list`, parses clean and makes the label-case rule throw a `TypeError`. That rule's crash is a separate defect, filed on its own; what changes here is that the scorer stops publishing a clean verdict it did not earn.
10+
11+
A crashed lint run is now recorded in every carrier a consumer might read, because reading any one of them has to be enough:
12+
13+
- **`lintError`** — a new optional string on `MetadataScore`, carrying the thrown message. Set only when the linter could not run; absent when it ran and reported errors, which is a lint verdict rather than a missing one. It reaches the CLI's published payload through `os lint --eval --json`, on `results[].score`.
14+
- **A synthetic `error` issue** (`rule: 'rubric/lint-crashed'`, exported as `LINT_CRASHED_RULE`) — so `issues`, `counts.errors` and `valid` carry the failure too. This is what makes the eval harness fail the case: its `passed` reads `counts.errors`, and would never have seen a new field. It still fails at `--eval-min 0`, where the score alone stops discriminating.
15+
- **`score: 0` / grade `F`** — the only channel `os lint --score --json` publishes, and the same refusal `unscorableScore()` already gives an eval case there was nothing to judge.
16+
17+
The schema half is untouched and still reported: `schemaErrors` and `counts.schemaErrors` say exactly what the parse found, which was the defensible half of the original intent.

packages/cli/src/lint/score.ts

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ export const SCORE_WEIGHTS = {
2727
suggestion: 1,
2828
} as const;
2929

30+
/**
31+
* The `rule` id on the synthetic issue raised when the linter throws.
32+
*
33+
* Exported so a consumer can tell "the linter reported a problem" from "the
34+
* linter never ran" by matching an id rather than prose.
35+
*/
36+
export const LINT_CRASHED_RULE = 'rubric/lint-crashed';
37+
3038
export interface MetadataScore {
3139
/** 0–100 quality score (higher is better). */
3240
score: number;
@@ -44,6 +52,18 @@ export interface MetadataScore {
4452
schemaErrors: string[];
4553
/** Lint issues (naming, labels, structure, data-model conventions). */
4654
issues: LintIssue[];
55+
/**
56+
* Set only when the lint half of the rubric could NOT run: `lintConfig` threw
57+
* and this carries the thrown message. Absent on every run where the linter
58+
* completed -- including one where it reported errors, which is a lint
59+
* verdict, not a missing one.
60+
*
61+
* Optional on purpose. It is the machine-readable half of the refusal below,
62+
* not its enforcement: a consumer that never reads it still cannot mistake a
63+
* crashed run for a clean one, because the same event is carried by `issues`,
64+
* `counts.errors`, `valid` and `score`.
65+
*/
66+
lintError?: string;
4767
}
4868

4969
function gradeFor(score: number): MetadataScore['grade'] {
@@ -72,12 +92,47 @@ export function scoreMetadata(stack: unknown): MetadataScore {
7292
: parsed.error.issues.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`);
7393

7494
// 2) Lint (naming/labels/structure + data-model conventions).
95+
//
96+
// A linter crash must not mask the schema verdict — the original intent, and
97+
// still right. What was not right is the number that came out of it: with
98+
// `issues = []` the penalty was 0, so a stack whose linter threw scored
99+
// 100 / A / `valid: true` with every count at zero — byte-for-byte the
100+
// verdict a genuinely clean stack gets, on a rubric half of which never ran.
101+
// "The linter found nothing" and "the linter never ran" are different facts.
102+
//
103+
// Reachable, and not exotically: a localized `label` (`{ en: …, 'zh-CN': … }`)
104+
// on an app, or on a view's `list`, is schema-valid and makes the label-case
105+
// rule throw. That crash is its own defect, filed separately; this function's
106+
// job is to never publish a clean verdict it did not earn.
107+
//
108+
// So the failure is recorded in every carrier a consumer might read, because
109+
// reading any ONE of them must be enough:
110+
// · `lintError` — the fact itself, typed, for a machine consumer;
111+
// · a synthetic `error` issue — so `issues`, `counts.errors` and `valid`
112+
// carry it too, which is what makes the eval harness fail the case: its
113+
// `passed` reads `counts.errors` and would never see a new field;
114+
// · `score` 0 / grade `F` — the only channel `os lint --score --json`
115+
// publishes, and the same refusal `unscorableScore()` (metadata-eval.ts)
116+
// already gives a case there was nothing to judge, for the same reason:
117+
// a clean number nothing earned is the worst possible output.
118+
// The schema verdict survives all of it — `schemaErrors` and
119+
// `counts.schemaErrors` still report exactly what the parse found.
75120
let issues: LintIssue[] = [];
121+
let lintError: string | undefined;
76122
try {
77123
issues = lintConfig(normalized) as LintIssue[];
78-
} catch {
79-
// A linter crash shouldn't mask the schema verdict — treat as no lint data.
80-
issues = [];
124+
} catch (err) {
125+
lintError = err instanceof Error ? err.message : String(err);
126+
issues = [
127+
{
128+
severity: 'error',
129+
rule: LINT_CRASHED_RULE,
130+
message:
131+
`The lint rubric did not run: ${lintError}. This verdict covers the schema ` +
132+
`parse only — no lint verdict was produced, so it must not be read as a clean one.`,
133+
path: '(lint)',
134+
},
135+
];
81136
}
82137

83138
const errors = bySeverity(issues, 'error');
@@ -90,7 +145,10 @@ export function scoreMetadata(stack: unknown): MetadataScore {
90145
warnings.length * SCORE_WEIGHTS.warning +
91146
suggestions.length * SCORE_WEIGHTS.suggestion;
92147

93-
const score = Math.max(0, Math.min(100, 100 - penalty));
148+
// A rubric that did not run has no score to report. 0 / `F` is not a penalty
149+
// dressed up as a measurement — it is the refusal, and it is the shape the
150+
// eval harness already uses for "there was nothing to judge".
151+
const score = lintError !== undefined ? 0 : Math.max(0, Math.min(100, 100 - penalty));
94152

95153
return {
96154
score: Math.round(score),
@@ -104,5 +162,6 @@ export function scoreMetadata(stack: unknown): MetadataScore {
104162
},
105163
schemaErrors,
106164
issues,
165+
...(lintError !== undefined ? { lintError } : {}),
107166
};
108167
}
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `scoreMetadata` must never publish a clean verdict for a rubric that did not
5+
* run.
6+
*
7+
* ## Why the linter is mocked here rather than driven
8+
*
9+
* The crash IS reachable on a schema-valid stack — a localized `label`
10+
* (`{ en: …, 'zh-CN': … }`) on an app, or on a view's `list`, parses clean and
11+
* makes the label-case rule throw a `TypeError`. That is a defect in the rule,
12+
* filed on its own; pinning it here would make this suite depend on a bug
13+
* staying unfixed, and the day someone repairs the rule these assertions would
14+
* go green for the wrong reason — or be deleted to make them pass.
15+
*
16+
* What this file pins is the SCORER's contract, which holds for any throw from
17+
* any rule: a crash is recorded, never swallowed into `issues: []`. So the
18+
* linter is replaced by one that throws on demand, and the control below runs
19+
* the same harness with a linter that returns cleanly — a mock that always
20+
* failed would satisfy every assertion here for no reason at all.
21+
*/
22+
23+
import { describe, expect, it, vi } from 'vitest';
24+
25+
const lint = vi.hoisted(() => ({
26+
/** When set, the stand-in `lintConfig` throws this instead of returning. */
27+
throws: null as unknown,
28+
}));
29+
30+
vi.mock('../src/commands/lint.js', async (importOriginal) => {
31+
const actual = await importOriginal<typeof import('../src/commands/lint.js')>();
32+
return {
33+
...actual,
34+
lintConfig: () => {
35+
if (lint.throws !== null) throw lint.throws;
36+
return [];
37+
},
38+
};
39+
});
40+
41+
const { scoreMetadata, LINT_CRASHED_RULE, SCORE_WEIGHTS } = await import('../src/lint/score.js');
42+
const { runMetadataEval } = await import('../src/lint/metadata-eval.js');
43+
44+
/** Schema-valid and, to the stand-in linter, clean. */
45+
const STACK = {
46+
objects: [
47+
{
48+
name: 'invoice',
49+
label: 'Invoice',
50+
sharingModel: 'private',
51+
fields: { name: { type: 'text', label: 'Invoice Number', required: true } },
52+
},
53+
],
54+
};
55+
56+
/** Schema-INVALID (`namespace` fails its pattern), so the parse half has a verdict. */
57+
const SCHEMA_INVALID_STACK = {
58+
manifest: { id: 'bad', namespace: 'X', version: '1.0.0', name: 'Bad', type: 'app' as const },
59+
};
60+
61+
function withLinterThrowing<T>(thrown: unknown, fn: () => T): T {
62+
lint.throws = thrown;
63+
try {
64+
return fn();
65+
} finally {
66+
lint.throws = null;
67+
}
68+
}
69+
70+
/**
71+
* The async twin. ⚠️ Not a stylistic variant of the above: the sync helper
72+
* restores `lint.throws` when `fn` RETURNS, which for an async `fn` is the
73+
* moment it hands back a pending promise — before a single line of the work
74+
* being measured has run. Awaiting inside the `try` is what keeps the stand-in
75+
* throwing for the whole run.
76+
*/
77+
async function withLinterThrowingAsync<T>(thrown: unknown, fn: () => Promise<T>): Promise<T> {
78+
lint.throws = thrown;
79+
try {
80+
return await fn();
81+
} finally {
82+
lint.throws = null;
83+
}
84+
}
85+
86+
describe('scoreMetadata — when the linter crashes', () => {
87+
it('CONTROL: the same harness with a linter that returns cleanly still scores 100 / A', () => {
88+
const r = scoreMetadata(STACK);
89+
expect(r.lintError).toBeUndefined();
90+
expect(r.score).toBe(100);
91+
expect(r.grade).toBe('A');
92+
expect(r.valid).toBe(true);
93+
expect(r.counts.errors).toBe(0);
94+
expect(r.issues).toEqual([]);
95+
});
96+
97+
it('refuses the verdict instead of scoring 100 / A / valid', () => {
98+
const r = withLinterThrowing(new TypeError('boom'), () => scoreMetadata(STACK));
99+
100+
// The headline: nothing about this may read like a clean stack.
101+
expect(r.score).toBe(0);
102+
expect(r.grade).toBe('F');
103+
expect(r.valid).toBe(false);
104+
});
105+
106+
it('records the crash in every carrier a consumer might read', () => {
107+
const r = withLinterThrowing(new TypeError('boom'), () => scoreMetadata(STACK));
108+
109+
expect(r.lintError).toBe('boom');
110+
expect(r.counts.errors).toBe(1);
111+
expect(r.issues).toHaveLength(1);
112+
expect(r.issues[0]).toMatchObject({ severity: 'error', rule: LINT_CRASHED_RULE });
113+
// The message must say the rubric did not RUN — "no issues found" is the
114+
// exact reading this whole change exists to prevent.
115+
expect(r.issues[0].message).toContain('did not run');
116+
expect(r.issues[0].message).toContain('boom');
117+
});
118+
119+
it('keeps the schema verdict, which the crash must not mask', () => {
120+
const r = withLinterThrowing(new TypeError('boom'), () => scoreMetadata(SCHEMA_INVALID_STACK));
121+
122+
expect(r.counts.schemaErrors).toBeGreaterThan(0);
123+
expect(r.schemaErrors.some((m) => m.includes('namespace'))).toBe(true);
124+
expect(r.lintError).toBe('boom');
125+
});
126+
127+
it('stringifies a non-Error throw rather than reporting "undefined"', () => {
128+
const r = withLinterThrowing('plain string failure', () => scoreMetadata(STACK));
129+
130+
expect(r.lintError).toBe('plain string failure');
131+
expect(r.issues[0].message).toContain('plain string failure');
132+
});
133+
134+
it('is not a lint error in disguise: a real lint error scores by the rubric and sets no lintError', () => {
135+
// One `error`-severity issue costs exactly its weight — the crash path is a
136+
// different claim from "the linter found one error", and the two must not
137+
// land on the same output.
138+
const r = scoreMetadata({
139+
objects: [{ name: 'BadName', label: 'Bad', fields: { name: { type: 'text', label: 'Name' } } }],
140+
});
141+
expect(r.lintError).toBeUndefined();
142+
expect(r.score).toBeGreaterThan(100 - SCORE_WEIGHTS.error * 2);
143+
});
144+
});
145+
146+
describe('the eval harness reads the refusal without a new field', () => {
147+
const CORPUS = [{ id: 'crashing_case', prompt: 'anything', fixture: STACK }];
148+
149+
it('CONTROL: the same case passes when the linter returns cleanly', async () => {
150+
const report = await runMetadataEval(CORPUS, { minScore: 75 });
151+
expect(report.results[0].passed).toBe(true);
152+
expect(report.ok).toBe(true);
153+
});
154+
155+
it('fails the case whose linter crashed, and contributes 0 to the mean', async () => {
156+
const report = await withLinterThrowingAsync(new TypeError('boom'), () =>
157+
runMetadataEval(CORPUS, { minScore: 75 }),
158+
);
159+
160+
expect(report.results[0].passed).toBe(false);
161+
expect(report.results[0].score.lintError).toBe('boom');
162+
expect(report.meanScore).toBe(0);
163+
expect(report.ok).toBe(false);
164+
});
165+
166+
it('still fails it when the score bar is lowered to 0 — the synthetic error is what holds', async () => {
167+
// `passed` reads `score >= minScore && counts.errors === 0 &&
168+
// counts.schemaErrors === 0`. At `--eval-min 0` the score half stops
169+
// discriminating, so the crash has to be an `error` in `counts` or the case
170+
// passes again. This is why the refusal is not carried by the number alone.
171+
const report = await withLinterThrowingAsync(new TypeError('boom'), () =>
172+
runMetadataEval(CORPUS, { minScore: 0 }),
173+
);
174+
175+
expect(report.results[0].score.score).toBeGreaterThanOrEqual(0);
176+
expect(report.results[0].score.counts.errors).toBe(1);
177+
expect(report.results[0].passed).toBe(false);
178+
expect(report.ok).toBe(false);
179+
});
180+
});

0 commit comments

Comments
 (0)