phyai-solve-pr-comments

name	phyai-solve-pr-comments
description	Triage and resolve review comments on a GitHub PR — fetch all comment surfaces (issue / inline / review), validate each suggestion against upstream source rather than trusting blindly, present findings to the user before editing, then make focused changes and re-run tests. Use when the user asks to "check PR N's comments", "address review feedback", "resolve PR comments", or similar.

Solve PR Comments

The point of this skill is to resolve PR review feedback correctly, not just compliantly. Bot suggestions (gemini-code-assist, copilot, codex) are often partially right or confidently wrong — they pattern-match local code without checking the upstream library contract. Your job is to verify each claim against ground truth, then surface the verdict to the user before touching code.

When to use

User says one of:

• "check PR N's comments"
• "address review feedback on PR N"
• "resolve / fix PR N comments"
• "look at the comments on PR N"
• "there are comments on PR N, fix them"

Skip this skill for:

• Asking what a comment says (just gh pr view)
• Re-running existing review (use /review instead)

Workflow

1. Fetch every comment surface — don't trust gh pr view --comments

gh pr view N --comments quietly drops content when GitHub's GraphQL flags Projects-classic deprecation. Hit the REST endpoints directly:

# Conversation comments (issue thread)
gh api repos/OWNER/REPO/issues/N/comments \
  --jq '.[] | {id, user: .user.login, body, created_at}'

# Inline review comments (the line-anchored ones)
gh api repos/OWNER/REPO/pulls/N/comments \
  --jq '.[] | {id, user: .user.login, path, line, side, body, created_at}'

# Overall reviews (the summary block on top of each review)
gh api repos/OWNER/REPO/pulls/N/reviews \
  --jq '.[] | {id, user: .user.login, state, body, submitted_at}'

If gh auth status says "token invalid" — stop and ask the user to re-auth. Don't try to work around it.

2. Triage before doing anything

For each comment, classify:

Class	Examples	Action
Real bug / perf issue	"race condition in shared dict", "per-forward fp32 cast"	Plan a fix, verify approach with user
Wrong claim	Bot extrapolated from a pattern that doesn't apply to this code	Reject — explain to user why it's wrong (cite upstream source)
Doc-only	Subtle contract that's not visible from code	Add docstring/comment, no code change
Style / nit	Naming, formatting	Skip unless user asks; not the point of review
Already fixed	Comment is on stale code	Mark as outdated, move on

3. Validate every claim against ground truth

This is the highest-leverage step. Before believing a bot:

• Library contract claims ("X needs fp32") -> read the upstream source. CUDA kernels usually live in .../site-packages/<pkg>/data/csrc/*.cu. Look for TORCH_CHECK / TVM_FFI_ICHECK / dispatch macros.

• Reference implementations -> grep .tmp/<reference-project>/ (e.g. sglang, vllm, lerobot) for how mature codebases handle the same kernel. SGLang's python/sglang/srt/layers/ is a particularly good reference for kernel wrappers. If the repo you need isn't already under .tmp/, clone it — git clone --depth 1 <upstream-url> .tmp/<name> is enough for a read-only consult; full history is rarely needed. Don't try to reason from training-data memory of the source.

  STRICT — DO NOT COPY CODE FROM REFERENCE REPOS. They exist only for correctness verification: confirming a kernel's dtype contract, double-checking a math formula, comparing dispatch logic. The fix you write must be authored from scratch in this project's style and abstractions. Pasting an SGLang/vLLM/lerobot block — even one that "looks like it fits" — is a license violation, breaks our layer hierarchy, and drags in dependencies we don't want. Read their code, understand the constraint, then write our own.

• Project tests -> read existing tests under tests/ before editing. They encode the contract the user already expects.

A bot's claim is worth nothing until you've checked it against the upstream contract. The bot may have pattern-matched a similar-looking issue from a different library; the right fix here may be the opposite of what they suggested, or there may be no fix at all (and the contract just needs to be documented).

4. Present findings before editing

Reply to the user with a tight table:

| # | File:Line | Issue | Verdict | Plan |
|---|---|---|---|---|
| 1 | foo.py:100 | claim X | Real | Pre-allocate fp32 in __init__ |
| 2 | bar.py:50  | claim Y | Wrong | Bot misread; weight dtype must match input. Add docstring note instead. |
| 3 | baz.py:200 | claim Z | Stale | Already fixed in commit abc123 |

Wait for the user to confirm scope. Do not silently apply every bot suggestion.

5. Make the changes — minimal and focused

• One concept per edit. Don't bundle "address comment + refactor + rename".
• Update docstrings for any contract you discovered (e.g., "flashinfer requires weight dtype == input dtype"). Future readers shouldn't have to re-derive what you just learned.
• If a bot suggestion has a code block (a suggestion: block in the comment body), still read it but adapt to local conventions; don't paste verbatim if it conflicts with project style.

6. Verify before reporting done

• Run the relevant test file (pytest tests/.../test_X.py) — not the whole suite, just the affected module.
• For perf changes (no parity test): write a tiny smoke test that exercises both old/new paths and asserts numerical match against a torch.nn.functional reference.
• For dtype changes: think about the load path too. Tensor.copy_ silently casts — does that propagate precision loss anywhere? Add a placement-load warning if surprising.

7. Reply on the PR (only if user asks)

If the user wants to push changes back as PR review replies:

gh api repos/OWNER/REPO/pulls/comments/COMMENT_ID/replies \
  -f body="Addressed in commit SHA — explained reasoning in the diff."

Don't auto-reply. The user may want to sanity-check before publishing.

Anti-patterns

• Pasting a bot's suggestion: block verbatim without checking whether the project already has a different pattern for that situation.
• "All five comments are valid, fixing all" — at least one is usually wrong or stale. If you don't find a wrong one, you didn't read carefully.
• Treating "medium priority" as "must-fix" — the bot's priority labels are heuristic. A medium-priority race condition is more important than a high-priority style nit.
• Editing without reading the test file first — tests document the intended behavior; an edit that breaks the test is usually wrong even if it matches the bot's suggestion.
• Silent dtype/shape changes — if your fix changes the dtype or shape of a parameter, the load path (apply_placements / state_dict loader) needs to handle the cast and ideally warn on mismatch.

Quick reference: gh CLI cookbook

# List PRs needing attention
gh pr list -R OWNER/REPO --search "review:required"

# Full PR snapshot (metadata + body, no comments)
gh pr view N -R OWNER/REPO --json number,title,state,body,author,headRefName

# Diff for the PR
gh pr diff N -R OWNER/REPO

# Get the commit on the PR head
gh pr view N -R OWNER/REPO --json headRefOid -q .headRefOid

# Resolve / mark conversation as resolved (REST not GraphQL — needs separate call)
# (gh CLI doesn't support this directly; use the GraphQL endpoint or the web UI)