Cr

/cr — Code Review

Automated code review for local branches, PRs, commits, and files. Detects review mode from arguments and routes to the appropriate review flow — either quick single-agent review with interactive fix selection, or multi-agent deep review with risk-based auto-fix.

All user-facing text matches the user's language. All questions and option selections MUST use your interactive dialog tool (e.g. AskUserQuestion) — never output options as plain text. Do not proceed until the user replies. When presenting multi-select options: ≤4 items → one question. >4 items → group by priority or category (each group ≤4 options), then present all groups as separate questions in a single prompt.

Route

Run pre-checks, then match the first applicable rule top-to-bottom:

1. git branch --show-current → record whether on main/master. 2. git status --porcelain → record whether uncommitted changes exist. 3. Check whether the current environment supports agent teams (multiple agents working in parallel and communicating with each other).

Each → means: Read the target file and follow it as the sole remaining instruction. Ignore all sections below. Do NOT review from memory or habit — each target file defines specific constraints on how to obtain diffs, apply fixes, and submit results.

---

Question

Ask a single question: "Agent Teams is available (multiple agents working in parallel). Enable multi-agent review with reviewer–verifier adversarial mechanism and auto-fix?" Provide 4 options:

Hand off

Pass $ARGUMENTS to the target file. For teams-review, also pass FIX_MODE (low / low_medium / full).

Bot Review

Non-interactive PR code review mode designed for CI/CD pipelines. Uses multi-agent adversarial review mechanism (reviewer–verifier) to ensure high-quality issue detection, then posts all confirmed issues as line-level PR comments. Does NOT apply fixes, approve, or merge — only reviews and comments.

For shared review mechanics (team setup, reviewer/verifier prompts, filter logic), see teams-review.md. This document covers bot-specific setup and output only.

Flow

Setup (validate PR, fetch diff)
  │
  │ (invalid PR / auth error / empty diff)
  ├──────────────────────────────────────→ Stop
  │
  ↓
Review (reviewer–verifier pipeline)
  │
  ↓
Filter (de-dup, worth-reporting, line numbers)
  │
  │ (no issues)
  ├──────────────────────────────────────→ Clean up → Success
  │
  ↓
Output (post PR comments, print summary)
  │
  ↓
Checklist Evolution (suggest new patterns)
  │
  ↓
Clean up (delete temp branch) ──────────→ Done

Unlike teams mode, bot mode is a single-pass pipeline — no fix loop, no user confirmation. Issues are reported as PR comments for human follow-up.

References

---

Arguments

$ARGUMENTS format: bot [target]

Where [target] is required and must be one of:

