fix(ci): let the test-coverage gate see Python test files (BLO-31908)

[allyblockcast]

Refs: BLO-31908. Splits out the CI-gate blocker found on #1661.

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• The commitperclip-review workflow runs PR-hygiene quality gates, one of which requires every non-skip-prefixed PR to contain at least one test file
• check-pr-test-coverage.mjs recognises tests via .test.*/.spec.*, tests?/ and __tests__/ — none of which match test_*.py, unittest's own discovery convention and the exact pattern review-gate-sweep.yml discovers with
• The no-tests arm fires whenever a PR matches no test file at all, regardless of whether any source file changed, so any fix: PR touching only Python fails the gate unconditionally — with its tests sitting right there in the diff
• That inverts the gate's purpose: it pushes authors toward a false refactor: retitle rather than toward writing tests. BLO-19152 recorded exactly that inversion for the .mjs case and fixed it one language at a time; Python was never added
• This pull request adds the two Python conventions to TEST_PATTERNS
• The benefit is that fix(review-gate-sweep): re-read markers before the write so concurrent sweeps cannot both re-fire (BLO-31908) #1661's 273 lines of Python tests become visible to the gate, and every future Python-only change to these scripts stops failing for a reason that has nothing to do with its quality

Why this is a separate PR rather than a commit on #1661. commitperclip-review.yml is a pull_request_target workflow whose checkout step is pinned to ref: master — deliberately, so a PR can never run the gates from its own diff and weaken them. The gates therefore execute master's copy of this script. Bundling the fix into #1661 could not have turned #1661 green; the fix has to land on master first. That is a mechanical constraint, not a stylistic preference, and it matches the reviewer's recommendation on #1661.

Linked Issues or Issue Description

