-
Notifications
You must be signed in to change notification settings - Fork 9
🚀 Feat: template conversational data augmentation + e2e tests + CodeRabbit #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
59420f5
wip: setup base pipeline with blocks
nicofretti a9166d6
add: skill for creating blocks
nicofretti 84ce41e
wip: fixing blocks
nicofretti 91aa398
fix: blocks data
nicofretti 4de867c
add: docs template
nicofretti 36f1827
wip: setup tests
nicofretti 86f46e3
add: coderabbit instructions
nicofretti 7dda212
add: coderabbit instructions
nicofretti b9b743a
add: coderabbit instructions
nicofretti 5aa20e7
add: coderabbit instructions
nicofretti e75bae9
fix: review
nicofretti bbfea64
wip: fixing skill + add type json-or-template
nicofretti cf66bbb
wip: fixing fields in blocks
nicofretti 2ecabbe
Merge branch '38-feat-template-conversational-data-augmentation' into…
nicofretti a767d28
fix: seed data + tests
nicofretti 2d599a8
wip: refactor blocks
nicofretti a9ce328
add: data augmentation pipeline fixes
nicofretti 15e3dbe
add: code review skill
nicofretti 6ab4051
add: coderabbit branches
nicofretti 84b82a7
add: coderabbit review
nicofretti 622aeb4
wip: fixing review
nicofretti c029212
fix: review E02
nicofretti 85334b4
fix: review E03
nicofretti ab717d8
fix: review E04
nicofretti db15bfc
Merge branch 'develop' into feat/e2e-tests
nicofretti 17fc547
fix: review E05
nicofretti 7152ea6
Merge remote-tracking branch 'refs/remotes/origin/feat/e2e-tests' int…
nicofretti af2e1d3
fix: review E06
nicofretti 96a2bc1
fix: review E07
nicofretti b1711df
fix: review E08
nicofretti 12c572c
fix: indent
nicofretti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| --- | ||
| name: address-pr-review | ||
| description: Use when you have PR review comments to address and want to evaluate each comment's validity before deciding to fix, reply, or skip | ||
| --- | ||
|
|
||
| # Address PR Review Comments | ||
|
|
||
| ## Overview | ||
|
|
||
| Interactive workflow: analyze PR review comment validity, recommend action, let user decide (fix/reply/skip). | ||
|
|
||
| ## When to Use | ||
|
|
||
| - PR has review comments needing evaluation before action | ||
| - Reviewer feedback might be incorrect or needs discussion | ||
| - Comments require varied responses (fix/reply/skip) | ||
| - Need to balance code quality with respectful reviewer engagement | ||
|
|
||
| ## When NOT to Use | ||
|
|
||
| - All comments are clearly valid and straightforward to fix | ||
| - No comments yet or doing pre-review self-review | ||
| - Comments only on non-code files without technical analysis needed | ||
|
|
||
| ## Workflow Overview | ||
|
|
||
| ```dot | ||
| digraph pr_review_flow { | ||
| "Fetch PR comments" [shape=box]; | ||
| "More comments?" [shape=diamond]; | ||
| "Show comment + file context" [shape=box]; | ||
| "Analyze validity" [shape=box]; | ||
| "Recommend action" [shape=box]; | ||
| "Ask user: Fix/Reply/Skip/Quit?" [shape=diamond]; | ||
| "Make code changes" [shape=box]; | ||
| "Draft reply" [shape=box]; | ||
| "Track as skipped" [shape=box]; | ||
| "Show summary" [shape=box]; | ||
|
|
||
| "Fetch PR comments" -> "More comments?"; | ||
| "More comments?" -> "Show comment + file context" [label="yes"]; | ||
| "More comments?" -> "Show summary" [label="no"]; | ||
| "Show comment + file context" -> "Analyze validity"; | ||
| "Analyze validity" -> "Recommend action"; | ||
| "Recommend action" -> "Ask user: Fix/Reply/Skip/Quit?"; | ||
| "Ask user: Fix/Reply/Skip/Quit?" -> "Make code changes" [label="Fix"]; | ||
| "Ask user: Fix/Reply/Skip/Quit?" -> "Draft reply" [label="Reply"]; | ||
| "Ask user: Fix/Reply/Skip/Quit?" -> "Track as skipped" [label="Skip"]; | ||
| "Ask user: Fix/Reply/Skip/Quit?" -> "Show summary" [label="Quit"]; | ||
| "Make code changes" -> "More comments?"; | ||
| "Draft reply" -> "More comments?"; | ||
| "Track as skipped" -> "More comments?"; | ||
| } | ||
| ``` | ||
|
|
||
| ## Fetching Comments | ||
|
|
||
| **CRITICAL**: Do NOT use `gh api --jq` directly - it truncates comment bodies. | ||
|
|
||
| Use the included script: | ||
|
|
||
| ```bash | ||
| # summary with counts and titles | ||
| python .claude/skills/address-pr-review/scripts/fetch_comments.py <PR> --summary | ||
|
|
||
| # show unresolved comments (default) | ||
| python .claude/skills/address-pr-review/scripts/fetch_comments.py <PR> | ||
|
|
||
| # single comment by ID | ||
| python .claude/skills/address-pr-review/scripts/fetch_comments.py <PR> --id <ID> | ||
|
|
||
| # all comments including resolved | ||
| python .claude/skills/address-pr-review/scripts/fetch_comments.py <PR> --all | ||
| ``` | ||
|
|
||
| ## Quick Reference | ||
|
|
||
| **Critical principle:** Reviewer may be wrong - analyze validity before recommending action. | ||
|
|
||
| | Phase | Actions | | ||
| |-------|---------| | ||
| | **Fetch** | Run `--summary` first to see counts<br>Then `--id <ID>` for each comment to analyze<br>Exit if no unresolved comments | | ||
| | **Per Comment** | Show: file:line, author, comment, ±10 lines context<br>Analyze: Valid/Nitpick/Disagree/Question<br>Recommend: Fix/Reply/Skip with reasoning | | ||
| | **Fix** | Minimal changes per llm/rules-*.md<br>Offer reply draft: `Fixed: [what]. [why]`<br>Show: `gh api --method POST repos/{owner}/{repo}/pulls/comments/$ID/replies -f body="..."` | | ||
| | **Reply** | Draft based on type: Question/Suggestion/Disagreement<br>Let user edit<br>Show gh command (never auto-post) | | ||
| | **Summary** | Processed X/N: Fixed Y, Replied Z, Skipped W<br>List: files modified, reply drafts, next steps | | ||
|
|
||
| ## Critical Principles | ||
|
|
||
| | Principle | Violation Pattern | | ||
| |-----------|-------------------| | ||
| | **Analyze first** | Accepting all feedback as valid without critical analysis | | ||
| | **Never auto-post** | Posting replies automatically instead of showing gh command | | ||
| | **One at a time** | Batch processing all comments without individual analysis | | ||
| | **Show context** | Making changes without displaying ±10 lines around code | | ||
| | **Minimal changes** | Large refactors in response to small comments | | ||
| | **Follow standards** | Ignoring llm/rules-*.md when fixing | | ||
| | **Respectful honesty** | Being defensive/dismissive when reviewer is wrong | | ||
| | **User control** | Posting drafts without letting user edit first | | ||
|
|
||
| ## Reply Formats | ||
|
|
||
| - Fix: `Fixed: [what]. [why]` | ||
| - Update: `Updated: [what]` | ||
| - Answer: `[explanation]` | ||
| - Acknowledge: `Good catch, [action/reason]` | ||
| - Disagree: `[respectful reasoning]` | ||
|
|
||
| ## Setup & Usage | ||
|
|
||
| Requires: `gh` CLI authenticated, GitHub remote configured | ||
|
|
||
| ```bash | ||
| # Start session | ||
| "use address-pr-review for PR <number>" | ||
|
|
||
| # Or list PRs first | ||
| "use address-pr-review" | ||
| ``` |
173 changes: 173 additions & 0 deletions
173
.claude/skills/address-pr-review/scripts/fetch_comments.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Fetch PR review comments with full body content. | ||
|
|
||
| Usage: | ||
| python fetch_comments.py <PR_NUMBER> # unresolved only | ||
| python fetch_comments.py <PR_NUMBER> --all # all comments | ||
| python fetch_comments.py <PR_NUMBER> --id <ID> # single comment | ||
| python fetch_comments.py <PR_NUMBER> --summary # counts only | ||
| """ | ||
|
|
||
| import json | ||
| import re | ||
| import subprocess | ||
| import sys | ||
| from typing import Any | ||
|
|
||
| RESOLVED_MARKERS = ["Addressed in commit", "Resolved in", "✅ Addressed"] | ||
| SEVERITY_PATTERN = re.compile(r"_([⚠️🛠️]+\s*[^_]+)_\s*\|\s*_([🟠🟡🔴]+\s*\w+)_") | ||
| TITLE_PATTERN = re.compile(r"\*\*([^*]+)\*\*") | ||
|
|
||
|
|
||
| def get_repo() -> str: | ||
| result = subprocess.run( | ||
| ["gh", "repo", "view", "--json", "owner,name", "-q", '.owner.login + "/" + .name'], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| if result.returncode != 0: | ||
| sys.exit(1) | ||
| return result.stdout.strip() | ||
|
|
||
|
|
||
| def fetch_comments(pr_number: str) -> list[dict[str, Any]]: | ||
| repo = get_repo() | ||
| result = subprocess.run( | ||
| ["gh", "api", f"repos/{repo}/pulls/{pr_number}/comments", "--paginate", "--slurp"], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| if result.returncode != 0: | ||
| print(f"failed to fetch comments: {result.stderr.strip()}", file=sys.stderr) | ||
| sys.exit(1) | ||
| # --slurp wraps paginated results in an outer array | ||
| pages = json.loads(result.stdout) | ||
| return [comment for page in pages for comment in page] | ||
|
|
||
|
|
||
| def is_resolved(comment: dict[str, Any]) -> bool: | ||
| body = comment.get("body", "") | ||
| return any(marker in body for marker in RESOLVED_MARKERS) | ||
|
|
||
|
|
||
| def parse_comment(comment: dict[str, Any]) -> dict[str, Any]: | ||
| """Extract essential info from comment body.""" | ||
| body = comment.get("body", "") | ||
|
|
||
| # extract severity | ||
| severity_match = SEVERITY_PATTERN.search(body) | ||
| severity = severity_match.group(2).strip() if severity_match else "" | ||
|
|
||
| # extract title (first bold text) | ||
| title_match = TITLE_PATTERN.search(body) | ||
| title = title_match.group(1).strip() if title_match else "" | ||
|
|
||
| # extract suggested fix (content between ```diff and ```) | ||
| diff_match = re.search(r"```diff\n(.*?)```", body, re.DOTALL) | ||
| suggested_fix = diff_match.group(1).strip() if diff_match else "" | ||
|
|
||
| # extract description (text after title, before <details>) | ||
| desc = "" | ||
| if title_match: | ||
| after_title = body[title_match.end() :] | ||
| details_pos = after_title.find("<details>") | ||
| if details_pos >= 0: | ||
| desc = after_title[:details_pos].strip() | ||
| else: | ||
| desc = after_title.strip() | ||
| else: | ||
| # no bold title - use full body as description | ||
| desc = body.strip() | ||
| if len(desc) > 500: | ||
| desc = desc[:500].rstrip() + "…" | ||
|
|
||
| # clean description of markdown artifacts | ||
| desc = re.sub(r"<!--.*?-->", "", desc, flags=re.DOTALL).strip() | ||
| desc = re.sub(r"\n{3,}", "\n\n", desc) | ||
|
|
||
| return { | ||
| "id": comment["id"], | ||
| "file": comment["path"], | ||
| "line": comment.get("line"), | ||
| "severity": severity, | ||
| "title": title, | ||
| "description": desc, | ||
| "suggested_fix": suggested_fix, | ||
| "resolved": is_resolved(comment), | ||
| } | ||
|
|
||
|
|
||
| def print_comment( | ||
| parsed: dict[str, Any], index: int | None = None, total: int | None = None | ||
| ) -> None: | ||
| prefix = f"[{index}/{total}] " if index and total else "" | ||
| loc = f"{parsed['file']}:{parsed['line']}" if parsed["line"] else parsed["file"] | ||
|
|
||
| print(f"\n{'=' * 60}") | ||
| print(f"{prefix}ID: {parsed['id']}") | ||
| print(f"Location: {loc}") | ||
| if parsed["severity"]: | ||
| print(f"Severity: {parsed['severity']}") | ||
| if parsed["title"]: | ||
| print(f"Issue: {parsed['title']}") | ||
| if parsed["description"]: | ||
| print(f"\n{parsed['description']}") | ||
| if parsed["suggested_fix"]: | ||
| print(f"\nFix:\n```diff\n{parsed['suggested_fix']}\n```") | ||
| print("=" * 60) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| if len(sys.argv) < 2: | ||
| print(__doc__) | ||
| sys.exit(1) | ||
|
|
||
| pr_number = sys.argv[1] | ||
| mode = sys.argv[2] if len(sys.argv) > 2 else "--unresolved" | ||
|
|
||
| if not pr_number.isdigit(): | ||
| print("PR number must be numeric") | ||
| sys.exit(1) | ||
| if mode == "--id" and len(sys.argv) <= 3: | ||
| print("missing id for --id") | ||
| sys.exit(1) | ||
|
|
||
| comments = fetch_comments(pr_number) | ||
| top_level = [c for c in comments if c.get("in_reply_to_id") is None] | ||
|
|
||
| if mode == "--id" and len(sys.argv) > 3: | ||
| target_id = int(sys.argv[3]) | ||
|
nicofretti marked this conversation as resolved.
|
||
| for c in top_level: | ||
| if c["id"] == target_id: | ||
| print_comment(parse_comment(c)) | ||
| sys.exit(0) | ||
| print(f"comment {target_id} not found") | ||
| sys.exit(1) | ||
|
|
||
| if mode == "--summary": | ||
| unresolved = [c for c in top_level if not is_resolved(c)] | ||
| resolved = len(top_level) - len(unresolved) | ||
| print(f"total: {len(top_level)}, resolved: {resolved}, unresolved: {len(unresolved)}") | ||
| if unresolved: | ||
| print("\nunresolved:") | ||
| for c in unresolved: | ||
| p = parse_comment(c) | ||
| loc = f"{p['file']}:{p['line']}" if p["line"] else p["file"] | ||
| sev = f" [{p['severity']}]" if p["severity"] else "" | ||
| title = f" - {p['title']}" if p["title"] else "" | ||
| print(f" {p['id']}: {loc}{sev}{title}") | ||
| sys.exit(0) | ||
|
|
||
| if mode == "--unresolved" or mode not in ["--all", "--id", "--summary"]: | ||
| top_level = [c for c in top_level if not is_resolved(c)] | ||
| print(f"showing {len(top_level)} unresolved comments") | ||
| else: | ||
| print(f"showing {len(top_level)} comments") | ||
|
|
||
| if not top_level: | ||
| print("no comments.") | ||
| sys.exit(0) | ||
|
|
||
| for i, c in enumerate(top_level, 1): | ||
| print_comment(parse_comment(c), i, len(top_level)) | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.