docs: investigation for #7597 (fleet_probe.py coverage-floor breach)

[bolichen97]

Summary

Diagnostic investigation of issue #7597 (fleet_probe.py reported at 14.8%, failing the per-file coverage floor). Deliverable is a written report archived at docs/task-specs/2026/09/task-fleet-probe-coverage-investigation/analysis.md, following the docs/task-specs/YYYY/MM/${task-id}/ convention in AGENTS.md and the analysis.md naming of the existing entries. No product code, tests, or CI/baseline files were changed — by design.

Disposition

needs-investigation / likely duplicate of the CI coverage-arm truncation class (#7516 / PR #7517), NOT an undertested file. The gate's printed remedy ("add tests, do not extend the baseline") does not apply here, so the report deliberately does not add fleet_probe.py to .github/coverage-baselines/backend.txt and does not write redundant tests.

Evidence (citations re-verified against the tree)

1. The tests already exist. class TestFleetProbe at test/test_pipeline_conductor_agent.py:179, whose _mod() loads the real script at :181; 20 test_* methods drive mod.main([...]) across argument-parsing, output-shaping, and failure paths. They arrived in c0dca4a8b itself, refuting "No commit since has added tests for it."
2. The load mechanism does not defeat coverage. Coverage attributes by file path (setup.cfg [coverage:paths] source = src/). Control case: credit_spend.py (same dir, same load_skill_script, same test file, TestCreditSpend at :453) is unbaselined and passes the gate; ~14 skill scripts use this loader and none are baselined.
3. The 34/229 signature matches module-top-level-only execution (imports, constants, def headers with bodies unattributed) — the fingerprint of covering tests not landing in the combined coverage data.
4. The truncation happens inside a green shard. The per-shard "Stage shard coverage data" step runs coverage combine 2>/dev/null || true (ci.yml:705), discarding the merge's exit status, and the guard that follows asserts only that .coverage exists (ci.yml:706) — never that the merge consumed every xdist worker's data file. A shard that loses a worker therefore uploads a well-formed but incomplete .coverage.<group> and stays green, and coverage-combine (ci.yml:973), which has no all-shards-present or per-shard content expectation, merges all four present artifacts and succeeds.

The report also rules out the shard wall at ci.yml:581-583 as the mechanism, because it cannot produce a per-file line-rate at all: a shard killed at the wall fails the backend-test matrix job, coverage-combine declares needs: [backend-test, changes] with an if: that does not use always() (ci.yml:975,979) so it is skipped, and the per-file backend check is gated on needs.coverage-combine.result == 'success' (ci.yml:1255). That the gate reported a number for fleet_probe.py at all — alongside a healthy 91.68% overall — proves coverage-combine succeeded and therefore that no shard died.

Second suggestion assessed

The claim that Coverage Gate is "effectively PR-only" is imprecise: ci.yml:3-7 triggers on both push-to-main and pull_request. The real gap is the concurrency block (ci.yml:12-20) yielding periodic rather than per-commit verdicts on main. This is a distinct CI-policy concern worth its own issue, not part of this diagnosis.

Recommended fix (for whoever picks up #7516/#7517)

Harden the per-shard staging step, not coverage-combine: at ci.yml:705, distinguish the benign no-op exit from a real merge failure instead of discarding both — the step is documented as a no-op when pytest-cov already combined the shard, and coverage combine exits non-zero with "No data to combine" in exactly that case, so the || true cannot simply be dropped — and assert the merge consumed the number of worker data files the run produced. An all-shards-present assertion in coverage-combine is worth having on its own merits but would not have caught this signature — all four shard artifacts were present, and the one that mattered was thin rather than absent.

Testing / verification

• Docs-only diff: ./scripts/docs-lint.sh and python3 scripts/docs_lint.py --test both pass, as do check_brand_name.py, check_changelog_history.py, and check_harness_parity.py against the merge base. docs/task-specs/ is in docs_lint.py's UNCURATED_PREFIXES, so the archive needs no index entry, but its links and line citations are still checked.
• Empirical coverage measurement was blocked by the sandbox network mode (INTEGRATIONS_ONLY / "Repository access only"): coverage and pytest_cov are absent and cannot be installed from PyPI. The disposition rests on static analysis, which is conclusive here due to the control case and the workflow's own gating.
• Baseline remains clean (no fleet_probe/credit_spend entries); test/test_pipeline_conductor_agent.py, fleet_probe.py, ci.yml, and setup.cfg are all unchanged.

Closes investigation for #7597.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed a95c8e94d1d03f4192c98ff5daa23c267c752a0d — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] a95c8e9

Verdict parsed from the review's SHA-scoped output markers for commit a95c8e94d1d03f4192c98ff5daa23c267c752a0d.

False positive or not applicable? A repository writer can comment:
/ai-review override fable a95c8e94d1d03f4192c98ff5daa23c267c752a0d: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of a95c8e94d1d03f4192c98ff5daa23c267c752a0d and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a95c8e9

False positive or not applicable? A repository writer can comment:
/ai-review override gpt a95c8e94d1d03f4192c98ff5daa23c267c752a0d: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of a95c8e94d1d03f4192c98ff5daa23c267c752a0d — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Spot-checks confirm the report's load-bearing citations: TestFleetProbe at test/test_pipeline_conductor_agent.py:179, the credit_spend control case at :453, both absent from the coverage baseline, the coverage combine 2>/dev/null || true + existence-only guard at ci.yml:705-706, and the combine/gate conditions at ci.yml:975/979/1255. The diff is a single archived analysis document in the docs/task-specs/YYYY/MM/ location AGENTS.md designates; no product code, tests, or CI files change, matching the description exactly. The diagnosis correctly refuses both symptom patches (baselining a covered file, writing duplicate tests), rules out the adjacent shard-wall mechanism with a valid argument, and routes the fix to the owning issue class with an appropriately hedged disposition.

Design-Verdict: PASS

A docs-only diagnosis that verifiably names the root cause, rejects both symptom patches, and routes the fix to the owning issue — right shape, right scope.

[DESIGN-REVIEWED] a95c8e9

[bolichen97]

• span=46da321044fc — "coverage-combine still succeeds" contradicts its failed backend-test dependency (INVESTIGATION.md:178) — FIXED.

GPT 5.6's rationale:

"coverage-combine still succeeds" contradicts its failed backend-test dependency, so this path cannot yield the reported coverage rate -> Fix: remove the shard-wall diagnosis.

Legitimate, and confirmable with stronger proof than the finding itself states. The report's chain — a shard killed at the timeout wall, coverage-combine still succeeding on the survivors, hence a 34/229 per-file reading — is unreachable: coverage-combine declares needs: [backend-test, changes] with an if: that does not use always() (ci.yml:975,979), so a dead shard SKIPS that job outright, and the per-file backend check is gated on needs.coverage-combine.result == 'success' (ci.yml:1255). The existence of a per-file 14.8% verdict therefore proves combine succeeded and no shard died — as does the 91.68% overall figure the report cites in its own Evidence 3.

The mechanism is replaced rather than merely deleted, because a docs PR whose entire purpose is a correct diagnosis cannot ship with a hole where its central claim was. The reachable intra-shard path is now the claim: the per-shard staging step runs coverage combine 2>/dev/null || true (ci.yml:705), discarding the merge's exit status, and the guard after it asserts only that .coverage EXISTS (ci.yml:706) — never that the merge consumed every xdist worker's data file. A shard that loses a worker therefore uploads a thin artifact and stays green. The report gains a "Why the shard wall is not the mechanism here" subsection carrying the ruling-out citations above; Recommendation 1 now prescribes hardening ci.yml:705 instead of an all-shards-present assertion in coverage-combine, which would not have caught this at all (all four shard artifacts were present, and the one that mattered was thin rather than absent); and the evidence index carries the new rows with the old ci.yml:581-583 shard-wall row annotated as ruled out.

[bolichen97]

• span=46da321044fc — the promised docs/task-specs/.../analysis.md is absent while five .agents/ files are added (INVESTIGATION.md:1) — FIXED.

GPT 5.6's rationale:

the promised docs/task-specs/.../analysis.md is absent while five .agents/ files are added -> Fix: retain only the report at the promised archive path.

Every factual half checks out. .agents/ exists nowhere in origin/main (git ls-tree -r origin/main returns no match), so the diff was establishing a brand-new top-level tree; AGENTS.md's Specification management section mandates docs/task-specs/YYYY/MM/${task-id}/; and both precedent entries — docs/task-specs/2026/08/pycache-unbounded-growth/analysis.md and docs/task-specs/2026/08/task-gateway-memory-6827/analysis.md — are exactly one markdown file with no harness JSON beside them.

Done as asked. The report now lives at docs/task-specs/2026/09/task-fleet-probe-coverage-investigation/analysis.md, named analysis.md to match both precedents, and the four harness files are dropped: task.json, context.json, features/FEAT-001.json, and the timestamped review note. Each is agent working state rather than a deliverable — task.json literally records "Left uncommitted per orchestrator instruction." while being committed, FEAT-001.json is the prompt that generated the report, and the review note endorses the very shard-wall mechanism this same review disproved, so keeping it would have shipped a self-contradiction. The diff is now that single markdown file and nothing else.

[bolichen97]

• Blocker — deliverable location and orchestrator state violate the repo's own archive convention — FIXED.

Design Review's rationale:

All five files land under a new top-level .agents/tasks/... ... while AGENTS.md mandates "Task specs go in docs/task-specs/YYYY/MM/${task-id}/" — the exact home of the precedent analyses (docs/task-specs/2026/08/task-gateway-memory-6827/analysis.md). Because .agents/ sits outside docs/, the report evades the docs-lint indexing rule and is unreachable from any index, defeating the PR's whole purpose (an archived, findable diagnosis).
Fix: move INVESTIGATION.md to docs/task-specs/2026/09/<task-id>/, index it per the directory README rule, drop the four harness-state files.

Legitimate on every claim I checked, and proportional — relocating the deliverable IS this PR's stated purpose done correctly, not a scope expansion. Verified independently: .agents/ is absent from origin/main, so merging would have established it as a precedent scratch tree on main; the mandate is in AGENTS.md; and the two precedent entries are each a single analysis.md.

The deliverable is now docs/task-specs/2026/09/task-fleet-probe-coverage-investigation/analysis.md and the four harness-state files are gone, so the whole .agents/ tree disappears from the diff. On indexing: docs/task-specs/README.md declares the YYYY/MM/<task-id>/ convention rather than enumerating entries — neither 2026/08 precedent is listed there — so conforming to the path is what makes the report reachable, and scripts/docs-lint.sh passes on the relocated file.

[bolichen97]

• Suggestion — post the report (or its Summary + Recommendation) to issue main is red: fleet_probe.py at 14.8% fails the per-file coverage floor on every rebased PR #7597 so the routing happens before this archive PR merges — ACKNOWLEDGED, not actionable within this diff.

Design Review's rationale:

The disposition's primary consumer is issue #7597 — post the report (or its Summary + Recommendation) to the issue thread so the routing happens even before this archive PR merges.

The observation is correct and the routing is worth doing: #7597's disposition turns entirely on this diagnosis, and nothing about it waits on the archive landing.

It is not something a change set can express, though. It asks for a comment on a separate issue thread, so no file edit satisfies it and it produces no reviewable artifact here. Acting on it from inside this PR would also fork the text: the report has been materially corrected since this review — the unreachable shard-wall mechanism was replaced with the reachable intra-shard one — so a copy pasted into #7597 would need re-pasting after every later correction, which is the duplication an archive path exists to prevent.

This PR's scope stays the archived, findable diagnosis at docs/task-specs/2026/09/task-fleet-probe-coverage-investigation/analysis.md. Routing #7597 is a one-comment follow-up that cites that path once it is on main, and it is recorded as owed work rather than dropped.

[chenmingwei23]

Approved: PR Readiness passed, all review lanes green.