tercel/code-forge/review

Code Forge — Review

⚡ Execution Entry Point

@../shared/execution-entrypoint.md

For this skill: start at Step 1 (Determine Review Mode). If you catch yourself about to say "falling back to manual review", STOP and go to the indicated step.

Comprehensive code review against reference documents and engineering best practices. Covers functional correctness, security, resource management, code quality, architecture, performance, testing, error handling, observability, maintainability, backward compatibility, and dependency safety.

The 15 dimensions answer one question: is the code that exists correct? They do not answer the orthogonal question: does every required behavior exist, and is each backed by a passing test? — the gap where "works in the demo" hides. The Acceptance Gate (see §Acceptance Gate) closes it: it runs the test suite for real and reconciles every P0/P1 acceptance criterion against a named passing test, hard-blocking merge on any uncovered P0 or any failing test. Pass --quick to skip the gate for a mid-work, analysis-only review.

Supports four modes:

• Feature mode: Review a single feature against its plan.md
• Project mode: Review the entire project against planning documents or upstream docs
• Feedback mode: Evaluate and respond to incoming code review comments (--feedback)
• GitHub PR mode: Post a 15-dimension review as a comment on a GitHub PR (--github-pr)

When to Use

• Feature implementation is complete or nearly complete
• Want to verify code quality before creating a PR
• Need a structured review against the original plan or documentation
• Want a holistic project-level quality check
• Received code review feedback and need to evaluate/respond to it (--feedback)
• Want to post a code review directly to a GitHub PR for team visibility (--github-pr)

When NOT to Use

This skill reviews source code only. Do not use it — and do not let a bare "review" in the request pull it in — when the review target is text rather than code:

Mixed request ("review this feature's code and its spec"): run this skill on the code part only, and say which part was routed elsewhere.

Examples

/code-forge:review user-auth             # Review a specific feature
/code-forge:review --project             # Full project review
/code-forge:review                       # Auto-detect features to review
/code-forge:review --feedback            # Evaluate incoming review comments
/code-forge:review --github-pr 123       # Post review to GitHub PR #123
/code-forge:review user-auth --save      # Review and save report to disk
/code-forge:review user-auth --quick     # Static 15-dim review only — skip the Acceptance Gate (no test run)

Workflow

Config → Determine Mode → Locate Reference → Collect Scope
  → Module Grouping: build scope dep-graph → pick axis from PA.2.2 pattern + cut_ratio
       · S1 by directory (plugins / components / independent packages)
       · S2 by call chain (layered / MVC / hexagonal — chain stays inside one agent)
       · S3 no split      (one dense blob — cutting it would split the evidence)
       · > 48 files or > 8 groups after boundary-safe merging ⇒ guardrail, ask the user
  → Fast path (< 3 files, OR 1 group, OR S3 ≤ 12 files):  Single sub-agent (all 15 dims)
  → Layered path (≥ 3 files AND ≥ 2 module groups):
       • Parallel per-module agents
           · Primary: full review (D1–D4, D6, D8–D9) on their own module files
           · Tier-2:  depth-1 expansion into cross-module callees that are ALSO in the diff
             (closes the blind spot where caller and callee live in different modules but both in scope)
       • Cross-module agent
           · D5, D7, D10–D15 + CROSS_MODULE_CONSISTENCY + SECOND_ORDER_REVIEW
           · Consumes aggregated METHOD_CHAINS (with X:-prefixed tier-2 inlined steps visible)
→ Merge + Deduplicate + Validate
→ Acceptance Gate (run tests fresh + reconcile every P0/P1 criterion → a named passing test; P0 gap or red test ⇒ BLOCKED)
→ Display Report → Update State → Summary

Context Management

The review analysis is offloaded to sub-agents to handle large diffs without exhausting the main context. For changes spanning multiple modules, parallel per-module agents each hold a bounded, module-scoped context window — while still being able to see one level into cross-module callees that are part of the same diff (tier-2 expansion). This closes the cross-module defensive-gap blind spot without re-introducing the full-diff context dilution that causes "whack-a-mole" defects.

