From c2aa77c182b08197a73897655d0afecf60705987 Mon Sep 17 00:00:00 2001 From: Qwynn Marcelle Date: Mon, 17 Aug 2026 15:09:15 -0400 Subject: [PATCH 1/2] feat(rules)!: deprecate the hygiene score and stop it certifying empty scans (META-284) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements ADR-003 amendment A-002, ratified 2026-08-03. computeHygieneScore([], 0) returned { value: 100, grade: 'A' }. No findings meant no penalty, no penalty meant a full score, a full score meant an A, and coverageRatio was computed but never consulted by the scoring path. A scan that observed nothing certified a repository as flawless, and that value reached a published artifact. The function returns HygieneScore | null now, and null when nothing was observed. null is not a bad grade; it is the absence of one, and a caller has to handle it rather than inherit an A. coverageRatio is number | undefined, because the old 0 was the default parameter arriving unchanged at every call site rather than a measurement. Both are source-level breaks for TypeScript readers, which is the intended alarm: that is exactly the code that would otherwise read absence as a pass. AuditResult.score is nullable for the same reason. Nothing is removed and no schema bytes change. ADR-003 §5 gives a normative-optional field a deprecation notice and a documented migration now, with removal at the next declared breaking boundary; the profile stays at 0.4, so this release is not it. generated.hygiene stays declared while a first-party producer still emits it — emission ceases first, then the field and exports go together. The old behavior was pinned by a test named 'clean repo scores 100'. An empty findings array is not a clean repository; it is an absence of evidence, and the two were indistinguishable in the return value. --- .changeset/quiet-moons-admit.md | 57 +++++++++++++ .../hygiene-score.invariants.test.ts | 82 +++++++++++++------ packages/rules/src/engine/hygiene-score.ts | 51 +++++++++++- .../__tests__/real-repos.integration.test.ts | 9 +- packages/rules/src/types.ts | 25 +++++- 5 files changed, 190 insertions(+), 34 deletions(-) create mode 100644 .changeset/quiet-moons-admit.md diff --git a/.changeset/quiet-moons-admit.md b/.changeset/quiet-moons-admit.md new file mode 100644 index 0000000..a1f51c7 --- /dev/null +++ b/.changeset/quiet-moons-admit.md @@ -0,0 +1,57 @@ +--- +"@workspacejson/rules": minor +--- + +Deprecate the hygiene score, and stop it certifying scans that observed nothing, +per ADR-003 amendment A-002. + +`computeHygieneScore([], 0)` returned `{ value: 100, grade: 'A' }`. No findings +meant no penalty; no penalty meant a full score; a full score meant an A. Nothing +in the function related the score to how much had been examined — `coverageRatio` +was computed and returned but never consulted by the scoring path. A scan that +looked at nothing certified a repository as flawless, and that value reached a +published artifact. + +**The function now returns `HygieneScore | null`,** and `null` when the scan +observed nothing: no findings, and no file-count denominator to say anything was +examined. `null` is not a bad grade. It is the statement that there is no score +to give, and a reader has to decide what to do about that instead of inheriting +an `A`. Where evidence exists — any finding, or a known denominator — the +arithmetic is unchanged. + +**`coverageRatio` is now `number | undefined`.** It was `0` whenever no total was +supplied, which is every current call site, so that zero was never a measurement +— it was the default parameter arriving unchanged. "Coverage was not measured" +and "coverage was zero" are different claims and no longer share a value. + +**Both are source-level breaks for TypeScript readers, which is why this is a +minor.** Code assigning the result to a bare `HygieneScore`, or `coverageRatio` +to a bare `number`, stops compiling. That is the intended alarm: it is exactly +the code that would otherwise read absence as a pass. `AuditResult.score` is +`HygieneScore | null` for the same reason — a caller handed no evidence needs +somewhere truthful to put that, and the previous non-nullable field left +fabricating a perfect score as the only way to satisfy it. + +**`computeHygieneScore`, `HygieneScore` and `AuditResult.score` are deprecated +and scheduled for removal at the next document-profile boundary.** A letter grade +is a judgement, and this standard is descriptive: it reports what a repository +*is*, not what a team must do about it. Scoring belongs to the consumer that +reads the descriptive fields. + +Migrating needs nothing that is not already public — `Finding.state`, +`.severity`, `.confidence` and `.temporalWeight` are the only inputs the function +ever had: + +```ts +const failures = findings.filter((f) => f.state === 'FAIL'); +const critical = failures.filter((f) => f.severity === 'critical'); +``` + +**Nothing is removed in this release and no schema bytes change.** Under ADR-003 +§5 a normative-optional field earns a deprecation notice and a documented +migration now, with removal at the next declared breaking boundary; the document +profile is unchanged at `generated.specVersion: "0.4"`, so this release is not +that boundary. `generated.hygiene` remains declared in the schema, because a +first-party producer still emits it and removing the declaration while that is +true would describe the artifact incorrectly. Emission ceases first, on the +producer's own schedule, and the field and exports go together afterwards. diff --git a/packages/rules/src/engine/__tests__/hygiene-score.invariants.test.ts b/packages/rules/src/engine/__tests__/hygiene-score.invariants.test.ts index d5c16c2..24c1713 100644 --- a/packages/rules/src/engine/__tests__/hygiene-score.invariants.test.ts +++ b/packages/rules/src/engine/__tests__/hygiene-score.invariants.test.ts @@ -21,13 +21,39 @@ function makeFinding(state: FindingState, severity?: Severity): Finding { // ── Invariants ──────────────────────────────────────────────────────────────── -describe('HygieneScore invariants', () => { - it('clean repo scores 100', () => { - const score = computeHygieneScore([]); - expect(score.value).toBe(100); - expect(score.grade).toBe('A'); +// A scan that observed nothing has no score. These are the assertions that used +// to run the other way: `computeHygieneScore([])` returned `{ value: 100, +// grade: 'A' }`, and a test named "clean repo scores 100" pinned it there. An +// empty findings array is not a clean repository — it is an absence of +// evidence, and the two were indistinguishable in the return value. +describe('HygieneScore — absence is not a pass', () => { + it('returns null when nothing was observed and no denominator was given', () => { + expect(computeHygieneScore([])).toBeNull(); + }); + + it('returns null for the exact input that certified an empty scan', () => { + // The defect as traced in the removal record: no findings, no files. + expect(computeHygieneScore([], 0)).toBeNull(); + }); + + it('scores a repository that was scanned and found clean', () => { + // No findings, but a known denominator — something WAS examined, so a score + // is defensible here in a way it is not above. + const score = computeHygieneScore([], 100); + expect(score).not.toBeNull(); + expect(score!.value).toBe(100); + expect(score!.grade).toBe('A'); + expect(score!.coverageRatio).toBe(0); }); + it('reports unmeasured coverage as undefined, not as zero', () => { + const score = computeHygieneScore([makeFinding('WARN', 'warning')]); + expect(score).not.toBeNull(); + expect(score!.coverageRatio).toBeUndefined(); + }); +}); + +describe('HygieneScore invariants', () => { it('score is always between 0 and 100', () => { for (let fails = 0; fails <= 20; fails += 1) { for (let warns = 0; warns <= 100; warns += 10) { @@ -35,34 +61,35 @@ describe('HygieneScore invariants', () => { ...Array.from({ length: fails }, () => makeFinding('FAIL', 'error')), ...Array.from({ length: warns }, () => makeFinding('WARN', 'warning')), ]; + // The empty corner is covered above and has no score by construction. + if (findings.length === 0) continue; const score = computeHygieneScore(findings); - expect(score.value).toBeGreaterThanOrEqual(0); - expect(score.value).toBeLessThanOrEqual(100); + expect(score).not.toBeNull(); + expect(score!.value).toBeGreaterThanOrEqual(0); + expect(score!.value).toBeLessThanOrEqual(100); } } }); it('any error caps score at 70', () => { const score = computeHygieneScore([makeFinding('FAIL', 'error')]); - expect(score.value).toBeLessThanOrEqual(70); + expect(score!.value).toBeLessThanOrEqual(70); }); it('grade boundaries are correct', () => { - expect(computeHygieneScore([]).grade).toBe('A'); - // 5 WARN findings: penalty = 5 * 3 = 15, score = 85, grade B const score85 = computeHygieneScore( Array.from({ length: 5 }, () => makeFinding('WARN', 'warning')), ); - expect(['A', 'B']).toContain(score85.grade); + expect(['A', 'B']).toContain(score85!.grade); }); it('is deterministic - same input always same output', () => { const findings = [makeFinding('FAIL', 'error'), makeFinding('WARN', 'warning')]; const score1 = computeHygieneScore(findings); const score2 = computeHygieneScore(findings); - expect(score1.value).toBe(score2.value); - expect(score1.grade).toBe(score2.grade); + expect(score1!.value).toBe(score2!.value); + expect(score1!.grade).toBe(score2!.grade); }); it('counts are accurate', () => { @@ -73,8 +100,8 @@ describe('HygieneScore invariants', () => { makeFinding('WARN', 'info'), // info-severity WARN still increments warnCount ]; const score = computeHygieneScore(findings); - expect(score.breakdown.failCount).toBe(2); - expect(score.breakdown.warnCount).toBe(2); + expect(score!.breakdown.failCount).toBe(2); + expect(score!.breakdown.warnCount).toBe(2); // Verify no legacy flat count fields const scoreView = score as unknown as Record; expect(scoreView.errorCount).toBeUndefined(); @@ -88,12 +115,12 @@ describe('HygieneScore invariants', () => { makeFinding('FAIL', 'error'), makeFinding('FAIL', 'error'), ]); - expect(twoErrors.value).toBeLessThanOrEqual(oneError.value); + expect(twoErrors!.value).toBeLessThanOrEqual(oneError!.value); }); it('critical FAIL caps score at 50', () => { const score = computeHygieneScore([makeFinding('FAIL', 'critical')]); - expect(score.value).toBeLessThanOrEqual(50); + expect(score!.value).toBeLessThanOrEqual(50); }); it('breakdown tracks all five states', () => { @@ -105,22 +132,25 @@ describe('HygieneScore invariants', () => { makeFinding('PREVIEW'), ]; const score = computeHygieneScore(findings); - expect(score.breakdown.failCount).toBe(1); - expect(score.breakdown.warnCount).toBe(1); - expect(score.breakdown.insufficientDataCount).toBe(1); - expect(score.breakdown.skipCount).toBe(1); - expect(score.breakdown.previewCount).toBe(1); + expect(score!.breakdown.failCount).toBe(1); + expect(score!.breakdown.warnCount).toBe(1); + expect(score!.breakdown.insufficientDataCount).toBe(1); + expect(score!.breakdown.skipCount).toBe(1); + expect(score!.breakdown.previewCount).toBe(1); }); - it('computeHygieneScore([]) returns correct empty breakdown', () => { - const score = computeHygieneScore([]); - expect(score.breakdown).toEqual({ + it('an all-zero breakdown is reachable only over a scanned repository', () => { + // This assertion used to be made against `computeHygieneScore([])`, which + // now has no score at all. The empty breakdown is still a real state — it + // just requires evidence that a scan happened. + const score = computeHygieneScore([], 100); + expect(score!.breakdown).toEqual({ failCount: 0, warnCount: 0, insufficientDataCount: 0, skipCount: 0, previewCount: 0, }); - expect(score.coverageRatio).toBe(0); + expect(score!.coverageRatio).toBe(0); }); }); diff --git a/packages/rules/src/engine/hygiene-score.ts b/packages/rules/src/engine/hygiene-score.ts index 94c55bf..d0c5243 100644 --- a/packages/rules/src/engine/hygiene-score.ts +++ b/packages/rules/src/engine/hygiene-score.ts @@ -7,10 +7,44 @@ const SEVERITY_WEIGHTS = { info: 2, } as const; +/** + * Compute a hygiene score from rule findings. + * + * @deprecated Scheduled for removal at the next document-profile boundary, per + * ADR-003 amendment A-002. A letter grade is a judgement, and this standard is + * descriptive: it reports what a repository *is*, not what a team must do about + * it. Scoring belongs to the consumer that reads the descriptive fields. + * + * To migrate, read the findings directly and apply your own weighting. Every + * input this function uses is already public — `Finding.state`, `.severity`, + * `.confidence` and `.temporalWeight` — so nothing is lost by moving the + * judgement to the side that owns it: + * + * ```ts + * const failures = findings.filter((f) => f.state === 'FAIL'); + * const critical = failures.filter((f) => f.severity === 'critical'); + * ``` + * + * Returns `null` when the scan observed nothing, rather than a perfect score + * over an empty observation. See the note on the return type below. + */ export function computeHygieneScore( findings: Finding[], - totalRepoFiles = 0, -): HygieneScore { + totalRepoFiles?: number, +): HygieneScore | null { + // A score asserted over an empty observation is a fabrication, not a pass. + // With no findings AND no known denominator, this function has been handed no + // evidence that anything was examined at all — the previous behavior returned + // `{ value: 100, grade: 'A' }` for exactly that input, which is how a scan + // that looked at nothing came to certify a repository as flawless in a + // published artifact. + // + // Absence is reported as absence. `null` is not a bad score; it is the + // statement that there is no score to give, and a caller has to decide what + // to do about that rather than inherit an 'A'. + const observedNothing = findings.length === 0 && (totalRepoFiles === undefined || totalRepoFiles === 0); + if (observedNothing) return null; + const breakdown = { failCount: 0, warnCount: 0, @@ -59,12 +93,21 @@ export function computeHygieneScore( const grade = value >= 95 ? 'A' : value >= 80 ? 'B' : value >= 65 ? 'C' : value >= 50 ? 'D' : 'F'; - // Coverage ratio: unique files appearing in evidence / total repo files + // Coverage ratio: unique files appearing in evidence / total repo files. + // + // `undefined` when no denominator was supplied, because "we did not measure + // coverage" and "coverage was zero" are different claims and the previous + // code reported both as `0`. Every current caller omits the argument, so that + // zero was never a measurement — it was the default arriving unchanged. const coveredFiles = new Set( findings.filter((f) => f.evidence.file).map((f) => f.evidence.file!), ); const coverageRatio = - totalRepoFiles > 0 ? coveredFiles.size / totalRepoFiles : 0; + totalRepoFiles === undefined + ? undefined + : totalRepoFiles > 0 + ? coveredFiles.size / totalRepoFiles + : 0; return { value, grade, breakdown, coverageRatio }; } diff --git a/packages/rules/src/testing/__tests__/real-repos.integration.test.ts b/packages/rules/src/testing/__tests__/real-repos.integration.test.ts index 4274ac8..b330dc5 100644 --- a/packages/rules/src/testing/__tests__/real-repos.integration.test.ts +++ b/packages/rules/src/testing/__tests__/real-repos.integration.test.ts @@ -44,8 +44,13 @@ describe('Real repo integration - workspace root', () => { const score = computeHygieneScore(findings); expect(durationMs).toBeLessThan(10_000); - expect(score.value).toBeGreaterThanOrEqual(0); - expect(score.value).toBeLessThanOrEqual(100); + // A real repository may legitimately produce no findings, and with no + // denominator supplied that is an unobserved scan rather than a perfect + // one. Assert the bounds only where there is a score to bound. + if (score !== null) { + expect(score.value).toBeGreaterThanOrEqual(0); + expect(score.value).toBeLessThanOrEqual(100); + } for (const finding of findings) { expect(finding.ruleId).toBeTruthy(); diff --git a/packages/rules/src/types.ts b/packages/rules/src/types.ts index 4eff521..e35f7de 100644 --- a/packages/rules/src/types.ts +++ b/packages/rules/src/types.ts @@ -220,6 +220,11 @@ export interface VrekoContext { // ─── Section 8: Score + Audit ───────────────────────────────────────────────── +/** + * @deprecated Scheduled for removal at the next document-profile boundary, per + * ADR-003 amendment A-002. A letter grade is prescriptive, and this standard is + * descriptive. See `computeHygieneScore` for the migration. + */ export interface HygieneScore { value: number; grade: 'A' | 'B' | 'C' | 'D' | 'F'; @@ -230,7 +235,14 @@ export interface HygieneScore { skipCount: number; previewCount: number; }; - coverageRatio: number; + /** + * Unique files appearing in finding evidence, over the total file count. + * + * `undefined` when no total was supplied — "coverage was not measured" and + * "coverage was zero" are different claims, and reporting the first as `0` + * made an unmeasured scan indistinguishable from an uncovered one. + */ + coverageRatio?: number | undefined; } // ─── Section 9: Keep ALL v0.1 types still needed downstream ────────────────── @@ -306,7 +318,16 @@ export interface AuditConfig { export interface AuditResult { findings: Finding[]; - score: HygieneScore; + /** + * @deprecated Follows `HygieneScore` out at the next document-profile + * boundary, per ADR-003 amendment A-002. + * + * `null` when the scan observed nothing. The field is nullable rather than + * required so that a caller handed no evidence has somewhere truthful to put + * that — the previous non-nullable type left fabricating a perfect score as + * the only way to satisfy it. + */ + score: HygieneScore | null; agentsMdPath: string; workspaceJsonFound: boolean; workspaceJsonStale: boolean; From 6fc0c65d04699b02387d5f5068df7e0fadfff854 Mon Sep 17 00:00:00 2001 From: Qwynn Marcelle Date: Mon, 17 Aug 2026 17:02:04 -0400 Subject: [PATCH 2/2] refactor(rules): extract the nested coverage conditional (Sonar S3358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three coverage states — not measured, measured as zero, measured as a ratio — were stacked in a nested ternary. Written as a guard instead, so each state is visible on its own line. No behavior change; build, typecheck and tests pass unchanged. --- packages/rules/src/engine/hygiene-score.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/rules/src/engine/hygiene-score.ts b/packages/rules/src/engine/hygiene-score.ts index d0c5243..1aea210 100644 --- a/packages/rules/src/engine/hygiene-score.ts +++ b/packages/rules/src/engine/hygiene-score.ts @@ -102,12 +102,14 @@ export function computeHygieneScore( const coveredFiles = new Set( findings.filter((f) => f.evidence.file).map((f) => f.evidence.file!), ); - const coverageRatio = - totalRepoFiles === undefined - ? undefined - : totalRepoFiles > 0 - ? coveredFiles.size / totalRepoFiles - : 0; + // Left `undefined` unless a denominator was actually supplied. Written as a + // guard rather than a nested conditional so that the three states — not + // measured, measured as zero, measured as a ratio — are each visible on their + // own line. + let coverageRatio: number | undefined; + if (totalRepoFiles !== undefined) { + coverageRatio = totalRepoFiles > 0 ? coveredFiles.size / totalRepoFiles : 0; + } return { value, grade, breakdown, coverageRatio }; }