From b9dab56379cda6900971036aad94da1ed51a5780 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 16 Aug 2026 22:51:44 +0200 Subject: [PATCH] ci: enforce the checks the contributor rules already required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four gates that were documented but unenforced, plus the flaky test that made a full-suite run untrustworthy. Rust lint/format: CLAUDE.md has required `cargo fmt` and `cargo clippy` before every commit for as long as the rule existed, yet neither ran anywhere in CI — the requirement rested on memory alone. Both now run in build-and-test.yml and build-release.yml. rustfmt and clippy are already baked into the builder image, so nothing is installed at job time. `cargo fmt --all -- --check` is strict immediately (the tree is clean). Clippy is advisory for now: ~51 pre-existing warnings mean `-D warnings` would fail on unrelated work, so the step carries a TODO to flip the flag once the backlog clears. A compile error still fails it, so it is not a no-op. Traceability threshold: MIN_THRESHOLD sat at 50 while real coverage was 86%, so nearly half the matrix could rot before the gate objected. Ratcheted to 82 with the policy written down — it only ever goes up, and is never lowered to make a red build pass. The same figure lives in MIN_COVERAGE_PERCENT so `traces:coverage` gates locally on the same bar, and a test fails if the two drift. Dangling IDs: a TRACES comment could name any well-formed ID and the extractor accepted it silently, so typos and renames that missed a call site passed unnoticed. `bun run traces:validate` cross-checks every traced ID against the table rows in requirements.md and fails with the referencing files listed. It spans UT/IT as well, which the coverage orphan list ignores by design. This currently reports DR-189 and UT-188, which are being defined separately. Flaky offlineCatalog test: the first dynamic import of the service paid ~1s to transform its dependency graph, charged to a test body against vitest's 5s default. Alone it passed; under suite-wide contention it timed out. The import is now warmed at collection time, so no test is timing the compiler — the timeout is deliberately unchanged. The store shim also drops subscribers from module instances discarded by resetModules, which previously leaked across tests. --- .gitea/workflows/build-and-test.yml | 26 ++++++ .gitea/workflows/build-release.yml | 16 ++++ .gitea/workflows/traceability-check.yml | 25 +++++- CLAUDE.md | 14 +++- docs/traceability-ci.md | 52 +++++++++--- docs/traces-quick-ref.md | 7 +- package.json | 1 + scripts/README.md | 12 ++- scripts/extract-traces.test.ts | 103 +++++++++++++++++++++-- scripts/extract-traces.ts | 106 +++++++++++++++++++++++- src/lib/services/offlineCatalog.test.ts | 24 +++++- 11 files changed, 355 insertions(+), 31 deletions(-) diff --git a/.gitea/workflows/build-and-test.yml b/.gitea/workflows/build-and-test.yml index fae13e8a..7e2876b7 100644 --- a/.gitea/workflows/build-and-test.yml +++ b/.gitea/workflows/build-and-test.yml @@ -61,6 +61,32 @@ jobs: bunx svelte-kit sync bun run test + # CLAUDE.md has required `cargo fmt` + `cargo clippy` before every commit + # for as long as the rule has existed, but nothing in CI checked either, + # so the requirement rested entirely on memory. Both components are baked + # into the builder image (Dockerfile.builder: `rustup component add + # rustfmt clippy`) — nothing is installed at job time. + - name: Check Rust formatting + run: | + cd src-tauri + cargo fmt --all -- --check + + # ⚠️ Advisory for now — clippy warnings do NOT fail this job yet. + # + # The tree carries ~51 pre-existing warnings; adding `-D warnings` today + # would paint CI red on unrelated work. A compile *error* still fails the + # step, so this is not a no-op: it stops new breakage and surfaces the + # backlog in every run. + # + # TODO: once the existing warnings are cleared, tighten this to + # cargo clippy --all-targets -- -D warnings + # Flip that flag — do not delete the step. Track progress with + # `cd src-tauri && cargo clippy --all-targets 2>&1 | grep -c '^warning'`. + - name: Run clippy (advisory) + run: | + cd src-tauri + cargo clippy --all-targets + - name: Run Rust tests run: | cd src-tauri diff --git a/.gitea/workflows/build-release.yml b/.gitea/workflows/build-release.yml index 534f32ed..5cde4636 100644 --- a/.gitea/workflows/build-release.yml +++ b/.gitea/workflows/build-release.yml @@ -54,6 +54,22 @@ jobs: bun run test --run continue-on-error: false + # Same gate as build-and-test.yml. A release must not ship from a tree + # that would fail the per-commit checks. rustfmt/clippy come from the + # builder image; nothing is installed here. + - name: Check Rust formatting + run: | + cd src-tauri + cargo fmt --all -- --check + continue-on-error: false + + # Advisory until the ~51 pre-existing warnings are cleared; see the longer + # note in build-and-test.yml. Tighten both to `-- -D warnings` together. + - name: Run clippy (advisory) + run: | + cd src-tauri + cargo clippy --all-targets + - name: Run Rust tests run: bun run test:rust continue-on-error: false diff --git a/.gitea/workflows/traceability-check.yml b/.gitea/workflows/traceability-check.yml index b42920cb..ff4e1ba6 100644 --- a/.gitea/workflows/traceability-check.yml +++ b/.gitea/workflows/traceability-check.yml @@ -81,8 +81,20 @@ jobs: exit 1 fi - # Check minimum threshold - MIN_THRESHOLD=50 + # Minimum coverage. RATCHET POLICY: this number only ever goes UP. + # + # It sits a few points under the coverage actually achieved, so a real + # regression trips it. It was 50 while true coverage was 86%, which + # meant nearly half the matrix could rot before CI said a word — a + # gate that cannot fail is not a gate. + # + # When coverage rises durably, raise this to just under the new figure + # (`bun run traces:coverage` prints it). Never lower it to make a red + # build pass — add the missing TRACES comments instead. + # + # Keep in sync with MIN_COVERAGE_PERCENT in scripts/extract-traces.ts; + # scripts/extract-traces.test.ts fails if the two drift apart. + MIN_THRESHOLD=82 if [ "$COVERAGE" -lt "$MIN_THRESHOLD" ]; then echo "❌ ERROR: Coverage ($COVERAGE%) is below minimum threshold ($MIN_THRESHOLD%)" exit 1 @@ -90,6 +102,15 @@ jobs: echo "✅ Coverage is acceptable ($COVERAGE% >= $MIN_THRESHOLD%)" + # Every ID named by a TRACES comment must be defined as a table row in + # docs/requirements.md. The extractor used to accept any well-formed ID + # silently, so a typo or a rename that missed a call site passed CI + # unnoticed (DR-189 and UT-188 lived in three source files, defined + # nowhere, for months). This covers UT/IT too, which the coverage + # orphan list above deliberately ignores. + - name: Validate requirement IDs + run: bun run traces:validate + - name: Check modified files if: github.event_name == 'pull_request' run: | diff --git a/CLAUDE.md b/CLAUDE.md index 30597d47..fd9f92db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,14 +101,22 @@ Tooling: bun run traces # extract traces (default format) bun run traces:json # JSON — e.g. | jq '.byType' or '.requirements."UR-005"' bun run traces:markdown # regenerate docs/traceability.md +bun run traces:coverage # coverage gate — exits non-zero below the threshold +bun run traces:validate # dangling-ID gate — every traced ID must be defined git diff --name-only | xargs grep -L "TRACES:" # find untraced changed files ``` +Every ID a `TRACES:` comment names must exist as a table row in +`docs/requirements.md` — `traces:validate` fails otherwise, so a typo or a +rename that missed a call site can no longer pass silently. + **CI is Gitea Actions** (`.gitea/workflows/`, remote `gitea.tourolle.paris`), not GitHub. `traceability-check.yml` fails the build if coverage drops below -**50%** (`MIN_THRESHOLD`); `build-and-test.yml` runs frontend + Rust tests and an -Android `cargo check`. See [docs/traceability-ci.md](docs/traceability-ci.md) and -[docs/traces-quick-ref.md](docs/traces-quick-ref.md). +**82%** (`MIN_THRESHOLD`, a *ratchet* — raise it as coverage climbs, never lower +it to make a build pass) or if any traced ID is undefined; `build-and-test.yml` +runs frontend + Rust tests, `cargo fmt --check`, an advisory `cargo clippy`, and +an Android `cargo check`. See [docs/traceability-ci.md](docs/traceability-ci.md) +and [docs/traces-quick-ref.md](docs/traces-quick-ref.md). ### Traces drive release notes diff --git a/docs/traceability-ci.md b/docs/traceability-ci.md index e1d4fb17..33853173 100644 --- a/docs/traceability-ci.md +++ b/docs/traceability-ci.md @@ -15,7 +15,7 @@ The CI/CD pipeline automatically validates that code changes are properly traced Traceability validation lives in `.gitea/workflows/traceability-check.yml`: - ✅ Automatic trace extraction -- ✅ Coverage validation against minimum threshold (50%) +- ✅ Coverage validation against minimum threshold (82%, ratcheted) - ✅ Modified file checking - ✅ Artifact preservation - ✅ Summary reports @@ -43,7 +43,7 @@ Extracts all TRACES comments from: ### 2. Coverage Thresholds The workflow checks: -- **Minimum overall coverage:** 50% +- **Minimum overall coverage:** 82% (`MIN_THRESHOLD`) Denominators are **derived from `docs/requirements.md` at run time** — they are never hardcoded here or in the workflow. Run `bun run traces:coverage` for the @@ -61,8 +61,39 @@ a `TRACES:` comment but is not defined in `requirements.md` is reported as **orphaned** and does not count toward coverage. UT/IT test identifiers are a separate taxonomy and are excluded entirely. -The workflow **fails** and blocks merge if coverage drops below 50% — or if it -computes above 100%, which can only mean the gate is miscounting. +The workflow **fails** and blocks merge if coverage drops below the threshold — +or if it computes above 100%, which can only mean the gate is miscounting. + +#### Ratchet policy + +`MIN_THRESHOLD` **only ever goes up.** It is deliberately set a few points below +the coverage actually achieved (82 against a real 86%), so a genuine regression +trips it. It previously sat at 50 while true coverage was 86%: nearly half the +matrix could have rotted before CI objected. + +When coverage rises durably, raise the threshold to just under the new figure. +**Never lower it to make a red build pass** — add the missing TRACES comments +instead. The same number lives in `MIN_COVERAGE_PERCENT` in +`scripts/extract-traces.ts` (so `bun run traces:coverage` gates locally on the +same bar); `scripts/extract-traces.test.ts` fails if the two drift apart. + +### 2b. Dangling requirement IDs + +```bash +bun run traces:validate +``` + +Every ID named by a `TRACES:` comment must be defined as a table row in +`docs/requirements.md`. The extractor used to accept any well-formed ID +silently, so a typo or a rename that missed a call site passed unnoticed — +`DR-189` and `UT-188` were referenced from three source files, defined nowhere, +for months. + +This check spans **all six** ID types (UR/IR/DR/JA/UT/IT), unlike the coverage +`orphaned` list above, which considers only the four requirement types so that +UT/IT noise cannot bury a real typo in the ratio's reporting. The workflow step +**fails the build** on any dangling ID and prints each offender with the files +that reference it. ### 3. Modified File Checking On pull requests, the workflow: @@ -120,13 +151,13 @@ TRACES: [UR-###, ...] | [IR-###, ...] | [DR-###, ...] | [JA-###, ...] ### On Push to Main Branch 1. ✅ Extracts all traces from code -2. ✅ Validates coverage is >= 50% +2. ✅ Validates coverage is >= 82% 3. ✅ Generates full traceability report 4. ✅ Saves report as artifact ### On Pull Request 1. ✅ Extracts all traces -2. ✅ Validates coverage >= 50% +2. ✅ Validates coverage >= 82% 3. ✅ Checks modified files for TRACES 4. ✅ Warns if new code lacks TRACES 5. ✅ Suggests proper format @@ -134,7 +165,8 @@ TRACES: [UR-###, ...] | [IR-###, ...] | [DR-###, ...] | [JA-###, ...] ### Failure Scenarios The workflow **fails** (blocks merge) if: -- Coverage drops below 50% +- Coverage drops below 82% +- A `TRACES:` comment names an ID `docs/requirements.md` does not define - JSON extraction fails - Invalid trace format @@ -174,7 +206,7 @@ made the broken CI arithmetic look plausible for so long. As of July 2026 overall coverage is ~86% (182/212). ### Targets -- **Short term** (Sprint): Maintain ≥50% overall +- **Short term** (Sprint): Maintain ≥82% overall (the current ratchet) - **Medium term** (Month): Reach 70% overall coverage - **Long term** (Release): Reach 90% coverage with focus on: - IR requirements (API clients) @@ -209,14 +241,14 @@ When submitting a pull request: - [ ] All new code has TRACES comments linking to requirements - [ ] TRACES format is correct: `// TRACES: UR-001 | DR-002` -- [ ] Workflow passes (coverage ≥ 50%) +- [ ] Workflow passes (coverage ≥ 82%) - [ ] No coverage regressions - [ ] Artifact traceability report was generated ## Troubleshooting ### "Coverage below minimum threshold" -**Problem:** Workflow fails with coverage < 50% +**Problem:** Workflow fails with coverage < 82% **Solution:** 1. Run `bun run traces:json` locally diff --git a/docs/traces-quick-ref.md b/docs/traces-quick-ref.md index 99a05c96..f9766e33 100644 --- a/docs/traces-quick-ref.md +++ b/docs/traces-quick-ref.md @@ -133,13 +133,14 @@ bun run traces:json | jq '.requirements."UR-005"' ### Before Committing 1. Ensure all new code has TRACES 2. Format is correct: `// TRACES: ...` -3. Requirements exist in README.md -4. No typos in requirement IDs +3. Requirements exist in `docs/requirements.md` — `bun run traces:validate` +4. No typos in requirement IDs (same command catches them) ## CI/CD Validation The workflow automatically checks: -- ✅ Coverage stays >= 50% +- ✅ Coverage stays >= 82% (a ratchet — raise it, never lower it) +- ✅ Every traced ID is defined in `docs/requirements.md` - ✅ New files have TRACES - ✅ JSON format is valid - ✅ Reports are generated diff --git a/package.json b/package.json index 39a369b0..403ef2e1 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "traces:json": "bun run scripts/extract-traces.ts --format json", "traces:markdown": "bun run scripts/extract-traces.ts --format markdown > docs/traceability.md", "traces:coverage": "bun run scripts/extract-traces.ts --format coverage", + "traces:validate": "bun run scripts/extract-traces.ts --format validate", "release:notes": "bun run scripts/release-notes.ts" }, "license": "MIT", diff --git a/scripts/README.md b/scripts/README.md index d4aa371c..f437bf27 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -69,7 +69,8 @@ Extract requirement IDs (TRACES) from source code and generate a traceability ma bun run traces # Generate markdown report bun run traces:json # Generate JSON report bun run traces:markdown # Save to docs/traceability.md -bun run traces:coverage # Coverage gate — exits non-zero below 50% +bun run traces:coverage # Coverage gate — exits non-zero below the ratchet +bun run traces:validate # Dangling-ID gate — every traced ID must be defined ``` The script scans all TypeScript, Svelte, and Rust files (plus `scripts/`) @@ -84,6 +85,12 @@ derived from `docs/requirements.md` at run time; they are never hardcoded. An ID that appears in a `TRACES:` comment but is not defined in `requirements.md` is reported as *orphaned* and does not count toward coverage (see DR-093). +**`bun run traces:validate` is the dangling-ID gate.** It fails if any traced ID +— including `UT`/`IT`, which coverage deliberately ignores — is not defined as a +table row in `requirements.md`, printing each offender with the files that +reference it. Without it the extractor accepted any well-formed ID silently, so +typos and renames that missed a call site went unreported for months. + > **Removed:** `check-req-coverage.sh`, `check-test-coverage.sh`, and > `find-req-implementations.sh` were deleted in July 2026. They read an > undocumented `@req:` tag convention parallel to `TRACES:`, grepped `src-tauri/` @@ -104,7 +111,8 @@ See [docs/traceability.md](../docs/traceability.md) for the latest generated map The traceability system is integrated with Gitea Actions CI/CD: - Automatically validates TRACES on every push and pull request -- Enforces minimum 50% coverage threshold +- Enforces a minimum coverage threshold (a ratchet: raise it, never lower it) +- Fails on dangling IDs — traced but undefined in `requirements.md` - Warns if new code lacks TRACES comments - Generates traceability reports automatically diff --git a/scripts/extract-traces.test.ts b/scripts/extract-traces.test.ts index 9f7e283c..735c32d3 100644 --- a/scripts/extract-traces.test.ts +++ b/scripts/extract-traces.test.ts @@ -14,7 +14,17 @@ */ import { describe, it, expect } from "vitest"; -import { countDefinedRequirements, computeCoverage } from "./extract-traces"; +import * as fs from "fs"; +import * as path from "path"; +import { + countDefinedRequirements, + computeCoverage, + findDanglingIds, + MIN_COVERAGE_PERCENT, +} from "./extract-traces"; + +// import.meta.dir is Bun-only; derive from import.meta.url under vitest. +const HERE = path.dirname(new URL(import.meta.url).pathname); describe("countDefinedRequirements", () => { it("counts a well-formed table row as a defined requirement", () => { @@ -82,6 +92,80 @@ Some prose explaining that UR-005 relates to DR-001 and JA-002. expect(defined.ids.has("DR-050")).toBe(true); expect(defined.ids.has("UR-999")).toBe(false); }); + + it("collects UT/IT rows separately, out of the coverage denominator", () => { + // §4 defines the test taxonomy. Those rows must be known (so a TRACES + // comment may name them) without ever moving the coverage ratio. + const md = ` +| UR-001 | A | High | Done | +| UT-001 | Player state transitions | DR-001 | Pending | +| IT-004 | Playback end-to-end | DR-002 | Pending | +`; + const defined = countDefinedRequirements(md); + expect(defined.total).toBe(1); + expect(defined.ids.has("UT-001")).toBe(false); + expect(defined.testIds.has("UT-001")).toBe(true); + expect(defined.testIds.has("IT-004")).toBe(true); + }); +}); + +describe("findDanglingIds", () => { + const defined = { + UR: 1, + IR: 0, + DR: 1, + JA: 0, + total: 2, + ids: new Set(["UR-001", "DR-001"]), + testIds: new Set(["UT-001"]), + }; + + it("flags a requirement ID that requirements.md does not define", () => { + expect(findDanglingIds(["UR-001", "DR-189"], defined)).toEqual(["DR-189"]); + }); + + it("flags an undefined UT/IT id, which the coverage orphan list cannot", () => { + // The gap this closes: computeCoverage deliberately ignores UT/IT, so + // UT-188 sat in three source files, defined nowhere, entirely unreported. + expect(computeCoverage(["UT-188"], defined).orphaned).toEqual([]); + expect(findDanglingIds(["UT-188"], defined)).toEqual(["UT-188"]); + }); + + it("accepts every ID that is defined, requirement or test", () => { + expect(findDanglingIds(["UR-001", "DR-001", "UT-001"], defined)).toEqual([]); + }); + + it("deduplicates and sorts, so one typo is reported once", () => { + expect( + findDanglingIds(["DR-189", "DR-189", "UR-999", "DR-189"], defined) + ).toEqual(["DR-189", "UR-999"]); + }); + + it("ignores IDs whose prefix is not a known trace type", () => { + // e.g. an unrelated "AB-123" caught by the loose ID regex. + expect(findDanglingIds(["AB-123"], defined)).toEqual([]); + }); +}); + +describe("coverage threshold", () => { + it("matches MIN_THRESHOLD in the Gitea traceability workflow", () => { + // Two files must agree on the gate: the script (local `traces:coverage`) + // and the workflow. Drift means the local gate and CI disagree about what + // passes, which is how the 50%-while-actually-86% slack went unnoticed. + const workflow = fs.readFileSync( + path.resolve(HERE, "../.gitea/workflows/traceability-check.yml"), + "utf-8" + ); + const match = workflow.match(/^\s*MIN_THRESHOLD=(\d+)\s*$/m); + expect(match).not.toBeNull(); + expect(Number(match![1])).toBe(MIN_COVERAGE_PERCENT); + }); + + it("is a ratchet: never lower it to make a red build pass", () => { + // Sanity bound. If coverage genuinely climbs, raise both numbers together. + expect(MIN_COVERAGE_PERCENT).toBeGreaterThanOrEqual(82); + expect(MIN_COVERAGE_PERCENT).toBeLessThanOrEqual(100); + }); }); describe("computeCoverage", () => { @@ -92,6 +176,7 @@ describe("computeCoverage", () => { JA: 0, total: 4, ids: new Set(["UR-001", "UR-002", "DR-001", "DR-002"]), + testIds: new Set(), }; it("computes coverage as traced ∩ defined over defined", () => { @@ -138,7 +223,15 @@ describe("computeCoverage", () => { }); it("reports 0% rather than NaN when nothing is defined", () => { - const empty = { UR: 0, IR: 0, DR: 0, JA: 0, total: 0, ids: new Set() }; + const empty = { + UR: 0, + IR: 0, + DR: 0, + JA: 0, + total: 0, + ids: new Set(), + testIds: new Set(), + }; const cov = computeCoverage([], empty); expect(cov.percent).toBe(0); expect(Number.isNaN(cov.percent)).toBe(false); @@ -163,12 +256,8 @@ describe("live requirements.md", () => { // (total 114) while the real file had grown to 211. Update these numbers // deliberately when requirements are added — that edit is the signal the // denominator is live rather than frozen. - const fs = require("fs"); - const path = require("path"); - // import.meta.dir is Bun-only; derive from import.meta.url under vitest. - const here = path.dirname(new URL(import.meta.url).pathname); const md = fs.readFileSync( - path.resolve(here, "../docs/requirements.md"), + path.resolve(HERE, "../docs/requirements.md"), "utf-8" ); const defined = countDefinedRequirements(md); diff --git a/scripts/extract-traces.ts b/scripts/extract-traces.ts index b45ca228..bf7d2cef 100644 --- a/scripts/extract-traces.ts +++ b/scripts/extract-traces.ts @@ -37,8 +37,27 @@ interface TracesData { /** Requirements *defined* in requirements.md — the coverage denominators. */ defined?: { UR: number; IR: number; DR: number; JA: number; total: number }; coverage?: CoverageResult; + /** Traced IDs of any type that requirements.md does not define. */ + dangling?: string[]; } +/** + * Minimum overall requirement coverage the traceability gate accepts. + * + * **Ratchet policy: this number only ever goes up.** It is set a few points + * below the coverage actually achieved, so a real regression trips it instead of + * being absorbed by slack. It sat at 50 while true coverage was 86%, which meant + * half the matrix could rot before CI noticed. When coverage rises durably, + * raise this to sit just under the new figure. Do **not** lower it to make a + * failing build pass — add the missing TRACES comments instead. + * + * `.gitea/workflows/traceability-check.yml` carries the same number as + * `MIN_THRESHOLD`; `scripts/extract-traces.test.ts` fails if the two drift. + * + * TRACES: | DR-093 + */ +export const MIN_COVERAGE_PERCENT = 82; + // Repo root, derived from this script's location (scripts/ -> repo root). // Must NOT be hardcoded to a developer's machine, or CI checkouts see no files. // @@ -222,7 +241,10 @@ export interface DefinedRequirements { DR: number; JA: number; total: number; + /** Requirement IDs (UR/IR/DR/JA) — the coverage denominator. */ ids: Set; + /** Test IDs (UT/IT) from §4. A separate taxonomy: never part of coverage. */ + testIds: Set; } export interface CoverageResult { @@ -247,11 +269,18 @@ export interface CoverageResult { */ export function countDefinedRequirements(markdown: string): DefinedRequirements { const ids = new Set(); - const ROW_ID = /^\|\s*(UR|IR|DR|JA)-(\d{3})\s*\|/; + const testIds = new Set(); + const ROW_ID = /^\|\s*(UR|IR|DR|JA|UT|IT)-(\d{3})\s*\|/; for (const line of markdown.split("\n")) { const match = line.match(ROW_ID); - if (match) ids.add(`${match[1]}-${match[2]}`); + if (!match) continue; + const id = `${match[1]}-${match[2]}`; + // UT/IT rows live in §4 and are collected separately: they must not enter + // the coverage denominator, but they still need to exist for a `TRACES:` + // comment to be allowed to name them (see findDanglingIds). + if (match[1] === "UT" || match[1] === "IT") testIds.add(id); + else ids.add(id); } const countOf = (type: string) => @@ -264,9 +293,39 @@ export function countDefinedRequirements(markdown: string): DefinedRequirements JA: countOf("JA"), total: ids.size, ids, + testIds, }; } +/** + * Every traced ID that requirements.md defines nowhere — a typo, a rename that + * missed a call site, or a reference to a deleted requirement. + * + * This is broader than `CoverageResult.orphaned`, which only ever considers the + * four requirement types because a UT/IT entry among the orphans would corrupt + * the coverage ratio's reporting. Dangling detection has no such constraint, so + * it checks all six ID types against both defined sets. Before it existed, the + * extractor accepted any well-formed ID silently: `DR-189` and `UT-188` were + * referenced from `controlsVisibility.ts` and `VideoPlayer.svelte` for months + * without being defined anywhere, and nothing reported it. + * + * TRACES: | DR-093 + */ +export function findDanglingIds( + tracedIds: string[], + defined: DefinedRequirements +): string[] { + const KNOWN_TYPE = /^(UR|IR|DR|JA|UT|IT)-\d{3}$/; + + const dangling = new Set( + tracedIds + .filter((id) => KNOWN_TYPE.test(id)) + .filter((id) => !defined.ids.has(id) && !defined.testIds.has(id)) + ); + + return [...dangling].sort(); +} + /** * Coverage is the *intersection* of traced and defined IDs over defined IDs. * @@ -408,6 +467,13 @@ function reportCoverage(data: TracesData, minThreshold: number): number { console.log(" Fix the TRACES comment or add the requirement."); } + if (data.dangling && data.dangling.length > 0) { + console.log(""); + console.log( + `⚠️ Dangling IDs (incl. UT/IT): ${data.dangling.join(", ")} — run \`bun run traces:validate\`.` + ); + } + // A ratio above 100% means the computation is broken (the condition that hid // the stale-denominator bug for so long). Fail loudly rather than report it. if (cov.percent > 100) { @@ -427,6 +493,37 @@ function reportCoverage(data: TracesData, minThreshold: number): number { return 0; } +/** + * Hard gate on dangling IDs: a `TRACES:` comment may only name an ID that + * requirements.md actually defines. Prints every offender with the files that + * reference it, so the fix is mechanical. + * + * TRACES: | DR-093 + */ +function reportDangling(data: TracesData): number { + const dangling = data.dangling ?? []; + + if (dangling.length === 0) { + console.log("✅ All traced IDs are defined in docs/requirements.md"); + return 0; + } + + console.log("❌ TRACES reference IDs that docs/requirements.md does not define:"); + console.log(""); + for (const id of dangling) { + const files = [ + ...new Set((data.requirements[id] ?? []).map((e) => e.file)), + ].sort(); + console.log(` ${id}`); + for (const file of files) console.log(` ${file}`); + } + console.log(""); + console.log("Fix each one by either:"); + console.log(" • correcting the ID in the TRACES comment (typo/rename), or"); + console.log(" • adding the requirement as a table row in docs/requirements.md."); + return 1; +} + // Main — guarded so this module stays importable from extract-traces.test.ts. if (import.meta.main) { const args = process.argv.slice(2); @@ -447,11 +544,14 @@ if (import.meta.main) { total: defined.total, }; data.coverage = computeCoverage(allTraced, defined); + data.dangling = findDanglingIds(allTraced, defined); if (format === "json") { console.log(generateJson(data)); } else if (format === "coverage") { - process.exit(reportCoverage(data, 50)); + process.exit(reportCoverage(data, MIN_COVERAGE_PERCENT)); + } else if (format === "validate") { + process.exit(reportDangling(data)); } else { console.log(generateMarkdown(data)); } diff --git a/src/lib/services/offlineCatalog.test.ts b/src/lib/services/offlineCatalog.test.ts index e863c129..83c002e9 100644 --- a/src/lib/services/offlineCatalog.test.ts +++ b/src/lib/services/offlineCatalog.test.ts @@ -23,6 +23,13 @@ const h = vi.hoisted(() => { fn(value); return () => subs.delete(fn); }, + // Drop subscribers left behind by module instances discarded via + // `vi.resetModules()`. Without this, every previously-imported copy of the + // service still reacts to `set()` and pushes its own visibility value. + reset(v: T) { + subs.clear(); + value = v; + }, }; } return { @@ -31,6 +38,21 @@ const h = vi.hoisted(() => { }; }); +// Prime the module graph once, at collection time, instead of inside a test. +// +// Every test re-imports the service after `vi.resetModules()` so it gets a fresh +// set of module-level subscriptions. The *first* of those imports also pays to +// transform the service and its dependency graph — around a second of real +// wall-clock work with a cold Vite cache. Charged to a test body that cost sat +// close enough to vitest's 5s default that suite-wide contention (many workers +// transforming at once) tipped this file into a timeout, while running the file +// alone always passed. Warming here moves the compile out of the timed region; +// the per-test re-imports that follow are cached and cost ~30ms. +// +// The timeout is deliberately left at the default: the point is to stop timing +// the compiler, not to give it a bigger budget. +await import("./offlineCatalog"); + vi.mock("$lib/stores/connectivity", () => ({ isConnected: { subscribe: h.isConnectedStore.subscribe }, })); @@ -50,8 +72,8 @@ vi.mock("$lib/stores/auth", () => ({ describe("pushCatalogVisibility resolves reachable || showCatalog (UT-068)", () => { beforeEach(() => { + h.isConnectedStore.reset(true); h.setShowServerCatalog.mockClear(); - h.isConnectedStore.set(true); vi.resetModules(); });