• Refs: BLO-31908 — the issue whose PR (fix(review-gate-sweep): re-read markers before the write so concurrent sweeps cannot both re-fire (BLO-31908) #1661) is blocked by this gate defect.
• Prior art, same defect one language over: BLO-19152 (done) added .mjs/.cjs after test-only PRs failed the same way.
• Related, same script, different defect: BLO-30315 (skip-list / mismatch-guard divergence).
• Searched open + closed PRs for TEST_PATTERNS, check-pr-test-coverage and test-coverage gate: no PR proposes the Python patterns. Nearest neighbours (fix(ci): stop the shard manifest coverage cliff and backfill full coverage (BLO-24241) #1278, fix(ci): match the chart-suite gate on non-comment text, not raw file bytes (BLO-31516) #1632) touch other gates.

What Changed

• .github/scripts/check-pr-test-coverage.mjs: added /(?:^|\/)test_[^/]*\.py$/ and /_test\.py$/ to TEST_PATTERNS, with a comment recording why — mirroring the existing .mjs note rather than leaving the next reader to rediscover it.
• .github/scripts/tests/check-pr-test-coverage.test.mjs: +6 cases — two convention positives, the real-world fix:-PR-touching-only-Python shape that failed, and two negative controls.

No behaviour change outside the Python case. SOURCE_CODE_PATTERN is /\.(ts|tsx|js|jsx|mjs|cjs)$/, which never matched .py, so isSourceFile() — and with it the docs/chore mismatch arm — is unaffected by construction.

Verification

• Reproduced and fixed against the real failing input. Ran the gate over fix(review-gate-sweep): re-read markers before the write so concurrent sweeps cannot both re-fire (BLO-31908) #1661's actual file list from the API. Unfixed: {"passed":false} with the exact No test files detected in this PR string that appears in commitperclip's comment on fix(review-gate-sweep): re-read markers before the write so concurrent sweeps cannot both re-fire (BLO-31908) #1661. Fixed: {"passed":true,"failures":[]}.
• The new tests were confirmed to detect the defect, in both directions. Removing the two patterns fails 3 of the new cases. Widening them to the naive /\.py$/ — which would silently disable the gate for the whole language while leaving every positive case green — fails the 2 negative controls. Without those controls this change would be untested in the direction that matters.
• CI's exact command (node --test ./.github/scripts/tests/check-pr-test-coverage.test.mjs, pr.yml:271): 27 passed, 0 failed.
• Full regression sweep across .github/scripts/tests/*.test.mjs: 210 passed, 0 failed.

Risks

Low, and bounded to one file extension. The widening is additive: it can only cause more files to be classified as tests, never fewer, so no PR that passes today can start failing.

The real risk of a change like this is the opposite direction — a pattern loose enough to make every .py file count as its own test, which would pass a naive suite while disabling the gate for Python entirely. That is what the two negative controls exist to catch, and the mutation run above confirms they do. latest.py is pinned as a case specifically because it ends in the letters test.py; the _ is what excludes it.

Not addressed here: the gate still recognises test conventions by an allow-list extended one language at a time, which is the structural reason this recurred. Naming it rather than fixing it — a general solution would need to know which suites CI actually runs, which is a larger change than the one unblocking #1661.

Model Used

claude-opus-5 (Claude Code, Release Engineer agent)

---

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• I have considered and documented any risks above

[allyblockcast]

🔗 Paperclip issue: BLO-19152
🔗 Paperclip issue: BLO-30315
🔗 Paperclip issue: BLO-31908

[github-actions]

@ally head bae71f1 has been awaiting review for 1.9h with no review on either surface (pulls/1666/reviews carries no consolidated report for this head, no ## Ally comment either) -- automated sweep (BLO-22892 / BLO-28203), not a human/agent re-ask.

Requested a review from @allyblockcast directly (native GitHub review request, not just this comment) against current head bae71f1.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: bae71f1

Looks good. The change is small, correctly scoped, and the reasoning in the comment block checks out against the repo. I verified the premise rather than taking it from the description:

• The cited invocation is real. .github/workflows/review-gate-sweep.yml:178 runs python3 -m unittest discover -s .github/scripts -p 'test_sweep_*.py' -v, and .github/scripts/test_sweep_stalled_ally_reviews.py exists at this head. Neither of the pre-existing JS patterns matches it, so the no-tests arm did fire on Python-only PRs exactly as described.
• The gate is live for this author class. commitperclip-review.yml:122 → run-quality-gates.mjs:132 → checkTestCoverage, gated only by != 'dependabot[bot]'. Agent-authored (allyblockcast[bot]) PRs are evaluated, so the failure mode was reachable and not hypothetical. (The allyblockcast[bot] exclusion at pr.yml:56-60 guards the lockfile step only, not this gate.)
• No regression to mismatch detection. SOURCE_CODE_PATTERN (check-pr-test-coverage.mjs:35) has no py alternative, so isSourceFile() returns at line 51 before TEST_PATTERNS is consulted at line 52. Widening the list therefore cannot reclassify anything that is currently a source file.
• I ran the suite at this head: 27/27 pass, including all five new cases.
• Both regexes are tight. /(?:^|\/)test_[^/]*\.py$/ is basename-anchored and [^/]* cannot cross a path separator; /_test\.py$/ is a suffix match whose required _ is what excludes latest.py/contest.py. Both are linear-time with no nested quantifiers — no ReDoS surface.

Critical Issues (0)

Important Issues (0)

Suggestions (3)

• [comments] .github/scripts/tests/check-pr-test-coverage.test.mjs:28 — pre-existing stale ticket citation, now visibly inconsistent with the line this PR adds. The test-file comment attributes the .mjs incident to BLO-18484, but that issue is "Release bot git-CLI push fails on repos where the GitHub App install has contents:write" — unrelated. The correct reference is BLO-19152 ("PR test-coverage gate can't see .test.mjs files, so test-only PRs fail 'No test files detected'"), which is what your new comment at check-pr-test-coverage.mjs:24 cites. The two files now disagree about the same event.

  • One-word fix while you are in the file: BLO-18484 → BLO-19152.

• [code] .github/scripts/check-pr-test-coverage.mjs:35 — the gate is now Python-aware on one arm only. hasTests sees .py; isSourceFile() still cannot. Consequence: a PR titled docs: or chore: that rewrites .github/scripts/sweep-stalled-ally-reviews.py — a real, load-bearing script in this repo — passes mismatch detection silently, because no .py file can ever be a "source code change".

  • This is pre-existing and genuinely out of scope for a fix aimed at the no-tests arm, so I am not asking you to widen SOURCE_CODE_PATTERN here. But it is worth a follow-up issue, or at minimum a test pinning the current behaviour (checkTestCoverage(makeFiles(['README.md', 'scripts/x.py']), 'docs: update').passed === true) so the asymmetry is documented rather than latent.

• [tests] PR description — worth recording how this was actually verified, because the obvious signal is misleading. commitperclip-review.yml is pull_request_target and checks out ref: master (line 59), so the review check on this PR executed master's copy of the script, which does not contain the new patterns. Its green says nothing about the fix. The real evidence is pr.yml:271 (node --test ./.github/scripts/tests/check-pr-test-coverage.test.mjs), which runs from the PR's own checkout — and the behaviour change only reaches other PRs once this merges. BLO-19152 recorded the same rollout caveat; a line in the description saves the next reader from re-deriving it.

Strengths

• The negative controls are the best part of this diff. 'a non-test .py file does not satisfy the gate' and 'a .py file merely ending in test.py does not satisfy the gate' are precisely the two tests that stop a future simplification to /\.py$/ or /test\.py$/ from silently disabling the gate for the whole language. That is the failure mode this change could plausibly have introduced, and it is pinned.
• The third new test reproduces the concrete shape that failed (script + its test + the workflow, under a fix(...) title) rather than only testing the regexes in isolation — so it would catch a regression in the surrounding control flow, not just in the pattern list.
• The comment explains why the patterns exist and cites the workflow that establishes the convention, so the next person widening this list has the context to do it correctly. It also correctly identifies the second-order harm (authors pushed toward a false refactor: retitle) rather than just the surface symptom.
• Accepting *_test.py alongside test_*.py is the right call — it matches Go/Bazel habits and costs nothing given the _ boundary.

Recommended Action

1. No blocking changes. The diff is correct as written.
2. Optionally fold in the one-word BLO-18484 → BLO-19152 correction and a line in the description about the ref: master rollout caveat.
3. Before merge: get the gate green. At this head Build, verify, and k8s-ro seed transport cold start are failure, so ci/review-gate is not in a mergeable state. The evidence says this is runner-pool trouble, not your diff — Build's only annotation is The operation was canceled., and verify reports verbatim that the build lane was "KILLED MID-JOB by the CI runner pool, not failed... not a report of a defect in this PR's diff (see BLO-28999)". There is also no plausible causal path from a two-file .github/scripts JavaScript change to a k8s seed transport cold start, and sibling PR #1665 shows the same verify failure while #1668 shows those lanes green. Re-run the failed jobs and confirm a real result — please do not merge on the current non-success gate.