Match a Side-Reviewer Ask to Its Head by the Push GitHub Logged - #4174

Summary

side_asked_on_head in scripts/pr_review_side.py decides whether CodeRabbit or Qodo was already asked for the current head. Until now it compared each ask comment's server createdAt with the head commit's committedDate. That date comes from the author's clock, so it can run behind the push or ahead of it.

• Backdated commit. A commit stamped 10:00 and pushed at 10:20 made a 10:05 ask for the previous head count for the new one. So the new head was never asked.
• Future-dated commit. A commit stamped 10:30 and pushed at 10:20 made a 10:25 ask for this head read as older. So the next wait asked again, a duplicate paid round.

The head's arrival is now the timestamp GitHub's repository activity log gives the push that left the head on its branch, read through gh api repos/<head repo>/activity?ref=refs/heads/<branch>. A jq projection prints the newest entry whose after is the head. The comment's createdAt is also a server stamp, so GitHub stamps both sides of the comparison.

This change reintroduces none of the defects the earlier fix rounds on #3881 hit:

• The log stamps a push when it arrives, before any check suite exists. So an ask made before the first suite still counts. A repository with no checks is asked once per head, not on every wait.
• No commit date is read anywhere, and committedDate leaves the pull request query.
• headRepository{ nameWithOwner } and headRefName, the fields pushed reads, are in Q_HELD, the query the asks' reads run.

Sometimes the push cannot be read: the read fails, the fork was deleted, or the log is missing the head. In that case a reviewer with an ask already on record is not asked again, and wait prints a note, because without the stamp that ask cannot be told from one for an earlier head. A reviewer with no ask on record is still asked, since there is nothing to duplicate. Neither bot gates the merge, so the worst case is one missed side round, never a wrong merge.

scripts/pr_review.py changes by its query and one wiring line, which hands gh_rest to side.ask. It stays at 2198 lines. scripts/pr_review_side.py is at 349 of its 350.

Verification

• tests/test_pr_review_policy.py, new TestSideAskServerStamp:
  • A backdated commit and a future-dated commit are each run through side.ask with the commit date set to mislead. Both cases fail with the old committedDate comparison restored.
  • The read builds the activity path with the branch URL-encoded. A failed read, a missing head repository, a missing branch, and a non-hex head all read as empty.
  • The real jq binary runs the projection against a constructed log. A head pushed twice resolves to the later push, and a log without the head prints nothing.
• An unread push with an ask on record asks nothing and prints the note. One with no ask on record still sends the first ask, and that case fails with the gate widened again. The wait-level cases that ask now answer the activity read.
• The merged-while-checks-settle case now carries a push stamp. It fails with its post-wait state check removed.
• Live read-only check against an open pull request in this repository. Q_HELD returns headRepository, and pushed returns the push's timestamp.
• For eight recent heads in this repository, the activity stamp came 1 to 2 seconds before the first check suite's createdAt. Each commit date was minutes earlier than either.
• The full suite passes except test_tooling_encoding, which caught the new jq spawn's missing encoding="utf-8". That is fixed, and the module passes. Run with uv run --no-project --with-requirements tests/requirements.txt python -m unittest discover -s tests -p 'test_*.py'. The pr_review modules and test_tooling_encoding pass after the last edit.
• ruff format, ruff check, mypy, and the prose gate against develop pass.

Local strict pass findings

The first pass over 2f77e8c raised four:

1. introduced, fixed in 11ede57. The merged-while-checks-settle test passed vacuously, because its payload carried no push stamp.
2. introduced, declined. If the activity log lagged the PR head, an on-open side ask could miss that wait. Measured on this repository's recent heads, the log comes before the first check suite. A lag would cost one missed side round with a printed note, which is neither silent nor destructive. Per GOVERNANCE.md "Trust Boundaries and Hardening Effort", it earns no retry.
3. introduced, declined. The scripts/README.md sentence "asked once per new head" does not name the unread-push skip. That sentence already omits the cap, cut-window, and closed-pull-request skips. The request_side_reviews docstring, which lists the skips, now names this one.
4. Design, declined. head_arrived still drives checks_hold's grace window. It answers a different question: when checks could have started on a read await_green polls. Moving it onto the push would add a REST read to every poll. A head with no suite and no check reading PENDING is how checks_hold already behaved.

The second pass over 11ede57 raised one:

5. introduced, fixed in 34f6585. The unread-push gate also withheld the first ask, even with no ask on record for an unread push to hide.

The third pass over 34f6585 raised one. It was handed to this pull request when the opening push's edit budget ran out:

6. introduced, fixed in bb41f02. No test pinned headRepository{ nameWithOwner } in Q_HELD. The fixtures inject it, so the suite would have passed with it dropped from the query while every live read came back empty. A case now pins each field the push read takes from Q_HELD, and it fails with that field removed.

The third pass also asked whether the activity log's timestamps carry fractional seconds. A live read shows they do not, for example 2026-11-13T19:11:58Z. Finding 2 has a live data point too: this branch's own push was in the activity log 4 seconds after git push returned.

The fourth pass over bb41f02 raised none.

Closes on promotion: #3882

🤖 Generated with Claude Code