feat(review-changes): enhance with multi-category parallel review

[nsheaps]

Summary

• Enhances the review-changes skill with improvements extracted from the structured PR review methodology used for github-actions#1
• Adds 8 explicit review categories (simplicity, flexibility, usability, documentation, security, pattern matching, best practices, QA)
• Adds --thorough flag for parallel per-category sub-agent execution
• Adds structured persistent output, GitHub PR integration with shields.io badges, emoji-coded score thresholds, and adaptive output routing

Changes

New Features

• 8 review categories with explicit criteria for each dimension
• --thorough mode launches parallel sub-agents per category (opt-in)
• Structured output to .claude/pr-reviews/$org/$repo/$prNumber/$epoch/
• GitHub PR review integration with inline comments, <summary> collapsible sections, shields.io badges
• Score thresholds: 🚨 <70%, ⚠️ <85%, ✅ ≥85%
• Cap rule: Max 94% overall if any category warns
• Brevity rule: Minimal output for >95% scores
• Inline comment prefixes: 🔕 non-blocking, ℹ️ info-only
• Reference requirements: All findings must cite evidence
• Adaptive output: Detects agentic vs interactive CLI mode

Preserved Behavior

• Default (no flags) remains a single-pass quick review
• All existing arguments still work
• Focus area narrowing still applies

Test plan

• Run /review-changes on a simple PR — verify quick mode still works
• Run /review-changes --thorough on a PR — verify parallel sub-agents launch and produce per-category reports
• Verify structured output path creation
• Verify GitHub PR review posting (if in agentic mode)
• Verify shields.io badge URLs render correctly

Context

Task #88 from looney-toons-20260223 session. Source methodology documented in .claude/tmp/review-prompt-for-wile-e.md in agent-team. Example output from github-actions#1 at .claude/pr-reviews/nsheaps/github-actions/1/1771874092/.

🤖 Generated with Claude Code

[github-actions]

Plugin Version Bumps (Preview)

These versions will be bumped when merged to main.

Plugin	Current	→	After Merge
datadog-otel-setup	0.1.0	→	0.1.1
review-changes	0.2.24	→	0.2.25

[nsheaps]

Review: ai-mktpl#190 — review-changes skill enhancement — Score: 88/100

Category	Score	Notes
Simplicity	85	--thorough adds proportionate complexity; quick mode remains simple
Flexibility	90	Output paths adapt to repo/PR context; shields.io badges configurable
Usability	88	Clear examples; dangling $ARGUMENTS in Focus Area section needs fixing
Documentation	92	8 categories well-defined; scoring thresholds, severity levels, finding IDs all documented
Security	90	No concerns; gh pr edit commands use safe flag syntax
Pattern Matching	87	Follows existing command frontmatter format; datadog cosmetic changes are noise
Best Practices	88	Score cap rule sensible; review lifecycle management is operationally mature
General QA	85	--thorough flag not stripped from $ARGUMENTS before use as focus area

✅ All categories ≥85% — Ready to merge (minor clarifications)

P3 — Medium (2 found)

Focus Area section — If provided, focus the review on: $ARGUMENTS — but $ARGUMENTS may include --thorough. The command must strip the flag before using remainder as focus area, but the spec doesn't say how. Add: "Strip --thorough from $ARGUMENTS if present; remainder is the focus area/file pattern."

Step 7 (AskUserQuestion) — This step fires unconditionally but will not work in agentic mode. Clarify: "Skip Step 7 when in agentic mode."

What's Done Well

• 8-category structure is consistent throughout the command and SKILL.md
• P1-P4 severity with finding IDs (security-1, simplicity-3) enables precise cross-referencing
• Score cap rule (any ⚠️ category caps overall at 94%) prevents gaming
• Review lifecycle management — dismissing old automated reviews, resolving fixed inline comments — is mature
• Shields.io badge format is well-specified with correct URL encoding

Verdict: Ready to merge. P3 findings are clarifications worth adding but not blocking.

Reviewed by Daffy D (qa) — full report: .claude/pr-reviews/nsheaps/ai-mktpl/190/1771900839/OVERALL-REPORT.md

[henry-nsheaps]

👍 Well-structured enhancement to the review-changes skill. Post-merge review confirms prior findings; recommend follow-up PR for corrections.

⚠️ minimizeComment used where resolveReviewThread is needed — correctness issue in Review Lifecycle docs (practices-1)
⚠️ $ARGUMENTS not stripped of --thorough before use as focus area (usability-3, concurring with @nsheaps)
⚠️ Step 7 AskUserQuestion fires unconditionally — won't work in agentic mode (usability-4, concurring with @nsheaps)
⚠️ plugin.json description is stale — still says "quality, security, performance, and maintainability" instead of 8 dimensions (patterns-1)
✅ 8-category framework is well-structured with clear, non-overlapping criteria
✅ P1-P4 severity with finding IDs enables precise cross-referencing
✅ Score cap rule and brevity rule are pragmatic design choices
✅ Expanded context gathering (base branch, commit messages) improves review quality
ℹ️ .claude/pr-reviews/ not in .gitignore — intent should be documented (qa-1)

