Add read-feedback-revise skill for addressing PR review comments

[igerber]

Summary

• Add new /read-feedback-revise skill that takes a GitHub PR comment URL, fetches the review feedback, analyzes it into prioritized action items, and enters plan mode to produce an implementation plan
• Supports all three GitHub comment types: issue comments (#issuecomment-), inline review comments (#discussion_r), and full PR reviews (#pullrequestreview-)
• 8-step read-only workflow: parse URL, fetch comment, fetch PR context, check branch state, read source files, analyze feedback, clarify ambiguities, then enter plan mode

Methodology references (required if estimator / math changes)

• N/A - no methodology changes. This is a new Claude Code skill (markdown instructions only).

Validation

• Tests added/updated: N/A - skill files are markdown instructions, not executable code
• Manual verification: Test with each of the three supported URL formats, invalid URLs, empty arguments, and ambiguous feedback scenarios

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

Generated with Claude Code

[github-actions]

Overall assessment: ⚠️ Needs changes

Executive Summary

• No estimator or methodology changes; Methodology Registry unaffected.
• gh api calls for review comments and changed files are not paginated, so large reviews/PRs can silently omit feedback and files. (P1)
• URL parsing step can drop the # fragment when query parameters exist, causing valid GitHub links to fail. (P2)
• Minor wording issue: headRefName is described as the PR “target” branch. (P3)

Methodology

• No findings.

Code Quality

• P1 — Impact: Large PRs or reviews (>30 files/comments) can be truncated, leading to missed action items and incomplete plans. — Fix: add --paginate (or --per-page 100 --paginate) to the review-comments and files endpoints while keeping the --jq '.[] | ...' output format. (.claude/commands/read-feedback-revise.md:79, .claude/commands/read-feedback-revise.md:93)
• P2 — Impact: Stripping query parameters by cutting at ? can remove the # fragment; valid GitHub comment URLs with query strings (e.g., notification links) will fail to parse. — Fix: parse with a URL parser and preserve the fragment while dropping only the query portion (or strip only the query between ? and #). (.claude/commands/read-feedback-revise.md:36)

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings.

Security

• No findings.

Documentation/Tests

• P3 — Impact: The prompt says the PR “targets” headRefName, but headRefName is the source/head branch; this can mislead users about which branch to check out. — Fix: reword to “PR head branch is …” and optionally mention baseRefName as the target branch. (.claude/commands/read-feedback-revise.md:102)

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 758f0e744cbd1c54fb8819b31ad19aa89026d5fe

---

Overall assessment: ⚠️ Needs changes

Executive summary

• No estimator or methodology changes; Methodology Registry is unaffected.
• Read-only constraint conflicts with the branch checkout instruction; clarify or move the checkout step to preserve the “no workspace changes” promise.
• URL parsing is brittle when fragments include query strings; add fragment normalization to avoid unnecessary AskUserQuestion loops.

Methodology

No findings.

Code Quality

• Severity: P3
  Impact: URLs with a fragment query (e.g., #discussion_r123?foo=bar) won’t match the fragment patterns because the instructions explicitly skip stripping ? after #, leading to avoidable “invalid URL” prompts.
  Concrete fix: Strip ? and subsequent parameters from the fragment before matching, or explicitly allow/ignore trailing ?… on fragments.
  Location: .claude/commands/read-feedback-revise.md:L36-L57

Performance

No findings.

Maintainability

• Severity: P2
  Impact: The “Steps 1–7 are read-only” constraint is contradicted by Step 4’s guidance to git checkout the PR branch, which can change the working tree and violates the stated contract. This creates ambiguity for users about what the skill is allowed to do.
  Concrete fix: Either (a) move branch-switching guidance to Step 8 (after plan approval), or (b) update the constraint language to explicitly allow branch switching as a permissible pre-plan action.
  Location: .claude/commands/read-feedback-revise.md:L18-L111

Tech Debt

No findings.

Security

No findings.

Documentation/Tests

No findings.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 743da8a43553d662db5a4ed2aac4c12ef3e84b3a

---

PR Review

Overall assessment: ✅ Looks good

Executive summary

• No estimator/identification/variance changes; methodology registry not implicated by this PR.
• Adds a new Claude command with a clear read-only workflow and explicit branch-check step.
• Minor inconsistency on invalid URL handling; align Step 1 with the error table for deterministic behavior.
• Edge case: outdated inline review comments can have line=null; add a fallback to avoid ambiguous locations.

Methodology

• No findings. This PR only adds a Claude command file; no estimator or math changes to cross‑check against docs/methodology/REGISTRY.md.

Code Quality

• P3 — Impact: Invalid URL handling is inconsistent (Step 1 says AskUserQuestion, error table says return an error), which can lead to divergent behavior depending on which section is followed. Concrete fix: pick one behavior and make both sections match (recommend AskUserQuestion to keep the flow). Location: .claude/commands/read-feedback-revise.md:L24-L58, .claude/commands/read-feedback-revise.md:L179-L185.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings.

Security

• No findings.

Documentation/Tests

• P3 — Impact: Inline review comments can have line: null for outdated comments; current jq output and the Step 6 format assume <path>:<line>, which can yield “null” or missing locations. Concrete fix: include a fallback field (e.g., .original_line or .position) in the jq output and update the output template to allow <path> with “line unavailable” when line is null. Location: .claude/commands/read-feedback-revise.md:L68-L80, .claude/commands/read-feedback-revise.md:L133-L155.