# Bot Review Triage

CI bots (Copilot, Codex, CodeRabbit, SonarCloud) post review passes as
inline review threads + a review summary. They produce volume — triage in
batch, but never bulk-resolve without replies (SKILL.md Iron Laws 6 + 8).

## Known bot logins

`copilot-*`, `codex`, `coderabbitai`, `sonarcloud`, `github-actions`,
`dependabot`, `*-ci`. Detect via `__typename == "Bot"` (GraphQL) or
`user.type == "Bot"` (REST) — see `gh-commands.md` for why the `[bot]`
login suffix is unreliable.

## Batch flow (`--bots-only`)

1. Fetch unresolved threads, filter `isBot == true`
2. Classify each finding:

| Verdict | Signal | Action |
|---------|--------|--------|
| **Real bug** | Reproducible, matches code behavior | Fix → reply with diff summary → resolve |
| **Real but deferred** | Valid, out of this PR's scope | Reply "tracked as follow-up: {ref}" → resolve |
| **False positive** | Bot misread the code | Reply with one-line explanation of why it's safe → resolve |
| **Iron Law conflict** | Bot suggests an Iron Law violation | Reply declining with the law + reasoning → resolve |

3. Present the verdict table to the user BEFORE posting anything
4. Post replies + resolve only after approval

## Codex thread anatomy (chatgpt-codex-connector)

Codex inline comments have a fixed shape — parse it, don't guess:

- Priority badge: `![P1 Badge](https://img.shields.io/badge/P1-orange...)`
  (P1 orange / P2 yellow / P3), then a **bold one-line title**, a detailed
  body, and a `Useful? React with 👍 / 👎.` footer.
- Mapping: P0/P1 → treat as code-change/blocker; P2 → verify-then-fix;
  P3 → nitpick. Codex P1s on Elixir code have proven accurate (Ecto
  schema-field crashes, tsquery guards) — verify, but don't dismiss.
- Reviews are per-commit (`Reviewed commit: <sha>` in the summary body) —
  after a force-push or big rebase, outdated codex threads are expected;
  handle via the standard outdated-thread rule.
- Reply + resolve works exactly like human threads. Optionally react 👍/👎
  on the finding itself — it trains the reviewer.
- A codex review summary with ZERO inline threads = clean pass (summary-only
  round); nothing to triage.