🖱️ Click to expand for full details

Code Quality & Structure

The main change (review-changes.md) is a substantial, well-organized enhancement to the review skill. The document grows from ~220 lines to ~460 lines, which is proportionate to the features added. The 8 review categories are clearly defined with non-overlapping criteria, and the document follows a logical flow: categories → process → scoring → output format → lifecycle management.

The two datadog-otel-setup file changes are purely cosmetic (table alignment in README, double→single quote style in YAML). These are fine and appear to be auto-formatter output swept into this branch.

Scoring Rationale

Quality (88%): Well-organized, comprehensive, good structure. Deducted for: stale plugin.json description (patterns-1), incorrect GraphQL mutation in lifecycle docs (practices-1), two usability gaps in argument handling and agentic mode (usability-3, usability-4). All findings are P3-P4.

Security (N/A): No executable code changed — this is a prompt/documentation file defining an agent workflow. No secrets, no injection surfaces.

Simplicity (85%): 462-line prompt is long but well-organized with clear headings and progressive disclosure. The --thorough mode adds proportionate complexity. Quick mode default preserves simplicity for common use. The document could be slightly more concise in the output format template sections (multiple near-identical code blocks for per-category, overall, and GitHub formats), but this repetition serves clarity for the agent consuming the prompt.

Confidence (85%): All findings verified against the actual merged code. Validated .gitignore state, plugin.json contents, and GraphQL mutation semantics against GitHub docs. Higher than my previous review (82%) since additional validation was performed.

Correctness Issue: minimizeComment vs resolveReviewThread

Carried forward from my previous review. The "Resolve Old Inline Comments" section (lines 416-418) instructs using minimizeComment to "resolve" threads, but this hides the comment entirely rather than marking it resolved. The correct mutation is resolveReviewThread. This will cause agents following this skill to destroy review context rather than collapsing it.

Focus Area / Argument Handling

Concurring with @nsheaps' review: the Focus Area section (line 447) uses $ARGUMENTS directly, but when --thorough is passed, $ARGUMENTS contains both the flag and the focus area. The skill needs to instruct agents to strip the flag before interpreting the remainder.

Agentic vs Interactive Mode

Concurring with @nsheaps' review: Step 7 fires AskUserQuestion unconditionally. In agentic/CI mode, there's no user to answer. Should be conditional or skipped.

Stale plugin.json Description

plugin.json at plugins/review-changes/.claude-plugin/plugin.json:4 still reads:

"Review code changes with detailed feedback on quality, security, performance, and maintainability"

This doesn't reflect the new 8-dimension model (simplicity, flexibility, usability, documentation, security, pattern matching, best practices, QA). The description is what users see in marketplace listings and help output.

Datadog Changes

Minor: the YAML file now uses single quotes for active values but the commented-out api_key lines still use double quotes. This creates an inconsistent quoting style within the file, but since the commented lines are examples and this is cosmetic, it's not worth blocking.

Recommended follow-ups (non-blocking):

• Fix minimizeComment → resolveReviewThread in Review Lifecycle Management section (practices-1)
• Add instruction to strip --thorough from $ARGUMENTS before use as focus area (usability-3)
• Clarify Step 7 behavior in agentic mode — skip or adapt AskUserQuestion (usability-4)
• Update plugin.json description to reflect the 8-dimension review model (patterns-1)
• Document whether .claude/pr-reviews/ artifacts are meant to be committed or gitignored (qa-1)

[henry-nsheaps]

🔕 P3 - Medium (usability-3): The Focus Area section uses $ARGUMENTS directly, but when --thorough is passed, $ARGUMENTS will contain both the flag and the focus area (e.g., --thorough src/api/). There's no instruction to strip --thorough from the arguments before using the remainder as the focus area.

Consider adding above line 447:

Strip --thorough from $ARGUMENTS if present; the remainder is the focus area/file pattern.

(Concurring with @nsheaps' finding from their review)

[henry-nsheaps]

🔕 P3 - Medium (usability-4): Step 7 fires unconditionally with AskUserQuestion, but in agentic mode (e.g., CI review bot or automated pipeline), there's no user to answer the question. This could cause the agent to hang or error.

Consider adding a conditional: "Skip Step 7 when running in agentic mode (no interactive user). In agentic mode, output findings and exit."

(Concurring with @nsheaps' finding from their review)