review-project

name	review-project
description	Use when reviewing a GitHub pull request in a project that follows the spec-first-feature convention (PRD + AC + bucket-based dispatch). Performs convention-aware PRD alignment, discipline rule checks (L107 binary AC, L108 citation cross-check, L109 migration+ORM same-PR, L081 ≤8 deliverable/bucket, L114 PRD §6 schema categorization), and offers an opt-in fix mode that applies mechanical fixes with per-fix user approval (never auto-commits or pushes).
license	MIT
metadata	{"author":"murat-aydogan","version":"1.0"}

review-project

Project-aware PR review skill. Unlike a generic code review, this one knows the project's PRD, knows the project's discipline rules (the Lxxx lessons in tasks/lessons.md), and never claims a PR aligns with the spec without verifying every acceptance criterion.

When to use

Invoke when the user asks for a PR review and either:

• The user explicitly names this skill ("/review-project", "review-project PR #98", "use review-project skill")
• The project has a docs/<feature>-prd.md + tasks/lessons.md shape (the spec-first-feature convention)
• The PR title looks like feat(<feature>): Bucket X — ... / docs(<feature>): ... / fix(<feature>): ...

If none of those apply, fall back to a generic code review instead — do not fabricate a PRD that does not exist.

Inputs

Ask the user for:

1. PR number (required). If not provided, run gh pr list and ask which.
2. Fix mode? (optional). Defaults to OFF (review-only). User explicitly opts in with phrases like "fix the findings", "apply auto-fixes", "--fix".

Step 1 — Identify the feature

Parse PR title with gh pr view <n> --json title. Expected format: feat(<feature>): Bucket X — ... or docs(<feature>): ... or fix(<feature>): ....

Extract <feature> slug. If parse fails → fall back to Step 2's generic review only and skip Steps 3–7.

Step 2 — Generic code review

Run gh pr view <n>, gh pr diff <n>. Analyze:

• Code correctness
• Style + project conventions
• Performance implications
• Test coverage
• Security considerations

Step 3 — PRD alignment check (CRITICAL)

If feature slug extracted in Step 1:

3a. Read PRD: docs/{feature}-prd.md. If not found, skip to Step 4.

3b. For each AC in PRD §7 (or whichever section holds acceptance criteria):

• Is it tested in this PR?
• Is test path mentioned in PR description's AC mapping table?
• Is test result BINARY (pass / fail)?
• Flag any AC marked "partial", "deferred", "TBD", "skipped" as L107 violation.

3c. For each Implementation Constraint in PRD §5 (e.g., §5.1, §5.2, …):

• Does the PR code match PRD's specification exactly?
• Example checks: format strings, field names, helper signatures, default values, enum values.
• Flag any deviation as PRD drift.

Step 4 — Schema discipline