- A clean pass can also be a plain bot COMMENT ("Codex Review: Didn't find
  any major issues" + `Reviewed commit: <sha>`) or a 👍 reaction on the
  trigger comment / PR body — all three mean the same thing.

## Common false-positive patterns (Elixir)

- **`nil[:key]` flagged as crash risk** — Access protocol on nil returns
  nil; nil-safe by design. Reply: "Access lookup on nil is nil-safe in
  Elixir (`nil[:key]` → `nil`); no guard needed."
- **"Unused variable" on pattern-match bindings** — bindings used for
  match assertion, not value. Prefix with `_` only if truly unused.
- **"Missing error handling" on `!` functions** — `Repo.get!`/`File.read!`
  crash intentionally per let-it-crash; supervised recovery is the design.
- **Atom-vs-string key confusion in test fixtures** — bots often suggest
  atomizing external/JSON data; that violates Iron Law #10 territory
  (`String.to_atom` on input). Decline.

## What NOT to do

- Never auto-resolve a bot pass to "clean up the PR" — each thread gets a
  reply first, even one line.
- Never accept a bot's code suggestion verbatim without reading the
  surrounding code — bots see the diff hunk, not the module.
- Never let a bot summary (review body) block on "resolution" — summaries
  are not threads and cannot be resolved; address inline findings instead.

# gh Command Reference — PR Review Threads

Verified against gh 2.94.0 and the live GitHub API (2026-06). Repo is
inferred from cwd, or parsed from a PR URL, or passed via `-R owner/repo`.

## The three comment surfaces

These are different endpoints with different reply mechanisms. Mixing them
up means replies silently land in the wrong place.

| Surface | Read | Resolvable? | Reply |
|---------|------|-------------|-------|
| Inline review comment (file/line) | GraphQL `reviewThreads` or REST `pulls/{n}/comments` | YES (thread) | REST `.../comments/{id}/replies` or GraphQL thread reply |
| Review summary ("Changes requested" body) | REST `pulls/{n}/reviews` | NO | Top-level conversation comment, or address inline and re-request review |
| Top-level conversation comment | REST `issues/{n}/comments` (a PR IS an issue) | NO | `POST repos/{o}/{r}/issues/{n}/comments -f body=...` |

## PR metadata

```bash
gh pr view "$PR" --json number,title,state,baseRefName,headRefName,url,author \
  --jq '{number,title,state,base:.baseRefName,head:.headRefName,url,author:.author.login}'
```

`gh pr view` accepts a number OR a URL and resolves the repo from the URL.

## Review threads with resolve state (GraphQL — the core query)

REST review comments have NO `isResolved` field and NO thread node ID.
The GraphQL query in SKILL.md Step 1 is the only way to get both. Key fields:

- `id` — `PRRT_...` thread node ID → used by resolve/unresolve and GraphQL reply
- `comments.nodes[].databaseId` — integer REST comment ID → used by REST reply
- `line` may be `null` on outdated threads → fall back to `originalLine`
- `--paginate` auto-follows `endCursor` because the query exposes
  `pageInfo { hasNextPage endCursor }` and accepts `$cursor`
- With `--paginate`, `--jq` runs PER PAGE. Add `--slurp` to merge pages into
  one array when counting/sorting across pages

## Reply

REST (preferred when `databaseId` is in hand — reply to the thread's ROOT comment):

```bash
gh api --method POST \
  "repos/$OWNER/$REPO/pulls/$PR/comments/$COMMENT_ID/replies" \
  -f body="$REPLY_TEXT" --jq '{reply_id: .id, in_reply_to: .in_reply_to_id}'
```

GraphQL (when only the thread ID is in hand):

```bash
gh api graphql \
  -f query='mutation($threadId:ID!,$body:String!){
    addPullRequestReviewThreadReply(input:{pullRequestReviewThreadId:$threadId, body:$body}){
      comment { id databaseId url } }}' \
  -F threadId="$THREAD_ID" -F body="$REPLY_TEXT"
```

## Resolve / unresolve

```bash
gh api graphql -f query='mutation($threadId:ID!){
  resolveReviewThread(input:{threadId:$threadId}){ thread { id isResolved } }}' \
  -F threadId="$THREAD_ID" --jq '.data.resolveReviewThread.thread'

gh api graphql -f query='mutation($threadId:ID!){
  unresolveReviewThread(input:{threadId:$threadId}){ thread { id isResolved } }}' \
  -F threadId="$THREAD_ID"
```

An invalid node ID returns a clean `NOT_FOUND` — safe to surface as an error.

## Review summaries + conversation comments

```bash
# Review summaries (incl. bot passes — Copilot/Codex post these)
gh api "repos/$OWNER/$REPO/pulls/$PR/reviews" \
  --jq '.[] | {reviewer: .user.login, isBot: (.user.type=="Bot"), state, body}'

# Top-level conversation: read + post (post = outward-facing, confirm first)
gh api "repos/$OWNER/$REPO/issues/$PR/comments" --jq '.[] | {user: .user.login, body}'
gh api --method POST "repos/$OWNER/$REPO/issues/$PR/comments" -f body="$SUMMARY"
```

## Bot detection

| Source | Field | Bot value |
|--------|-------|-----------|
| GraphQL | `author.__typename` | `"Bot"` |
| REST | `user.type` | `"Bot"` |
| REST | `user.login` suffix | `[bot]` — **NOT reliable**: `pulls/{n}/comments` reports `Copilot` (no suffix) while `pulls/{n}/reviews` reports `copilot-pull-request-reviewer[bot]` |

Use the type fields, not the login suffix.

## Edge cases

- **Outdated threads** (`isOutdated: true`): line moved or was deleted;
  `line` is often null → use `originalLine`. Default action: reply
  "addressed in {commit}, line has since moved" + resolve.
- **Multi-round reviews**: re-running is idempotent — the unresolved-only
  filter drops handled threads; GitHub's `isResolved` IS the state.
- **Summary-only feedback** (CHANGES_REQUESTED with no inline threads):
  can't be resolved; reply as conversation comment and/or change code —
  the review state flips on re-request.
- **No threads / all resolved**: report "No unresolved review threads on
  PR #N", but still surface CHANGES_REQUESTED summaries.

# PR Review Response Patterns

Templates and patterns for responding to PR review comments.

## Response Templates by Category

### Code Change Responses

**Accepting a suggestion:**

```
Good catch! Updated to use {suggestion}. {Brief explanation of why it's better if not obvious.}
```

**Accepting with modification:**

```
Agreed on the concern about {issue}. I went with {alternative} instead because {reason}. Let me know if you'd prefer your original suggestion.
```

**Declining with explanation:**

```
Considered this, but {reason for current approach}. Specifically, {concrete detail — e.g., "this pattern avoids N+1 queries when the association is loaded"}. Happy to discuss further.
```

**Iron Law conflict:**

```
This would violate Iron Law #{n}: {description}. The current code uses {pattern} to avoid {consequence}. See {reference} for the rationale.
```

### Question Responses

**Explaining a decision:**

```
{Direct answer}. I chose this approach because {reason}. {Optional: alternative considered and why rejected.}
```

**Explaining Elixir/Phoenix patterns:**

```
This uses {pattern name} — {1-sentence explanation}. In Phoenix, {brief context for why this is idiomatic}. See {HexDocs link} for more detail.
```

### Nitpick Responses

**Quick acknowledgment:**

```
Fixed, thanks!
```

**Style preference:**

```
Updated. I'll follow this convention going forward.
```

**Disagreement on style (rare):**

```
I see the preference for {their style}. This codebase uses {current style} consistently in {examples}. Happy to align either way — what do you think?
```

### Discussion Responses

**Architecture trade-off:**

```
Good point about {concern}. Current approach optimizes for {priority}. The trade-off is {downside}. We could mitigate with {option} if it becomes an issue. Want me to open an issue to track?
```

**Alternative suggestion:**

```
Interesting approach! Comparing:
- Current: {pros/cons}
- Suggested: {pros/cons}
I lean toward {choice} because {reason}, but open to your perspective.
```

## Common Elixir Review Patterns

### Pattern: "Use with instead of nested case"

Reviewers often suggest `with` chains. Check if it improves
readability — `with` is better for 3+ clauses, worse for 1-2.

### Pattern: "Missing typespec"

If the project uses typespecs consistently, add them. If not,
acknowledge and note it as a follow-up.

### Pattern: "Could use pattern matching"

Usually correct for Elixir. Replace `if map[:key]` with
function head pattern matching when possible.

### Pattern: "N+1 query concern"

Always take seriously. Check with `Repo.preload` or
`from(..., preload: [...])`. Reference Ecto Iron Law #5.

### Pattern: "Missing error handling"

Check if the caller expects `{:ok, _}/{:error, _}` or if
the function should raise. Match the context's convention.

### Pattern: "Test coverage"

If reviewer flags missing tests, check what's needed using
the testing-reviewer criteria (public functions, handle_events,
workers).

## Tone Guidelines

- **Be grateful**: Reviewers spend time improving your code
- **Be specific**: Reference exact lines, functions, patterns
- **Be brief**: Don't over-explain accepted changes
- **Be honest**: If you don't know, say so and research
- **Avoid defensive language**: "Actually..." or "Well..."
- **Use code blocks**: Show the fix, don't just describe it

## Handling Conflicts Between Reviewers

When multiple reviewers give conflicting feedback:

1. Identify the core concern each reviewer has
2. Find a solution that addresses both concerns
3. If irreconcilable, present both options and ask the PR author to decide
4. Never silently pick one reviewer's suggestion over another

## Batch Response Strategy

When a PR has many comments:

1. Group related comments (same file, same concern)
2. Address with a single response referencing all locations
3. Use "Addressed in {commit SHA}" for code changes
4. Leave "Will address in follow-up" for non-blocking items

## Anti-patterns to Avoid

- **Rubber-stamping**: Don't accept every suggestion blindly
- **Arguing style**: If it's not an Iron Law, defer to team convention
- **Ignoring context**: Read the full diff, not just the commented line
- **Over-promising**: Don't commit to changes you haven't verified
- **Emoji-only responses**: Always include at least a brief text acknowledgment