takt/builtins/en/policies/review.md

Review Policy

Define the shared judgment criteria and behavioral principles for all reviewers.

Principles

Principle	Criteria
Fix immediately	Never defer minor issues to "the next task." Fix now what can be fixed now
Eliminate ambiguity	Vague feedback like "clean this up a bit" is prohibited. Specify file, line, and proposed fix
Fact-check	Verify against actual code before raising issues. Do not speculate
Practical fixes	Propose implementable solutions, not theoretical ideals
Boy Scout	If a changed file has problems, have them fixed within the task scope

Scope Determination

Situation	Verdict	Action
Problem introduced by this change	Blocking	REJECT
Existing problem in a changed file	Blocking	REJECT (Boy Scout rule)
Structural problem in the changed module	Blocking	REJECT if within scope
Problem in an unchanged file	Non-blocking	Record only (informational)
Refactoring that greatly exceeds task scope	Non-blocking	Note as a suggestion

Judgment Criteria

REJECT (Request Changes)

REJECT without exception if any of the following apply.

• New behavior without tests
• Bug fix without a regression test
• Use of any type
• Fallback value abuse (?? 'unknown')
• Explanatory comments (What/How comments)
• Unused code ("just in case" code)
• Direct mutation of objects/arrays
• Swallowed errors (empty catch blocks)
• TODO comments (not tracked in an issue)
• Duplicated code in 3+ places (DRY violation)
• Method proliferation doing the same thing (should be absorbed by configuration differences)
• Specific implementation leaking into generic layers (imports and branching for specific implementations in generic layers)
• Missing cross-validation of related fields (invariants of semantically coupled config values left unverified)

Warning

Not blocking, but improvement is recommended.

• Insufficient edge case / boundary value tests
• Tests coupled to implementation details
• Overly complex functions/files
• Unclear naming
• Abandoned TODO/FIXME (those with issue numbers are acceptable)
• @ts-ignore or eslint-disable without justification

APPROVE

Approve when all REJECT criteria are cleared and quality standards are met. Never give conditional approval. If there are problems, reject.

Fact-Checking

Always verify facts before raising an issue.

Do	Do Not
Open the file and check actual code	Assume "it should be fixed already"
Search for call sites and usages with grep	Raise issues based on memory
Cross-reference type definitions and schemas	Guess that code is dead
Distinguish generated files (reports, etc.) from source	Review generated files as if they were source code

Writing Specific Feedback

Every issue raised must include the following.

• Which file and line number
• What the problem is
• How to fix it

❌ "Review the structure"
❌ "Clean this up a bit"
❌ "Refactoring is needed"

✅ "src/auth/service.ts:45 — validateUser() is duplicated in 3 places.
     Extract into a shared function."

Boy Scout Rule

Leave it better than you found it.

In Scope

• Existing problems in changed files (unused code, poor naming, broken abstractions)
• Structural problems in changed modules (mixed responsibilities, unnecessary dependencies)

Out of Scope

• Unchanged files (record existing issues only)
• Refactoring that greatly exceeds task scope (note as a suggestion, non-blocking)

Judgment

Situation	Verdict
Changed file has an obvious problem	REJECT — have it fixed together
Redundant expression (a shorter equivalent exists)	REJECT
Unnecessary branch/condition (unreachable or always the same result)	REJECT
Fixable in seconds to minutes	REJECT (do not mark as "non-blocking")
Fix requires refactoring (large scope)	Record only (technical debt)

Do not tolerate problems just because existing code does the same. If existing code is bad, improve it rather than match it.

Detecting Circular Arguments

When the same kind of issue keeps recurring, reconsider the approach itself rather than repeating the same fix instructions.

When the Same Problem Recurs

1. Check if the same kind of issue is being repeated
2. If so, propose an alternative approach instead of granular fix instructions
3. Even when rejecting, include the perspective of "a different approach should be considered"

Rather than repeating "fix this again," stop and suggest a different path.