If PR contains migration files (backend/app/db/migrations/versions/*.py or project equivalent):

• L109: ORM mirror in same PR? (Look for changes to db/models.py or equivalent.)
• L114: PRD §6 schema categorization stated? Categories are typically (a) var-tam / (b) var-eksik / (c) yok.

If db/models.py adds a new model without a matching migration → flag.

Step 5 — Citation cross-check

For each path:line citation found in the PR description:

• Run git ls-files <path> — exists in main?
• Working-tree-only artifacts → L108 violation.

Step 6 — Bucket scope check (L081)

If PR is a Bucket dispatch:

• Count deliverables in PR description (numbered list).
• ≤ 8 per bucket. If > 8 → flag bucket split needed.

Step 7 — Backward compatibility

If PRD says "dual-stack window yok" or "Faz 0 no behavior change":

• Verify flag check is atomic (no mid-state dual-accept inside one request).
• Verify existing tests still pass (no regression).

Step 8 — Lessons cross-reference

If tasks/lessons.md exists:

• Scan PR for patterns that match documented lessons.
• Example: PR adds a migration without ORM → L109 explicit mention.
• Example: PR cites a file that's not in main → L108 explicit mention.
• Reference lesson IDs in findings (e.g., "violates L107").

Output format

# Review: PR #<n> — <title>

## Decision: ✅ APPROVE | ⚠️ CHANGES REQUESTED | 🛑 BLOCKER

## PRD alignment
| AC | Tested? | Path | Status |
| ... |

## PRD drift
- (list if any)

## Discipline violations
- L107 / L108 / L109 / L081 / L114 (referenced explicitly)

## Generic code review
- (built-in step 2 output)

## Recommended actions
| # | Severity | Action | Category |

The Category column is mandatory — Step 9 fix mode reads it:

• AUTO_FIX — mechanical, can be safely applied (missing ORM column, broken citation path, missing PRD §6 line, missing test stub, lint error)
• NEEDS_DECISION — user decision required (PRD drift: which value is correct? L081 bucket split: how to split? L107 deferred AC: defer or test now?)
• MANUAL_ONLY — security trade-off, architectural change, scope shift — fix mode never touches these

BLOCKER decision reserved for: silent bugs (e.g., type-mismatch swallowed by try/except), data loss risk, AC mapping that fails binary test.

Step 9 — Fix mode (opt-in)

Only enter Step 9 if the user explicitly opted into fix mode. Otherwise stop after the review output above.

9a — Build the fix plan

For every row in Recommended actions, write a concrete proposal:

#	Finding	Cat	Proposed fix	Files touched
1	L109: migration v1_foo adds column bar, db/models.py untouched	AUTO_FIX	Add bar: Mapped[str] = mapped_column(String, nullable=False) to class Foo	backend/app/db/models.py:142
2	L107: AC #3 marked "deferred"	NEEDS_DECISION	(decide first: defer or test?)	—
3	PRD §5.2 format "{user}/{id}" differs from code "{id}-{user}"	NEEDS_DECISION	(PRD or code: which is correct?)	docs/-prd.md OR backend/.../foo.py
4	L108: PR description cites services/baz.py:42, not in git ls-files	AUTO_FIX	Remove or correct the citation in PR description	(PR description edit)

9b — Get approval

Ask the user (concise):

"Which AUTO_FIX numbers should I apply? e.g. 1,4, all, or none. For NEEDS_DECISION items (#2, #3), can you answer the questions separately?"

• none → jump to 9d; only ask NEEDS_DECISION questions
• all → apply every AUTO_FIX
• Numbered list → apply only those

9c — Apply approved fixes

For each approved AUTO_FIX:

1. Branch check: Is the current branch the PR's branch? If not, ask the user whether to run gh pr checkout <n>. Never switch branches without approval.
2. Edit: Read the target file, then Edit it. Each patch must be a single logical change.
3. Show the diff after the edit (git diff <file>).
4. Verify when possible:
   • Python edit → ruff check <file> or python -c "import <module>"
   • Migration + ORM addition → import smoke test
   • PR description edit → gh pr edit <n> --body-file <tmp>
5. Never commit automatically. Leave the change unstaged so the user reviews the diff and commits manually.
6. Never push, merge, amend, or force-push. This mode is read + edit only, scoped to the local working tree.
7. If a fix fails (file structure changed, conflict, lint failure), abort that fix, log it, continue with the others.

9d — Report

## Fix mode summary

### Applied (N)
| # | Finding | File | Diff summary |

### Skipped (M)
| # | Finding | Reason |

### Awaiting decision (NEEDS_DECISION)
| # | Question | Options |

### Not auto-fixable (MANUAL_ONLY)
| # | Finding | Why manual |

### Next step
- Review the diffs: `git diff`
- Commit: `git commit -m "fix(review): apply <findings>"`
- Re-run: invoke this skill again without fix mode to verify clean

Fix-mode safety rules (non-negotiable)

• Never commit — the user commits
• Never push / merge / force-push / amend
• Branch checkout requires explicit user approval each time
• Multi-file changes → one Edit per file, one diff per file
• PRD edits are allowed only after NEEDS_DECISION is answered; always confirm which version is correct before touching the PRD
• Creating a test file is NEEDS_DECISION, not AUTO_FIX — test shape (mock vs integration, which path) is a user judgment call

Conventions assumed (hard-coded)

docs/{feature}-prd.md
docs/SYSTEM-{feature}-scan.md
docs/SYSTEM-{feature}-scan-followup.md
tasks/lessons.md

If these paths do not exist, downgrade to a generic code review and call that out explicitly in the output.

Related lessons (typical referrals)

• L081 — ≤ 8 deliverables per bucket
• L107 — Acceptance criteria must be binary (pass / fail), no "partial" / "deferred"
• L108 — Every path:line citation must exist in git ls-files
• L109 — Migration + ORM mirror in the same PR
• L114 — PRD §6 must categorize schema state: var-tam / var-eksik / yok
• L120 — "SHIPPED" requires foundation + wiring + end-to-end runtime path, not just foundation
• L122 — Subagent "CRITICAL" claims about auth must be spot-checked before publication