Skip to content

fix(coverage): expose a de-duplicated first-party Cobertura aggregation (#815) - #829

Merged
drmoisan merged 6 commits into
epic/review-residuals-2026-09-08-integrationfrom
bug/coverage-aggregation-double-counts-method-rows-815-exec
Sep 9, 2026
Merged

drmoisan merged 6 commits into
epic/review-residuals-2026-09-08-integrationfrom
bug/coverage-aggregation-double-counts-method-rows-815-exec

Conversation

@drmoisan

@drmoisan drmoisan commented Sep 9, 2026

Copy link
Copy Markdown
Owner

fix(coverage): expose a de-duplicated first-party Cobertura aggregation (#815)

Summary

  • Adds Get-CoberturaFirstPartyCoverageSummary, which aggregates Cobertura line and branch counters across first-party packages while counting each (class, source line number) pair exactly once. The previous descendant-axis idiom counted a source line once per <line> element, so a line appearing under both the class rollup and one or more <methods> entries was counted repeatedly.
  • Measured against a committed real Cobertura document, the defect doubles the branch counters exactly and inflates line counters by roughly a factor of two. The derived percentages barely move: branch percentage is bit-for-bit identical at 79.24%, line percentage differs by 0.01 point. This is a count defect, not a rate defect.
  • Adds Format-CoberturaFirstPartyCoverageSummary (pure formatter) and Get-CoberturaFirstPartyCoverageReport (composition), so wiring the report into the coverage entry point costs exactly one line.
  • Adds seven Pester tests, including a differential assertion that reproduces the old descendant-axis selection over the same fixture and asserts the corrected counts are strictly lower.
  • Repairs a structurally invalid mocked Cobertura stub in two pre-existing test files. No assertion was weakened and no production validation was relaxed.

Why

spec.md records the root cause: the aggregation idiom selected .//line, a descendant axis that reaches <line> elements nested under <methods> as well as those in the class-level <lines> rollup. In the field-initializer-across-constructors shape confirmed at issue #670, one source line appears once in the class rollup and again under each constructor, so it is counted three or four times.

The corrected function delegates to the existing Get-CoberturaPackageLineSummary, which already de-duplicates by line number, rather than re-deriving the counting rule. That keeps one implementation of the de-duplication invariant instead of two.

What Changed

Core fix

  • scripts/vscode/Invoke-MSTestWithCoverage.FirstParty.ps1 (new) — the three functions. Pure: no I/O, no mutation of the input document. Throws the same Cobertura XML does not contain a <packages> node. message as Get-CoberturaCoverageSummary for shape parity.
  • scripts/vscode/Invoke-MSTestWithCoverage.Helpers.ps1 — one added line dot-sourcing the new file, matching the existing split precedent used by PackageRate.ps1 and Threshold.ps1. Helpers.ps1 sat at 469 lines against a 500-line ceiling, so a new file was the only placement that holds the ceiling.
  • scripts/vscode/Invoke-MSTestWithCoverage.ps1 — one added line emitting the report, placed so the existing Done. Coverage artifact: line is retained.

Tests

  • tests/scripts/vscode/Invoke-MSTestWithCoverage.FirstParty.Tests.ps1 (new) — seven tests over an in-memory here-string fixture. No temporary files.
  • tests/scripts/vscode/Invoke-MSTest.RunSettings.Tests.ps1 and tests/scripts/vscode/Invoke-MSTestWithCoverage.AssemblyDiscovery.Tests.ps1 — the mocked ConvertTo-KoverageCoberturaXml returned <coverage line-rate="0.8" />, a stub with no <packages> node. See Risks below.

Docs

  • spec.md — all 14 acceptance criteria checked off.
  • plan.2026-09-08T23-49.md — all 48 tasks checked off.
  • Evidence artifacts under the feature folder covering baselines, the fail-before/pass-after pair, the scope and threshold gates, and the QA loop.

Architecture / How It Fits Together

Invoke-MSTestWithCoverageMain writes the processed Cobertura document, asserts the line-coverage threshold, then calls Get-CoberturaFirstPartyCoverageReport on the same in-memory string and emits the returned line. That composition function casts to [xml], calls the aggregation, and hands the result to the pure formatter. Every computation and every emitted string lives in a unit-tested pure function; the entry point holds only the one-line call.

The allowlist is derived, not hard-coded: ProjectNames defaults to (Get-KoverageProjectAllowlist), evaluated at parameter binding.

Verification

Completed

PowerShell toolchain, run in order (format, then analyze, then test; type checking is not applicable per .claude/rules/powershell.md step 3 and is recorded rather than omitted):

Gate Result
PoshQC format over both folders ok, scoped git status --porcelain empty
PSScriptAnalyzer, scripts/vscode 16 findings, equal to the recorded pre-change baseline of 16; zero attributable to this change
PSScriptAnalyzer, tests/scripts/vscode 0 findings
Pester, tests/scripts/vscode 103 tests, 0 errors, 0 failures, against a baseline of 96
New module line coverage 96.97% against the 90% floor for newly added modules
Folder line coverage 78.56% to 79.47%

The loop restarted once from format after the stub repair described under Risks, then completed a clean single pass.

Fail-before/pass-after: the test file was authored before any production code existed and recorded red (FAILED=6, unresolved-command failures). After the fix it records PASSED=7 FAILED=0.

Differential counts over the fixture, executed rather than asserted from the plan:

Quantity Descendant-axis (defect) De-duplicated (fix)
LinesValid / LinesCovered 8 / 6 4 / 3
BranchesValid / BranchesCovered 16 / 10 8 / 4
LineRate 0.75 0.75

LineRate is identical under both computations, which is why the tests assert counts and not rates: a rate assertion would pass whether or not the fix is correct.

Corroboration against a committed real Cobertura document (66,158 de-duplicated lines):

Quantity Descendant-axis De-duplicated
LinesValid / LinesCovered 133,485 / 112,865 66,158 / 55,940
BranchesValid / BranchesCovered 33,624 / 26,642 16,812 / 13,321
Line percentage 84.55% 84.56%
Branch percentage 79.24% 79.24%

Recommended

  • mcp__drm-copilot__run_poshqc_test with scan_folders set to tests/scripts/vscode.
  • Run the coverage entry point end to end and confirm the added report line renders alongside the retained Done. Coverage artifact: line.

Backward Compatibility / Migration Notes

No breaking change. The three new functions are additive. The entry point gains one output line and retains its existing output. No public function signature changed, no file was renamed or removed, and no threshold, analyzer severity or policy requirement was altered.

Risks and Mitigations

The plan's coverage prediction for the wiring line was wrong, and it surfaced as a real test failure. Plan decision D3 asserted that no unit test reaches the post-processing block past the -NoExecute early return. Six pre-existing tests do reach it, mocking ConvertTo-KoverageCoberturaXml to return <coverage line-rate="0.8" />. The existing threshold assertion tolerates that stub because it reads only the root attribute; the new report does not, because it matches Get-CoberturaCoverageSummary and requires a <packages> node. The first test pass went red with 6 failures.

The repair made the stub structurally valid by adding an empty <packages /> at three sites in Invoke-MSTest.RunSettings.Tests.ps1 and one in Invoke-MSTestWithCoverage.AssemblyDiscovery.Tests.ps1, updating the one exact-match assertion quoting it in lockstep. Net zero lines in both files. The alternative — relaxing the new function's <packages> requirement — was rejected, because it would have diverged the new function from the existing Get-CoberturaCoverageSummary contract to accommodate a malformed test stub.

Both files are outside the Write Set spec.md names, which is a scope deviation and is called out here deliberately. Both sit under tests/scripts/vscode/, so the scope-boundary criterion is unaffected. The batch now touches 3 production and 3 test files, exactly at the .claude/rules/powershell.md cap. The largest file in either folder is 496 lines, under the 500 ceiling.

Consequence: the added wiring line is covered rather than uncovered, so the entry point's missed-line count is 10 before and 10 after.

Recorded findings, deliberately not actioned in this PR

  • The corrected first-party line figure of 84.56% clears the 80% floor the script gate enforces and sits below the 85% floor stated in .claude/rules/general-unit-test.md and .claude/rules/quality-tiers.md. The gap exists under both computations (84.55% defective), so this correction neither created nor widened it.
  • CLAUDE.md sets an 80% line floor while the two rules files set 85%, and the only automated script gate enforces 80. This divergence predates the issue. No threshold was lowered, weakened or deleted to make any gate pass.

Review Guide

  1. scripts/vscode/Invoke-MSTestWithCoverage.FirstParty.ps1 — the aggregation, the rounding and zero-denominator fallback, and the delegation to Get-CoberturaPackageLineSummary.
  2. tests/scripts/vscode/Invoke-MSTestWithCoverage.FirstParty.Tests.ps1 — the fixture, and the private differential helper that pins the old behaviour.
  3. The two one-line production wiring changes.
  4. The two stub repairs in the pre-existing test files (mechanical, net zero lines).
  5. Evidence artifacts, if the numbers above need auditing.

Follow-ups

  • Issue 828 (Bug: claude-md-cut3-names-uninvoked-coverage-command #828) records a separate documentation defect found during this work: CLAUDE.md section CUT3 step 4 names a vstest.console.exe ... /EnableCodeCoverage invocation that the repository deliberately never issues, because the built-in collector conflicts with the outer dotnet-coverage instrumentation. It is filed rather than fixed here: CLAUDE.md ownership between this repository and drm-copilot must be settled first, since everything under .claude/ other than agent-memory/ is pushed down with no templating.
  • The 80-versus-85 percent line-floor divergence noted above has no issue of its own yet.

GitHub Auto-close

🤖 Generated with Claude Code

https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh

drmoisan and others added 6 commits September 9, 2026 11:00
#815)

Atomic plans quote a first-party coverage aggregate for their QA gates, and
the repository exposed no committed, callable way to obtain the four covered
and valid line and branch counts. Plan authors therefore wrote the
aggregation by hand, and the variant that propagated selects <line> elements
on the descendant axis, which matches both the class-level rollup and the
method-level view of the same source line and so counts most rows twice.

Add Get-CoberturaFirstPartyCoverageSummary,
Format-CoberturaFirstPartyCoverageSummary and
Get-CoberturaFirstPartyCoverageReport in a new
scripts/vscode/Invoke-MSTestWithCoverage.FirstParty.ps1, dot-sourced from
Invoke-MSTestWithCoverage.Helpers.ps1 so a caller that dot-sources Helpers
alone still resolves them. The aggregation obtains per-class figures by
calling the existing Get-CoberturaPackageLineSummary rather than re-deriving
the de-duplication rule, so exactly one implementation of the counting rule
exists. Its allowlist defaults to Get-KoverageProjectAllowlist rather than
restating the nine production assembly names. Wire one line into
Invoke-MSTestWithCoverageMain so the entry point reports the aggregate,
retaining its existing "Done. Coverage artifact:" line.

Add tests/scripts/vscode/Invoke-MSTestWithCoverage.FirstParty.Tests.ps1 with
seven tests over an in-memory here-string fixture that repeats line numbers
across constructor rows at differing multiplicities. The tests assert counts
and no rate: the line rate is 0.75 under both computations over that fixture,
so a rate assertion would pass whether or not the fix is correct. A private
test-scoped helper reproduces the descendant-axis selection and the test
asserts its totals are strictly greater than the delivered function's.

Measured: over the committed Cobertura document from issue 798, the
descendant-axis computation reports 133485/112865 lines and 33624/26642
branches where the de-duplicated computation reports 66158/55940 and
16812/13321. The defect is a count defect, not a rate defect.

No threshold, analyzer severity or policy requirement is changed.

Refs #815
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
…ally valid (#815)

Six pre-existing tests drive Invoke-MSTestWithCoverageMain past its -NoExecute
early return with ConvertTo-KoverageCoberturaXml mocked to return the minimal
stub '<coverage line-rate="0.8" />'. That stub carries no <packages> node.
Assert-CoberturaLineCoverageThreshold tolerates it because it reads only the
root line-rate attribute, but the first-party report added for #815 rejects a
document with no <packages> node, matching Get-CoberturaCoverageSummary, so the
six tests began failing with "Cobertura XML does not contain a <packages> node."

The real ConvertTo-KoverageCoberturaXml always emits a <packages> element, so
the stub was less structurally valid than the output it stands in for. Add an
empty <packages /> element to it in both files, and update the one exact-match
assertion that quotes the stub so it continues to compare the full string.

No assertion is weakened, no production behaviour is relaxed, and neither file
gains or loses a line: RunSettings.Tests.ps1 stays at 496 and
AssemblyDiscovery.Tests.ps1 at 99.

Refs #815
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
…dence

Phase 4 gates: the AC3 descendant-axis controlled comparison with its positive
control and case-sensitivity control, the AC4 allowlist-derivation gate, the AC2
invariant and trace-unreachability gate, the AC11 before-and-after file line
counts with the committed three-dot numstat re-verification, the AC12
threshold-unchanged gate pinned by SHA-256, the AC13 scope-boundary listing, and
the AC14 handoff of the CLAUDE.md CUT3 wording mismatch, marked POSTING BLOCKED
because no promotion MCP tool is present in the executor's tool surface.

Phase 5 QA loop: format, type check recorded NOT APPLICABLE for PowerShell,
analyze over both folders, test, coverage measurement and the coverage
comparison. The loop ran twice; the first pass failed at the test stage and the
second completed clean.

Two findings recorded rather than accommodated. First, plan decision D3 asserts
that no unit test reaches the post-processing block of
Invoke-MSTestWithCoverageMain past its -NoExecute early return. Six pre-existing
tests do reach it, which is what caused the first test-stage failure, and the
consequence is that the added wiring line is covered rather than uncovered: the
entry point's missed LINE count is 10 before and 10 after, so the
baseline-plus-one allowance AC10 grants was not needed. Second, the repository
carries an 80 percent line floor in CLAUDE.md and an 85 percent floor in the two
rules files while the only automated script gate enforces 80; that divergence
predates this issue and no threshold was changed.

Measured: the new module reaches 96.97 percent line coverage against a 90 percent
floor, Helpers.ps1 covered lines rose 191 to 192, and folder line coverage rose
from 78.56 to 79.47 percent. PoshQC test reports 103 tests, 0 errors, 0 failures
against a baseline of 96.

Refs #815
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
Each criterion was checked off individually in spec.md, citing the evidence
artifact that discharges it, per the acceptance-criteria-tracking skill. spec.md
is the sole acceptance-criteria source because the work mode is full-bug;
user-story.md is absent and that absence is correct by design.

AC14 is left unchecked. Its middle clause asks for "a pointer to a separate
promotion or issue raised for it". No promotion and no issue was raised, because
the MCP promotion route is not present in this executor's tool surface, so the
handoff artifact carries the POSTING BLOCKED marker with that reason and the
intended promotion text in full. The other two clauses of AC14 hold: the
mismatch is recorded with its corroborating in-code citation, and P4-T7 verified
individually that CLAUDE.md appears in neither the anchored diff nor the
porcelain status. Plan task P6-T14 would have accepted the POSTING BLOCKED
artifact as sufficient for its own check-off, but checking the criterion off
would assert that a promotion exists when none does, so both the criterion and
that plan task are left unchecked and the residual is handed to the orchestrator.

Refs #815
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
…n checklist

The P6-T16 listing was captured after the Phase 6 commit and before this
artifact was written, because the artifact and the task's own check-off both
land inside the feature folder and would otherwise appear in the listing the
task asserts is empty of that folder. This commit is the fixpoint closure: it
persists that artifact and the final plan checkbox state so the worktree ends
clean.

P6-T14 remains unchecked. Its acceptance requires AC14 to read [x], and AC14's
middle clause asks for a pointer to a raised promotion that does not exist. The
reason is recorded in evidence/other/p4-t8-claude-md-cut3-handoff.md and
evidence/issue-updates/p6-t15-ac-status.md. Every other plan task from P0-T1
through P6-T16 is checked off against verified acceptance conditions.

Refs #815
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
The executor recorded AC14 as PARTIAL because the MCP route for creating a
potential entry and opening a GitHub issue is absent from its tool surface, so
the criterion's middle clause -- a pointer to a separately raised issue -- had
no referent. The orchestrator holds that route and exercised it.

Issue 828 (Bug: claude-md-cut3-names-uninvoked-coverage-command) records the
CLAUDE.md CUT3 step 4 mismatch: the section names a vstest.console.exe
/EnableCodeCoverage invocation the repository deliberately never issues, since
the built-in collector conflicts with the outer dotnet-coverage instrumentation.
The issue body is 3383 bytes with zero unfilled template placeholders, and
carries the ownership caveat that CLAUDE.md ownership between this repository
and drm-copilot must be settled before any edit.

CLAUDE.md itself remains unmodified on this branch. The two files under
docs/features/potential/ sit inside the fifth permitted prefix of plan decision
D6, whose Deviation 2 anticipates exactly this case, so the AC13 scope gate is
unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
@drmoisan
drmoisan merged commit 732b84d into epic/review-residuals-2026-09-08-integration Sep 9, 2026
drmoisan added a commit that referenced this pull request Sep 9, 2026
… gate

Feature 815 merged as PR #829 (merge commit 732b84d), verified against
gh pr view and the remote integration tip rather than the child's report;
the fan-in introduced zero deletions.

The integration CI run dispatched after 813's merge concluded success, which
is the first gate this epic's integrated tree actually received. A second run
is dispatched against 732b84d.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh
@drmoisan
drmoisan deleted the bug/coverage-aggregation-double-counts-method-rows-815-exec branch September 12, 2026 13:53
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