feat(engine)!: SKIP stops outranking OK, coverage gets its own axis

A repo's `status` is now the worst JUDGED finding, and `SKIP` only when
nothing was judged. `SKIP` used to rank between `OK` and `WARN`, so one
un-runnable check spoke for every check that ran: 0 ERROR, 0 WARN and a
dozen OK headlined as "skipped". Five repos in org-ops census 05, `okr`
among them with the most OK in the org, reading as unread.

"`SKIP` is never a pass" survives in the half of the rule that carries
it — an unregistered repo, or an empty finding set, still says SKIP,
because there is nothing else to be worst of.

Fixing the status alone would have traded "clean repos look skipped" for
"skipped checks look clean". So `notChecked` rides beside it: in --json,
and as a `· N not checked` qualifier on the summary line. Absent means an
older engine, not zero.

Measured across all 21 local clones from ONE saved sweep, so before and
after come from the same findings rather than two sweeps of a moving org:
343 findings before, 343 after. 8 repos moved, every one SKIP -> OK.
1 ERROR and 3 WARN before, 1 ERROR and 3 WARN after — the counts that
decide whether a repo needs work did not move.

BREAKING: consumers reading `.status` see a changed value domain.

147 -> 154 tests.
This commit is contained in:
Kjell Tore Guttormsen 2026-08-09 21:29:08 +02:00
commit 10ad1254ab
7 changed files with 162 additions and 14 deletions

View file

