fix(core): a coverage figure is the union across a tree's supported hosts (#521) - #538
Merged
Merged
Conversation
…osts (#521) `tech-stack.md` said the coverage script "takes no arguments so it is identical on every platform". True of the COMMAND, misleading about the REPORT - which is what a reader decides with, and what `tdd` Phase 5 gates on. Code behind a platform fork cannot execute off its own platform, so a single-host report scores the other platform's arm as uncovered no matter how well tested it is. Measured: plugins/ca/tools reads 66.18% branches on Windows against 65.35% on Linux, and exec.ts alone 87.50% against 76.38% - an 11-point gap that is entirely awaitTaskkill plus the win32 treeKill arm on one side and the POSIX arm on the other. Neither number is wrong; both are incomplete. That matters beyond arithmetic. treeKill is a process-containment path, so under a single-host rule a GENUINE gap in it is indistinguishable from the platform artifact - the figure stops measuring test quality and starts measuring how much of the tree is POSIX. AC-1: tech-stack.md no longer implies the report is platform-independent, and separates the identical command from the host-dependent result. AC-2: the rule is that a quoted figure is the UNION across a tree's supported hosts, per tree, and any figure quoted in an issue, ADR or phase record names the host it came from. #511's closing measurement is restated under it: the single-host baseline table is now labelled windows-only and marked superseded as a basis for a threshold decision, rather than silently reinterpreted. AC-3: includes/maturity-coverage.md carries the rule and tdd Phase 5 defers to it, so the two places that describe how the threshold is applied agree. Both are generated surface, so ca-codex and ca-pi receive the identical text. Per-tree, because it only earns its cost where code forks: ca/tools and ca-pi/tools fork, ca-sandbox/tools and site/ do not, and a union of two identical reports is the same report. CI computes the union for plugins/ca/tools - one matrix cell per host emitting a vitest BLOB report, merged with `vitest --merge-reports`. Verified that merging is a genuine union rather than the last report winning: two disjoint suites, 7 + 65 branches merging to 72 and 18 + 69 lines to 87. The merge job reports how many hosts contributed and says so when only one did, so a partial figure is never mistaken for the union. ADVISORY, deliberately. DECISION-0031 weighed Available: the coverage gate is orchestrator-run, these jobs only produce the figure it quotes, and enforcing them would let a Windows runner flake (#535 is live) block a merge on a number no check consumes. They are awaited by ci-passed so a hang stays visible and never enforced; the sanctioned-advisory assertion is extended to name them, which is a deliberate contract change rather than a fix for a red build. ca-pi/tools is in scope for the RULE but has no union job yet (#537), and tech-stack.md says so rather than implying coverage it does not have. The split is mechanical, not a design question: the audited-graph contract attributes a job's graph by its working directory, which only resolves when that directory is literal rather than matrix-templated, and compensating with extra audit steps breaks the contract pinning the exact audit inventory. Both contracts are right. ca 2.10.1 -> 2.10.2, ca-codex 0.3.0 -> 0.3.1, ca-pi 0.1.38 -> 0.1.39: the shared surface projects into all three payloads. Claude-Session: https://claude.ai/code/session_01WJgVfZw7J81PB7mwpHyUxx
…blob when it did (#521) Two flaws the job's own first run exposed on windows-latest. 1. `--reporter=blob` REPLACES the console reporter, so a failing cell logged "Process completed with exit code 1" and nothing else - no summary, no failing test name, no assertion. `--reporter=default` is now requested alongside it. A cell that cannot say what broke is not worth having. 2. The artifact upload ran only on success, so the FAILING host's blob - the one most worth having - was skipped exactly when it was needed. It is now `if: always()`, which also stops the merge job reporting a one-host figure as though the other host had nothing to add. Claude-Session: https://claude.ai/code/session_01WJgVfZw7J81PB7mwpHyUxx
…539) A red check with no explanation trains readers to ignore it - the habit this repo fights and the reason #504/#515 were filed rather than re-run. The cell is advisory and blocks nothing, the failure predates the job, and #539 has the diagnosis. Says so where someone seeing red will actually look, and says not to silence it with continue-on-error. Claude-Session: https://claude.ai/code/session_01WJgVfZw7J81PB7mwpHyUxx
…521) test_ci_has_pi_path_output_matrix_and_independent_version_guard proves the ca-pi-latest advisory receipt is load-bearing by replacing its `if: always()` with `if: success()` and asserting the contract notices. The replace was first-occurrence over the WHOLE file. `if: always()` is a generic line, so that assertion silently stopped testing the Pi receipt the moment an unrelated job above it grew one - which #521's coverage-union upload does, deliberately, so a FAILING host still publishes its blob. Six ca-pi cells went red on every platform, correctly: the guard was telling me its mutation no longer reached its subject. Scoped to the ca-pi-latest job body, plus an assertion that the mutation actually changed something - a no-op replace would otherwise read as a pass. Verified load-bearing: downgrading the real receipt to `success()` still turns this suite red. Same defect class as the survivors mutation testing found in #515: a check that appears to pin a property while hitting an unrelated line. Claude-Session: https://claude.ai/code/session_01WJgVfZw7J81PB7mwpHyUxx
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 #521. Implements DECISION-0031.
The claim that was wrong
tech-stack.mdsaid the coverage script "takes no arguments so it is identical on every platform." True of the command. Misleading about the report — which is what a reader decides with, and whattddPhase 5 gates on.Code behind a platform fork cannot execute off its own platform, so a single-host report scores the other platform's arm as uncovered no matter how well tested it is:
plugins/ca/toolsbranchesexec.tsbranchesThat 11 points is entirely
awaitTaskkillplus the win32treeKillarm on one side, and the POSIX arm on the other. Neither number is wrong; both are incomplete.It matters beyond arithmetic.
treeKillis a process-containment path — so under a single-host rule a genuine gap in it is indistinguishable from the platform artifact. The figure stops measuring test quality and starts measuring how much of the tree is POSIX.The rule (AC-2)
A quoted coverage figure is the union across a tree's supported hosts. Any figure quoted in an issue, ADR, or phase record names the host it came from.
Per tree, because it only earns its cost where code actually forks:
plugins/ca/toolsexec.tsonprocess.platformplugins/ca-pi/toolsplugins/ca-sandbox/toolssite/A union of two identical reports is the same report.
#511's closing measurement is restated, not silently reinterpreted: the single-host baseline table is now labelled windows-only and marked superseded as a basis for a threshold decision.
AC-1 and AC-3
tech-stack.mdseparates the identical command from the host-dependent result.includes/maturity-coverage.mdcarries the rule;tddPhase 5 defers to it and now requires the measuring host to be named. Those are the two places that describe how the threshold is applied, and they agree.Both are generated surface, so
ca-codexandca-pireceive identical text.The union is real, not assumed
I verified
--merge-reportsgenuinely recomputes coverage across blobs rather than letting the last one win — two disjoint suites, measured separately and merged:7 + 65 = 72, 18 + 69 = 87. Exactly additive.
The merge job reports how many hosts contributed and says so when only one did, so a partial figure is never mistaken for the union.
Advisory by design
DECISION-0031 weighed Available explicitly. The coverage gate is orchestrator-run; these jobs only produce the figure it quotes. Enforcing them would let a Windows runner flake — #535 is live — block a merge on a number no check consumes.
So they are awaited by
ci-passed(a hang stays visible) and never enforced. The sanctioned-advisory assertion is extended to name them, which is a deliberate contract change with the reason written down, not a fix for a red build. A new test asserts they never become required.Why ca-pi split out (#537)
Mechanical, not a design question. The audited-graph contract attributes a job's dependency graph by its working directory, which only resolves when that directory is literal — a
matrix.tree-templated one resolves to., which nothing audits. Compensating with extranpm auditsteps breaks a second contract pinning the exact audit inventory acrossci.ymlanddocs.yml.Both contracts are right, and satisfying them means one job per tree.
tech-stack.mdsays ca-pi has no union job yet rather than implying coverage it does not have.Verification
test_ci_impact.py52 pass, including two new contracts: the union must measure on both hosts and actually merge; and it must never become a required check.build-surface --checkin sync across claude/codex/pi;build-host-packages --checkconsistent.test_release_lib83,test_public_codex_docs3,test_public_pi_docs13 — all pass.origin/main: ca2.10.1 → 2.10.2, ca-codex0.3.0 → 0.3.1, ca-pi0.1.38 → 0.1.39with root metadata regenerated, ca-sandbox unchanged.Note on the diff
The two contracts above rejected my first two designs before this one — a templated matrix over trees, then compensating audit steps. Both were caught by existing tests rather than by review, which is the outcome those tests were written for.