Skip to content

fix(coverage): every surface states the same coverage (REQ-387, #956 items 1–2) - #1019

Merged
avrabe merged 7 commits into
mainfrom
fix/req-387-coverage-agreement
Sep 30, 2026
Merged

avrabe merged 7 commits into
mainfrom
fix/req-387-coverage-agreement

Conversation

@avrabe

@avrabe avrabe commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Why

#956 items 1–2. CoverageEntry::percentage() returned 100 for an empty scope, and its doc comment said display code MUST check is_empty_scope() first. Only the CLI did. The dashboard, static export, JSON API and MCP painted a green 100% for a project whose sources: loaded nothing. None of them applied coverage.unmodelled-rules either, so a project got one answer from rivet coverage and another everywhere else.

What

  • API-compatible: main is already 0.40.0, so a public break fails the semver gate. percentage_opt(), accounted_percentage_opt(), overall_coverage_opt() and ClosureEntry::percentage_opt() return None for an empty scope. The 100-returning methods remain but are #[deprecated], so clippy -D warnings rejects any rivet use of them. The compiler forced every surface over.
  • One rendering: coverage::format_percentage / coverage_band render None as n/a / null, in a neutral colour, never green.
  • One definition of project coverage: coverage::compute_project_coverage applies the unmodelled-rules declarations. The serve API, the dashboard coverage and stats pages, and MCP use it and carry the declared reason.
  • Same bug class found and fixed in three more places:
    • the export matrix's row coverage (100% for an empty row);
    • rivet context, which told an AI agent "Overall: 100.0%" and "All rules at 100% coverage" for a project with nothing loaded;
    • --fail-under, whose JSON said "passed": true in the run that exited 1 for an empty load.
  • Kani: the proofs now prove None exactly when total is 0, and bounds otherwise.

Verification

  • One fixture (empty scope plus a declared unmodelled rule) is asserted across the CLI (coverage JSON, the gate, rivet context), the serve API and dashboard, and MCP. Six negative controls (reintroducing 100 on each surface, dropping the reason, the gate) each fail a named test.
  • Before this, the whole suite passed; no test covered what these surfaces showed for an empty scope.
  • fmt, clippy 1.97 --all-targets -D warnings, cargo test --workspace --no-fail-fast (2638), rivet validate, rivet docs check, yamllint.

Residual, bound by the public API (recorded on REQ-387)

  • The snapshot format stores an f64, so it keeps 100 for an empty scope.
  • Core's single-page export shows n/a but not the unmodelled reason.
  • Item 3 (MCP validate parity) and the lower tier remain.

Refs #956.

🤖 Generated with Claude Code

https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

📐 Rivet artifact delta

Change Count
Added 0
Removed 0
Modified 1
Downstream impacted (depth ≤ 5) 0

Graph

graph LR
  REQ_387["REQ-387"]:::modified
  classDef added fill:#d4edda,stroke:#28a745,color:#155724
  classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
  classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
  classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Loading
Modified
ID Changes
REQ-387

📎 Full HTML dashboard attached as workflow artifact rivet-delta-pr-1019 — download from the workflow run.

Posted by rivet-delta workflow. The graph shows only changed artifacts; open the HTML dashboard (above) for full context.

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: fcd2665 Previous: d2c7c37 Ratio
store_insert/10000 20260990 ns/iter (± 1026131) 12998695 ns/iter (± 636331) 1.56
link_graph_build/10000 33752074 ns/iter (± 1306418) 24773408 ns/iter (± 1563623) 1.36
diff/10000 9405194 ns/iter (± 604730) 7659528 ns/iter (± 249548) 1.23

This comment was automatically generated by workflow using github-action-benchmark.

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.97717% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rivet-core/src/coverage.rs 90.19% 10 Missing ⚠️
rivet-core/src/snapshot.rs 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

avrabe added a commit that referenced this pull request Sep 30, 2026
Codecov reported 77% of #1019's changed lines exercised; the export coverage page, its overall badge and the embed coverage table had no test for the n/a path. Each now asserts n/a in a neutral colour, and that a scored rule keeps its number; rendering 100 for None, or colouring it green, fails them.

Verifies: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
@avrabe
avrabe force-pushed the fix/req-387-coverage-agreement branch from 622f07f to fcd2665 Compare September 30, 2026 11:37
avrabe and others added 5 commits September 30, 2026 16:46
…items 1-2)

