binks-review

name	binks-review
description	Address Binks automated code review comments on a PR. Fetches comments, assesses validity, fixes valid issues, replies with feedback, reacts with thumbs up/down, and resolves threads. Triggers on: 'address binks review', 'handle binks comments', 'binks reviewed PR #N', or when user asks to respond to binks-code-reviewer feedback.

Binks Review Handler

Handles the full lifecycle of responding to binks-code-reviewer automated review comments on a PR.

Inputs

• PR number or URL (required) — e.g. 586245, https://meteorite.shopify.io/repos/shop/world/pulls/2013456
• Repo (optional, defaults to shop/world)

Workflow

1. Identify unresolved Binks threads

Resolve the PR system first per ../_shared/pr-system.md — GitHub and Meteorite PRs have separate number namespaces and different CLIs. Binks runs on both, and its threads carry the same <!-- binks-thread:<hash> --> marker either way, so the assessment logic in steps 2–3 is identical — only the fetch/reply/react/resolve mechanics differ.

Always start by checking thread resolution status.

PR_SYS=gh — fetch review threads via GraphQL to get resolution state and the root comment ID for each thread, then cross-reference with the REST comments to filter down to only unresolved Binks findings.

# Step 1a: Get thread resolution status and root comment IDs
gh api graphql -f query='
query {
  repository(owner: "{owner}", name: "{repo}") {
    pullRequest(number: {pr}) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              databaseId
              author { login }
            }
          }
        }
      }
    }
  }
}'

From this response, build a set of unresolved Binks comment IDs — threads where isResolved == false AND the root comment author is binks-code-reviewer.

# Step 1b: Fetch full comment bodies only for unresolved threads
gh api repos/{owner}/{repo}/pulls/{pr}/comments \
  --jq '.[] | select(.user.login == "binks-code-reviewer[bot]") | "ID: \(.id)\nPath: \(.path)\nLine: \(.line)\nBody:\n\(.body)\n---"'

Filter: Only process comments whose ID appears in the unresolved set from step 1a. Skip all resolved threads entirely — they've already been addressed (either in this session or a previous one).

PR_SYS=gs — there is no GraphQL endpoint. gs pr view <number> --comments prints every thread inline: each file:line, a thread: <uuid> line, and the comment body. One call replaces steps 1a and 1b.

gs pr view <number> --comments

Record the thread: <uuid> for each Binks finding alongside its comment id — gs pr resolve takes that UUID, not a GraphQL PRRT_… node id, and there is no second call that recovers it. Identify Binks threads by the <!-- binks-thread:<hash> --> marker and the comment author.

Filter: if --comments output does not mark resolution state, skip any thread that already carries a reply from you — that is the only resolved-vs-unresolved signal available on this path.

If all Binks threads are already resolved, report that and stop.

2. Assess each unresolved finding

For each unresolved Binks comment, do not assume it's correct. Read the relevant code and verify:

1. Validity: Is the claim actually correct? Check the code paths mentioned.
2. Importance: Even if valid, does it matter? Distinguish real bugs from stylistic nitpicks or overstated severity.
3. Recommendation: Fix it, acknowledge but skip, or push back — with reasoning.

3. Fix valid issues

For findings assessed as valid and worth fixing:

1. Make the code change
2. Add tests covering the fixed behavior
3. Run dev typecheck to verify (if available in the worktree)
4. Stage, amend, submit:

   git add -A && git commit --amend --no-edit
   git push --force-with-lease
   devx ci run
   

4. React on the original comment

React with 👍 or 👎 on each Binks comment based on your assessment:

PR_SYS=gh

# thumbs up (valid finding)
gh api repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions -f content='+1'

# thumbs down (invalid or not worth fixing)
gh api repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions -f content='-1'

PR_SYS=gs

# thumbs up (valid finding)
gs pr react <number> <comment_id> --thumbs-up

# thumbs down (invalid or not worth fixing)
gs pr react <number> <comment_id> --thumbs-down

5. Reply with feedback

Reply in-thread to each Binks comment. Structure the reply as:

1. What you did: describe the fix, or explain why no change is needed
2. Feedback on the finding: assess the quality of Binks' analysis — was the conclusion correct? Was the explanation precise or did it miss the real issue? Call out red herrings, imprecise reasoning, overstated severity, or cases where the finding was spot-on.

PR_SYS=gh

gh api repos/{owner}/{repo}/pulls/{pr}/comments \
  -f body="<reply text>" \
  -F in_reply_to={comment_id}

PR_SYS=gs — the body comes from a file, so write the reply to a temp file first (this also avoids escaping backticks and code fences).

gs pr comment <number> --reply-to <comment_id> --body-file /tmp/binks-reply-<comment_id>.md

6. Resolve threads

After replying, resolve each thread you addressed:

PR_SYS=gh — use the thread IDs captured in step 1a; no need to re-fetch.

gh api graphql -f query='mutation { resolveReviewThread(input: {threadId: "{thread_id}"}) { thread { isResolved } } }'

PR_SYS=gs — use the thread: <uuid> values captured in step 1.

gs pr resolve <number> <thread_uuid>

Reply tone

Follow the AGENTS.md guidelines for responding to Binks reviews:

• Don't assume the finding is correct — assess first
• Be direct about whether the fix was warranted
• Give calibration feedback: was the analysis precise? Did it miss the real issue? Was severity overstated?
• Keep it concise — one paragraph for the fix, one for the feedback

Example reply (valid finding, good analysis)

Fixed — added a rescue CSV::MalformedCSVError at the method level. Malformed CSVs now produce an error row instead of a 500.

Feedback on this finding: Spot on. The analysis correctly identified that this tool is for debugging broken batch uploads, making malformed CSVs a likely input. The error propagation reasoning was accurate and severity was appropriate.

Example reply (valid finding, overstated severity)

Fixed — non-dimensional sizes now report exists when content is found, instead of falling through to the aspect ratio check.

Feedback on this finding: The core observation is correct — composite sizes were misreported as incomplete. However, the severity is overstated. This is a read-only admin debug tool, not part of the import pipeline. A false incomplete wouldn't cause re-imports — it would just mean manual verification of those rows.

Example reply (invalid finding)

No change needed — the code already handles this case via the validate_row_inputs guard on line 168.

Feedback on this finding: The concern about unhandled nil is incorrect. The early return in validate_row_inputs ensures platform is non-nil before reaching the taxonomy parse step. The analysis missed the control flow from the extracted method.

Common Rationalizations

Check yourself against these before taking shortcuts:

Rationalization	Reality
"Binks is usually wrong, I'll dismiss this without checking"	Binks catches real bugs — PR #878 had 3 valid findings across 3 rounds, each a genuine correctness issue. Always read the code before assessing. Dismiss only with evidence.
"I'll address this in a follow-up"	Follow-ups get lost. If the fix is small (< 30 min) and the code is already checked out, fix it now. Only defer if the fix requires a separate design decision.
"This is just a style nit, thumbs-down and move on"	Binks doesn't flag style nits — it flags correctness and safety issues. If it looks like a nit, you probably misread the finding. Re-read the code path it's pointing at.
"The finding references code I didn't change, so it's not relevant"	Binks analyzes the PR's behavioral impact, not just the diff lines. A change in file A can break an invariant in file B. Verify the claim.
"A git command failed while fixing — I'll work around it"	Stop. Diagnose. Ask the user. Don't improvise with alternative git commands or skip the fix. A broken worktree state will compound when Binks fires again after the next push.