Task: Review Changes

Overview

Act as QA engineer + lead engineer simultaneously. Review only the current changes (diff from the task branch or unstaged/staged changes) against the original request and plan. Verify task completion AND code quality in a single pass.

Context

Input sources:

• Git diff (git diff, git diff --cached, git diff <base>..HEAD).
• The original User Request (from chat history).
• The Plan (task management tool or documents/whiteboard.md).
• Project conventions (AGENTS.md, linter/formatter configs). </context>

Rules & Constraints

Instructions

<step_by_step>

1. Empty Diff Guard

   • Run git diff --stat, git diff --cached --stat, and git status --short.
   • If there are NO changes (no diff, no staged files, no untracked files), report "No changes to review" and STOP.

2. Gather Context

   • Create a review plan in the task management tool.
   • Collect the diff: git diff (unstaged), git diff --cached (staged), or git log --oneline <base>..HEAD + git diff <base>..HEAD for branch-based changes.
   • Read the original user request and the plan (whiteboard / task list).
   • Look for project conventions in AGENTS.md and config files. If these files do not exist, rely on conventions visible in the diff and surrounding code.

3. QA: Task Completion

   • Map each requirement/plan item to concrete changes in the diff.
   • Flag requirements with no corresponding changes as [critical] Missing.
   • Flag plan items marked "done" but not present in diff as [critical] Phantom completion.
   • Check for regressions: do changed files break existing functionality?

4. QA: Hygiene

   • Temp artifacts: New temp_*, *.tmp, *.bak, debug console.log/ print statements, hardcoded secrets or localhost URLs.
   • Unfinished markers: New TODO, FIXME, HACK, XXX introduced in this diff (distinguish from pre-existing ones).
   • Dead code: Commented-out blocks, unused imports/variables/functions added in this diff.

5. Code Review: Design & Architecture

   • Responsibility: Does each changed file/module stay within its stated responsibility? Flag scope creep.
   • Coupling: Are new dependencies (imports, API calls) justified? Flag tight coupling or circular dependencies.
   • Abstraction: Is the level of abstraction appropriate? Flag over-engineering (unnecessary interfaces, premature generalization) and under-engineering (god-functions, duplicated logic).

6. Code Review: Implementation Quality

   • Naming: Are new identifiers (vars, funcs, types) clear and consistent with project conventions?
   • Error handling: Are errors handled explicitly? Flag swallowed exceptions, missing error paths, generic catch-all handlers.
   • Edge cases: Are boundary conditions (null, empty, overflow, concurrent access) handled?
   • Types & contracts: Are type signatures precise? Flag any, untyped parameters, missing return types (where project conventions require them).
   • Tests: Do new/changed behaviors have corresponding tests? Are existing tests updated for changed behavior?

7. Code Review: Readability & Style

   • Consistency: Do changes follow the project's established patterns (file structure, naming, formatting)?
   • Comments: Are non-obvious decisions explained? Flag misleading or stale comments.
   • Complexity: Flag functions > 40 lines or cyclomatic complexity spikes introduced in this diff.
   • Clarity: Flag clarity sacrificed for brevity — nested ternaries, dense one-liners, overly compact expressions. Explicit code is preferred over clever short forms.

8. Run Automated Checks

   • If the project has a check command (deno task check, npm run lint, make check, etc.), run it and include results.
   • If no check command is found, explicitly note "No automated checks configured" in the report — do not silently skip.
   • If tests exist, run them and report failures.

9. Final Report Output a structured report with the verdict on the FIRST line:

   ## Review: [Approve | Request Changes | Needs Discussion]
   
   ### QA Findings
   - [severity] file:line — description
   
   ### Code Review Findings
   - [severity] file:line — description
   
   ### Automated Checks
   - [pass|fail|skipped] command — summary
   
   ### Summary
   - Requirements covered: X/Y
   - Critical issues: N
   - Warnings: N
   - Nits: N
   

   If no issues: short confirmation "Changes look good. All requirements covered, no issues found."

</step_by_step>

Verification