Add pr-sanity: cheap checks that run on every PR (#200) by realmarcin · Pull Request #201 · CultureBotAI/TraitMech

realmarcin · 2026-08-02T03:50:17Z

Closes #200.

The gap

Before #196, every workflow in this repo sat behind a paths: filter. A PR touching only docs/**, README.md, NEXT_TASKS.md, or a brand-new workflow file matched none of them and ran nothing.

gh pr checks prints no checks for that case. It reads like "nothing to verify". It means "nothing was verified".

PR #194 is a live instance — two files, zero checks — which is pointed, since #194's own description is about agent workflows that silently do not run.

#196 gave the repo a floor: vendored-sync has no filter, so every PR now gets at least one real check. But it verifies vendored files, not the diff in front of it. This verifies the diff.

What it checks

Four checks, stdlib + PyYAML, no network, ~0.5s:

check	catches
`WORKFLOW_INVALID`	a workflow that does not parse, or has no `on:` / `jobs:`
`NO_UNFILTERED_CI`	no workflow triggers on `pull_request` without a `paths:` filter
`CONFLICT_MARKER`	an unresolved merge-conflict marker in a tracked file
`BROKEN_LINK`	a relative Markdown link pointing at a path that does not exist

WORKFLOW_INVALID matters more than it looks: GitHub does not fail loudly on a malformed workflow, it silently never runs it — the same bug class as the rest of this PR.

NO_UNFILTERED_CI is self-referential on purpose. It is #200's invariant expressed as a test: if someone later adds a paths: filter to the last unfiltered workflow, this fails loudly instead of quietly returning the repo to "some PRs run nothing". There is also a test asserting the live repo satisfies it.

Testing, given what is being fixed

The failure mode here is "a check that silently verifies nothing", so a vacuous check would be self-defeating. Every check is therefore tested in both directions — it fires on the real defect, and stays quiet on the legitimate lookalike:

<<<<<<< HEAD flagged; ======= not flagged, because a bare ======= is a Markdown setext heading underline and matching it would false-positive on ordinary prose.
broken relative link flagged; resolving links, #anchors, https://, mailto:, and root-relative /docs/x.md all clean.
all-filtered workflows trip NO_UNFILTERED_CI; one unfiltered workflow satisfies it.
CLI exit codes asserted end to end (0 clean, 1 with findings).

I also confirmed against the live repo that the link check is not vacuous: it resolves 31 real relative links across 44 markdown files, all currently passing. A "0 findings" result that came from matching nothing would be exactly the disease.

14 new tests, 166 total.

Also

Wired into just qc as well as its own unfiltered workflow, so it runs locally. The duplicate CI run on qc-matching PRs costs half a second and buys local coverage.

Two pre-existing gates caught my own mistakes while building this, which is a good sign for them: audit-justfile-paths failed until scripts/pr_sanity.py was tracked, and the same untracked-file condition failed a test.

🤖 Generated with Claude Code

Before #196 every workflow sat behind a paths: filter, so a PR touching only docs/, README.md, or a brand-new workflow file ran nothing at all — `gh pr checks` printed "no checks", which reads like "nothing to verify" and means "nothing was verified". PR #194 is a live instance: two files, zero checks. #196 gave the repo a floor by adding one unfiltered workflow, but vendored-sync verifies vendored files, not the diff in front of it. This verifies the diff. Four checks, stdlib + PyYAML, no network, ~0.5s: WORKFLOW_INVALID a workflow that does not parse, or has no `on:`/`jobs:`. GitHub does not fail loudly on a malformed workflow — it silently never runs it, which is this whole bug class. NO_UNFILTERED_CI no workflow triggers on pull_request without a paths filter. Self-referential on purpose: if someone later filters the last unfiltered workflow, this fails instead of quietly returning the repo to "some PRs run nothing". CONFLICT_MARKER an unresolved merge-conflict marker in a tracked file. BROKEN_LINK a relative Markdown link pointing at a missing path. Only <<<<<<< and >>>>>>> are treated as conflict markers. A bare ======= is a legitimate Markdown setext heading underline, so matching it would false-positive on ordinary prose; there is a test pinning both directions. Because the failure being fixed is "a check that silently verifies nothing", every check is tested in both directions — fires on the defect, quiet on the legitimate lookalike — and I confirmed against the live repo that the link check is not vacuous: it resolves 31 real relative links across 44 markdown files, all of which pass. 14 new tests, 166 total. Wired into `just qc` as well as its own workflow, so it runs locally too. The duplicate CI run on qc-matching PRs costs half a second and is worth the local coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

check_workflows returned [] when .github/workflows did not exist, treating "no CI at all" as "nothing to check". That is the same silent-skip shape this script exists to catch: `just qc` would have gone green on a repo whose CI had been deleted. A missing directory is now the NO_UNFILTERED_CI finding in its most complete form. Filed from the same review, not fixed here: - #202 — the link check does not skip fenced code blocks, so a link written as an example inside a fence is treated as real. No live impact (0 findings today), and fence-aware parsing risks silently skipping real links, which would be worse than the false positive it removes. - #203 — setup-uv is pinned at v3, v5 and v7 across six workflows. This PR followed the qc.yaml majority (v3) rather than resolving the spread, so the decision belongs in its own change. 15 pr-sanity tests, 167 total, qc green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

realmarcin · 2026-08-02T03:52:40Z

Review

Adversarial pass over my own diff. One fix, two filed, plus a limitation worth stating rather than hiding.

It demonstrably works

This PR runs 4 checks (pr-sanity, pytest, qc, vendored-sync). Before #196 a PR shaped like this one would have run some of them; a PR touching only docs/** would have run none. That is the gap closed.

Fixed in `44a7e7a`

check_workflows returned [] when .github/workflows did not exist. Probing it, I found the invariant was skipped rather than failed on a repo with no CI directory — so just qc would go green on a repo whose CI had been deleted. That is precisely the silent-skip shape this script exists to catch, sitting inside the script itself. A missing directory is now NO_UNFILTERED_CI in its most complete form, with a test.

Filed, not fixed

pr-sanity's link check does not skip fenced code blocks #202 — the link check does not skip fenced code blocks. A link written as an example inside a fence is treated as real. Zero impact today (0 findings across 44 files), but it would surface as a CI failure on an unrelated PR, and pr-sanity running unfiltered means a false positive blocks any PR. Not fixed here because fence-aware parsing must handle indented fences and matching fence lengths, and getting it wrong would silently skip real links — worse than the false positive, given the point of this script is to not verify nothing.
Three different major versions of astral-sh/setup-uv across workflows (v3, v5, v7) #203 — setup-uv is pinned at v3, v5 and v7 across six workflows. This PR followed the qc.yaml majority (v3), so it adds a fourth v3 rather than resolving the spread. Beyond consistency, different majors resolve different uv versions, so uv sync can behave differently between jobs in the same PR.

A limitation I cannot engineer away

NO_UNFILTERED_CI cannot catch its own deletion. If someone deletes pr-sanity.yaml, nothing runs to complain that nothing runs. I verified the two cases it does catch — all-workflows-filtered, and no-workflows-directory — but a self-referential check has this floor by construction. The honest mitigation is that deleting a workflow file is a visible diff, unlike adding a paths: filter, which is a two-line change that silently shrinks coverage. That second case is the one this catches, and it is the one that actually happened (#184, #200).

State

pr-sanity self-checks clean in ~0.5s, just qc green, 167 tests (15 for this script). Not merging — your call.

Copilot

Pull request overview

Adds a fast “catch-all” CI gate (pr-sanity) intended to run on every PR (including docs-only and new-workflow PRs) to prevent the “no checks ran” failure mode described in #200.

Changes:

Add scripts/pr_sanity.py implementing cheap repo-wide checks (workflow YAML validity, “unfiltered PR workflow exists” invariant, conflict markers, broken relative Markdown links).
Add unit tests covering the checks and CLI exit codes.
Wire the check into both GitHub Actions (.github/workflows/pr-sanity.yaml) and local just qc via a new just pr-sanity recipe.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`scripts/pr_sanity.py`	Implements the pr-sanity checks and CLI output/exit codes.
`tests/test_pr_sanity.py`	Adds end-to-end and unit tests for the new script and invariant.
`justfile`	Adds a `pr-sanity` recipe and includes it in `qc`.
`.github/workflows/pr-sanity.yaml`	Adds an unfiltered workflow to run pr-sanity on every PR/push.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        if isinstance(triggers, dict) and "pull_request" in triggers:
+            pr = triggers["pull_request"]
+            if pr is None or (isinstance(pr, dict) and not pr.get("paths")):
+                unfiltered.append(rel)


+    for path in files:
+        if path.suffix not in TEXT_SUFFIXES or not path.is_file():
+            continue


+def test_one_unfiltered_workflow_satisfies_the_invariant(tmp_path):
+    root = _repo(tmp_path)
+    (root / ".github/workflows/a.yaml").write_text(FILTERED_WF)
+    (root / ".github/workflows/b.yaml").write_text(UNFILTERED_WF)
+    assert "NO_UNFILTERED_CI" not in _checks(check_workflows(root))
+
+
+def test_pull_request_with_empty_paths_list_counts_as_unfiltered(tmp_path):
+    root = _repo(tmp_path)
+    (root / ".github/workflows/a.yaml").write_text(
+        "name: x\non:\n  pull_request:\n    branches: [main]\njobs:\n"
+        "  a:\n    runs-on: ubuntu-latest\n    steps: [{run: \"true\"}]\n"
+    )
+    assert "NO_UNFILTERED_CI" not in _checks(check_workflows(root))
+


The first CI run of pr-sanity failed with 6 BROKEN_LINK findings that my local run could not produce. Both causes were real. 1. A genuinely broken link. .claude/skills/manage-identifiers/SKILL.md pointed at ../schema-gap-analysis/skill.md, but #190 renamed that file to SKILL.md and missed this reference. macOS resolves the lowercase path on its case-insensitive filesystem, so it looked fine locally and only failed on a Linux runner. Fixed the link, and made the check case-exact by comparing the final component against the real directory listing — the same mismatch is now caught on macOS too, rather than waiting for CI. 2. Links that escape the repo. README's ../CultureMech, ../MediaIngredientMech and ../CommunityMech, and the skills' ../../../../kg-microbe/... references, point at sibling fleet checkouts. They resolve on a machine with the fleet cloned and never on a runner, so checking them makes the result depend on what happens to be in the parent directory. Now skipped: this verifies links *within* the repo. That second fix exposed a third bug, found by probing rather than by CI. The first _within() compared a possibly-relative candidate against an absolute root, which always raises ValueError — so every in-repo link would have been classified as external and skipped, leaving the link check silently verifying nothing. Exactly the failure this script exists to catch, inside the script. Both sides now go through abspath, and there is a test. Counted after the fix: 26 in-repo links verified, 5 external skipped. The earlier "31 links all pass" was itself a false negative — 5 of those only resolved because the sibling repos happen to be checked out here. 18 pr-sanity tests, 170 total, qc green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

realmarcin · 2026-08-02T03:57:13Z

Review follow-up: the check earned its keep by failing on its own first run

pr-sanity's first CI run failed with 6 BROKEN_LINK findings my local run could not reproduce. Both causes were real, and one of them was a live bug in the repo.

1. A genuinely broken link, hidden by macOS

.claude/skills/manage-identifiers/SKILL.md:176 pointed at ../schema-gap-analysis/skill.md. #190 renamed that file to SKILL.md and missed this reference. macOS resolves the lowercase path on its case-insensitive filesystem, so it looked fine on every local run; Linux does not, so CI caught it.

Link fixed. I also made the check case-exact — it now compares the final path component against the real directory listing, so the same class of mismatch fails on macOS too instead of waiting for a runner. Verified directly: _exists_exact returns False for skill.md and True for SKILL.md on this machine, where plain exists() returns True for both.

2. Links that escape the repo

README's ../CultureMech, ../MediaIngredientMech, ../CommunityMech and the skills' ../../../../kg-microbe/... point at sibling fleet checkouts. They resolve on a machine with the fleet cloned and never on a runner, so checking them makes the result depend on what happens to be in the parent directory. Now skipped — this verifies links within the repo.

3. A bug that fix exposed, found by probing rather than by CI

The first _within() compared a possibly-relative candidate against an absolute root. That always raises ValueError, so every in-repo link would have been classified as external and skipped — the link check would have gone permanently, silently vacuous while reporting "all clear".

That is precisely the failure this script exists to catch, sitting inside the script. It would not have been caught by CI, because "0 findings" is indistinguishable from "working" unless you count. Both sides now go through abspath, with a test.

Counted after the fix: 26 in-repo links verified, 5 external skipped. My earlier claim of "31 links all pass" was itself a false negative — 5 of those only resolved because the sibling repos happen to be checked out here.

State

All four checks green (pr-sanity 19s, qc, pytest, vendored-sync), mergeable=CLEAN. 18 pr-sanity tests, 170 total.

Still filed and not fixed: #202 (fenced code blocks), #203 (setup-uv version spread).

Not merging — your call.

Two of the file's numbers did not reproduce, and a whole work thread was missing. - Header: no open PRs; merged through #210. Replaced the prose issue list with a table of all 11 open issues; recorded #184/#199/#200/#202/#204 as closed. - Section 4: the "predicates 85% / nodes 62%" figures do not reproduce under any metric. The repo's own dry-run scripts give 63% and 35%; point at them as ground truth instead of restating a number here. The "quality floor" claim was also too generous — `positively regulates` (37 edges) and `negatively regulates` (16) are exact RO labels sitting ungrounded while their paraphrases `promotes`/`inhibits` are already mapped. - Section 5: "1 of 220 traits done" was wrong arithmetic. 220 is what is still fragmented, re-measured today, across 220 of 477 files. - Section 7 (new): the CI/agent-workflow thread — #194, #196, #201, #206, #207, #210 — plus the seven small review issues it left open. - Section 8 (new): the paid Edison sweep's manifest says 353 ok, but research/ is gitignored and only 11 reports survive here. Resume detection is file-existence based, so a re-run would re-bill 342 completed calls. - Section 9 (new): the grounding backfill that follows from the section 4 correction, gated by the existing blocking label-correspondence check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The backlog had drifted a day and carried numbers that did not reproduce. Docs-only; the PR touches `NEXT_TASKS.md` and nothing else. Corrected: - Section 4's grounding coverage was recorded as "predicates 85% / nodes 62%". The repo's own scripts give 63% and 35%; the hardcoded percentages are replaced by the commands that produce them. - Section 5 presented four different measurements — 220 fragmented graphs, 219 baseline files, 1314 findings, 1264 stranded nodes — as though they were one. Now a table with a source per row. - Section 8 read "353 ok, 8 fail" as 8 unfinished traits. Every `fail:1` row is a retry that later succeeded; `fail − ok` is empty, so zero traits are outstanding. - Section 2 called the cross-Mech vendored-sync sweep pending. It landed: MediaIngredientMech#160 and CommunityMech#280/#278 are closed and both spokes carry the unfiltered workflow. Only the CultureMech `label-correspondence` half may remain. Added: section 7 for the CI/agent-workflow thread (#194, #196, #201, #206, #207, #210, #216), section 8 for the paid Edison sweep whose 353 reports are gitignored and absent from a fresh clone, section 9 for the grounding backfill. Filed along the way: #214 (residual reports drift with nothing to catch it), #217 (no workflow-conventions page), #218 (enforce the concurrency rule in pr-sanity), #220 (`audit-graphs` cannot see a graph splitting into two trait-bearing components — `morphology/dumbbell_shaped.yaml` is the live instance, and it is why 220 and 219 disagree). #215 was found and fixed the same way, in #216. The open-issue table was cross-checked against `gh issue list` before merge: 15 rows, exact match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Seven workflows were spread across three majors — v3 x4, v5, v7 x2 — which got worse rather than better since #203 was filed, because #210 added claude-code-review.yml on v7. Nothing pinned v3 deliberately; #201 chose it by copying qc.yaml, the majority at the time. v7 is the right target, and not merely because it is newest. It is the newest major *tag* that exists: v8 removed the update-major-minor-tags workflow, so v8 and v9 ship releases but publish no `vN` ref. Anything above v7 would have to be pinned to a full version or a SHA, which is a different decision from this one. Checked before bumping rather than after: - Every call site passes only `enable-cache: true`, plus `version: "latest"` on label-correspondence. Both inputs are present in v7's action.yml (23 inputs; enable-cache defaults to `auto`, version to empty). - v7 + `enable-cache: true` is already exercised in this repo — claude-code-review.yml has run it green repeatedly today — so the combination is not theoretical. - v9 changes the `prune-cache` default to false; v7 still defaults it true. Not a concern here, but it is the kind of thing that makes a later jump to a SHA pin worth doing deliberately. All seven workflows still parse and keep their on:/jobs: keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Seven workflows were spread across three majors, and it had drifted further since the issue was filed — #210 added claude-code-review.yml on v7, taking it from v3x4/v5/v7 to v3x4/v5/v7x2. Nothing pinned v3 deliberately; #201 picked it by copying qc.yaml, the majority at the time. v7 is not merely the newest that works, it is the newest major *tag that exists*: v8 removed the update-major-minor-tags workflow, so v8 and v9 ship releases but publish no floating `vN` ref. Anything above v7 requires a full-version or SHA pin, which is a separate decision — filed as #224 along with the observation that this repo already SHA-pins the one action that reads untrusted PR content. Checked before bumping rather than after: - Every call site passes only `enable-cache: true`, plus `version: "latest"` on label-correspondence. Both inputs exist in v7's action.yml. - v7 with `enable-cache: true` was already running green in this repo via claude-code-review.yml, so compatibility was not theoretical. - v9 flips the `prune-cache` default to false; v7 does not. Also drops `version: "latest"` from label-correspondence, the only call site setting it — that job could otherwise pick up a new uv release ahead of the other six on the same commit. Nothing pins a uv version (no [tool.uv] required-version, no .uv-version), and the action documents an omitted `version` as "the version in pyproject.toml or 'latest'", so removal is behaviourally identical and makes all seven resolve uv the same way. Remaining asymmetry, deliberately untouched and noted in #224: curation-history.yaml passes no `with:` at all and so runs uncached. That is a caching choice, not a version one. Because this touches qc, pytest, validate-strict, pr-sanity and label-correspondence, CI exercised the bump on itself. Closes #203. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings August 2, 2026 03:50

Copilot started reviewing on behalf of realmarcin August 2, 2026 03:50 View session

This was referenced Aug 2, 2026

pr-sanity's link check does not skip fenced code blocks #202

Closed

Three different major versions of astral-sh/setup-uv across workflows (v3, v5, v7) #203

Closed

Copilot AI reviewed Aug 2, 2026

View reviewed changes

realmarcin merged commit b0374f0 into main Aug 2, 2026
4 checks passed

realmarcin deleted the feat/pr-sanity-catchall branch August 2, 2026 05:08

This was referenced Aug 2, 2026

Add pr-shepherd (comment-only) and document the agent identity gate #194

Merged

Skip fenced code blocks in the link check, and report unclosed fences (#202) #207

Merged

Reconcile NEXT_TASKS.md (2026-08-02) #213

Merged

realmarcin mentioned this pull request Aug 3, 2026

Standardise astral-sh/setup-uv on v7 (#203) #222

Merged

Navigation Menu

Uh oh!

Add pr-sanity: cheap checks that run on every PR (#200) - #201

Add pr-sanity: cheap checks that run on every PR (#200)#201
realmarcin merged 3 commits into
mainfrom
feat/pr-sanity-catchall

realmarcin commented Aug 2, 2026

Uh oh!

realmarcin commented Aug 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

realmarcin commented Aug 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

realmarcin commented Aug 2, 2026

The gap

What it checks

Testing, given what is being fixed

Also

Uh oh!

realmarcin commented Aug 2, 2026

Review

It demonstrably works

Fixed in 44a7e7a

Filed, not fixed

A limitation I cannot engineer away

State

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

realmarcin commented Aug 2, 2026

Review follow-up: the check earned its keep by failing on its own first run

1. A genuinely broken link, hidden by macOS

2. Links that escape the repo

3. A bug that fix exposed, found by probing rather than by CI

State

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fixed in `44a7e7a`