feat(rules)!: deprecate the hygiene score and stop it certifying empty scans (META-284)

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

packages/rules/src/engine/__tests__/hygiene-score.invariants.test.ts

@@ -21,48 +21,75 @@ 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) {
         const findings = [
           ...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<string, unknown>;
     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);
   });
 });

packages/rules/src/engine/hygiene-score.ts

@@ -1,4 +1,4 @@
 import type { Finding, HygieneScore } from '../types.js';
 
 const SEVERITY_WEIGHTS = {
   critical: 15,
@@ -7,10 +7,44 @@
   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,23 @@
   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;
+  // 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 };
 }

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();

packages/rules/src/types.ts

@@ -220,6 +220,11 @@
 
 // ─── 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 @@
     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 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;