# Review Comments > Fetch PR review comments, address each one in code, and post resolution replies - Skill: `matthiasn/review-comments` (Agent Skill) - Install (CLI): `npx skillmds@latest add matthiasn/review-comments` - Raw SKILL.md: https://api.skillmd.com/api/skills/matthiasn/review-comments/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: matthiasn (https://skillmd.com/u/matthiasn) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/matthiasn/review-comments --- # Review PR Comments Fetch all review comments from a pull request, address each one (fix code, add docs, or explain the rationale), and reply to each comment on GitHub with the resolution. ## Steps 1. **Fetch comments** — use `gh api` to get all review comments: ```bash gh api repos/{owner}/{repo}/pulls/$ARGUMENTS/comments \ --jq '.[] | {id, path, line, body, in_reply_to_id}' ``` Filter to top-level comments only (`in_reply_to_id == null`) — those are the ones that need responses. 2. **Understand each comment** — read the referenced file and line to understand the concern. Group related comments if they touch the same issue. 3. **Address each comment** — make the appropriate code change (fix, refactor, add docs, add tests). If you disagree with a suggestion, prepare a clear rationale. 4. **Reply to each comment** — post a reply using: ```bash gh api repos/{owner}/{repo}/pulls/$ARGUMENTS/comments -X POST \ -f body="" \ -F in_reply_to= ``` Keep replies concise: state what was done (e.g., "Fixed. Replaced generic fallback with explicit throw.") or explain why it was left as-is. 5. **Close every codecov gap — target 100% patch coverage.** The Codecov bot posts a PR comment/check; treat its uncovered lines as review comments that MUST be resolved with tests. **Every line codecov marks uncovered has to be covered — no exceptions for "pre-existing" files.** If a file shows up in the PR diff with uncovered lines, cover them even if you didn't write them. a. **Find what's actually uncovered.** The bot summary only gives totals; pull the per-line data from the Codecov API and intersect it with the PR's added lines (this is exactly what `codecov/patch` scores): ```bash # Per-line diff coverage (head_cov == 0 → miss): curl -s "https://api.codecov.io/api/v2/github/{owner}/repos/{repo}/compare?pullid=$ARGUMENTS" \ > /tmp/cc.json # Then: for each file with has_diff, list lines where # coverage.head == 0 && is_diff, and intersect with the file's # git-diff added-line numbers (git diff --unified=0 origin/main...HEAD). ``` Codecov ignores are in `codecov.yml` (`*.g.dart`, `*.freezed.dart`, `l10n/*.dart` here) — skip those. b. **Codecov is often stale/partial — trust local coverage for new code.** Coverage is sharded; if any shard failed or the run is mid-flight, the bot shows a partial picture (e.g. "1% of diff") that inflates the miss list with trivial lines (`@override`, const ctors). Regenerate locally and intersect with the git-diff added lines for the authoritative set: ```bash fvm flutter test --coverage # parse coverage/lcov.info: DA:,0 == uncovered ``` Run enough test dirs that every diff file is genuinely exercised (a file covered only by tests you didn't run shows as a false miss). c. **Write real tests for each uncovered line** — mirror the sibling case already tested (e.g. a new `.map`/`switch` arm → copy the `aiConfig` case for `savedTaskFilter`; a debounce-cancel line → emit *two* notifications so the timer is non-null when cancelled). Extend parameterized `variantCases`/`variantsByBucket` tables rather than duplicating whole test bodies. d. **Uncoverable private-ctor lines** (`const FooKeys._();` in a static-only keys class) can't be hit and dart coverage ignores no inline comment — convert the class to `abstract final class FooKeys { ... }` so the constructor line disappears entirely. e. **Re-run coverage until the (added ∩ uncovered) set is empty**, then run the affected suites to confirm still-green. 6. **Verify** — run analyzer and affected tests to confirm all changes compile and pass. 7. **Babysit the PR until it is actually done.** Opening a PR and replying once is not the end of the job. A PR is finished only when all three hold at the same time: - **mergeable** — `gh pr view --json mergeable` is `MERGEABLE` - **all green** — every check passed, not merely "not failing": zero `pending` and zero failures, including `codecov/patch` - **all replied** — every top-level review comment has a reply, with a real fix or a stated reason for declining **Pushing requires authorization.** Commits, rebases and pushes need explicit user or orchestrator approval (AGENTS.md, "Security & Configuration"). Being asked to *address review comments* authorizes the code changes, not the push — confirm before the first push of a session, and never force-push a branch you did not create in this session. `--force-with-lease` guards against clobbering a concurrent update; it is not a substitute for approval. Reviews arrive *after* pushes, so pushing fixes restarts the loop: the reviewer re-reviews the new commit and may file new findings. Bots also rate-limit and arrive late (CodeRabbit will say "next review available in N minutes" and skip the run entirely). Keep watching until the three conditions hold together. Poll on the structured status rather than the display columns — the table format is human-facing, and a failed API or auth call prints to stderr and would otherwise read as "no pending checks". Note `gh pr checks` has **no** `--json` flag (checked on gh 2.45); the structured source is `gh pr view --json statusCheckRollup`. Verify whatever command you poll with actually works *before* wrapping it in an `until` loop: a command that errors makes the loop exit immediately and every subsequent report a lie. For a CheckRun, `status` is the lifecycle (`COMPLETED`) and `conclusion` carries the verdict (`SUCCESS` / `FAILURE` / `CANCELLED`) — a failed check is `COMPLETED`, so keying on `status` alone reports a red run as done and green. Read the conclusion for completed checks and the state for StatusContexts: ```bash # one "\t" line per check; empty output means the query # failed, not that everything passed — so treat it as not-done. rollup() { gh pr view "$1" --json statusCheckRollup --jq ' .statusCheckRollup[] | if .status == "COMPLETED" then .conclusion elif .status then .status else .state end + "\t" + (.name // .context)' } # Capture once per iteration and check the exit status: an errored or # empty result is "not done", never "done and green". while :; do out=$(rollup ) || { echo "poll failed"; sleep 30; continue; } [ -n "$out" ] || { echo "empty rollup — treating as not done"; sleep 30; continue; } printf '%s\n' "$out" | grep -qE '^(IN_PROGRESS|QUEUED|PENDING)' || break sleep 30 done bad=$(printf '%s\n' "$out" | grep -vE '^(SUCCESS|NEUTRAL|SKIPPED)') [ -z "$bad" ] && echo "all green (${#out} bytes of verdicts)" || printf '%s\n' "$bad" ``` Three ways these snippets lie if written casually, all worth guarding: an errored command inside `$( )` yields empty output that a `grep -q pending` reads as "nothing pending"; `grep … || echo "all green"` turns *no output at all* into a pass; and a background poller whose result is never collected lets the summary be written before it finishes. Capture the output, check the status, and `wait` for the poller before reporting. Cross-check the total against `gh pr checks ` before declaring green: the honest failure mode here was a poller that exited on its first iteration and reported "settled" while 15 checks were still queued. Run that in the background (`run_in_background: true`, or `… &` with the PID kept) so replying to comments proceeds concurrently rather than blocking on CI. Then re-check for comments filed against the new commits — including top-level ones with no reply yet: `gh api` returns **one page (30 comments)** unless `--paginate` is passed, and `--jq` then runs per page — so a comment and its reply landing on different pages makes an answered comment look unanswered, and a comment on a later page look absent. Fetch every page and aggregate once with `jq -s`: ```bash # `set -o pipefail` so an API failure fails the pipeline instead of # producing an empty list that reads as "nothing unanswered". ( set -o pipefail gh api --paginate repos/{owner}/{repo}/pulls//comments --jq '.[]' | jq -s -r ' [.[] | select(.in_reply_to_id != null) | .in_reply_to_id] as $replied | [.[] | select(.in_reply_to_id == null)] as $top | "top-level: \($top | length), answered: \([$top[] | select(.id as $i | $replied | index($i))] | length)", ($top[] | select(.id as $i | ($replied | index($i)) | not) | "UNANSWERED \(.id) \(.user.login) \(.path)")' ) || echo "comment query FAILED — do not report all-replied" ``` Print the counted totals, not just the unanswered lines: "top-level: 26, answered: 22" is checkable, whereas empty output is indistinguishable from a query that never ran. Post replies with `-F body=@file` rather than an inline shell string. Review bodies contain backticks, quotes and code fences; nested shell quoting silently mangles them, and a failed POST inside a loop can still look like it succeeded. Report the real state — "27 pass, 1 pending" is the honest answer while a check is still running, not "all green". ## Guidelines - Address ALL comments — do not skip any. - Make real code fixes, not just reply text. - Run the analyzer and formatter after all fixes. - Run affected tests to verify fixes. - Keep reply text concise and factual. - If a comment is from a bot review (e.g., CodeRabbit, Gemini), still address valid points but use your judgement on noise. - Coverage is not optional: every codecov-flagged line in the diff must end up covered (goal 100% patch), pre-existing or not. Prefer real behavioural tests; only restructure code (e.g. `abstract final class`) for genuinely uncoverable lines.