Project Analysis

Before reviewing code, understand the project's architecture and tech stack:

@../shared/project-analysis.md

Execute PA.1 (Project Profile) and PA.2 (Architecture Analysis). This informs:

• Which review dimensions apply (D14 Accessibility only for frontend)
• Language-specific checks (Rust unsafe blocks, Go unchecked errors, Python type hints)
• Architecture-specific checks (layer boundary violations, circular dependencies)
• The Project Profile determines which patterns are expected vs. suspicious
• Module grouping strategy (Step 3F.3 / 3P.3). PA.2.2's architecture pattern decides how the review scope is split across sub-agents — by directory, by call chain, or not at all. Carry architecture_pattern and PA.2.3's module-level dependency edges forward into grouping; do not re-derive them from path strings.

Review Severity Levels

All issues use a 4-tier severity system, ordered by merge-blocking priority. Severity is assigned strictly per the definitions in `references/suppression-gates.md` §Severity Levels and re-verified by Gate 3 before the finding is emitted.

Full severity definitions, gate-drop rules, and speculative-phrasing rule live in references/suppression-gates.md §Severity Levels. Every review sub-agent MUST read it before emitting findings — with one exception: the acceptance reconciliation agent (3G.3) never reads it, since its polarity is inverted.

Call-Graph Discipline (Mandatory Pre-Analysis)

Before applying any dimension, the review sub-agent MUST build a call graph for every public method in the review scope. This is procedural — dimensions are applied to the graph, not to the surface method body. Surface reading is blind to a class of bugs where a method looks correct yet silently skips a validation, a state mutation, or a raise.

Three-tier expansion rule (summary):

• Tier 1 — same-module private helpers → FULL recursive inlining, prefix steps with helper_name →
• Tier 2 — cross-module callees whose file is ALSO in the diff → depth-1 inlining, prefix with X:Module.method →. Capped at 8 distinct files per agent (the tier-2 expansion budget); spend the budget on the callees with the most call sites, defer the rest to TIER2_DEFERRED.
• Tier 3 — stdlib, third-party, or helpers outside the review scope → ext_call leaf, no expansion

The graph enumerates every validation, mutation, raise, iterate/subscript/deserialize (external-input path), including steps inside all inlined bodies.

Why tier 2 is capped and tier 1 is not. Tier-1 inlining is bounded by the agent's own group (4 primary files, 6 in a 33+-file scope, occasionally more after a count-driven merge — see 3F.3.4) and is what catches defensive gaps inside that group — never cap it. Tier-2 eligibility is the whole diff, so on a large change it grows without bound and would put the entire diff into every per-module agent's context, re-creating the dilution the layered path exists to prevent. Deferred tier-2 callees are forwarded to the cross-module agent, which has the global view and owns cross-boundary analysis by definition — the work moves, it is not dropped. Full rules: references/call-graph-discipline.md §Tier-2 expansion budget.

Full protocol — mandatory reading for every agent that builds or consumes a call graph (fast-path, per-module, and cross-module; NOT the acceptance agent): references/call-graph-discipline.md. It defines the inlining convention, the complete what-to-enumerate list, and the under-flagging anti-rationalization table (countering "skip inlining, the method is short" / "stop at _private_helper" / "cross-module = ext_call leaf"). Skipping inlining produces a pre-analysis failure; the orchestrator rejects chains where a same-scope private helper appears as an opaque call step.

Output format: the METHOD_CHAINS section defined in the agent's role-specific format file (references/format-single.md / format-per-module.md / format-cross-module.md). An empty or missing METHOD_CHAINS → the orchestrator rejects the report and re-invokes.

Finding Suppression Gate (Mandatory Pre-Emission Check)

Before the sub-agent writes ANY finding into the output YAML, it MUST route the finding through five gates. Gates guard against the speculative / quota-filling / unverified-claim noise that the dimensional framework otherwise incentivizes.

Gate summary (full definitions in `references/suppression-gates.md`):