@ -54,7 +54,13 @@ export function readEngineCommit(dir = join(HERE, '..')) {
}
}
const LEVELS = ['OK', 'SKIP', 'WARN', 'ERROR'];
// The JUDGEMENT lattice, and `SKIP` is deliberately not in it. A skip is not a
// severity — it is the absence of a verdict, so it cannot be the worst of a set
// that contains real ones. It used to sit between OK and WARN here, which meant
// a repo with 0 ERROR, 0 WARN and a dozen OK headlined as "skipped": five repos
// in org-ops census 05, `okr` among them with the most OK in the org. Coverage
// is carried on its own axis instead — see `notCheckedOf`.
const LEVELS = ['OK', 'WARN', 'ERROR'];
// Findings carry a level AND a bucket, and the two are independent axes.
// The level says how sure and how loud; the bucket says what KIND of problem it
@ -893,12 +899,25 @@ export function checkInternalLinks({ files, present }) {
return findings;
}
// Worst of the judged findings; `SKIP` only when there is nothing to be worst
// OF. That second half is what keeps "`SKIP` is never a pass" true: an
// unregistered repo, or an empty finding set, still says so plainly. What the
// rule no longer does is let one un-runnable check speak for twelve that ran.
export function levelOf(findings) {
let worst = 'OK';
let worst = null;
for (const f of findings ?? []) {
if (LEVELS.indexOf(f.level) > LEVELS.indexOf(worst)) worst = f.level;
if (f.level === 'SKIP') continue;
if (worst === null || LEVELS.indexOf(f.level) > LEVELS.indexOf(worst)) worst = f.level;
}
return worst;
return worst ?? 'SKIP';
}
// The coverage axis, counted rather than left for each consumer to re-derive
// from `findings`. Same reason `buckets` is precomputed beside `status`: a
// number nobody can see reads exactly like a check that silently stopped
// running.
export function notCheckedOf(findings) {
return (findings ?? []).filter((f) => f.level === 'SKIP').length;
}
export function bucketsOf(findings) {
@ -920,6 +939,7 @@ export function classifyRepo(
klass: null,
traits: [],
status: 'SKIP',
notChecked: 1,
buckets: { broken: 0, missing: 0, weakening: 0 },
findings: [{
level: 'SKIP',
@ -947,7 +967,15 @@ export function classifyRepo(
...checkDescription(description, register),
];
return { name, klass, traits, status: levelOf(findings), buckets: bucketsOf(findings), findings };
return {
name,
klass,
traits,
status: levelOf(findings),
notChecked: notCheckedOf(findings),
buckets: bucketsOf(findings),
findings,
};
}
// ---------------------------------------------------------------- I/O shell
@ -1155,11 +1183,17 @@ const BUCKET_TITLE = {
// engine is visible, not just correctable in hindsight.
const MARK = { OK: '✓', WARN: '!', ERROR: '✗', SKIP: '·' };
// Both axes on the one line a sweep actually reads. Letting `status` mean
// judgement fixed "clean repos look skipped"; printing a bare OK next to a
// check that never ran would trade it for "skipped checks look clean", which is
// the worse direction. Absent `notChecked` is neither zero nor a crash — a
// result from before this axis existed prints the old line, not "undefined".
export function headerLine(result, engineVersion, engineCommit = null) {
const klass = result.klass ? ` [${result.klass}]` : '';
const traits = result.traits?.length ? ` {${result.traits.join(', ')}}` : '';
const sha = engineCommit ? ` @${String(engineCommit).slice(0, 7)}` : '';
return `${MARK[result.status]} ${result.name}${klass}${traits}${result.status} (repo-standard v${engineVersion}${sha})`;
const coverage = result.notChecked > 0 ? ` · ${result.notChecked} not checked` : '';
return `${MARK[result.status]} ${result.name}${klass}${traits}${result.status}${coverage} (repo-standard v${engineVersion}${sha})`;
}
// `engineCommit` is always present, null when underivable: an ABSENT key means

View file

@ -567,17 +567,49 @@ test('every registered repo names a class that actually exists', () => {
// -------------------------------------------------------------- aggregation
test('levelOf ranks ERROR above WARN above SKIP above OK', () => {
test('levelOf ranks ERROR above WARN above OK', () => {
assert.equal(levelOf([{ level: 'OK' }, { level: 'WARN' }, { level: 'ERROR' }]), 'ERROR');
assert.equal(levelOf([{ level: 'OK' }, { level: 'WARN' }]), 'WARN');
assert.equal(levelOf([{ level: 'OK' }, { level: 'SKIP' }]), 'SKIP');
assert.equal(levelOf([{ level: 'OK' }]), 'OK');
assert.equal(levelOf([]), 'OK');
});
// The defect this replaced: SKIP outranked OK, so a repo with 0 ERROR, 0 WARN
// and a dozen OK headlined as "skipped". Reported by org-ops against five repos
// in census 05; `okr` has the most OK in the org and read as unread. SKIP is not
// a severity — it is the absence of a judgement, and it cannot be the worst of a
// set that contains real ones.
test('a SKIP does not outrank an OK — it is absence of judgement, not a severity', () => {
assert.equal(levelOf([{ level: 'OK' }, { level: 'SKIP' }]), 'OK');
assert.equal(levelOf([{ level: 'SKIP' }, { level: 'WARN' }]), 'WARN');
assert.equal(levelOf([{ level: 'SKIP' }, { level: 'ERROR' }]), 'ERROR');
});
// The other half of the same rule, and the half that keeps "`SKIP` is never a
// pass" true: when there is nothing else to be worst OF, SKIP stands.
test('SKIP still wins when nothing was judged at all', () => {
assert.equal(levelOf([{ level: 'SKIP' }]), 'SKIP');
assert.equal(levelOf([{ level: 'SKIP' }, { level: 'SKIP' }]), 'SKIP');
});
// An empty finding set means no check ran, which is the same state as an
// all-SKIP set — not a clean bill. It cannot occur for a registered repo, so
// this pins a deliberate choice rather than an observed case.
test('no findings at all is SKIP, not OK — nothing ran', () => {
assert.equal(levelOf([]), 'SKIP');
});
test('an unregistered repo is SKIP, not a pass — the gate refuses to guess a class', () => {
const r = classifyRepo({ name: 'stranger', files: {}, present: [], description: null }, REGISTER);
assert.equal(r.status, 'SKIP');
assert.equal(r.notChecked, 1);
});
// Coverage is its own axis, carried explicitly rather than left for each
// consumer to re-derive from `findings`. An exemption nobody can see reads
// exactly like a check that silently stopped running.
test('notChecked counts the skipped checks beside the status, on every result', () => {
const r = classifyRepo({ name: 'stranger', files: {}, present: [], description: null }, REGISTER);
assert.equal(typeof r.notChecked, 'number');
});
test('a fully compliant plugin repo classifies OK', () => {
@ -1483,6 +1515,28 @@ test('headerLine falls back to the version alone when the commit is underivable'
assert.doesNotMatch(line, /null|undefined/);
});
// Letting `status` mean judgement fixes "clean repos look skipped"; printing an
// unqualified OK beside a check that never ran would trade it for "skipped
// checks look clean", which is the worse direction. The one line org-ops reads
// has to carry both axes.
test('headerLine qualifies a pass that had checks it could not run', () => {
const line = headerLine({ name: 'okr', klass: 'plugin', traits: [], status: 'OK', notChecked: 1 }, '0.7.0');
assert.match(line, /OK/);
assert.match(line, /1 not checked/);
});
test('headerLine says nothing about coverage when every check ran', () => {
const line = headerLine({ name: 'okr', klass: 'plugin', traits: [], status: 'OK', notChecked: 0 }, '0.7.0');
assert.doesNotMatch(line, /not checked/);
});
// An older result object has no `notChecked` at all. Absent is not zero and not
// a crash — it prints the pre-0.7.0 line rather than the word "undefined".
test('headerLine tolerates a result from before the coverage axis existed', () => {
const line = headerLine({ name: 'voyage', klass: 'plugin', traits: [], status: 'OK' }, '0.7.0');
assert.doesNotMatch(line, /not checked|undefined|NaN/);
});
test('withEngineVersion stamps the commit next to the version', () => {
const stamped = withEngineVersion({ name: 'x', status: 'OK', findings: [] }, '0.5.0', 'a7276e6f0d1e2b3c');
assert.equal(stamped.engineVersion, '0.5.0');