fix(cli): count the tests a metric could not measure, not only the candidates the report dropped (#185) - #207
Merged
Merged
Conversation
…ndidates the report dropped (#185) `ExcludedLowEvidence` was incremented in exactly one place — the coordinator's own reporting floor — so it counted only candidates a provider had already offered. Every decline inside a provider was invisible, and that is where most of them happen. A test whose runs all recorded a zero median normalises nothing, so `DurationProvider` could compute neither of its statistics and declined both; the test then landed silently inside `healthy`, which tells a reader it was looked at and is fine. It was not looked at. `ProviderReport` gains a per-kind `NotMeasured` tally beside its family size, and every provider fills it. `Examination.NotPosed` is deleted in favour of `AwaitingRuns` and `Unreadable`, which turned all six call sites into compile errors and forced each to say which it was — the two are different news and only the first is answered by waiting. Six baseline runs where the comparison needs seven empties as the store fills; a zero median never will. Per kind and never totalled. Summing counts one test once per question its data could not answer, intersecting collapses to nothing because pass and fail are always readable, and either total would move with `--kind` — which is the one thing a count of what could not be measured must not do. `DurationProvider.Unstable` is where the acceptance criterion lives: `RobustDispersion.Of` answers zero both for a sample too short to have a spread and for one whose median is not positive, so the dispersion gate was reading an unmeasured test as a perfectly steady one. The preconditions are now computed before it. `healthy` is deliberately unchanged. Subtracting the unmeasured would make a headline count move with `--kind`; `SummaryDto.Healthy` now documents that it means "no finding was raised" rather than "checked and fine". `SharedFailure` and `BrokenFixture` publish nothing rather than zero — they are counted in signature groups, not tests. The three retry kinds share one figure, taken at the precondition they all read, because the chain that picks between them never asks whether a later kind was measurable. Rendered as one line above the fence naming the three largest kinds, and published per kind as `summary.notMeasured`. Schema 1.14 -> 1.15. `docs/internals/finding-populations.md` records the per-kind decisions, per that file's own rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMQcZLBGFTd8wz3mcfn9vc
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The contract and rendering changes are consistently implemented across providers/coordinator/envelope (with schema bump) and are backed by comprehensive targeted tests for both JSON and text output behavior.
Pull request overview
This PR fixes a reporting blind spot in the CLI’s analysis summary by adding a per-finding-kind “not measured” tally (split into awaitingRuns vs unreadable) so tests a provider cannot compute metrics for no longer vanish into “healthy”, and the JSON/report output can explain coverage gaps per metric kind.
Changes:
- Extend the provider/coordinator contract to carry per-kind
NotMeasuredcounts, and project them into the JSON envelope assummary.notMeasured(schema1.14→1.15). - Update CLI text rendering to surface an “nothing to measure” line (unreadable-only) and ensure empty reports don’t incorrectly read as success when all questions were unanswerable.
- Add broad unit test coverage across providers, coordinator behavior (including
--kindfiltering stability), and output rendering/contract.
File summaries
| File | Description |
|---|---|
| tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs | Adds tests asserting Vanished publishes awaiting-runs vs unreadable semantics (unreadable structurally zero). |
| tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs | Adds tests for window-wide “no recorded clock” behavior and for “measured but no finding” not being counted. |
| tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs | Verifies rendered output includes (or omits) the “nothing to measure” line and ordering/truncation rules. |
| tests/Xping.Cli.Tests/Report/RetryProviderTests.cs | Adds tests that retry kinds share one not-measured figure and do not count readable uneventful tests. |
| tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs | Updates schema assertion to 1.15 and checks summary.notMeasured presence. |
| tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs | Adds tests for unreadable-only counting when concurrency never varies and for “measured but no finding”. |
| tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs | Adds tests for --kind stability of the not-measured tally, floor-vs-tally exclusivity, and provider-failure behavior. |
| tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs | Adds tests for outage windows being counted as awaiting runs and for avoiding double-reporting clustered failures. |
| tests/Xping.Cli.Tests/Report/DurationProviderTests.cs | Adds acceptance coverage for the zero-median/unmeasurable duration case and related non-counted/measured cases. |
| tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs | Updates schema assertions to 1.15. |
| src/Xping.Cli/Report/Rendering/TextReportRenderer.cs | Renders an unreadable-only “nothing to measure” line and improves empty-report messaging when kinds were unanswerable. |
| src/Xping.Cli/Report/Providers/VanishedProvider.cs | Tracks and publishes per-kind not-measured counts for Vanished (awaiting-runs only). |
| src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs | Publishes not-measured counts; distinguishes awaiting-runs vs unreadable at decline sites and handles “no clocks” window-wide. |
| src/Xping.Cli/Report/Providers/RetryProvider.cs | Publishes a shared not-measured count for all three retry kinds at their shared precondition. |
| src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs | Publishes not-measured counts and reclassifies “insufficient concurrency variation” as unreadable. |
| src/Xping.Cli/Report/Providers/IFindingProvider.cs | Introduces NotMeasuredCount, expands ProviderReport to include per-kind not-measured tallies, and refines Examination outcomes. |
| src/Xping.Cli/Report/Providers/FailureModeProvider.cs | Publishes shared not-measured counts for per-test failure-mode kinds while keeping group-based kinds absent. |
| src/Xping.Cli/Report/Providers/DurationProvider.cs | Adds per-kind not-measured tracking and preconditions so “unmeasurable duration” cases are counted rather than silently treated as healthy. |
| src/Xping.Cli/Report/FindingCoordinator.cs | Aggregates and filters provider NotMeasured tallies per kind alongside candidates/family sizes, returning them in AnalysisResult. |
| src/Xping.Cli/Report/Contract/ReportEnvelope.cs | Bumps schema version to 1.15 and adds summary.notMeasured + DTOs with clarified healthy meaning. |
| src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs | Projects AnalysisResult.NotMeasured into the envelope with stable enum-declaration ordering and correct key spelling. |
| docs/internals/finding-populations.md | Documents per-kind not-measured semantics, reasons, and why certain kinds publish nothing. |
| docs/cli/command-reference.md | Updates JSON contract docs and example to include summary.notMeasured and schema 1.15. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #185.
The problem
AnalysisResult.ExcludedLowEvidencewas incremented in exactly one place —FindingCoordinator's reporting floor — so it counted only candidates a provider had already offered. Every decline inside a provider was invisible, and that is where most of them happen.The sharpest case, and the one the issue was filed from: a test whose runs all recorded a zero median normalises nothing, so
DurationProvidercan compute neither of its statistics and declines both. That test then landed silently insidehealthy, which tells a reader it was looked at and is fine. It was not looked at. This is #167's fourth acceptance criterion — the one PR #184 explicitly deferred for want of a mechanism.The shape of the fix
Per kind, and never totalled. Summing across kinds counts one test once per question its data could not answer; intersecting them collapses to nothing, because a test's pass and fail are always readable; and either total would move with
--kind, which is the one thing a count of what could not be measured must not do. Per kind it cannot move — the six providers own disjoint kind sets.Two reasons, not one.
Examination.NotPosedis deleted in favour ofAwaitingRunsandUnreadable. Deleting it turned all six call sites into compile errors and forced each to declare which it was. The question at every site is whether another run of the same shape fixes it: Duration's 7/3 arm floors empty as the store fills, a zero median never will. Folding both into one figure would have repeated the issue's own category error one level down.ProviderReportgains a per-kindNotMeasuredtally beside its family size, filtered by--kindwith the same condition the family and the candidates get. Every provider fills it.Acceptance, checked against a real store
DurationProvider.Unstablekilled it at the dispersion gate, becauseRobustDispersion.Ofanswers0both for a sample too short to have a spread and for one whose median is not positive — an unmeasured test read as a perfectly steady one. The preconditions now run before that gate.awaitingRunsvsunreadable, published separately per kindDurationRegression: {awaitingRuns: 0, unreadable: 3}is identical in the full report and under--kind DurationRegressionThe sample suite now shows what it was hiding — this previously printed
16 healthyand a greenNo findings.:Judgement calls
healthyis unchanged. Subtracting the unmeasured would make a headline count move with--kind.SummaryDto.Healthynow documents that it means "no finding was raised", not "checked and fine".unreadablehalf is rendered. The counts line already says "awaiting more runs" in a different unit — tests here, candidates there — and two waiting figures on one screen is the confusion this change exists to remove.awaitingRunsis in the JSON.SharedFailureandBrokenFixturepublish nothing rather than zero. They are counted in signature groups, not tests. Absence says the kind keeps no such tally; a published zero says it looked and could read everything.Exhausted ?? Deepening ?? Maskedstops at the first kind that fires, so whether a later one was measurable is a question the algorithm never asks; answering it for the tally alone would mean running all three on every fingerprint.ParallelSensitiveProvider'stest == nullstays a tested fingerprint, unlike the same condition elsewhere:CochranArmitage.Ofhas already run by then, so the comparison genuinely was made and the family has to say so.Contract
Schema
1.14→1.15.summary.notMeasuredis keyed by the kind asfindings[].kindand--kindspell it:docs/internals/finding-populations.mdgains a section recording the per-kind decisions, per that file's own rule.Testing
964 CLI tests pass (24 new), 901 SDK Core tests pass, solution builds clean under
TreatWarningsAsErrors+AnalysisMode=All. Two runs over an unchanged store still serialise byte-identically.New tests include the acceptance case (
ATestWhoseRunsAllRecordedAZeroMedianIsCountedRatherThanSilentlyCalledHealthy), its negative (a steady test is counted nowhere),--kindstability, floor-vs-tally exclusivity, and one coverage pair per provider.🤖 Generated with Claude Code
https://claude.ai/code/session_01TMQcZLBGFTd8wz3mcfn9vc