fix(review-pr): batch thread replies and resolves into one GraphQL request - #143
Open
DJRHails wants to merge 3 commits into
Open
fix(review-pr): batch thread replies and resolves into one GraphQL request#143DJRHails wants to merge 3 commits into
DJRHails wants to merge 3 commits into
Conversation
…quest The resolve step posted a REST reply plus a resolveReviewThread call per finding, two content-creating requests each in a tight loop (and every REST reply mints an implicit review). With the fleet posting through one account, a busy hour trips GitHub's per-user content-creation throttle and every worker's gh pr create 403s with 'was submitted too quickly' (2026-09-02). Send all replies and resolves as one aliased mutation document instead, and back off with a wake rather than retrying on a 403.
DJRHails
commented
Sep 2, 2026
DJRHails
left a comment
DJRHails
left a comment
Owner
Author
There was a problem hiding this comment.
Automated review — findings posted inline. Each thread is resolved as its fix lands.
P2 (fixed): the resolve step told workers to "check every alias came back non-null" but not what to do when one is null. `gh api graphql` exits 1 whenever any alias errors, yet GraphQL executes the remaining aliases and they land, so re-sending the file duplicates every reply that already succeeded. Split the failure handling into the two shapes: HTTP 403 (nothing landed: 60 s once, then a wake) versus HTTP 200 with per-alias errors (re-send only the null aliases, once). P3 (fixed): reply bodies were plain GraphQL string literals, so one unescaped quote or backslash in a dismissal reasoning fails the whole document. Switched the example to block strings and said why; verified against the live schema that a block-string body with quotes and a backslash parses. P4 (fixed, trivial): dropped the now-unused `databaseId` from the thread fetch query. It only served the REST `in_reply_to` path this PR removes. Verification: the skill's graphql block, with placeholders filled, parses and executes against api.github.com (only NOT_FOUND on the placeholder ids); all prek hooks pass on the file.
DJRHails
commented
Sep 2, 2026
DJRHails
left a comment
DJRHails
left a comment
Owner
Author
There was a problem hiding this comment.
One more finding from measuring the batched flow live on this PR.
P2 (fixed): the resolve step claimed the implicit review was a REST artefact the GraphQL reply avoids. Measured live on dotfiles#143: each batched `addPullRequestReviewThreadReply` still minted its own empty COMMENTED review, so batching cuts requests (the content-creation throttle) but not PullRequestReviewEvents. Say so, so a worker does not mis-diagnose the echoes that still arrive.
Owner
Author
Review SummaryDirect single-pass review (one markdown file, 61 changed lines). No agent fan-out. Findings
Verification
Commits
Verdict: approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
On 2026-09-02 (12:35–13:00 UTC) every worker's
gh pr createthrough the maintainer token 403'd withwas submitted too quickly(createPullRequest) whilegh api rate_limitshowed core and graphql at 5000/5000 (reporting run). That is GitHub's per-user content-creation throttle (~80 requests/min, 500/hour), shared by the whole fleet because everything posts as one account.The DJRHails events feed for the 12:00 hour shows where the budget went: ~155
PullRequestReviewEvents in bursts of 15–20 at 2 s intervals, almost all on touchstone PRs. Those bursts are this skill's resolve step: for each finding it POSTs a REST reply (which mints an implicit review) and then a single-threadresolveReviewThreadmutation — two content-creating requests per finding, in a loop. 23 pr-reviewer runs were created in that hour against a usual 1–11.What
addPullRequestReviewThreadReply+resolveReviewThreadper thread) sent as a singlegh api graphql -F query=@filecall. Twenty findings go from 40 requests to 1, and from 20 implicit reviews (20 webhook echoes) to 0.The document shape was checked against the live GraphQL schema (parses; only
NOT_FOUNDon placeholder ids). Thefinding:F<n>tokens, dismissed-stays-open rule and gantry sign-off are unchanged.Companion: https://github.com/DJRHails/gantry/pull/997 (the gantry
BASE.mdnote on the shared throttle). The lean rewrite in #144 now carries the same batched block. Note for the in-flight lean rewrite of this skill onfeat/lean-review-pr-skill: it keeps the same per-thread loop (its lines 178–179) and should carry this block too.via gantry