Item 1: CoverageEntry::percentage() returned 100 for an empty scope and its
doc said display code MUST check is_empty_scope() first; only the CLI did.
The dashboard, static export, MCP tool and JSON API painted a green 100%.
New percentage_opt / accounted_percentage_opt / overall_coverage_opt (and
ClosureEntry::percentage_opt) return None for an empty scope; the legacy
methods stay for API compatibility but are #[deprecated], so clippy
-D warnings makes any rivet use of them a CI failure. Every surface now
renders None as n/a / null through coverage::format_percentage and
coverage::coverage_band. The same class was found and fixed in the export
matrix's row coverage, rivet context ("Overall: 100.0%", "All rules at
100% coverage" for an agent), and --fail-under, whose JSON said
"passed": true in the run that exited 1 for an empty load.

Item 2: coverage::compute_project_coverage applies coverage.unmodelled-rules;
the serve API, dashboard coverage and stats pages, and MCP use it, and
carry the declared reason as rivet coverage does.

Tests assert agreement on one fixture across the CLI (coverage JSON, the
gate, rivet context), the serve API and dashboard, and MCP; six negative
controls (reintroducing 100 on each surface, dropping the reason, the gate)
each fail a named test. Kani proofs now prove None exactly at total 0.

Residual, API-bound: the snapshot format stores an f64 and keeps the legacy
100 for an empty scope; core export's single-page renderer shows n/a but
not the unmodelled reason. Both need a public API change.

Implements: REQ-387
Verifies: REQ-387
Refs: #956

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
Implements: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
…e (REQ-387)

The Grafana-oriented API specs asserted every percentage is a number, so they failed once an empty-scope rule reported null instead of 100. They now assert the contract for every rule: null exactly when total is 0, otherwise a number in [0, 100]. The /api/v1/coverage spec previously checked only the first rule, which passed by luck. The full Playwright suite passes locally, 406 of 406. The CHANGELOG notes the change for API consumers.

Verifies: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
Codecov reported 77% of #1019's changed lines exercised; the export coverage page, its overall badge and the embed coverage table had no test for the n/a path. Each now asserts n/a in a neutral colour, and that a scored rule keeps its number; rendering 100 for None, or colouring it green, fails them.

Verifies: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
…ontract (REQ-387)

The PR-diff mutation gate reported 15 surviving mutants: the deprecated
percentage methods (kept for API consumers, no longer called by rivet)
had no test of their legacy contract, and coverage_band's boundaries were
unpinned. Tests now cover both. One survivor was equivalent: at magnitude
100, f64::EPSILON is below resolution, so `100.0 - EPSILON` was 100.0; the
band now tests `>= 100.0`, which is exact since covered == total gives
exactly 100.0. A clean local run over those functions: 62 of 62 caught, no
mutant built against a stale binary.

Verifies: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
@avrabe
avrabe force-pushed the fix/req-387-coverage-agreement branch from 2eb7cd8 to 8a1ffda Compare September 30, 2026 14:47
avrabe and others added 2 commits September 30, 2026 17:50
The REQ-387 rewrite of proof_coverage_percentage_bounds bound pct inside a match arm and used it after the match, so the harness did not compile under cfg(kani); the ordinary build never compiles it. The zero-total case now returns early, and cargo kani --harness proof_coverage_percentage_bounds reports 1 of 1 verified.

Implements: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
The PR-diff mutation job reached its 45-minute limit after 73 of 90 mutants (none missed). A local clean run of the remaining ones found 9 survivors in export.rs: the matrix row percentage arithmetic and its empty-row guard, and the single-page index and coverage sections, which no test rendered. Two in-module tests now assert a 1-of-2 row renders 50.0% and that both single-page sections show the overall label the coverage report computes (n/a for an empty scope). Rerun with the tests: 18 caught, 1 unviable, 0 missed; no mutant log built against a stale binary.

Verifies: REQ-387

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9
@avrabe

avrabe commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

PR-diff mutation on cdfe334 reached the job's 45-minute limit after 73 of 90 mutants: 56 caught, 17 unviable, 0 missed. I ran the 17 it didn't reach locally, in a clean worktree with no shared target dir. Nine survived, all in export.rs: the matrix row percentage and its empty-row guard, render_section_index and render_section_coverage. 7b7a026 adds two in-module tests; the rerun gives 18 caught, 1 unviable, 0 missed, and none of the 19 builds reused a stale binary. Kani failed twice with exit 143 (runner shutdown mid-compile). The harness compile error fixed in cdfe334 was real; cargo kani --harness proof_coverage_percentage_bounds verifies locally with 0.67.0.

🤖 Generated with Claude Code

https://claude.ai/code/session_015HMQUV3u86jN2hmCtXNTc9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant