[john-the-dev]

What

Owner-requested: enforce a 95% minimum unit-test coverage bar. New CI job Coverage Gate: every PR must cover ≥ 95% of the Python lines it adds or changes, measured by running the full standalone suite (tests/**/*.test.py, same discovery as ci.yml) under coverage.py and judging the PR's diff with diff-cover. The whole-tree number is printed informationally in every run.

Why diff coverage instead of a global 95% floor

Measured global coverage today is 31% (10,570 statements, 7,323 missed) — the legacy surface (bridges with live APIs, GUI automation, launchd installers) predates the test push. A global floor at 95% would hard-red every open PR on day one and block all merges. The diff gate applies the full 95% bar to all new code: the protected number can't get worse, every merge converges the tree upward, and nobody gets punished for debt they didn't create. docs/testing-coverage.md documents the flip-to-global step for when the tree gets there. (If you want the hard global floor anyway, it's a one-line change — but I'd recommend against shipping that today.)

Pieces

• scripts/coverage-gate.sh — instrumented suite run (parallel-mode per test file, combined), informational tree report, then diff-cover --fail-under=95 vs base. Fast path: PRs touching no .py skip the instrumented run entirely. Runnable locally with the same command CI uses.
• .coveragerc — scope src/ scripts/ skills/; excludes tests, __main__ blocks, pragma: no cover.
• .github/workflows/coverage-gate.yml — PR-triggered, fetch-depth: 0 (diff-cover needs the base), dev-deps installed inline (same pattern as ci.yml's detect-secrets).
• docs/testing-coverage.md — the rule, local usage, pragma policy, the subprocess-boundary caveat (prefer importlib over subprocess in tests so coverage sees the module), TS follow-up note.

Validation (local, full suite)

• 112-file Python suite runs green under instrumentation.
• Uncovered scratch module → gate fails (exit 1) naming the exact missing lines.
• Same module + test → gate passes (100% diff coverage).
• No-Python diff → fast-path skip.

Follow-up (not in this PR)

TypeScript gating: c8 around the existing tsx --test run + diff-cover on lcov output. Bash: no practical coverage tooling — excluded with rationale in docs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R4cEk3Nzp6kVVowwQaN1J1

[john-the-dev]

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines). Whole-tree coverage (informational, from today's full-suite run): 31%.

---

(Posted manually this once — the automated sticky comment activates after this PR merges: GitHub only runs workflow_run workflows from the main branch's copy, so the comment pipeline in this PR can't fire on itself. On PRs that touch Python, this comment shows the diff-coverage percentage and, on failure, the exact uncovered lines per file.)

[john-the-dev]

Example of the automated comment on a Python-touching PR (real output from this PR's local validation runs — a scratch module was added, gated, covered, re-gated):

---

When the PR's changed lines are under-covered (gate fails, CI red):

Coverage Gate

❌ Diff coverage BELOW the 95% bar — uncovered changed lines listed below. Whole-tree (informational): 31%.

Diff Coverage

Diff: origin/main...HEAD

• src/scratch_uncovered.py (0.0%): Missing lines 1-4

Summary

• Total: 4 lines
• Missing: 4 lines
• Coverage: 0%

src/scratch_uncovered.py

Lines 1-5

! 1 def classify(n):
! 2     if n < 0:
! 3         return "negative"
! 4     return "positive"

---

Same change after a test covering it was added (gate passes, CI green):

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 31%.

Diff Coverage

Diff: origin/main...HEAD

• src/scratch_uncovered.py (100%)

Summary

• Total: 4 lines
• Missing: 0 lines
• Coverage: 100%

---

The !-prefixed lines are the exact changed lines the suite never executed — the author sees precisely what to cover. One sticky comment per PR, updated on every push.

[qingyun-wu]

Well-designed gate overall — the diff-coverage choice (95% on changed lines, whole-tree informational) is the right call for a tree at 31%, and the two-stage comment pattern (gate on pull_request with read-only token, comment via workflow_run that never checks out PR code) is the textbook-safe way to post fork-PR comments. The .coveragerc source mode also closes the classic loophole: a brand-new untested module still lands in coverage.xml at 0%, so diff-cover counts it.

One required change — coverage-comment.yml can't download the artifact:

permissions:
  pull-requests: write

Cross-run artifact download (download-artifact@v4 with run-id: + github-token:) requires actions: read on the token, and declaring a permissions block sets every unlisted scope to none — so as written, the download step 403s on every fire and no comment is ever posted. Fix:

permissions:
  pull-requests: write
  actions: read

(Refs: download-artifact cross-run discussion, GITHUB_TOKEN permissions docs.)

One verification ask: this PR touches no .py, so its own gate run took the no-Python fast path (the 13s check) — the instrumented-suite + diff-cover path has never executed in CI. Before merge, please paste a local bash scripts/coverage-gate.sh run from a branch with a real Python change (one passing, ideally one failing) so we know the full path works, since the next Python PR becomes the guinea pig otherwise.

Two non-blocking notes:

• Test-only PRs behave correctly by accident of design: tests/* is omitted from coverage, and diff-cover ignores files absent from the report rather than treating them as 0% — worth one line in docs/testing-coverage.md so nobody "fixes" the omit later and breaks test-only PRs.
• Policy (echoing the #pr-triage thread): 95% on changed lines in bridge/GUI code will force mock-heavy tests of low value. Consider launching warn-only (--fail-under on a pure-logic allowlist, report-only elsewhere) and letting the owner flip it to blocking once the sticky comment has built some track record. The escape-hatch paragraph in the doc already acknowledges this — starting soft just makes it structural.

Happy to re-review once the permission fix lands.

[qingyun-wu]

LGTM.

• Diff-coverage over a global floor is the right call with the tree at 31%: the 95% bar lands on every new line without redding unrelated PRs for legacy debt, and the informational whole-tree number keeps the ratchet visible. The flip-to-global-floor path is documented for when the tree converges.
• The two-stage comment pattern is the security-correct one: the gate runs on pull_request (fork PRs get a read-only token), the commenter runs on workflow_run with pull-requests: write only and never checks out PR code — avoiding the pull_request_target foot-gun. I considered the residual artifact-trust surface (a fork could stamp an arbitrary pr-number.txt, steering the sticky comment to another PR) and agree it's the accepted trade-off, since workflow_run.pull_requests is empty for fork PRs — which is exactly why the artifact exists. Token can only comment; content is markdown, never executed.
• Gate script details check out: set -euo pipefail; the no-Python fast path saves the instrumented suite run (and fetch-depth: 0 is present for diff-cover's base comparison); test failures under instrumentation are collected across all files and fail the gate with a clear "suite must be green first" verdict; parallel = True + combine matches the one-process-per-standalone-test reality; sticky-comment marker + paginate avoids comment spam.
• Self-demonstrating: the new "diff coverage >= 95% (python)" check ran green on this PR itself, and all existing checks pass.
• Docs cover the local run path, the pragma policy, the subprocess-boundary caveat (import-the-module guidance), and the owner escape hatch — good reviewer ergonomics.

[github-actions]

@cla-assistant check