Detect PRs that received no CI at all (#345)

[realmarcin]

PR #344 produced zero pull_request workflow runs — not failures, not skips. gh pr checks printed "no checks reported", mergeStateStatus stayed CLEAN, and the PR looked
mergeable with nothing having evaluated it.

First: the diagnosis in #345 was wrong, and is corrected there

I filed it blaming GitHub's 300-file limit on evaluating paths: filters. But
pr-sanity.yaml and vendored-sync.yaml carry no paths: filter at all — deliberately,
per #184/#200 — and neither fired either. Querying by SHA rather than by branch confirms it:

gh api ".../actions/runs?head_sha=<#344 head>"
  -> every run is event=workflow_dispatch    (only the ones I triggered by hand)
  -> ZERO runs with event=pull_request

A limit on filter evaluation cannot explain a workflow with no filter to evaluate. PR #343
(2 files), opened minutes earlier from the same clone, got 5 pull_request runs. The file
count correlates; I have not shown it causes.

So this detects the silence, not a hypothesised cause

The rule: does an open PR have at least one check triggered by the pull request itself?
That stays meaningful however the runs go missing.

The workflow triggers on push to main, not on pull_request — and that's the substance,
not a detail. A workflow that runs on pull_request cannot detect a PR where
pull_request events aren't arriving. Pushes to main demonstrably do fire, so it reports
from a vantage point that works. No schedule:, per the kill-switch reasoning
pr-shepherd.yml sets out.

workflow_dispatch runs deliberately don't count as evidence. Dispatching by hand is
what you do after noticing — counting it would make the check green on exactly the PRs it
exists to find. A test pins that specifically.

What was checked

• Run against the live repo: it flags Normalise the corpus so the round-trip claim becomes enforceable (#322) #344 and nothing else.
• Fetching is kept out of the rule, so it's testable with no network or token — 6 tests on
  the rule, main() shells out to gh.
• just qc green · just pr-sanity clean · 443 tests pass · audit-qc-paths accepts
  the new workflow · audit-justfile-paths caught the untracked script before CI did.

Not in qc: it needs network and gh auth, which qc deliberately does not.

Partially addresses #345 — the detector lands, the root cause stays open.

🤖 Generated with Claude Code

[culturebot-reviewer]

Claude finished @realmarcin's task in 3m 46s —— View job

---

Review

The reasoning is sound and the correction to #345's diagnosis is the right call — detecting the silence rather than a hypothesised cause is what makes this durable. Triggering on push to main is correct and well justified. One real defect, plus a ceiling on what green means here that I think belongs in the docstring.

No trait data, CURIEs, evidence snippets, or causal graphs touched, so the usual grounding/evidence checks don't apply.

---

🟡 gh pr list silently caps at 30 — in a tool whose whole purpose is catching silent omissions

scripts/audit_pr_checks_present.py:61 calls gh pr list with no --limit. gh's default is 30 (-L, --limit int … (default 30)). Past 30 open PRs, the 31st onward are never fetched, never checked, and the audit prints open PRs: 30 and exits 0 — reporting a clean bill of health for a set it never looked at. That is the exact failure shape #345 is about, reproduced inside the detector.

Same class, much narrower: line 66's actions/runs?head_sha=… also takes the API's 30-per-page default. head_sha bounds it to one commit so it rarely bites, but a PR that accumulated many hand-dispatches (precisely the #344 remediation path) can push a genuine pull_request run off page one. That direction produces a false positive, so it's the safe failure — worth &per_page=100 anyway since you're touching the line.

Both are one-flag fixes: --limit 500 and &per_page=100. Fix this →

---

🔵 Green here is weaker than the failure message implies — claude-code-review.yml alone satisfies the rule

.github/workflows/claude-code-review.yml fires on pull_request: [opened, synchronize] with no paths: filter, so essentially every PR in this repo gets at least one pull_request run from the reviewer bot regardless of what the gating workflows do. A run is recorded even when the job is skipped by its if: gates.

So the rule can only detect total event silence — which is #344's shape, and the PR title says "at all", so this is honestly scoped. But it means the one hypothesis you explicitly left open (paths-filter evaluation degrading on large PRs) would produce a partial silence — pytest, qc, validate-strict all mute, reviewer bot firing — and this check stays green through it. Worth a line in the module docstring saying so, so the next person doesn't read a passing audit-pr-checks as "the gating workflows ran."

If you later want the stronger property, the shape is a named set of required workflows checked by name rather than event, not a change to this rule.

🔵 NON_EVIDENCE_EVENTS is a denylist where the stated contract is an allowlist

Line 34 / line 46: the docstring says "triggered by the pull request itself," but the implementation is "anything except workflow_dispatch and schedule." That currently counts push, repository_dispatch, merge_group, pull_request_target, and dynamic as evidence. Nothing in the repo produces those against an open PR's head SHA today — every workflow's push: is branches: [main] — so this is latent, not live. But the failure direction is the bad one: a new trigger added later silently starts counting as evidence, with no test failing. {"pull_request", "pull_request_target"} as an allowlist states the actual rule and fails closed. Fix this →

🔵 A red X lands on a main commit and stays there

Exiting 1 (line 91) fails the job on a main commit for a condition about a different PR. It's an alarm that fires where someone will see it, which is the point — but the mark is permanent on that commit's history even after the offending PR is fixed or closed, and it dilutes the "this commit was verified" reading of main's check list that docs/WORKFLOW_CONVENTIONS.md argues for under Never cancel on main. There's also a narrow race: a PR opened seconds before a merge has no runs yet and reads as an offender. If you keep exit 1 (defensible), consider skipping PRs younger than a few minutes.

🔵 Concurrency key doesn't match the documented idiom

.github/workflows/pr-checks-present.yaml:34-37 groups on github.ref with cancel-in-progress: false. Since this workflow only ever runs on main plus dispatch, that's not superseding — it's queueing, so a manual dispatch waits behind an in-flight push run for no benefit. WORKFLOW_CONVENTIONS.md says to give anything that isn't genuinely superseding a unique key (github.run_id). The comment on line 36 explains why you're not cancelling, which is right; the shared key is the part that doesn't buy anything.

🔵 Hardcoded repo default

Line 75 defaults --repo to CultureBotAI/TraitMech while the workflow has ${{ github.repository }} available. Consistent with new_history_record.py and render_trait_pages.py, so take or leave it.

---

Not verified: I could not run pytest or just pr-sanity — Bash is restricted in this environment, so I'm taking the "443 tests pass / just qc green" claims in the description at face value. The test file itself reads correctly and covers the dispatch trap well; test_hand_dispatched_runs_do_not_count_as_evidence is the right thing to pin.

Requesting changes on the 30-PR cap. The rest is yours to weigh.

• Read repo conventions (README.md, docs/WORKFLOW_CONVENTIONS.md)
• Read the diff (script, tests, workflow, justfile)
• Verify detection logic against every workflow's actual triggers
• Post review + verdict
  · branch fix/345-large-pr-no-ci

[culturebot-reviewer]

🟡 One real defect: scripts/audit_pr_checks_present.py:61 calls gh pr list with no --limit, so it silently caps at gh's default of 30 open PRs — the 31st onward are never fetched, never checked, and the audit exits 0 reporting clean on a set it never looked at. That is #345's failure shape reproduced inside the detector. Line 66's runs query likewise takes the API's default per_page=30. Both are one-flag fixes.

Also worth a docstring line: claude-code-review.yml fires on pull_request with no paths filter, so the reviewer bot alone satisfies the rule on essentially every PR. The check therefore detects total event silence only — which is #344's shape and matches the title, but a partial silence (the paths-filter hypothesis you left open) passes through green.

Smaller notes in the full review comment: denylist vs allowlist for NON_EVIDENCE_EVENTS, the permanent red X on a main commit for another PR's condition, and a concurrency key that queues rather than supersedes.

The correction to #345's diagnosis and the choice to trigger on push-to-main are both right, and the dispatch-doesn't-count test pins exactly the trap that matters.

[realmarcin]

All six taken. The 🟡 is the one that mattered, and it's a fair hit — the detector had the bug it detects.

🟡 gh pr list capping at 30

Right, and painfully on the nose. Past 30 open PRs the rest were never fetched and this printed a clean bill of health for a set it never looked at. --limit 500 now, and the comment says the cap is explicit rather than inherited, since inheriting a default is how it happened.

Took the &per_page=100 on the runs query too. You're right it's the safe direction (false positive), but a PR with many hand-dispatches is precisely the #344 remediation path, so it's the case most likely to hit it.

🔵 Denylist → allowlist

Taken. {pull_request, pull_request_target}. Your point about the failure direction is the deciding one: nothing produces push/merge_group against an open PR head today, but a trigger added later would have silently started counting with no test failing. Two new tests pin it, including that a push run alone is not evidence.

🔵 Green is weaker than the message implies

Good catch, and worth stating rather than quietly relying on. claude-code-review.yml fires on pull_request with no paths: filter and records a run even when its if: gates skip the job — so this detects total silence only. The docstring now has a WHAT THIS DOES NOT CATCH section saying a partial silence passes, that a green audit-pr-checks must not be read as "the gating workflows ran", and that the stronger property needs a named required-workflow set rather than a tweak to this rule.

🔵 Fresh PRs racing a merge

Taken — MIN_AGE_MINUTES = 10, with a test. Without it this would have false-alarmed on any merge that happened to land just after someone opened a PR, which is the fastest way to get an alarm ignored.

I kept exit 1. The permanent red X on a main commit is a real cost, but a passing job nobody reads is exactly what #345 is about.

🔵 Concurrency key

Right — not superseding, so the shared key bought nothing and did queue. Now github.run_id, with the comment split so it's clear the no-cancel part is deliberate and the shared key was the accident.

🔵 Hardcoded repo

Now defaults from GITHUB_REPOSITORY.

Verified live

Still flags #344; no longer flags #346 now its checks have arrived; and runs clean against CommunityMech via --repo — which is the fleet-wide use that fix enables, and there are four such PRs across the fleet today.

just qc green · just pr-sanity clean · 449 tests pass · ruff clean.

[culturebot-reviewer]

Claude finished @realmarcin's task in 3m 58s —— View job

---

Review

All six taken, and the 🟡 is properly fixed — --limit 500 with the comment saying the cap is explicit rather than inherited, which is the part that will stop it recurring. I re-checked each point against 7d6b849b and have no blocking findings. Four 🔵s below, all in the same "the fix is right, here's where its edge still is" family.

No trait data, CURIEs, evidence snippets, or causal graphs touched.

---

Verified against the previous round

• gh pr list --limit 500 (scripts/audit_pr_checks_present.py:89) and &per_page=100 (line 98) — both explicit, both commented with why.
• Allowlist — EVIDENCE_EVENTS = frozenset({"pull_request", "pull_request_target"}) (line 51), with test_a_push_event_run_alone_is_not_evidence and test_an_unknown_future_trigger_is_not_evidence pinning the closed direction. That's the right pair: the second is the one that fails when someone adds a trigger.
• WHAT THIS DOES NOT CATCH (lines 21–28) — says plainly that a green audit-pr-checks must not be read as "the gating workflows ran", and that the stronger property is a different rule. That's the honest version.
• MIN_AGE_MINUTES = 10 — see 🔵 Curate DOI-backed nutrient physiology causal graphs #1, the clock is off but the gate is right.
• github.run_id (pr-checks-present.yaml:39) with the comment split so the no-cancel half stays deliberate. Matches WORKFLOW_CONVENTIONS.md Concurrency.
• GITHUB_REPOSITORY default (line 111).

I also checked the thing a new no-paths: workflow could break: NO_UNFILTERED_CI counts a workflow only if "pull_request" in trigger_names(triggers) (scripts/pr_sanity.py:462). This one triggers on push-to-main, so it does not inflate the #200 floor from 2 to 3. Good — a workflow that counted toward "unfiltered CI" while running on no PR would have been the exact false-green shape the doc warns about. Action pins match every other call site.

---

🔵 The age gate measures the wrong clock

collect() derives age_minutes from pr["createdAt"] (line 100), but evidence is fetched for headRefOid (line 98). A PR opened last week and pushed twenty seconds ago has a brand-new head SHA with no runs against it yet — and age_minutes in the thousands, so the grace period doesn't apply. Push is a much more frequent event than open, so the case the gate covers is the rarer of the two.

Narrow in practice: checkout + setup-just + uv sync buys a couple of minutes before gh is called, and GitHub registers run records within seconds of a push, so the real window is small. Latent, not live. The head commit's committedDate is the clock you want (--json commits, take the last). Where that's older than the push — rebase, cherry-pick — it errs toward checking, so it's never worse than today. Fix this →

🔵 PRs skipped for youth vanish from the report

offenders() continues at line 69, and main() prints only open PRs: N and unchecked: M. A PR excluded by the age gate appears in neither number as excluded — the output reads as full coverage of all N. That's a small instance of the shape you just fixed one level up, and WORKFLOW_CONVENTIONS.md argues the general form of it under Verify the check ran. One stderr line (skipped (too new): #N) closes it. Fix this →

🔵 100 is still a cap — server-side filtering removes the question

gh api without --paginate returns page one, so per_page=100 is a bigger ceiling, not the absence of one. Safe direction, as established. But you don't need pagination at all here: ?head_sha=X&event=pull_request&per_page=1 filters server-side, so hand-dispatched runs can't crowd a genuine one off the page under any count. One extra call if you want pull_request_target honored too. Worth noting --paginate is not the drop-in alternative — combined with -q it emits one JSON document per page, and json.loads would break on the second. Fix this →

🔵 --limit 500 traded a silent undercount for a loud timeout

collect() makes one gh api call per open PR, sequentially, under timeout-minutes: 10. At 500 PRs it would not finish — and a timed-out job is red, which at a glance is indistinguishable from "found offenders". That is the right trade (loud beats silent, and this repo is nowhere near it), but the fleet-wide --repo use you cited is where it would first show. Take or leave.

---

Not verified locally: Bash is restricted in this environment, so I read rather than ran. CI on this head corroborates the claims independently — pytest, qc, pr-sanity and vendored-sync are all green on 7d6b849b.

Approving.

• Read the new diff (script, tests, workflow, justfile)
• Verify each of the six points is actually addressed
• Check for defects introduced by the fixes
• Confirm the new no-paths: workflow doesn't inflate the NO_UNFILTERED_CI floor
• Post review + verdict
  · branch fix/345-large-pr-no-ci

[culturebot-reviewer]

All six taken; the 30-PR cap fix is correct and the --limit comment states it as explicit rather than inherited, which is what stops it recurring. Allowlist has the right two tests, the WHAT THIS DOES NOT CATCH section is the honest version, and the concurrency key now matches WORKFLOW_CONVENTIONS.md. I also checked that this new no-paths: workflow does not inflate the #200 floor — NO_UNFILTERED_CI counts a workflow only if it triggers on pull_request, and this one triggers on push-to-main, so it correctly does not count.

Four 🔵s in the full comment, none blocking: age_minutes is derived from createdAt while evidence is keyed on headRefOid, so a long-open PR pushed seconds ago gets no grace period (the head commit's committedDate is the clock you want); PRs skipped by the age gate are invisible in the output, so it reads as full coverage; per_page=100 is a bigger ceiling rather than none, and &event=pull_request would remove the question server-side; and --limit 500 traded a silent undercount for a possible 10-minute timeout, which is the right trade but worth knowing.

No trait data, CURIEs, evidence snippets, or causal graphs touched. Bash was restricted here so I read rather than ran, but pytest/qc/pr-sanity/vendored-sync are green on this head.

[realmarcin]

Took three of the four. Approved already, but one of them was the same failure shape this tool exists to catch — not one to leave in this tool of all tools.

🔵 Skipped PRs vanishing — fixed

You're right that it's the shape I'd just fixed one level up. offenders() continued past a youth-excluded PR and main() printed only open/unchecked, so the output read as full coverage of a set it hadn't fully checked. partition() now returns (offenders, skipped) and main() names the skipped ones.

🔵 The age gate's clock — fixed

Correct, and the frequency argument is the decisive part: push is far more common than open, so createdAt covered the rarer case. Now measured from the head commit, falling back to createdAt if the lookup fails — which, as you note, errs toward checking.

🔵 Server-side filtering — fixed

&event=...&per_page=1. You're right that per_page=100 was a bigger ceiling rather than the absence of one. Also took the warning about --paginate + -q emitting one document per page; I didn't go that route.

🔵 --limit 500 vs the 10-minute timeout — accepted, and cheaper now

Agreed it's the right trade. It's also less exposed than when you raised it: evidence short-circuits on the first matching event, and the commit-date lookup happens only for a PR that would otherwise be reported. A healthy PR now costs one request, not three, so the fleet-wide case you flagged is where it'd bite, and it's further away.

One thing worth recording

The server-side change introduced a real bug: gh api -q unwraps to a bare scalar, and json.loads("2026-08-06T21:47:27Z") raises. The unit tests didn't catch it — they feed offenders() fixtures and never exercise collect(). The same separation that makes the rule testable leaves the fetching untested, and only running it against the live repo found it. Added _gh_text() for raw scalars.

Verified live after the fix: still flags #344 alone, runs clean against CommunityMech via --repo.

just qc green · just pr-sanity clean · 451 tests pass.

[culturebot-reviewer]

Claude finished @realmarcin's task in 3m 5s —— View job

---

Review

Round 3 takes three of the four 🔵s and each is correctly done. One new defect came in with the fix, in the part the author already flagged as the untested seam.

No trait data, CURIEs, evidence snippets, or causal graphs touched.

---

🟡 The documented createdAt fallback does not exist — and its absence discards the report

scripts/audit_pr_checks_present.py:136-139:

# ... Falls back to createdAt if the commit lookup fails.
iso = _gh_text(["api", f"repos/{repo}/commits/{sha}",
                "-q", ".commit.committer.date"]) or pr["createdAt"]

_gh_text() does not return on failure — it raise SystemExit(2) at line 93. So a failed commit lookup does not fall back to createdAt; it terminates the process. The or guard only fires on empty stdout, which is not what either failure produces:

• Non-zero gh exit (rate limit, transient 5xx, a head SHA the commits API won't resolve) → SystemExit(2), never reaching the or.
• Missing field → gh api -q prints the literal string null, which is truthy, so datetime.fromisoformat("null") raises ValueError — an unhandled traceback, not a fallback.

The fallback is unreachable by either route.

The consequence is worse than the dead branch. collect() builds collected in-loop and nothing is printed until main() returns from it, so a single failed lookup on any PR throws away the offender list already computed for all the others. The audit dies with gh ... failed: on stderr and exit 2 — loud, so this isn't a silent-green failure — but a #344-shape PR sitting in that list never gets named. And the lookup only happens for PRs that would otherwise be reported, so the request most likely to fail is the one attached to a genuine offender.

It's a two-line fix in either direction: give _gh_text a default (return "" on non-zero, and treat "null" as empty), or delete the sentence and say it aborts. The --repo fleet-wide use you cited is where a per-PR transient becomes likely rather than theoretical. Fix this →

---

Verified against round 2

• Skipped PRs visible — partition() returns (bad, young) (line 73) and main() prints both the count and a skipped (too new): #N line per PR (lines 161-164). Closed.
• Head-commit clock — age now derives from .commit.committer.date for the head SHA (line 138). Right clock; see the 🟡 for the fallback.
• Server-side filtering — ?head_sha=…&event=…&per_page=1 per event, break on the first hit (lines 122-128). sorted(EVIDENCE_EVENTS) puts pull_request first, so the common case costs one request and the ceiling is gone rather than raised. -q .total_count returns a bare int, which json.loads handles correctly — unlike the string case that broke.
• _gh_text / _gh_json split — the right shape, and the docstring at line 88 names the exact trap (json.loads("2026-08-06T21:47:27Z") raises) so the next person doesn't collapse them back together.
• --limit 500 vs timeout-minutes: 10 — cheaper than when it was raised, as you say: one request per healthy PR, and the commit lookup only for PRs that would be reported.

🔵 collect() is still the untested half, and the new test doesn't reach it

You named this yourself — the scalar-unwrap bug was found only by running against the live repo. But test_a_long_open_pr_with_a_freshly_pushed_head_is_skipped (line 101) calls offenders([_pr(3, [], age_minutes=0.5)]), which is the same assertion as test_a_freshly_opened_pr_is_skipped at line 72 with a different literal. Both pin the age gate; neither touches the age source, and the clock change lives entirely in collect(). That test would have passed identically before this commit.

A dozen lines monkeypatching _gh_json/_gh_text would pin the three things that have no coverage at all: that a pull_request hit short-circuits before querying pull_request_target, that age_minutes is None when evidence was found, and that the date path parses a bare scalar rather than JSON. That last one is the bug that shipped. Fix this →

🔵 partition() checks youth before evidence

Lines 68-72: the age branch is evaluated first, so a PR younger than 10 minutes that does have pull_request runs lands in young and is printed as skipped (too new) rather than passing. It can't happen live — collect() sets age only when found is empty — but that makes partition()'s output correct only because its caller encodes the same assumption, and partition() is the half that's meant to be independently testable. Checking evidence first and falling through to the age gate is the same three lines with no coupling.

---

Not verified locally: Bash is restricted in this environment, so I read rather than ran — the "451 tests pass / just qc green / just pr-sanity clean" claims are taken at face value, and CI on this head is the independent check.

Requesting changes on the 🟡. The two 🔵s are yours to weigh, and neither blocks.

• Read the round-3 diff (f6ec10a7)
• Verify the three taken 🔵s are addressed
• Check the new _gh_text / partition() / commit-date paths for defects
• Post review + verdict
  · branch fix/345-large-pr-no-ci

[culturebot-reviewer]

🟡 One defect in f6ec10a7: the "Falls back to createdAt if the commit lookup fails" comment at scripts/audit_pr_checks_present.py:137 describes behaviour the code does not have. _gh_text() raises SystemExit(2) on a non-zero gh exit (line 93), so a failed commit lookup aborts the whole audit rather than falling back — and it aborts before main() prints the offenders collect() had already found. One transient 5xx or rate-limit on PR #7 discards the report about #344. The or pr['createdAt'] guard only catches empty stdout; gh api -q on a missing field prints the literal null, which then reaches fromisoformat and tracebacks, so the fallback is unreachable by either route. Either implement it (catch the failure and return "") or delete the sentence.

The three taken 🔵s are all correctly done: partition() returns and main() names the youth-skipped PRs, the clock is the head commit, and &event=...&per_page=1 removes the ceiling rather than raising it. The _gh_text/_gh_json split for -q-unwrapped scalars is the right shape and the comment on it will stop the next person from re-introducing it.

Two 🔵s in the full comment: collect() is now ~40 lines of untested shell-out that has already harboured one live-only bug, and the new test_a_long_open_pr_with_a_freshly_pushed_head_is_skipped exercises the age gate rather than the clock source, so it would not have caught the change it is named for; and partition() checks youth before evidence, which is only correct because collect() sets age solely for evidence-less PRs.

No trait data, CURIEs, evidence snippets, or causal graphs touched. Bash is restricted here so I read rather than ran.

[realmarcin]

The 🟡 is exact, and it's my comment that was wrong — twice over, in two different ways I hadn't distinguished.

🟡 The fallback that couldn't fire

_gh_text raises SystemExit(2) on non-zero exit, so a failed commit lookup never reached the or — it terminated the process and discarded the report on every other PR, which is the run's entire job. And gh api -q prints the literal null for a missing field, which is truthy, so that path reached datetime.fromisoformat("null") and raised.

Added _gh_text_opt(), deliberately distinct from _gh_text: this is the one call site where a transient failure must degrade, not abort. It returns None on non-zero exit and filters the literal "null" rather than trusting an or. Parsing moved into _age_minutes(), which returns None on an unparseable value — and partition() treats unknown age as "do not skip", the safe direction, since skipping is what suppresses a report.

🔵 Evidence before youth

Right. A young PR that already has runs is simply fine, and calling it "skipped (too new)" understated the coverage the run achieved. Youth now only excuses an absence.

🔵 collect() being the untested half

This is the one I should have done last round when I noticed it rather than just writing it down. Both real defects in this file have lived there — json.loads on a bare scalar, then this fallback — and both were found by running it, not by the suite, because the tests fed offenders() fixtures and never crossed the gh boundary.

Five tests now stub that boundary and cover: head-commit date used, fallback on lookup failure, survival of a "null" timestamp, short-circuit once evidence is found, and the evidence-before-youth ordering.

Canaried: reverting _gh_text_opt back to _gh_text fails two of them.

just qc green · 456 tests pass · live run unchanged (flags #344 alone).

[culturebot-reviewer]

Claude finished @realmarcin's task in 2m 15s —— View job

---

Review

Round 4 fixes the 🟡 properly — the fallback is now reachable by both routes it was unreachable by, and _gh_text_opt / _age_minutes / partition degrade in the right direction at each step. No blocking findings. Three 🔵s, all about the seam you identified yourself.

No trait data, CURIEs, evidence snippets, or causal graphs touched.

---

Verified against round 3

• _gh_text_opt (scripts/audit_pr_checks_present.py:102-120) — returns None on non-zero exit instead of SystemExit(2), and filters the literal "null" before it can reach fromisoformat. Both routes that made the documented fallback unreachable are closed. Keeping it as a separate function rather than adding a default= to _gh_text is the better call: it makes "this one call site must degrade" a property of the call site, not a flag someone can forget to pass.
• _age_minutes (line 131) — None on unparseable, and partition() line 74 treats unknown age as do not skip. Right direction: skipping is what suppresses a report, so an unknown clock must not buy silence.
• Evidence before youth — partition() checks EVIDENCE_EVENTS first (line 71), so partition() is now correct on its own inputs rather than only under collect()'s calling convention. test_a_young_pr_that_already_has_runs_is_not_reported_as_skipped pins it.
• collect() stubs — five tests now cross the gh boundary. test_collect_falls_back_to_created_at_when_the_commit_lookup_fails is a genuine canary: revert line 180 to _gh_text and the _gh_text_opt monkeypatch stops applying, the real subprocess runs against o/r, and it exits 2.

---

🔵 The clock-source test does not discriminate the clock source

test_collect_uses_the_head_commit_date_when_available (tests/test_audit_pr_checks_present.py:131) sets commit_date="2020-06-01" against createdAt="2020-01-01" and asserts pr["age_minutes"] > 0. Both dates are six years past, so that assertion holds identically whichever one is used. Change line 180-181 to plain iso = pr["createdAt"] and this test stays green — the comment on line 135 ("Measured from the 2020-06 commit, not the 2020-01 PR open date") asserts something the code below it doesn't check.

This is the same shape flagged last round on test_a_long_open_pr_with_a_freshly_pushed_head_is_skipped, which you fixed by adding collect() coverage — the coverage arrived but the discriminating assertion didn't come with it. One line closes it: run collect() twice, once with commit_date="" (forcing createdAt), and assert the commit-date age is strictly smaller. Fix this →

🔵 The "null" filter is the one branch the "null" test cannot reach

test_collect_survives_an_unparseable_timestamp (line 146) monkeypatches _gh_text_opt — the function that does the filtering — so its stub hands "null" straight through. What actually runs is _age_minutes's ValueError path returning None. The line-150 comment says "null is filtered by _gh_text_opt, so createdAt is used"; in this test it isn't filtered and createdAt isn't used. Outcome is the same either way (age None → not skipped → reported), so nothing is wrong live — but line 120's out in ("", "null") and line 118's return None both have zero coverage, and stubbing the boundary is precisely how they escaped it.

Given the stated history — two defects in this file, both in the fetch layer, both found only by running it — the missing test is the small one that patches subprocess.run and asserts _gh_text_opt returns None for a non-zero exit and for stdout "null". Fix this →

🔵 _gh_text is now dead

Nothing calls it — the only remaining reference is _gh_text_opt's docstring naming it for contrast (line 105). Its docstring carries the useful warning (json.loads("2026-08-06T21:47:27Z") raises), so deleting it loses that; but an uncalled function is where a warning goes to rot, and the next person to need a raw-scalar fetch may reach for it without the _opt reasoning. Either fold the warning into _gh_json and drop _gh_text, or leave a one-line note saying it's kept as the strict counterpart. Take or leave.

---

Not verified locally: Bash is restricted to git in this environment, so pytest and just qc would not run — the "456 tests pass / just qc green" claims and the live-run result are taken at face value, with CI on this head as the independent check. The three findings above are from reading the fixtures against the code, not from executing them.

Approving.

• Read the round-4 diff (8ceeb33e)
• Verify the 🟡 fallback is actually reachable now
• Check the two 🔵s (collect() coverage, evidence-before-youth)
• Look for defects introduced by the fixes
• Post review + verdict
  · branch fix/345-large-pr-no-ci

[culturebot-reviewer]

Round 4 fixes the 🟡 correctly. _gh_text_opt returns None on a non-zero gh exit instead of SystemExit(2), and filters the literal null that gh api -q prints for a missing field, so both routes that made the documented createdAt fallback unreachable are closed. _age_minutes returns None on an unparseable value and partition() treats unknown age as 'do not skip', which is the safe direction since skipping is what suppresses a report. Keeping _gh_text_opt as a separate function rather than a default= flag on _gh_text is the better shape — it makes 'this call site must degrade' a property of the call site. Evidence-before-youth is done, and the fallback test is a real canary: reverting to _gh_text makes the monkeypatch stop applying and the test exits 2.

Three 🔵s in the full comment, none blocking. Two are about the seam you named yourself. test_collect_uses_the_head_commit_date_when_available asserts only age_minutes > 0, which holds for both the 2020-06 commit date and the 2020-01 createdAt, so it would stay green if collect() were reverted to pr['createdAt'] — the coverage arrived without the discriminating assertion. And test_collect_survives_an_unparseable_timestamp monkeypatches _gh_text_opt, the function that does the null filtering, so what it exercises is _age_minutes's ValueError path; lines 118 and 120 have no coverage, and the test's inline comment describes behaviour it doesn't reach. Outcome is safe either way. Third: _gh_text is now dead code, referenced only from _gh_text_opt's docstring.

No trait data, CURIEs, evidence snippets, or causal graphs touched. Bash is restricted to git here so I read rather than ran; CI on this head is the independent check on the test claims.