feat(skill): update code review instruction

Author: codeaholicguy

commands/code-review.md

@@ -2,13 +2,20 @@
 description: Pre-push code review against design docs.
 ---
 
-Perform a local code review **before** pushing changes.
+Perform a **holistic** local code review **before** pushing changes. Go beyond the diff — review how changes integrate with the broader codebase.
 
-1. **Gather Context** — If not already provided, ask for: feature/branch description, list of modified files, relevant design doc(s) (e.g., `docs/ai/design/feature-{name}.md`), known constraints or risky areas, and which tests have been run. Also review the latest diff via `git status` and `git diff --stat`.
+1. **Gather Context** — If not already provided, ask for: feature/branch description, relevant design doc(s) (e.g., `docs/ai/design/feature-{name}.md`), known constraints or risky areas, and which tests have been run. Run `git status` and `git diff --stat` to identify modified files.
 2. **Use Memory for Context** — Search memory for project review standards and recurring pitfalls: `npx ai-devkit@latest memory search --query "code review checklist project conventions"`.
 3. **Understand Design Alignment** — For each design doc, summarize architectural intent and critical constraints.
-4. **File-by-File Review** — For every modified file: check alignment with design/requirements and flag deviations, spot logic issues/edge cases/redundant code, flag security concerns (input validation, secrets, auth, data handling), check error handling/performance/observability, and identify missing or outdated tests.
-5. **Cross-Cutting Concerns** — Verify naming consistency and project conventions. Confirm docs/comments updated where behavior changed. Identify missing tests (unit, integration, E2E). Check for needed configuration/migration updates.
-6. **Store Reusable Knowledge** — Save durable review findings/checklists with `npx ai-devkit@latest memory store ...`.
-7. **Summarize Findings** — Categorize each finding as **blocking**, **important**, or **nice-to-have** with: file, issue, impact, recommendation, and design reference.
-8. **Next Command Guidance** — If blocking issues remain, return to `/execute-plan` (code fixes) or `/writing-test` (test gaps); if clean, proceed with push/PR workflow.
+4. **Holistic Codebase Review** — For each modified file, use targeted grep/glob on exported names (functions, types, constants) to trace callers and dependents. Read only relevant sections (signatures, call sites, type defs) — skip files with no shared interface. Then check:
+   - **Consistency**: Scan 1–2 similar modules to verify the change follows established patterns.
+   - **Duplication**: Search for existing utilities the new code could reuse or now duplicates.
+   - **Contract integrity**: Verify type signatures, API contracts, and config/DB schemas remain consistent at integration boundaries.
+   - **Dependency health**: Check for circular dependencies or version conflicts from new/changed imports.
+   - **Breaking changes**: Are public APIs, CLI flags, env vars, or config keys changed in ways that break existing consumers? Check downstream dependents.
+   - **Rollback safety**: Can this change be safely reverted? Flag irreversible migrations, one-way data format changes, or state transitions that cannot be undone.
+5. **File-by-File Review** — For every modified file: check alignment with design/requirements and flag deviations, spot logic issues/edge cases/redundant code, flag security concerns (input validation, secrets, auth, data handling), check error handling/performance/observability, and identify missing or outdated tests.
+6. **Cross-Cutting Concerns** — Verify naming consistency and project conventions. Confirm docs/comments updated where behavior changed. Identify missing tests (unit, integration, E2E). Check for needed configuration/migration updates.
+7. **Store Reusable Knowledge** — Save durable review findings/checklists with `npx ai-devkit@latest memory store ...`.
+8. **Summarize Findings** — Categorize each finding as **blocking**, **important**, or **nice-to-have** with: file, issue, impact, recommendation, and design reference. Include findings from both the diff and the broader codebase analysis.
+9. **Next Command Guidance** — If blocking issues remain, return to `/execute-plan` (code fixes) or `/writing-test` (test gaps); if clean, proceed with push/PR workflow.

packages/cli/templates/commands/code-review.md

@@ -2,13 +2,20 @@
 description: Pre-push code review against design docs.
 ---
 
-Perform a local code review **before** pushing changes.
+Perform a **holistic** local code review **before** pushing changes. Go beyond the diff — review how changes integrate with the broader codebase.
 
-1. **Gather Context** — If not already provided, ask for: feature/branch description, list of modified files, relevant design doc(s) (e.g., `{{docsDir}}/design/feature-{name}.md`), known constraints or risky areas, and which tests have been run. Also review the latest diff via `git status` and `git diff --stat`.
+1. **Gather Context** — If not already provided, ask for: feature/branch description, relevant design doc(s) (e.g., `{{docsDir}}/design/feature-{name}.md`), known constraints or risky areas, and which tests have been run. Run `git status` and `git diff --stat` to identify modified files.
 2. **Use Memory for Context** — Search memory for project review standards and recurring pitfalls: `npx ai-devkit@latest memory search --query "code review checklist project conventions"`.
 3. **Understand Design Alignment** — For each design doc, summarize architectural intent and critical constraints.
