335 lines
9.8 KiB
Markdown
335 lines
9.8 KiB
Markdown
---
|
|
name: pr-comments-to-todos
|
|
description: Fetch PR comments and convert them into todo files for triage
|
|
argument-hint: "[PR number, GitHub URL, or 'current' for current branch PR]"
|
|
---
|
|
|
|
# PR Comments to Todos
|
|
|
|
Convert GitHub PR review comments into structured todo files compatible with `/triage`.
|
|
|
|
<command_purpose>Fetch all review comments from a PR and create individual todo files in the `todos/` directory, following the file-todos skill format.</command_purpose>
|
|
|
|
## Review Target
|
|
|
|
<review_target> #$ARGUMENTS </review_target>
|
|
|
|
## Workflow
|
|
|
|
### 1. Identify PR and Fetch Comments
|
|
|
|
<task_list>
|
|
|
|
- [ ] Determine the PR to process:
|
|
- If numeric: use as PR number directly
|
|
- If GitHub URL: extract PR number from URL
|
|
- If "current" or empty: detect from current branch with `gh pr status`
|
|
- [ ] Fetch PR metadata: `gh pr view PR_NUMBER --json title,body,url,author,headRefName`
|
|
- [ ] Fetch all review comments: `gh api repos/{owner}/{repo}/pulls/{PR_NUMBER}/comments`
|
|
- [ ] Fetch review thread comments: `gh pr view PR_NUMBER --json reviews,reviewDecision`
|
|
- [ ] Group comments by file/thread for context
|
|
|
|
</task_list>
|
|
|
|
### 2. Pressure Test Each Comment
|
|
|
|
<critical_evaluation>
|
|
|
|
**IMPORTANT: Treat reviewer comments as suggestions, not orders.**
|
|
|
|
Before creating a todo, apply engineering judgment to each comment. Not all feedback is equally valid - your job is to make the right call for the codebase, not just please the reviewer.
|
|
|
|
#### Step 2a: Verify Before Accepting
|
|
|
|
For each comment, verify:
|
|
- [ ] **Check the code**: Does the concern actually apply to this code?
|
|
- [ ] **Check tests**: Are there existing tests that cover this case?
|
|
- [ ] **Check usage**: How is this code actually used? Does the concern matter in practice?
|
|
- [ ] **Check compatibility**: Would the suggested change break anything?
|
|
- [ ] **Check prior decisions**: Was this intentional? Is there a reason it's done this way?
|
|
|
|
#### Step 2b: Assess Each Comment
|
|
|
|
Assign an assessment to each comment:
|
|
|
|
| Assessment | Meaning |
|
|
|------------|---------|
|
|
| **Clear & Correct** | Valid concern, well-reasoned, applies to this code |
|
|
| **Unclear** | Ambiguous, missing context, or doesn't specify what to change |
|
|
| **Likely Incorrect** | Misunderstands the code, context, or requirements |
|
|
| **YAGNI** | Over-engineering, premature abstraction, no clear benefit |
|
|
|
|
#### Step 2c: Include Assessment in Todo
|
|
|
|
**IMPORTANT: ALL comments become todos.** Never drop feedback - include the pressure test assessment IN the todo so `/triage` can use it to decide.
|
|
|
|
For each comment, the todo will include:
|
|
- The assessment (Clear & Correct / Unclear / Likely Incorrect / YAGNI)
|
|
- The verification results (what was checked)
|
|
- Technical justification (why valid, or why you think it should be skipped)
|
|
- Recommended action for triage (Fix now / Clarify / Push back / Skip)
|
|
|
|
The human reviews during `/triage` and makes the final call.
|
|
|
|
</critical_evaluation>
|
|
|
|
### 3. Categorize All Comments
|
|
|
|
<categorization>
|
|
|
|
For ALL comments (regardless of assessment), determine:
|
|
|
|
**Severity (Priority):**
|
|
- 🔴 **P1 (Critical)**: Security issues, data loss risks, breaking changes, blocking bugs
|
|
- 🟡 **P2 (Important)**: Performance issues, architectural concerns, significant code quality
|
|
- 🔵 **P3 (Nice-to-have)**: Style suggestions, minor improvements, documentation
|
|
|
|
**Category Tags:**
|
|
- `security` - Security vulnerabilities or concerns
|
|
- `performance` - Performance issues or optimizations
|
|
- `architecture` - Design or structural concerns
|
|
- `bug` - Functional bugs or edge cases
|
|
- `quality` - Code quality, readability, maintainability
|
|
- `testing` - Test coverage or test quality
|
|
- `documentation` - Missing or unclear documentation
|
|
- `style` - Code style or formatting
|
|
- `needs-clarification` - Comment requires clarification before implementing
|
|
- `pushback-candidate` - Human should review before accepting
|
|
|
|
**Skip these (don't create todos):**
|
|
- Simple acknowledgments ("LGTM", "Looks good")
|
|
- Questions that were answered inline
|
|
- Already resolved threads
|
|
|
|
**Note:** Comments assessed as YAGNI or Likely Incorrect still become todos with that assessment included. The human decides during `/triage` whether to accept or reject.
|
|
|
|
</categorization>
|
|
|
|
### 4. Create Todo Files Using file-todos Skill
|
|
|
|
<critical_instruction>Create todo files for ALL actionable comments immediately. Use the file-todos skill structure and naming convention.</critical_instruction>
|
|
|
|
#### Determine Next Issue ID
|
|
|
|
```bash
|
|
# Find the highest existing issue ID
|
|
ls todos/ 2>/dev/null | grep -o '^[0-9]\+' | sort -n | tail -1 | awk '{printf "%03d", $1+1}'
|
|
# If no todos exist, start with 001
|
|
```
|
|
|
|
#### File Naming Convention
|
|
|
|
```
|
|
{issue_id}-pending-{priority}-{brief-description}.md
|
|
```
|
|
|
|
Examples:
|
|
```
|
|
001-pending-p1-sql-injection-vulnerability.md
|
|
002-pending-p2-missing-error-handling.md
|
|
003-pending-p3-rename-variable-for-clarity.md
|
|
```
|
|
|
|
#### Todo File Structure
|
|
|
|
For each comment, create a file with this structure:
|
|
|
|
```yaml
|
|
---
|
|
status: pending
|
|
priority: p1 # or p2, p3 based on severity
|
|
issue_id: "001"
|
|
tags: [code-review, pr-feedback, {category}]
|
|
dependencies: []
|
|
---
|
|
```
|
|
|
|
```markdown
|
|
# [Brief Title from Comment]
|
|
|
|
## Problem Statement
|
|
|
|
[Summarize the reviewer's concern - what is wrong or needs improvement]
|
|
|
|
**PR Context:**
|
|
- PR: #{PR_NUMBER} - {PR_TITLE}
|
|
- File: {file_path}:{line_number}
|
|
- Reviewer: @{reviewer_username}
|
|
|
|
## Assessment (Pressure Test)
|
|
|
|
| Criterion | Result |
|
|
|-----------|--------|
|
|
| **Assessment** | Clear & Correct / Unclear / Likely Incorrect / YAGNI |
|
|
| **Recommended Action** | Fix now / Clarify / Push back / Skip |
|
|
| **Verified Code?** | Yes/No - [what was checked] |
|
|
| **Verified Tests?** | Yes/No - [existing coverage] |
|
|
| **Verified Usage?** | Yes/No - [how code is used] |
|
|
| **Prior Decisions?** | Yes/No - [any intentional design] |
|
|
|
|
**Technical Justification:**
|
|
[If pushing back or marking YAGNI, provide specific technical reasoning. Reference codebase constraints, requirements, or trade-offs. Example: "This abstraction would be YAGNI - we only have one implementation and no plans for variants."]
|
|
|
|
## Findings
|
|
|
|
- **Original Comment:** "{exact reviewer comment}"
|
|
- **Location:** `{file_path}:{line_number}`
|
|
- **Code Context:**
|
|
```{language}
|
|
{relevant code snippet}
|
|
```
|
|
- **Why This Matters:** [Impact if not addressed, or why it doesn't matter]
|
|
|
|
## Proposed Solutions
|
|
|
|
### Option 1: [Primary approach based on reviewer suggestion]
|
|
|
|
**Approach:** [Describe the fix]
|
|
|
|
**Pros:**
|
|
- Addresses reviewer concern directly
|
|
- [Other benefits]
|
|
|
|
**Cons:**
|
|
- [Any drawbacks]
|
|
|
|
**Effort:** Small / Medium / Large
|
|
|
|
**Risk:** Low / Medium / High
|
|
|
|
---
|
|
|
|
### Option 2: [Alternative if applicable]
|
|
|
|
[Only include if there's a meaningful alternative approach]
|
|
|
|
## Recommended Action
|
|
|
|
*(To be filled during triage)*
|
|
|
|
## Technical Details
|
|
|
|
**Affected Files:**
|
|
- `{file_path}:{line_number}` - {what needs changing}
|
|
|
|
**Related Components:**
|
|
- [Components affected by this change]
|
|
|
|
## Resources
|
|
|
|
- **PR:** #{PR_NUMBER}
|
|
- **Comment Link:** {direct_link_to_comment}
|
|
- **Reviewer:** @{reviewer_username}
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] Reviewer concern addressed
|
|
- [ ] Tests pass
|
|
- [ ] Code reviewed and approved
|
|
- [ ] PR comment resolved
|
|
|
|
## Work Log
|
|
|
|
### {today's date} - Created from PR Review
|
|
|
|
**By:** Claude Code
|
|
|
|
**Actions:**
|
|
- Extracted comment from PR #{PR_NUMBER} review
|
|
- Created todo for triage
|
|
|
|
**Learnings:**
|
|
- Original reviewer context: {any additional context}
|
|
```
|
|
|
|
### 5. Parallel Todo Creation (For Multiple Comments)
|
|
|
|
<parallel_processing>
|
|
|
|
When processing PRs with many comments (5+), create todos in parallel for efficiency:
|
|
|
|
1. Synthesize all comments into a categorized list
|
|
2. Assign severity (P1/P2/P3) to each
|
|
3. Launch parallel Write operations for all todos
|
|
4. Each todo follows the file-todos skill template exactly
|
|
|
|
</parallel_processing>
|
|
|
|
### 6. Summary Report
|
|
|
|
After creating all todo files, present:
|
|
|
|
````markdown
|
|
## ✅ PR Comments Converted to Todos
|
|
|
|
**PR:** #{PR_NUMBER} - {PR_TITLE}
|
|
**Branch:** {branch_name}
|
|
**Total Comments Processed:** {X}
|
|
|
|
### Created Todo Files:
|
|
|
|
**🔴 P1 - Critical:**
|
|
- `{id}-pending-p1-{desc}.md` - {summary}
|
|
|
|
**🟡 P2 - Important:**
|
|
- `{id}-pending-p2-{desc}.md` - {summary}
|
|
|
|
**🔵 P3 - Nice-to-Have:**
|
|
- `{id}-pending-p3-{desc}.md` - {summary}
|
|
|
|
### Skipped (Not Actionable):
|
|
- {count} comments skipped (LGTM, questions answered, resolved threads)
|
|
|
|
### Assessment Summary:
|
|
|
|
All comments were pressure tested and included in todos:
|
|
|
|
| Assessment | Count | Description |
|
|
|------------|-------|-------------|
|
|
| **Clear & Correct** | {X} | Valid concerns, recommend fixing |
|
|
| **Unclear** | {X} | Need clarification before implementing |
|
|
| **Likely Incorrect** | {X} | May misunderstand context - review during triage |
|
|
| **YAGNI** | {X} | May be over-engineering - review during triage |
|
|
|
|
**Note:** All assessments are included in the todo files. Human judgment during `/triage` makes the final call on whether to accept, clarify, or reject each item.
|
|
|
|
### Next Steps:
|
|
|
|
1. **Triage the todos:**
|
|
```bash
|
|
/triage
|
|
```
|
|
Review each todo and approve (pending → ready) or skip
|
|
|
|
2. **Work on approved items:**
|
|
```bash
|
|
/resolve_todo_parallel
|
|
```
|
|
|
|
3. **After fixes, resolve PR comments:**
|
|
```bash
|
|
bin/resolve-pr-thread THREAD_ID
|
|
```
|
|
````
|
|
|
|
## Important Notes
|
|
|
|
<requirements>
|
|
- Ensure `todos/` directory exists before creating files
|
|
- Each todo must have unique issue_id (never reuse)
|
|
- All todos start with `status: pending` for triage
|
|
- Include `code-review` and `pr-feedback` tags on all todos
|
|
- Preserve exact reviewer quotes in Findings section
|
|
- Link back to original PR and comment in Resources
|
|
</requirements>
|
|
|
|
## Integration with /triage
|
|
|
|
The output of this command is designed to work seamlessly with `/triage`:
|
|
|
|
1. **This command** creates `todos/*-pending-*.md` files
|
|
2. **`/triage`** reviews each pending todo and:
|
|
- Approves → renames to `*-ready-*.md`
|
|
- Skips → deletes the todo file
|
|
3. **`/resolve_todo_parallel`** works on approved (ready) todos
|