From e0a0399275285e85734f5b80ff234603ca7a6252 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sat, 30 May 2026 22:36:09 +0200 Subject: [PATCH 1/2] fix(coverage): relabel per-rule "totals" so they don't read as artifact counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug-hunt findings REQ-110 (HTML) + REQ-111 (JSON), 3/3 lens-confirmed. The coverage overview summed per-RULE denominators (an artifact in N rules counts N times) but labelled the result "artifacts covered" / exposed it under the JSON key `total` — the same key `stats` uses for the distinct-artifact count. Two different cardinalities, one label. Per the relabel decision (numbers unchanged): - HTML overview: "{n} / {m} coverage checks across R rules" (was "artifacts covered"). - coverage --format json `overall`: `checks_covered` / `checks_total` (the ambiguous `total`/`covered` keys are removed from `overall`). Per-rule `entries[]` keep `covered`/`total`, correct at rule scope. Regression test asserts the `checks_*` keys exist and the bare `total`/ `covered` keys are absent from `overall`. Fixes: REQ-110, REQ-111 Verifies: REQ-110, REQ-111 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 15 +++++++++++++++ rivet-cli/src/main.rs | 12 ++++++++---- rivet-cli/src/render/stats.rs | 10 +++++++--- rivet-cli/tests/cli_commands.rs | 26 ++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e160231..e9edac3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ ## [Unreleased] +### Fixed + +- **REQ-110 / REQ-111 — coverage "totals" no longer masquerade as artifact + counts.** The dashboard overview rendered `{covered} / {total} artifacts + covered` and the `coverage --format json` `overall` object exposed + `total`/`covered`, but both aggregate *per-rule* denominators — an artifact + satisfying N traceability rules is counted N times — which is a different + cardinality from the distinct-artifact `total` the `stats` command reports + under the same key. The numbers are unchanged (per the relabel decision); + the label is now honest: the HTML reads "coverage checks" and the JSON + `overall` exposes `checks_covered` / `checks_total` (the ambiguous + `total`/`covered` keys are removed from the `overall` object — a JSON + consumer change). Per-rule `entries[]` keep `covered`/`total`, which are + correct at rule scope. + ## [0.13.3] — 2026-05-29 Theme: **serve dashboard correctness — three user-reported fixes**. diff --git a/rivet-cli/src/main.rs b/rivet-cli/src/main.rs index a12c761..e247088 100644 --- a/rivet-cli/src/main.rs +++ b/rivet-cli/src/main.rs @@ -6099,17 +6099,21 @@ fn cmd_coverage( }) }) .collect(); - let total: usize = report.entries.iter().map(|e| e.total).sum(); - let covered: usize = report.entries.iter().map(|e| e.covered).sum(); + // REQ-111: these aggregate per-RULE denominators (an artifact satisfying + // N rules is counted N times) — NOT the distinct-artifact `total` that + // `stats` JSON reports. Emit under `checks_*` keys so the same key name + // `total` never carries two different cardinalities across commands. + let checks_total: usize = report.entries.iter().map(|e| e.total).sum(); + let checks_covered: usize = report.entries.iter().map(|e| e.covered).sum(); let external_boundary: usize = report.entries.iter().map(|e| e.external_boundary).sum(); let overall_pct = (report.overall_coverage() * 10.0).round() / 10.0; let mut output = serde_json::json!({ "command": "coverage", "rules": rules_json, "overall": { - "covered": covered, + "checks_covered": checks_covered, "external_boundary": external_boundary, - "total": total, + "checks_total": checks_total, "percentage": overall_pct, }, }); diff --git a/rivet-cli/src/render/stats.rs b/rivet-cli/src/render/stats.rs index 7e85283..7c28026 100644 --- a/rivet-cli/src/render/stats.rs +++ b/rivet-cli/src/render/stats.rs @@ -234,8 +234,12 @@ pub(crate) fn render_stats(ctx: &RenderContext) -> String { } else { "#c62828" }; - let total_covered: usize = cov_report.entries.iter().map(|e| e.covered).sum(); - let total_items: usize = cov_report.entries.iter().map(|e| e.total).sum(); + // REQ-110: these are per-RULE check sums (an artifact in N rules counts + // N times), NOT distinct artifacts. The store-artifact total lives on the + // "Artifacts" stat card. Label them "coverage checks" so the two + // different "totals" are not conflated. + let checks_covered: usize = cov_report.entries.iter().map(|e| e.covered).sum(); + let checks_total: usize = cov_report.entries.iter().map(|e| e.total).sum(); let cov_delta = bl.map_or(String::new(), |s| { delta_pct_badge(overall, s.coverage.overall) }); @@ -249,7 +253,7 @@ pub(crate) fn render_stats(ctx: &RenderContext) -> String {
\ \
\ - {total_covered} / {total_items} artifacts covered across {} rules\ + {checks_covered} / {checks_total} coverage checks across {} rules\
\ \ \ diff --git a/rivet-cli/tests/cli_commands.rs b/rivet-cli/tests/cli_commands.rs index 5f43729..aaddf2c 100644 --- a/rivet-cli/tests/cli_commands.rs +++ b/rivet-cli/tests/cli_commands.rs @@ -1722,6 +1722,32 @@ fn coverage_json() { parsed.get("rules").and_then(|v| v.as_array()).is_some(), "coverage JSON must contain 'rules' array" ); + + // REQ-111: the overall aggregate sums per-rule denominators (an artifact in + // N rules counts N times), a different cardinality from `stats` JSON's + // distinct-artifact `total`. It must be exposed under disambiguated + // `checks_*` keys and must NOT reuse the bare `total`/`covered` names that + // would collide semantically with the stats command. + let overall = &parsed["overall"]; + assert!( + overall.get("checks_total").and_then(|v| v.as_u64()).is_some(), + "coverage overall must expose 'checks_total'" + ); + assert!( + overall + .get("checks_covered") + .and_then(|v| v.as_u64()) + .is_some(), + "coverage overall must expose 'checks_covered'" + ); + assert!( + overall.get("total").is_none(), + "coverage overall must NOT use the ambiguous key 'total' (REQ-111)" + ); + assert!( + overall.get("covered").is_none(), + "coverage overall must NOT use the ambiguous key 'covered' (REQ-111)" + ); } // ── rivet matrix ─────────────────────────────────────────────────────── From cc95800326eca2d798bbd51a4d17e4bc3ee8f43d Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sun, 31 May 2026 09:16:20 +0200 Subject: [PATCH 2/2] style(coverage): rustfmt the REQ-110/111 test assertions Trace: skip --- rivet-cli/tests/cli_commands.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rivet-cli/tests/cli_commands.rs b/rivet-cli/tests/cli_commands.rs index aaddf2c..cfde622 100644 --- a/rivet-cli/tests/cli_commands.rs +++ b/rivet-cli/tests/cli_commands.rs @@ -1730,7 +1730,10 @@ fn coverage_json() { // would collide semantically with the stats command. let overall = &parsed["overall"]; assert!( - overall.get("checks_total").and_then(|v| v.as_u64()).is_some(), + overall + .get("checks_total") + .and_then(|v| v.as_u64()) + .is_some(), "coverage overall must expose 'checks_total'" ); assert!(