Conducty Code Review

Conducty Code Review — Whole-Branch Review

The in-cycle reviewers in [[conducty-execute]] (spec-review, full-review) verify each prompt against its own acceptance criteria. They do not see the branch as a unit. conducty-code-review does: it reads the diff between the branch and its base, looks at the cumulative change as a single artifact, and produces a holistic review note in the vault.

[!important] Read [[conducty-obsidian]] first Output goes to the vault as Code Reviews/Code Review YYYY-MM-DD HHmm.md, linked from the plan note that produced the changes.

When to Use

• After a plan finishes and the changes are about to ship
• Before [[conducty-ship]] — code-review is a precondition for ship
• On request: "review my changes", "review this PR", "is this branch ready"
• On a PR URL — fetch the diff via gh pr diff <id> and review

Skip when:

• Plan is still mid-execution (use checkpoint instead)
• Single-prompt review is sufficient — that's spec-review/full-review inside [[conducty-execute]]

Workflow

Step 1: Establish the Diff

# Local branch vs main
git diff $(git merge-base HEAD main)..HEAD

# Or for a PR
gh pr diff <pr-number>

Record:

• Base ref + tip ref + commit count
• Files changed (count + list)
• Lines added/removed
• The active plan note(s) that produced these changes (read from [[Plans Index]] — pick plans whose execution window matches the commit range)

If the diff is huge (>1000 lines or >40 files), warn the user and ask whether to slice the review (per-module, per-commit, or per-prompt).

Step 2: Anchor in the Plan(s)

Read the plan note(s) that produced the changes. Note:

• The acceptance criteria for each prompt
• The no-go zones declared
• Any failure patterns logged during execution

The diff should match the plan. Anything in the diff not in the plan is scope creep and gets flagged.

Step 3: Walk the Diff With Five Lenses

Apply each lens to the full diff, not file-by-file. Each lens produces findings.

Lens 1 — Spec alignment:

• Did the branch actually implement every acceptance criterion across all plan prompts?
• Anything in the diff that wasn't in any prompt? (scope creep)
• Anything in the no-go zones that got touched anyway?

Lens 2 — Correctness:

• Logic errors, off-by-one, null/undefined handling, race conditions
• Are error paths handled or swallowed?
• Boundary cases: empty inputs, very large inputs, concurrent access
• Idempotency where it matters (retries, requeue, replays)

Lens 3 — Security:

• Input validation at boundaries (user input, external APIs, deserialization)
• Auth/authz enforcement on every new endpoint or sensitive action
• Secrets in code or logs (grep -nE 'sk_|api_key|password|secret' diff)
• SQL/command injection, XSS, SSRF, path traversal
• Dependency additions — new package.json/Cargo.toml/requirements.txt entries flagged for vuln review

Lens 4 — Architecture & coupling:

• Did the change respect the project's bounded contexts? (Read [[Context {Project} Architecture]] if it exists)
• New cross-module imports that weren't there before
• Circular dependencies introduced
• New singletons / global state
• Any module growing past a clean responsibility

Lens 5 — Tests & maintainability:

• Tests added match the criteria? Or just smoke tests?
• Tests verify behavior, not mock behavior
• Failure messages diagnostic
• Dead code / commented-out blocks left behind
• TODO/FIXME comments without an owner or follow-up issue
• Naming clarity; comments justify why, not what