-4. **File-by-File Review** — For every modified file: check alignment with design/requirements and flag deviations, spot logic issues/edge cases/redundant code, flag security concerns (input validation, secrets, auth, data handling), check error handling/performance/observability, and identify missing or outdated tests.
-5. **Cross-Cutting Concerns** — Verify naming consistency and project conventions. Confirm docs/comments updated where behavior changed. Identify missing tests (unit, integration, E2E). Check for needed configuration/migration updates.
-6. **Store Reusable Knowledge** — Save durable review findings/checklists with `npx ai-devkit@latest memory store ...`.
-7. **Summarize Findings** — Categorize each finding as **blocking**, **important**, or **nice-to-have** with: file, issue, impact, recommendation, and design reference.
-8. **Next Command Guidance** — If blocking issues remain, return to `/execute-plan` (code fixes) or `/writing-test` (test gaps); if clean, proceed with push/PR workflow.
+4. **Holistic Codebase Review** — For each modified file, use targeted grep/glob on exported names (functions, types, constants) to trace callers and dependents. Read only relevant sections (signatures, call sites, type defs) — skip files with no shared interface. Then check:
+   - **Consistency**: Scan 1–2 similar modules to verify the change follows established patterns.
+   - **Duplication**: Search for existing utilities the new code could reuse or now duplicates.
+   - **Contract integrity**: Verify type signatures, API contracts, and config/DB schemas remain consistent at integration boundaries.
+   - **Dependency health**: Check for circular dependencies or version conflicts from new/changed imports.
+   - **Breaking changes**: Are public APIs, CLI flags, env vars, or config keys changed in ways that break existing consumers? Check downstream dependents.
+   - **Rollback safety**: Can this change be safely reverted? Flag irreversible migrations, one-way data format changes, or state transitions that cannot be undone.
+5. **File-by-File Review** — For every modified file: check alignment with design/requirements and flag deviations, spot logic issues/edge cases/redundant code, flag security concerns (input validation, secrets, auth, data handling), check error handling/performance/observability, and identify missing or outdated tests.
+6. **Cross-Cutting Concerns** — Verify naming consistency and project conventions. Confirm docs/comments updated where behavior changed. Identify missing tests (unit, integration, E2E). Check for needed configuration/migration updates.
+7. **Store Reusable Knowledge** — Save durable review findings/checklists with `npx ai-devkit@latest memory store ...`.
+8. **Summarize Findings** — Categorize each finding as **blocking**, **important**, or **nice-to-have** with: file, issue, impact, recommendation, and design reference. Include findings from both the diff and the broader codebase analysis.
+9. **Next Command Guidance** — If blocking issues remain, return to `/execute-plan` (code fixes) or `/writing-test` (test gaps); if clean, proceed with push/PR workflow.

skills/dev-lifecycle/references/code-review.md

@@ -1,12 +1,19 @@
 # Phase 8: Code Review
 
-Final pre-push review. Check `git status -sb` and `git diff --stat`.
+Final pre-push **holistic** review. Go beyond the diff — review how changes integrate with the broader codebase. Check `git status -sb` and `git diff --stat`.
 
 1. **Gather context** — feature description, modified files, design docs, risky areas, tests already run.
 2. **Verify design alignment** — summarize architectural intent, check implementation matches.
-3. **File-by-file review** — correctness, logic/edge cases, redundancy, security, performance, error handling, test coverage.
-4. **Cross-cutting** — naming conventions, documentation updates, missing tests, config/migration changes.
-5. **Summarize** — blocking issues, important follow-ups, nice-to-haves. Per finding: file, issue, impact severity, recommendation.
-6. **Final checklist** — design match, no logic gaps, security addressed, tests cover changes, docs updated.
+3. **Holistic codebase review** — for each modified file, grep exported names (functions, types, constants) to trace callers and dependents. Read only relevant sections (signatures, call sites, type defs) — skip files with no shared interface. Then check:
+   - **Consistency**: scan 1–2 similar modules for pattern alignment.
+   - **Duplication**: search for existing utilities the new code could reuse or now duplicates.
+   - **Contract integrity**: verify type signatures, API contracts, config/DB schemas remain consistent at integration boundaries.
+   - **Dependency health**: check for circular dependencies or version conflicts from new imports.
+   - **Breaking changes**: public APIs, CLI flags, env vars, or config keys changed in ways that break existing consumers.
+   - **Rollback safety**: can this be safely reverted? Flag irreversible migrations or one-way data/state changes.
+4. **File-by-file review** — correctness, logic/edge cases, redundancy, security, performance, error handling, test coverage.
+5. **Cross-cutting** — naming conventions, documentation updates, missing tests, config/migration changes.
+6. **Summarize** — blocking issues, important follow-ups, nice-to-haves. Per finding: file, issue, impact severity, recommendation. Include findings from both diff and broader codebase analysis.
+7. **Final checklist** — design match, no logic gaps, security addressed, integration points verified, tests cover changes, docs updated.
 
 **Done**: If checklist passes, ready to push and create PR. If blocking issues → back to Phase 4 (fix code) or Phase 7 (add tests).