kantan-review-feature

name	kantan-review-feature
description	Use when implementation is complete, before reporting or marking any feature as done, and before finishing a feature in a Rails + React app. Mandatory review step of the Kantan workflow — never skip it. Performs an expert Rails + React code review of the changes against the plan, the repo conventions, and universal best practices.

Review a Feature

Act as an expert Rails + React reviewer. Review the implemented changes against the approved plan before the feature is finished. This step makes no commits and ships no code — it produces findings.

Gather context

1. Read the plan at <backend-root>/.kantan-dev/plans/YYYYMMDD_feature_name.md (and the idea, for intent).
2. Read each involved repo's conventions file — AGENTS.md if present, else CLAUDE.md — for the backend and the target frontend.
3. Collect the full change set in each involved repo from the working tree (changes are not committed): run git status and git diff, and include untracked/new files. Review everything that changed.

Review your own work as a stranger's

This step exists to re-open the reasoning you used while implementing. Every choice you made — a fix you deferred, an offense you left, a diff you kept small — is in scope here, not settled.

• Only the user can settle a question. Treat something as out of scope for review only if you can cite where the user decided it: a line in the idea, the approved plan, or an explicit instruction. Cite the source.
• Your own earlier justification is not a citation. Rationale you wrote in a plan you authored, a summary, a todo, or an earlier turn carries no standing. Re-derive it or flag it.
• If you delegate the review to a subagent or fresh context, the brief carries inputs only — paths to the idea, plan, conventions, and the change set. Never a list of items "not to flag", never your reasoning for a choice. A reviewer handed your conclusions cannot find the fault in them.

Review against the plan first

• Every task / acceptance criterion in the plan is implemented — nothing missing.
• Nothing beyond the plan's scope crept in (flag scope drift).
• The plan's stated verification steps actually pass.

Review for quality

Check the changes against the repo's conventions and these universals — repo conventions win:

Backend (Rails)

• Specs exist and are meaningful for new models/services/jobs/endpoints; external HTTP stubbed; suite green.
• Fat service / thin controller; jobs delegate to services.
• N+1s: missing eager-loads / with_attached_*, missing .distinct on has_many :through filters, per-row queries in loops/serializers.
• Security: authn/authz on new endpoints, strong params, no plaintext secrets/PII, no mass-assignment holes.
• DB: NOT NULL / foreign-key constraints + indexes on foreign keys; reversible migrations.
• Collection endpoints paginate.

Frontend (React)

• Matches the repo's stack and style (state lib, styling, quotes/width) — not imposed from memory.
• Stable list keys (no bare index); effect dependencies correct; no stale closures; cleanup where needed.
• HTTP via the repo's wrapper; API paths centralized; FormData uploads do not set Content-Type.
• Loading/error states handled; PropTypes/types present; no stray console.log or dead code.

Contract (backend ↔ frontend)

• Frontend payloads, param names, and response handling match the backend endpoints (shapes, nesting, status codes).

Run the tools yourself

Anything a tool can settle must come from running that tool during the review — not from an earlier run, a memory, or the implementer's say-so. In each involved repo run the repo's test command and its linter/formatter, and record the exact command and its result in the report.

• Every offense or failure the tool reports is a finding. An offense on a line this branch touched is Major. An offense elsewhere in a file this branch touched is a finding too — downgrade it only after proving it pre-dates the branch (stash the change or run the tool on the merge base) and recording that comparison. Unproven means it is yours.
• A finding you don't intend to fix is still a finding. "Pre-existing", "out of scope", "the fix would bury the real change", "better as its own cleanup" are recommendations to the user, not verdicts you may issue. Record them and let the user decide.

Write it in plain English

Write every document and chat message from this skill in ASD-STE100 Simplified Technical English — a restricted form of English built for technical documents that a non-native reader must get right on one reading. Later agent sessions read these files too.

• Active voice, named actor. "The service rejects the request" — not "the request is rejected".
• One idea per sentence. Aim for 20 words or fewer.
• One word, one meaning. Pick a term and repeat it verbatim; never vary it for elegance — endpoint stays "endpoint", never "route" or "API surface".
• Explain each technical term once, where it first appears. Use it bare after that.
• Present tense; imperative for steps. "Run the suite" — not "the suite should be run".
• Never simplify code. Identifiers, paths, commands, and quoted output stay exactly as they are.

Report findings

Produce a report grouped by severity. For each finding give repo path:line, what is wrong, and the fix.

• Critical — bugs, security holes, data loss, or missing plan items. Blocks finishing.
• Major — convention violations, N+1s, missing tests, contract mismatches. Blocks finishing.
• Minor / Nit — style, naming, small improvements. Advisory.

Two further sections are required whenever they have content — and an item belongs in one of them only if it meets that section's bar:

• Settled by the user — not flagged. Only decisions attributable to the user, each naming its source (idea / approved plan / a specific instruction). Verify each is genuinely implemented as described rather than accepting the assertion. Cannot cite a source? It is not settled — file it under a severity heading above instead.
• Deferred — needs your call. Everything the review found but did not fix, each with the cost of fixing it now and your recommendation. Proven-pre-existing tool offenses go here. These are surfaced to the user, never resolved by you.

Save the report to <backend-root>/.kantan-dev/reviews/YYYYMMDD_feature_name.md (same slug as the idea/plan) and end it with an explicit verdict line:

• Verdict: APPROVED — only when zero Critical/Major findings remain and the test and lint runs recorded above are green, or every remaining offense is proven pre-existing and listed under "Deferred".
• Verdict: BLOCKED — otherwise, with the open blocking findings listed above it.

This file is required by kantan-finish-feature — finishing cannot proceed without it.

Resolve before finishing

If there are Critical or Major findings, fix them — loop back to kantan-backend-tdd / kantan-frontend, following TDD and the repo's conventions — then re-run tests and linters and re-check. After each re-check, update the review file and its verdict. Only once no blocking findings remain (Verdict: APPROVED), proceed to kantan-finish-feature.

Do NOT run git add or git commit. Leave changes for the user to review.