• PR number (e.g., 123) — review the specified PR
• PR URL (e.g., https://github.com/.../pull/123) — review the specified PR

If [target] is empty or not a valid PR reference, print error message and stop.

---

Prerequisites

This mode requires gh CLI with valid GitHub authentication. The gh CLI reads credentials from these sources (in priority order):

1. GH_TOKEN environment variable 2. GITHUB_TOKEN environment variable 3. gh auth login cached credentials

In CI environments, set GH_TOKEN or GITHUB_TOKEN with the following permissions:

• repo — read repository and PR information
• pull_requests: write — post PR comments

---

Phase 1: Setup

Check prerequisites

Verify gh CLI is available and authenticated:

gh auth status

If this command fails, print error message with setup instructions and stop.

Validate PR

Parse [target] from $ARGUMENTS (everything after bot). If it's a URL, extract the PR number.

gh repo view --json nameWithOwner --jq .nameWithOwner
gh pr view {number} --json headRefName,baseRefName,headRefOid,state,body

Record OWNER_REPO, PR_BRANCH, BASE_BRANCH, HEAD_SHA, STATE, PR_BODY.

Validation:

• If commands fail → print error message and stop.
• If URL contains {owner}/{repo} that doesn't match OWNER_REPO → print

"Cross-repo PR review not supported" and stop.

• If STATE is not OPEN → print "PR is not open" and stop.

Fetch PR diff

git fetch origin pull/{number}/head:pr-{number}
git fetch origin {BASE_BRANCH}
MERGE_BASE=$(git merge-base origin/{BASE_BRANCH} pr-{number})
git diff $MERGE_BASE pr-{number}

If diff is empty → print "No changes to review." and stop.

If diff exceeds 10000 lines → print "PR too large for automated review ({N} lines). Please request a manual review." and stop with exit code 2.

If diff exceeds 200 lines, first run git diff --stat for overview, then read per file using git diff $MERGE_BASE pr-{number} -- {file}.

Fetch existing PR comments

gh api repos/{OWNER_REPO}/pulls/{number}/comments

Store as EXISTING_COMMENTS for de-duplication in Phase 3.

Module partition

See teams-review.md → Phase 1: Scope → Module partition.

---

Phase 2: Review

See teams-review.md → Phase 2: Review (Team setup, Verification pipeline), with the following bot-specific adjustments:

Reviewer prompt additions

In addition to the base reviewer prompt from teams-review.md:

• PR context: PR_BODY content to understand the stated motivation. Verify

the implementation actually achieves what the author describes. If PR_BODY is empty, skip motivation verification and focus on code-level review only.

• Output format (overrides base format from teams-review.md):

[file:line] [A1/B2/C3] — [description] — [key lines] (include checklist item ID, e.g., A1, B2, C3)

After review

Apply missing report detection per teams-review.md → After review: if a reviewer's system notification shows "completed" but no SendMessage report was received, send a follow-up message requesting its report before proceeding.

Close all agents (send shutdown_request + TeamDelete) → Phase 3. Unlike teams mode, bot mode does not retain reviewers as fixers since it only comments and never applies fixes.

---

Phase 3: Filter — coordinator only

See teams-review.md → Phase 3: Filter (entry conditions, stance), with the following bot-specific adjustments. Bot mode skips the Existence check step (teams-review.md § 3.2) — verifiers already validated issue existence through their CONFIRM verdicts during the review phase.

3.1 De-dup

• Remove cross-reviewer duplicates (same location, same topic)
• Remove issues already in EXISTING_COMMENTS (same file, same line range,

similar description)

3.2 Worth reporting

Consult judgment-matrix.md for worth-fixing criteria. Discard issues that are not worth reporting (e.g., pure style preferences, speculative optimizations).

3.3 Determine line numbers

For each confirmed issue, determine the exact line number in the new file (right side of diff). Read the actual file in the PR branch to confirm — do not derive from diff hunk offsets alone.

GitHub PR review API only accepts lines within a diff hunk (additions or unchanged context lines shown in the diff). If a confirmed issue targets a line that exists in the new file but falls outside any diff hunk, find the nearest line within the same file's diff hunks and reference the original location in the comment body. If no hunk exists for that file, add to ORPHAN_ISSUES.

3.4 Handle deleted lines

GitHub PR review comments can only target lines that exist in the new file (additions or unchanged lines within the diff hunk). For issues involving deleted lines:

• If the issue is about why the deletion is wrong (e.g., removed necessary

code), target the nearest surviving line in the same hunk and mention the deleted line in the comment body.

• If no suitable line exists in the hunk, add to ORPHAN_ISSUES list for

standalone comment (see Phase 4).

---

Phase 4: Output

If no issues found

Print success message and skip to Phase 6 (Clean up):

Code review passed. No issues found.

If issues found

Submit all confirmed issues as line-level PR review comments using gh api. Only individual issue comments are posted to the PR — the summary is printed to stdout only and NOT submitted to the PR. If multiple issues target the same line, they are merged into a single comment.

gh api repos/{OWNER_REPO}/pulls/{number}/reviews --input - <<'EOF'
{
  "commit_id": "{HEAD_SHA}",
  "event": "COMMENT",
  "comments": [
    {
      "path": "relative/file/path",
      "line": 42,
      "side": "RIGHT",
      "body": "[A2] Description of the issue — suggested fix"
    },
    {
      "path": "relative/file/path",
      "line": 88,
      "side": "RIGHT",
      "body": "[B1] First issue — fix suggestion\n[C3] Second issue on same line — fix suggestion"
    },
    ...
  ]
}
EOF

Comment body format:

• Single issue: [{priority}] {description} — {suggested fix}
• Multiple issues on same line: each issue on its own line within the same body

Where {priority} is the checklist item ID (e.g., A2, B1, C7).

Field requirements:

• commit_id: HEAD SHA of the PR branch (HEAD_SHA)
• path: relative to repository root
• line: line number in the new file (right side of diff)
• side: always "RIGHT"
• body: concise description with specific fix suggestion when possible; merge

multiple issues on the same line into one body with line breaks

After submitting line comments, print (not post to PR) a summary to stdout:

Code review found {N} issue(s). Comments posted to PR #{number}.

Issues:
1. [A2] src/foo.cpp:42 — null pointer dereference risk
2. [B1] src/bar.cpp:88 — unnecessary copy in loop
...

Summary:
- A (Correctness & Safety): X issues
- B (Refactoring & Optimization): Y issues
- C (Conventions & Documentation): Z issues

Error handling: If the gh api call fails (network error, auth failure, rate limit), print the error to stderr and exit with code 2. Posting failure is a fatal error that requires attention.

Orphan issues

If ORPHAN_ISSUES (from Phase 3.4) is not empty, post a separate PR comment listing these issues:

gh pr comment {number} --body '### ⚠️ Issues Outside Diff Hunks

The following issues were found but could not be attached to specific diff lines
(the affected code was deleted or falls outside diff hunks in this PR):

1. [A2] `src/foo.cpp` (deleted line 42) — removed necessary null check
2. [B1] `src/bar.cpp` (deleted line 88) — deleted error handling code
...'

If this comment fails to post, print the error but continue — orphan issues are supplementary information.

---

Phase 5: Checklist evolution

Skip this phase if no issues were found in Phase 3 or no new patterns are identified.

Review all confirmed issues from this session. If any represent a recurring pattern not covered by the current checklist, follow checklist-evolution.md Step 1 (Draft candidates) to identify new patterns.

Since bot mode cannot modify repository files directly, post a separate PR comment (not a line-level review comment) proposing the checklist update:

gh pr comment {number} --body '### 📋 Suggested Checklist Update

The following pattern was detected but is not covered by the current checklist:

- **ID**: `[suggested-id]`
- **Pattern**: [description]
- **Why it matters**: [rationale]

Consider adding this to `.codebuddy/skills/cr/references/code-checklist.md`.'

If the comment fails to post, skip silently — do not block the review workflow.

---

Phase 6: Clean up

All agents were already shut down at the end of Phase 2. This phase only cleans up the temporary git branch:

git branch -D pr-{number} 2>/dev/null || true

---

Exit Codes

For CI/CD integration, the bot review process uses the following exit codes:

Checklist Evolution

Rules for updating review checklists. Goal: keep checklists minimal and high-signal — each item should direct AI attention to a distinct class of real issues, not catalog every possible bug pattern.

Step 1: Draft candidates

For each uncovered pattern, draft a candidate item. ALL rules below MUST be satisfied — violation makes the candidate invalid:

1. One assertive phrase describing the expected state (not a question) 2. Generic: applies across files, not tied to a specific variable, function, or bug 3. Atomic: one checkable concern per item (not "X and Y") 4. No overlap: if the issue is a specific case of an existing item, do NOT add it 5. Place under the most specific existing category; create a new category only when no existing one fits 6. Each category stays within 3–8 items. Below 3, merge into a related category. Above 8, first try merging overlapping items; only split if each resulting sub-category has a distinct focus expressible in 2–3 words 7. Prefer fewer, broader items — the checklist is a prompt for attention directions, not an exhaustive bug catalog

When uncertain whether a new item overlaps with an existing one, do NOT add it.

Step 2: User confirmation

Present candidates via multi-select. Each option label is the candidate item text. Unchecked candidates are discarded. If all are discarded, stop.

Step 3: Insert

Insert accepted items into the checklist file at the appropriate position per the category and priority rules above.

Code Review Checklist

Review in priority order: A (highest impact) → B → C. The reviewer prompt specifies which levels to check. Test code: only check for obvious implementation errors, plus A8 (Test Correctness).

Project rules loaded in context override this checklist.

---

A. Correctness, Safety & Structural Integrity

Issues that directly affect runtime behavior or silently degrade codebase quality while passing all tests.

A1. Dead Code & Unreachable Paths

Scope: the entire changeset (not just individual hunks). Search the project

for call sites, enum value usage, and reachability before clearing an item.

• New function / method has at least one actual call site — verify with project-wide

search (a function that is only defined but never called is dead code)

• No conditional branch that can never execute (e.g., checking a removed enum value,

condition always true/false by contract, type narrowing that eliminates a case)

• No parameter accepted but never read in the function body
• No variable assigned but never subsequently used
• No leftover scaffolding from a previous approach that the current code supersedes

(e.g., old helper now bypassed by a new code path, compatibility shim for a removed call site)

• switch / if-else covers only values that actually occur at runtime — no

speculative "just in case" branches for states the system cannot produce

A2. Incomplete Refactoring

When any function signature, return type, enum, or shared data structure changes

in this diff, search the entire project for every consumer and verify each

one is updated. Partial updates are a top-priority finding.

• All call sites of a changed function use the new signature correctly (no silent

fallback to a default-parameter overload that hides the omission)

• Renamed / removed enum values are not referenced anywhere else in the project
• Old code paths that the refactoring intended to replace are actually removed, not

left alongside the new path

• Shared types / structs modified in this diff — all readers and writers updated

consistently

• No mix of old and new API style in the same changeset (e.g., half the callers

pass the new parameter, half still use the old form)

• When a variant / union / tagged type gains or loses a member, every visit /

switch / pattern-match site handles the full set — no silent fall-through to a default branch that masks a missing case

A3. Code Duplication & Consolidation

"Duplication" includes near-identical logic with renamed variables, same algorithm

re-derived in a different file, and same constant / formula hard-coded in

multiple places. The reviewer must search the project — duplication usually

spans files, not lines.

• No two code blocks (across the entire project, not just the diff) that perform

the same logical operation — extract to a shared function or call the existing one

• No "almost identical" blocks that differ only in variable names or minor details —

parameterize or template the shared logic

• Constants / magic numbers / formulas that appear in 2+ locations consolidated into

a single named definition (e.g., a multiplier used in several files should be one constexpr constant, not repeated literals)

• Configuration-like data (default values, thresholds, format strings) that logically

belongs together not scattered across unrelated files — centralize into a config struct, namespace, or file

• New helper / utility function does not duplicate an existing one elsewhere in the

project — search for similar names and similar logic before accepting

A4. Asymmetric & Inconsistent Implementation

When two or more code paths handle the same kind of object or operation (e.g.,

standalone text vs. text-box text, import vs. export, two similar node types),

compare them side-by-side.

• Parallel code paths use the same validation / guard strategy (e.g., if one path

null-checks before use, the analogous path must too — unless the contract provably differs)

• Error-handling style consistent across sibling functions (same error → same

response, not silently swallowed in one place and propagated in another)

• Lifecycle operations symmetric: every acquire has a release, every open has a

close, every register has an unregister — and the pairing style is the same across analogous code paths

• Data transformation (e.g., coordinate conversion, unit scaling) uses the same

formula / helper everywhere — no ad-hoc re-derivation in a single call site

• New code follows the project's established pattern for the same kind of task

(e.g., if the project uses callbacks for async, don't introduce a different async mechanism without justification)

A5. Semantic Drift

Check whether names still match reality in the final state of the diff.

This catches cases where code evolved but identifiers kept their original names.

• Variable / field names still accurately describe the value they hold after all

changes in this diff (e.g., a variable named visibleCount must not store an index or a flag)

• Function / method names still accurately describe their current behavior — if

the body changed meaning, the name must follow

• When a function's contract changes (accepts wider input, returns different type,

has new side effects), its name reflects the new contract — not the old one

A6. Code Correctness

• Return values / out-parameters set correctly in all branches (including error paths)
• Conditional logic free of && / || mix-ups, missing negation, precedence errors
• switch/case covers all branches with no unintended fall-through

A7. Boundary Conditions

For internal (non-public-API) functions: if callers provably guarantee a

precondition (e.g., non-null, non-empty, within range), the guard is

unnecessary — do not flag. Verify the guarantee by reading actual callers.

>

Conversely, a defensive check that silently swallows an illegal state (e.g.,

if (!ptr) return; when null indicates a real bug upstream) can mask the

root cause. Flag when a guard hides what should be an error.

• Division-by-zero protected (both float and integer)
• Empty container checked before front() / back() / operator[]
• Null / nil / undefined dereference guarded
• Integer overflow / underflow handled (especially unsigned subtraction)
• Array / string bounds checked
• Defensive early-return or default value does not silently hide an upstream bug —

if the guarded condition should never happen, prefer an assertion or error over a silent return

A8. Test Correctness

Applies when the diff includes new or modified test code. Priority A because a

wrong test is worse than no test — it provides false confidence.

• Test expected values derived from independent reasoning (specification, manual

calculation, known reference) — not copied from the implementation under test

• Tests verify behavior (given input X, output should be Y) rather than

merely exercising the code path without meaningful assertions

• Test does not re-implement the production algorithm to compute the expected

result (if the algorithm is wrong, both production and test will agree)

• Boundary / error test cases assert the specific expected outcome — not just

"does not crash"

A9. Comment–Code Synchronization

Stale comments are a correctness hazard: future maintainers (including AI) trust

them and propagate the wrong understanding. Promoted to A-level because the harm

compounds silently. For comment completeness (missing descriptions, missing

design-intent notes), see C6.

• When a function body changes behavior, its doc-comment / header comment is updated

in the same commit

• Inline comments adjacent to changed lines still describe the current logic — not

a previous version

• TODO / FIXME / HACK comments reference issues that still exist — remove those

that have been resolved

A10. Error Handling

• I/O operation results checked for errors
• Parse results validated before use
• External input validated for legality
• Failed calls have reasonable fallback / safe return
• Promises / async calls properly awaited with error handling

A11. Injection & Sensitive Data

• User input sanitized before DOM insertion (innerHTML, dangerouslySetInnerHTML,

v-html, [innerHTML], document.write, etc.)

• URL parameters, localStorage, postMessage data validated before use
• No hard-coded API keys, tokens, or credentials in client-side code

A12. Resource Management

• Manually allocated resources released on all paths (prefer RAII)
• File handles / system resources properly closed
• Lock acquire and release properly paired
• Database connections / network sockets released in finally / defer blocks
• Event listeners, timers, subscriptions, observers cleaned up on unmount / scope

exit

A13. Memory Safety

• No use-after-move (moved-from object not reused)
• No dangling reference / pointer to local variable
• Container element reference not used after container modification

A14. Thread Safety

Only flag when the access pattern is clearly unsafe.

• Shared mutable state accessed without lock or atomic
• Callback / closure captures a reference or pointer whose lifetime may end before

invocation

• Condition variable wait without predicate (spurious wakeup)

---

B. Refactoring & Optimization

Improvements to code quality, performance, and maintainability.

B1. Performance

• Container space pre-allocated when size is predictable
• No unnecessary deep copies (only flag when semantic equivalence is certain)
• Loop-invariant expressions hoisted outside loops
• Frequent string concatenation inside loops optimized
• No unnecessary temporary object construction
• Large objects passed by const& instead of by value
• No unnecessary re-renders from missing memoization, unstable references, or inline

object/function creation in props

• No full imports of large dependencies when only a small part is used

B2. Code Simplification

• Deep nested if/else simplified with early return
• Redundant conditional checks merged or eliminated
• Overly long functions split into single-responsibility sub-methods
• Over-engineered abstraction (base class with one subclass, interface with one

implementation, template instantiated one way, wrapper that just forwards) replaced with direct implementation

B3. Module Architecture

Only flag when the diff introduces a new dependency or moves code across module

boundaries.

• Module responsibilities clear with no boundary violations
• Dependency direction reasonable
• No circular dependencies

B4. Interface Usage

• Called APIs used according to their design intent and documentation
• No use of deprecated interfaces

B5. Interface Changes

Flag only — describe the change and its scope for the coordinator to assess.

• Public API signature or class interface changes identified and described

B6. Test Coverage

Flag only — report for awareness, do not auto-fix.

• Changed logic paths have corresponding test cases
• Boundary conditions have test coverage
• Error paths have test coverage

B7. Regression Risk

Flag only — report for awareness, do not auto-fix.

• Modification impact on other callers assessed
• Behavior changes consistent across all target platforms

B8. Rendering Correctness

• List items rendered with stable, unique key (not array index)
• Side effects correctly placed in lifecycle hooks / useEffect with proper dependency

arrays

• Component state derived correctly (no stale closures, no out-of-sync derived state)

---

C. Conventions & Documentation

Coding standards and documentation consistency.

C1. Project Conventions

• Naming follows project's existing style (per loaded project rules)
• Variable names semantically clear, no unnecessary abbreviations
• Names in new code consistent with style in the same file
• Variables assigned initial value at declaration (per project rules)
• Class member variables initialized at declaration or in constructor
• Code complies with language usage restrictions in project rules
• New code consistent with existing patterns in the project

C2. File Organization

• Function order in implementation files matches declaration order
• Header files have appropriate include guards
• Include / import dependencies necessary and reasonable

C3. Type Safety

• No implicit narrowing conversions (large type → small type)
• No signed / unsigned mixed comparisons
• Magic numbers extracted as named constants (unless context already makes meaning

clear)

C4. Const Correctness

• Methods that don't modify state marked const
• Unmodified parameters passed as const references
• Unmodified local variables declared const

C5. Documentation Consistency

• Type names / enum names in code consistent with project documentation
• Value ranges in comments consistent with specification documents

C6. Public API Comments

• Public API comments accurately describe current behavior, parameters, return values
• Value ranges, constraints, error conditions in comments match implementation
• Public APIs have sufficient parameter and return value descriptions
• Design intent explanations present where code alone is insufficient

C7. Accessibility

• Images have meaningful alt text (empty alt for decorative images)
• Form inputs have associated labels
• Interactive elements keyboard-navigable with semantic HTML

---

Exclusion List

Project rules override this exclusion list. If project rules have explicit requirements

for an excluded issue type, that type is not excluded — review per project rules.

1. Pure style preferences within formatting tool scope (not required by project rules) 2. Formatting already handled by project formatting tools (indentation, whitespace, etc.) 3. Suggestions based on assumed future requirements, not current code 4. Code following project's existing style but not matching some external standard 5. Priority C issues in test code (unless project rules require otherwise) 6. "Better alternative" suggestions for existing stable, bug-free code 7. Missing guards in internal functions when callers provably guarantee the precondition (only applies to non-public-API code; verify by reading actual call sites)

Diagnosis

Analyze the most recent /cr session in this conversation to find defects in the skill files themselves — checklist gaps, ambiguous instructions, missing exclusion rules, etc. The goal is to make the skill more accurate and reliable, NOT to re-review the project code.

Work entirely from the session context. Only read a skill file when you need to confirm the exact wording of a rule before suggesting a change.

Prerequisites

If no /cr session exists in the current conversation, inform the user and stop.

Analyze

Scan the entire session from start to finish for these signals and report findings. Do not stop after finding the first issue — exhaustively check every user message and system notification. Key evidence includes user rollbacks of auto-fixes, manual corrections or overrides the user had to provide, steps the AI deviated from, and user interventions to unblock a stalled flow (e.g., the user asking "why did you stop?" or manually prompting the AI to continue). For each finding, state which skill file to change and what the change should be.

False positives

Issues reported or auto-fixed that the user rejected, reverted, or corrected. For each: what was wrong, and which checklist item, exclusion rule, or judgment-matrix rule should be added or revised.

Judgment errors

Issues where the user disagreed with the risk level or worth-fixing decision (e.g., reverted an auto-fix, or explicitly overrode a skip). For each: what the session assigned vs what it should have been, and how to revise judgment-matrix.md.

Flow deviations

Steps the AI skipped, reordered, or executed incorrectly. For each: which step in which file was violated, and how to clarify the instruction.

Other improvements

Anything else observed in the session that points to a concrete skill file change — e.g., redundant steps, missing guardrails, unclear wording. Only include if the change is specific and actionable.

Apply

If any finding has a concrete file edit, present all actionable edits via multi-select. Each option label is a one-line summary of the edit. Unchecked edits are discarded.

Apply selected edits.

Document Review Checklist

Review in priority order: A (highest impact) → B → C. The reviewer prompt specifies which levels to check. Project rules loaded in context override this checklist.

---

A. Accuracy

Issues where the document contains incorrect, contradictory, or incomplete information.

A1. Code-Document Accuracy

• Described behaviors match actual code implementation
• Parameter names, types, default values consistent with code
• Return values and error conditions accurately documented
• Described algorithms / processing steps consistent with implementation
• Version numbers, format identifiers, constants correct
• Value ranges and constraints accurate
• Enum values and meanings consistent with code definitions
• File format structures (byte offsets, field sizes, etc.) correct

A2. Internal Consistency

• Different sections describing the same concept agree with each other
• Constraints and rules consistent across the document (no contradictions like

"must be >= 0" in one section with a negative default in another)

• Same rule appearing in multiple places identical in meaning

A3. Completeness

• All public APIs / features documented
• Newly added features or parameters reflected in the document
• Removed or deprecated features marked accordingly
• Edge cases and limitations documented
• All conditional branches exhaustively covered (no undocumented "else" cases)
• Sequential steps complete with no missing intermediate steps
• Undefined behaviors identified (input combinations with no documented result)

A4. Reference Validity

Do not attempt to verify URL reachability.

• Internal cross-references point to existing sections or files
• External links / URLs well-formed and not obviously outdated
• Referenced file paths, tool names, command examples actually exist

---

B. Clarity & Structure

Improvements to readability, unambiguity, and organization.

B1. Ambiguity Detection

• No descriptions interpretable in multiple ways
• Conditional statements precise ("should" vs "must", "may" vs "will")
• Boundary conditions clearly stated (inclusive vs exclusive, "at least" vs "exactly")

B2. Simplification

Only flag when the same information is stated more than once in different sections.

• Redundant paragraphs or sections repeating the same information consolidated

B3. Logical Flow

• Information presented in logical order
• Related topics grouped together
• Forward references minimized

B4. Examples & Illustrations

• Existing examples correct and consistent with described behavior

B5. Terminology Consistency

• Terms used consistently throughout the document
• Terms match codebase usage (class names, function names, enum values)
• Abbreviations defined on first use

---

C. Formatting & Style

Polish and stylistic consistency.

C1. Formatting Consistency

• Headings, lists, tables formatted consistently
• Code snippets properly formatted and syntax-highlighted

C2. Grammar & Wording

• No grammatical errors or awkward phrasing
• Writing style consistent throughout
• Tone appropriate for target audience

C3. Section Organization

• Sections appropriately sized (not too long or too short)
• Table of contents (if present) consistent with actual sections
• Deprecated or obsolete sections cleaned up

---

Exclusion List

Project rules override this exclusion list.

1. Pure formatting preferences not affecting readability 2. Stylistic rewrites that don't improve clarity or accuracy 3. Suggestions based on assumed future requirements, not current content 4. Suggestions to add content beyond the document's stated scope 5. Removing default or verbose attributes from examples — examples in specs and tutorials are intentionally detailed to demonstrate available options

Judgment Matrix

Risk Level Assessment

Risk level is per-issue, not per-type — the same category (e.g., rename) can be low or high risk depending on scope and impact.

Handling by Risk Level

Special rule for "full" mode: issues that would change test baselines (screenshot comparisons, golden files) are always deferred for user confirmation, regardless of risk level.

Worth Fixing?

Code-checklist and doc-checklist define what to look for. This section defines whether to fix a discovered issue.

Decision principles

1. Must fix — The issue affects runtime correctness, safety, or security. 2. Fix when clear — The issue improves code quality (performance, simplification, architecture). Fix only when the solution is unambiguous and does not introduce new risk. Performance changes require both high confidence in semantic equivalence and a net benefit after weighing the gain against added code complexity. 3. Fix when inconsistent — The issue involves naming, initialization, comments, or file organization. Fix only when it violates project rules loaded in context or contradicts the surrounding code's established patterns. 4. Always skip — Pure style preferences (not violating any consistency rule), suggestions based on assumed future requirements rather than current code, and alternative implementation rewrites for stable code that has no correctness issue.

Exceptions

• Duplicate code extraction: fix when identical logic is clearly duplicated

(not by count threshold — judge by complexity and maintenance cost).

• Public API signature changes that are not bug fixes: fix only when justified

by clear benefit to API consumers. Always high risk.

• Test coverage gaps and regression risks are flagged, not fixed — report

them for the user's awareness rather than auto-fixing.

Anti-patterns (Do NOT Fix)

Patterns that frequently produce false positives. Skip unless there is strong evidence of an actual bug:

• Speculative optimizations — build system tweaks, caching additions, or

conditional guards with no proven failure or measured bottleneck.

• Documentation example "simplification" — removing attributes, parameters,

or steps from examples that are intentionally verbose for pedagogical purposes. This is NOT the same as removing redundant code.

• Behavior changes disguised as bug fixes — if a proposed fix changes

observable behavior (not just implementation details), verify the original behavior is actually a bug, not an intentional design choice. When intent cannot be confirmed from the diff context alone, flag but do not fix.

Local Review

Single-agent review for local changes. Reviews the diff, presents confirmed issues, and lets the user interactively choose which ones to fix.

References

---

Step 1: Scope

Determine the diff to review based on $ARGUMENTS and working tree state:

• Empty `$ARGUMENTS`, uncommitted changes exist: scope is

uncommitted changes only. Fetch with git diff HEAD (staged + unstaged tracked files). Also check for untracked files with git status --porcelain (?? lines) and read their contents for review.

• Empty `$ARGUMENTS`, no uncommitted changes: find the base branch by

checking common base branches in order: main, master. Use the first one that exists. Fetch the branch diff:

  git merge-base origin/{base_branch} HEAD
  git diff <merge-base-sha>

Also check for untracked files with git status --porcelain (?? lines).

• Commit hash (e.g., abc123): validate with git rev-parse --verify,

then git show.

• Commit range (e.g., abc123..def456 or abc123...def456): validate both

endpoints. Fetch the diff including both endpoints:

  git diff A~1..B

• File/directory paths: verify all paths exist on disk, then read file

contents.

If diff is empty → show usage examples and exit: /cr (uncommitted changes or current branch), /cr a1b2c3d, /cr a1b2c3d..e4f5g6h, /cr src/foo.cpp, /cr 123, /cr https://github.com/.../pull/123.

---

Step 2: Review

Review the diff. Apply code-checklist.md to code files, doc-checklist.md to documentation files. When changed lines depend on surrounding context, read the relevant sections or related definitions as needed. Untracked files have no diff — review their full contents as new code.

For each issue found:

• Provide a code citation (file:line + snippet) from the current tree.
• Self-verify by re-reading the code — confirm or withdraw.
• If a cited path/line no longer exists, locate the correct file/path via git diff --name-only or file search before reporting.

Output rule: only present the final confirmed issues to the user. Do not output analysis process, exclusion reasoning, or issues that were considered but ruled out.

---

Step 3: Filter

Consult judgment-matrix.md for risk level assessment, worth-fixing criteria, and special rules. Discard issues that are not worth reporting.

If no issues remain after filtering → report "no issues found" and exit.

---

Step 4: Report and fix

Present a summary of what was reviewed, then list all confirmed issues. Ask which ones to fix via multi-select. Each option's label is the issue summary (e.g., [risk] file:line — description). Follow the grouping rule in SKILL.md: ≤4 items → one question; >4 items → group by priority or category (each group ≤4 options), then present all groups as separate questions in a single prompt.

If the user selects any issues, apply the fixes.

---

Step 5: Checklist evolution

Review all confirmed issues from this session. If any represent a recurring pattern not covered by the current checklist, read checklist-evolution.md and follow its steps.

PR Review

PR review uses Worktree mode — fetch the PR branch locally so review can read related code across modules, at the exact version of the PR branch. This is critical for review accuracy.

References

---

Step 1: Create worktree

If $ARGUMENTS is a URL, extract the PR number from it.

Clean up leftover worktrees from previous sessions:

for dir in /tmp/pr-review-*; do
    [ -d "$dir" ] || continue
    n=$(basename "$dir" | sed 's/pr-review-//')
    git worktree remove "$dir" 2>/dev/null
    git branch -D "pr-${n}" 2>/dev/null
done

Validate PR target:

gh repo view --json nameWithOwner --jq .nameWithOwner
gh pr view {number} --json headRefName,baseRefName,headRefOid,state,body

Record OWNER_REPO. Extract: PR_BRANCH, BASE_BRANCH, HEAD_SHA, STATE, PR_BODY. If either command fails, inform the user and abort. If $ARGUMENTS is a URL containing {owner}/{repo}, verify it matches OWNER_REPO. If not, inform the user that cross-repo PR review is not supported and abort. If STATE is not OPEN, inform the user and exit.

If current branch equals `PR_BRANCH` and HEAD equals `HEAD_SHA`, skip worktree creation — the code is already local.

Otherwise, create a worktree:

git fetch origin pull/{number}/head:pr-{number}
git worktree add --no-track /tmp/pr-review-{number} pr-{number}
cd /tmp/pr-review-{number}

If worktree creation fails, inform the user and abort.

---

Step 2: Collect diff and context

git fetch origin {BASE_BRANCH}
git merge-base origin/{BASE_BRANCH} HEAD
git diff <merge-base-sha>

If the diff exceeds 200 lines, first run git diff --stat to get an overview, then read the diff per file using git diff -- {file} to avoid output truncation.

If diff is empty → clean up worktree and exit.

Fetch existing PR review comments for de-duplication:

gh api repos/{OWNER_REPO}/pulls/{number}/comments

---

Step 3: Review

Internal analysis:

1. Based on the diff, read relevant code context as needed to understand the change's correctness (e.g., surrounding logic, base classes, callers). 2. Read PR_BODY to understand the stated motivation. Verify the implementation actually achieves what the author describes. 3. Apply code-checklist.md to code files, doc-checklist.md to documentation files. Use judgment-matrix.md to decide whether each issue is worth reporting. 4. Check whether issues raised in previous PR comments have been fixed. 5. For each potential issue, perform a second-pass verification: re-read the surrounding code and check — is there a guard or early return elsewhere that handles this? Does the call chain guarantee preconditions? Am I misunderstanding lifetime or ownership? 6. Discard all ruled-out issues. Keep only issues confirmed to exist. 7. De-duplicate confirmed issues against existing PR comments.

Output rule: only present the final confirmed issues to the user. Do not output analysis process, exclusion reasoning, or issues that were considered but ruled out.

---

Step 4: Clean up and report

If a worktree was created, clean it up:

cd -
git worktree remove /tmp/pr-review-{number}
git branch -D pr-{number}

Present results to user:

• Summary: one paragraph describing the purpose and scope of the change.
• Overall assessment: code quality evaluation and key improvement directions.
• Issue list (or "no issues found" if clean).

If no issues → ask whether to submit an approval review AND merge the PR:

1. Submit Approval:

   gh api repos/{OWNER_REPO}/pulls/{number}/reviews --input - <<'EOF'
   {
     "commit_id": "{HEAD_SHA}",
     "event": "APPROVE"
   }
   EOF

2. Merge (squash):

   gh pr merge {number} --squash --delete-branch

If the user declines, do nothing. Skip the comment submission below.

If issues found → present confirmed issues to user in the following format:

{N}. [{priority}] {file}:{line} — {description of the problem and suggested fix}

Where {priority} is the checklist item ID (e.g., A2, B1, C7). Then ask the user to select which issues to submit using a single multi-select question where each option's label is the issue summary (e.g., [A2] file:line — description). User checks multiple options in one prompt. Unchecked issues are skipped.

Must use gh api + heredoc. Do not use gh pr comment, gh pr review, or any command that creates non-line-level comments:

gh api repos/{OWNER_REPO}/pulls/{number}/reviews --input - <<'EOF'
{
  "commit_id": "{HEAD_SHA}",
  "event": "COMMENT",
  "comments": [
    {
      "path": "relative/file/path",
      "line": 42,
      "side": "RIGHT",
      "body": "Description of the issue and suggested fix"
    }
  ]
}
EOF

• commit_id: HEAD SHA of the PR branch
• path: relative to repository root
• line: line number in the new file (right side of diff). Must be

determined during Step 3 by reading the actual file in the worktree — do not derive from diff hunk offsets.

• side: always "RIGHT"
• body: concise, in the user's conversation language, with a specific fix

suggestion when possible

Summary of issues found / submitted / skipped.

---

Step 5: Checklist evolution

Review all confirmed issues from this session. If any represent a recurring pattern not covered by the current checklist, read checklist-evolution.md and follow its steps.

Teams Review

You are the coordinator. Create an Agent Team and dispatch reviewer, verifier, and fixer agents. Never modify files directly. Read code only for arbitration, diagnosis, and fix verification.

Always process all auto-fixable issues before involving the user. Do NOT pause to ask the user anything until Confirm (Phase 5) or Report (Phase 6).

The reviewer–verifier adversarial pair is the core quality mechanism: reviewers find issues, verifiers challenge them. This two-party check significantly reduces false positives. Reviewers and verifiers MUST NOT see each other's output or share conversation history.

Input from SKILL.md

• FIX_MODE: low | low_medium | full

References

Flow

Scope → Review → Filter → Fix/Validate → Confirm → Report

• Filter routes auto-fixable issues to Fix/Validate; remaining go to Confirm.

If nothing to fix or confirm, skip directly to Report.

• Confirm ↔ Fix/Validate loop until no pending issues remain.

---

Phase 1: Scope

Determine the diff to review based on $ARGUMENTS:

• Empty arguments: find the base branch by checking common base branches

in order: main, master. Use the first one that exists. Fetch the branch diff:

  git merge-base origin/{base_branch} HEAD
  git diff <merge-base-sha>

• Commit hash (e.g., abc123): validate with git rev-parse --verify,

then git show.

• Commit range (e.g., abc123..def456 or abc123...def456): validate both

endpoints. Fetch the diff including both endpoints:

  git diff A~1..B

• File/directory paths: verify all paths exist on disk, then read file

contents.

If diff is empty → show usage examples and exit: /cr (uncommitted changes or current branch), /cr a1b2c3d, /cr a1b2c3d..e4f5g6h, /cr src/foo.cpp, /cr 123, /cr https://github.com/.../pull/123.

Associated PR comments

If gh is available, check whether the current branch has an open PR:

gh pr view --json number,state --jq 'select(.state == "OPEN") | .number' 2>/dev/null

If an open PR exists, fetch its line-level review comments:

gh api repos/{owner}/{repo}/pulls/{number}/comments

Store as PR_COMMENTS for verification in the review step.

Build baseline

Skip if doc-only. If no build/test commands can be determined, warn that fix validation will be skipped. Otherwise run build + test. Fail → abort.

Module partition

Partition files in scope into review modules for parallel review. Each module is a self-contained logical unit. Split large files by section/function group; group related small files together. Classify each module as code, doc, or mixed.

Issue tracking

The coordinator tracks all issues in memory throughout the session. Each issue has:

• Brief description
• Status: pending | approved | fixed | failed | skipped
• Risk: low | medium | high
• File: file path:line
• Proposed fix (medium/high risk only)

---

Phase 2: Review

Team setup

Create a team for the session.

• One reviewer agent (reviewer-N) per module.
• One verifier agent (verifier), shared across all modules.

Module merging: if the total diff is ≤1000 changed lines AND ≤20 files, merge all modules into a single reviewer. The overhead of multiple agents (startup, coordination, forwarding) outweighs the parallelism benefit at this scale.

Reviewer prompt

Stance: thorough — discover as many real issues as possible, self-verify before submitting.

Each reviewer receives:

• Scope: file list + changed line ranges for its module. Reviewers fetch

diffs and read additional context themselves as needed — coordinator does NOT pass raw diff or file contents.

• Checklist: code-checklist.md for code, doc-checklist.md for doc, both

for mixed.

• Evidence requirement: every issue must have a code citation (file:line + snippet) from the current tree.
• Checklist exclusion: see the exclusion section in the corresponding

checklist. Project rules loaded in context take priority.

• Self-check: before submitting, re-read the relevant code and verify each

issue. Mark as confirmed or withdrawn. Only submit confirmed issues. If a cited path/line no longer exists, locate the correct file/path via git diff --name-only or file search before reporting.

• Output format: [file:line] [A/B/C] — [description] — [key lines]

PR comment reviewer (when PR_COMMENTS exist): one additional agent to verify PR review comments against current code. Same output format, same verification pipeline.

Verification (pipeline)

Stance: adversarial — default to doubting the reviewer, actively look for reasons each issue is wrong. Reject with real evidence, confirm if it holds up. This step is mandatory — the coordinator MUST NOT skip it or perform verification itself. Exception: if every reviewer explicitly reports zero issues (LGTM / no issues found), skip verification and proceed directly to Phase 3.

The verifier runs as a pipeline — it does not wait for all reviewers to finish. As each reviewer sends a report via SendMessage, the coordinator MUST forward it to the verifier immediately. Forwarding rules:

• Quote verbatim: wrap the reviewer's original SendMessage content in a

quote block and send it as-is.

• No rewriting: do not summarize, reorganize, merge multiple reviewer

reports into one message, or add coordinator commentary.

• One forward per reviewer: each reviewer report is a separate message to

the verifier.

• Completion signal: after forwarding the last reviewer's report, send a

separate message to the verifier stating: "All N reviewer reports have been forwarded. Please finalize your verdicts for all issues above." (Replace N with the actual reviewer count.)

Include the following verbatim in every verifier's prompt:

You are a code review verifier. Your stance is adversarial — default to doubting the
reviewer's conclusion and actively look for reasons why the issue might be wrong. Your
job is to stress-test each issue so that only real problems survive.

For each issue you receive:

1. Read the cited code (file:line) and sufficient surrounding context.
2. Actively try to disprove the issue: Is the reviewer's reasoning flawed? Is there
   context that makes this a non-issue (e.g., invariants guaranteed by callers, platform
   constraints, intentional design)? Does the code actually behave as the reviewer
   claims? Look for the strongest counter-argument you can find.
   - For cross-module "inconsistency" claims: read the type definitions at both
     locations — different libraries may use different conventions (coordinate systems,
     sign conventions) that make surface-level differences correct. REJECT if the
     reviewer's evidence is limited to "these two snippets look different" without
     confirming both operate under the same conventions.
3. Output for each issue:
   - Verdict: REJECT or CONFIRM
   - Reasoning: for REJECT, state the concrete counter-argument. For CONFIRM, briefly
     note what you checked and why no valid counter-argument exists.

Important constraints:
- Your counter-arguments must be grounded in real evidence from the code. Do not
  fabricate hypothetical defenses or invent caller guarantees that are not visible in
  the codebase.
- A CONFIRM verdict is not a failure — it means the reviewer found a real issue and
  your challenge validated it.
- Reviewer reports arrive incrementally via the coordinator. Do NOT produce a final
  summary until the coordinator explicitly tells you all reports have been forwarded.
  Process each report as it arrives, but wait for the completion signal before
  concluding.
- The coordinator will tell you the total number of reviewers in the completion
  signal. If you have not received that many reports, do NOT finalize — wait for
  the remaining forwards.

After review

Missing report detection: if a reviewer's system notification shows "completed" but the coordinator has not received a SendMessage report from that reviewer, immediately send a follow-up message to the reviewer requesting its report. Do not wait or assume the report was lost — the agent may have exhausted its turn limit before sending.

Before entering Phase 3, confirm: (1) all reviewers have submitted their final reports; (2) the verifier has given a CONFIRM/REJECT verdict for every forwarded finding, OR all reviewers reported zero issues and verification was skipped.

Keep all agents alive — do not close any agents mid-session. Reviewers are reused as fixers in Phase 4. All agents are cleaned up in Phase 6 (Report).

---

Phase 3: Filter — coordinator only

Your stance here is neutral — trust no single party. Treat reviewer reports and verifier rebuttals as equally weighted inputs. Use your project-wide view to consider cross-module impact, conventions, and architectural intent that local reviewers may miss.

3.1 De-dup

Remove cross-reviewer duplicates (same location, same topic).

3.2 Existence check

3.3 Risk level

Consult judgment-matrix.md for risk level assessment, worth-fixing criteria, handling by risk level, and special rules.

Fix approach (Medium/High only): specify the chosen approach and reasoning. Record in the issue's Proposed field. Low risk: single obvious fix, no guidance.

3.4 Route

All confirmed issues are recorded with risk level.

• Cross-module impact: if a fix requires updates outside the fixer's module,

add it to the current fix queue and assign to the appropriate fixer.

Always auto-fix eligible issues first — do NOT present pending issues to the user before all auto-fixable issues have been processed and validated. Phase 4 if auto-fix queue is non-empty. Otherwise jump to Phase 5 if pending issues exist, or Phase 6 if none.

---

Phase 4: Fix/Validate

Fix

Stance: precise — apply each fix completely and correctly, never expand scope. The coordinator MUST NOT apply fixes directly.

Agent assignment: reuse reviewers as fixers when available — each reviewer already has context on the files it reviewed. The coordinator MUST assign by explicit file list and ensure a file is owned by only one fixer at a time:

• Issue in a file that a reviewer already read → assign to that reviewer.
• Otherwise → create a fixer agent (or fixer-cross for cross-module issues).
• Multi-file renames → single atomic task assigned to one agent.

One agent may receive multiple fix tasks if it covers several files. Avoid assigning the same file to multiple agents to prevent concurrent edit conflicts.

Each fixer receives:

• Issue description + file path(s) + line range(s) (fixers read files themselves)
• Fix approach from Proposed field (Medium/High risk)
• Fixer rules (include verbatim in every fixer prompt):

Fix rules:
1. After fixing each issue, immediately: git commit --only <files> -m "message"
2. Only modify files explicitly assigned by the coordinator. Never use git add .
3. If a fix requires changes to unassigned files, stop and report to the coordinator
   for re-assignment.
4. Commit message: English, under 120 characters, ending with a period.
5. When in doubt, skip the fix rather than risk a wrong change.
6. Do not run build or tests.
7. Do not modify public API function signatures or class definitions (comments are OK),
   unless the coordinator's issue description explicitly requires an API signature fix.
8. After each fix, check whether the change affects related comments or documentation
   within your assigned files (function/class doc-comments, inline comments describing
   the changed logic). If so, update them in the same commit as the fix.
   Cross-module documentation updates (README, spec files, other modules) are handled
   separately by the coordinator.
9. When done, report the commit hash for each fix and list any skipped issues with
   the reason for skipping.

Each fixer commits per issue (one commit per fix — never combine multiple issues into a single commit).

Verify fixes (coordinator)

Wait for all fixers. Before running build + test, the coordinator reads each fixer's commit diff and verifies: 1. The fix correctly addresses the original issue 2. No new issues introduced (naming inconsistencies, missing updates in surrounding code, logic errors) 3. Fix scope matches the issue — no unintended changes

If a problem is found, send the fixer a correction request with specific details (max 1 retry). If the retry fails or the fixer is unavailable, revert and mark failed.

Build/test validate

Run build + test.

Revert scope: only revert commits produced by fixers in this phase. Never revert commits unrelated to the current fixes — other users or tools may commit concurrently during the fix phase. Identify fixer commits by the commit hashes reported by fixers; any other commits on the branch are out of scope.

• Skip if no build/test commands available or doc-only modules.
• Pass → mark issues fixed.
• Fail → bisect among fixer commits only to find the failing commit, revert

it, re-validate remaining before blaming others (one bad commit may cause cascading failures). Per failing issue: retry via the original fixer agent with failure details (max 2 retries), or revert and mark failed.

After validation

If Phase 5 approves further fixes, reuse existing reviewers as fixers (or create new fixer agents if needed) and re-enter Phase 4.

---

Phase 5: Confirm

Present pending + failed issues grouped by risk (high → low), sorted by file path within each group: [number] [file:line] [risk] [reason] — [description]

Then present issues via multi-select. Each option label is the issue summary (e.g., [risk] file:line — description). Checked → approved, unchecked → skipped.

If the user replies with a bulk instruction (e.g., "fix all", "skip the rest"), apply it only to issues at or below the current FIX_MODE threshold. Issues above the threshold still require individual confirmation.

• All skipped → Phase 6.
• Any approved → Phase 4 (Fix/Validate). After validation, if more

pending/failed remain, return here (Phase 5). If nothing remains, proceed to Phase 6.

---

Phase 6: Report

Send shutdown_request to all remaining agents in parallel (single message with multiple SendMessage calls). Then call TeamDelete immediately without waiting for individual shutdown responses.

Summary:

• Issues found / fixed / skipped / failed
• Rolled-back issues and reasons
• Final test result
• Issues from PR comments (when PR_COMMENTS existed)
• Note: "To verify fix quality, run /cr again."

Checklist evolution

Review all confirmed issues from this session. If any represent a recurring pattern not covered by the current checklist, read checklist-evolution.md and follow its steps.