name: review description: > Use ONLY when reviewing SOURCE CODE — a code change, a feature implementation, a whole repository, or a GitHub pull request. Runs 15-dimension code quality analysis PLUS an Acceptance Gate (runs the test suite fresh and reconciles every P0/P1 acceptance criterion against a named passing test, hard-blocking merge on any uncovered P0 or failing test) for features or entire projects (generate mode), structured evaluation and response to incoming code review comments (feedback mode via --feedback flag), or automated PR review posted as a GitHub comment (--github-pr flag). Do NOT use for reviewing documents or prose — PRD, SRS, tech design, feature spec, README, plan, proposal, paper, or any "review this .md / this doc / these docs" request, in any language (e.g. "review 文档", "审查文档", "看下这篇设计文档"). Document review belongs to spec-forge:review (specs and project docs), theory-forge (theory/academic papers), or plain reading — the word "review" alone is NOT a trigger for this skill. metadata: version: "1.0.1"

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`):

Output requirement — `evidence` field:

• blocker / critical: evidence is MANDATORY, non-empty — concrete trigger input, observable wrong behavior, and (for D1/D2) the trust-boundary argument.
• warning: evidence SHOULD be present when non-obvious; REQUIRED when the finding makes a Gate 5 trigger claim.
• suggestion: evidence MAY be present; REQUIRED when the finding makes a Gate 5 trigger claim.

Gate 5 trigger phrases (any of these in title/description requires a verification artifact): zero references, never called, dead code, unused, only used in, only referenced in, duplicates X, copy of X, parallel implementation, reimplements, grep returns/shows/finds, N lines exceed. Paraphrases of grep output are insufficient — paste the actual matched lines. Dead-code claims require cross-directory grep (src + tests).

Full gate definitions, drop rules, and the over-flagging anti-rationalization table (countering "the input could be malformed if an attacker controls it" / "I haven't found anything in D8 yet — I should produce something" / "It's just a rename for clarity — that's a legitimate suggestion" etc.) live in `references/suppression-gates.md`. 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.

Acceptance Gate (Mandatory — Dynamic Verification + Coverage Reconciliation)

This gate runs on every review unless `--quick` is passed. It is executed in Step 3G, after the dimensional review and before the report is displayed.

Why it exists. The 15 dimensions and the call-graph discipline verify that the code that exists is correct. Nothing in them verifies that every required behavior exists and is backed by a test that actually ran and passed. That is the precise gap between "works in a demo" and "passes product acceptance" — happy-path code that the reviewer confirms is correct, while the boundary / error / negative / NFR behaviors the spec required were never implemented or never tested. The Acceptance Gate is the only check that asks the second question.

Polarity warning — this gate is the OPPOSITE of the Finding Suppression Gate. The five suppression gates and the CANDIDATE_INVENTORY scratchpad exist to DROP unproven findings on the correctness review (where over-flagging is the failure mode). The Acceptance Gate's job is to REPORT ABSENCE (where under-flagging is the failure mode). The suppression gates, the scratchpad, and the Gate-5 evidence-artifact requirement DO NOT apply to acceptance findings. A required behavior with no passing test is itself the finding — never drop it for lack of a reachable-trigger argument, a grep artifact, or a "concrete observable wrong behavior." The absence IS the evidence.

Two parts.

1. Dynamic verification (absorbs the `code-forge:verify` discipline). Actually run the project's test suite, fresh, and read the complete output. No completion claim from memory, from a previous run, or from "the impl agent said it passed." A skipped / todo / pending test never counts as passing.
2. Coverage reconciliation. Enumerate the REQUIRED behaviors from the authoritative source and map each to a named test that exists, executed, passed, and demonstrably exercises that behavior. Unmapped P0 ⇒ blocker; unmapped P1 ⇒ critical.

Authoritative source resolution (first found wins). Record which one was used.

1. docs/{feature}/srs.md — every P0/P1 functional requirement's acceptance criteria, plus measurable NFRs.
2. docs/{feature}/test-cases.md — every P0/P1 test case (TC-ID + title + expected result).
3. Feature mode: plan.md ## Acceptance Criteria. Project mode: the docs-backed criteria located in Step 2P, or docs/features/*.md ## Acceptance Criteria.
4. None found → bare acceptance: skip reconciliation, still run dynamic verification, and note that no authoritative source was found (point the user at /spec-forge:srs or /spec-forge:test-cases).

Coverage rule. A required behavior is covered ONLY when a test (a) exists, (b) executed in this run, (c) passed, and (d) demonstrably exercises the behavior — matched by TC-ID embedded in the test name, or by behavior/assertion when no TC-ID convention exists. A passing test that only asserts the happy path (or "no error thrown") while the behavior requires boundary / error / negative assertions is weak — treat as uncovered for that sub-aspect. to-be-automated never counts.

Verdict. ACCEPTANCE_GATE = BLOCKED when any test failed, OR any P0 behavior is uncovered/weak, OR no runnable suite exists while P0 behaviors are defined. Otherwise PASS (bare acceptance with a green suite is PASS (unreconciled)). When BLOCKED, the orchestrator sets overall `merge_readiness = rework_required` regardless of the dimensional findings, and the gate failures are listed at the TOP of the report — above every dimensional finding. The gate result renders as the first section of the report (see references/report-template.md §Acceptance Gate).

Review Dimensions Reference

The 15 dimensions are split across two reference files by agent role — each agent reads only the one it needs:

Quick summary by tier:

• Tier 1 (Must-Fix): D1 Functional Correctness, D2 Security, D3 Resource Management
• Tier 2 (Should-Fix): D4 Code Quality, D5 Architecture, D6 Performance, D15 Simplification & Anti-Bloat, D7 Test Coverage
• Tier 3 (Recommended): D8 Error Handling, D9 Observability, D10 Standards
• Tier 4 (Nice-to-Have): D11 Backward Compat, D12 Maintainability, D13 Dependencies, D14 Accessibility (frontend only)

Dimension Application Rules:

• D1–D3: Always apply. Potential merge blockers.
• D4–D7, D15: Always apply. Should-fix items.
• D8–D10: Always apply. Flag as warnings/suggestions.
• D11–D13: Always apply but expect mostly suggestions.
• D14: Apply ONLY if project_type is "frontend" or "fullstack".
• D15 (Simplification & Anti-Bloat): Always apply. Mandatory in every mode (feature, project, GitHub PR). This is the primary defense against incremental bloat from skill-driven workflows — sub-agents MUST grep for existing equivalents before accepting any new symbol, MUST verify external callers exist for every new top-level symbol, and MUST flag scope creep beyond plan.md. Never skip D15 even on small changes.

When spawning review sub-agents, instruct each to read ONLY its role's dimension file per the table above. Never point a per-module agent at dimensions-cross.md or vice versa — the split is what keeps each agent's context bounded.

Detailed Steps

@../shared/configuration.md

Step 1: Determine Review Mode

Parse the user's arguments to determine which mode to use.

1.0 Scope Guard — Code Only (check this FIRST)

Before any mode detection, confirm the review target is source code. If the request names a document as the thing to review — a .md/.txt/.rst/.docx/.pdf file, a PRD, SRS, tech design, feature spec, README, plan, proposal, or paper — STOP immediately. Do not run any dimension, sub-agent, or gate.

Print one line and end the skill:

code-forge:review reviews source code. For document review use spec-forge:review

(specs/docs), theory-forge (theory papers), or just ask me to read it directly.

Exceptions that are still in scope (do NOT stop):

• A .md file passed as the reference document for a code review (plan.md, feature

spec used as the acceptance baseline) — the review target is still the code.

• Docs-as-code changes inside a diff being reviewed (a PR that touches both code and docs).

If the target is ambiguous (e.g. bare "review this" with no code context), ask one question instead of assuming code.

1.0a --github-pr Flag Provided

If the user passed --github-pr (e.g., /code-forge:review --github-pr or /code-forge:review --github-pr 123):

→ GitHub PR Mode — Read and follow skills/review/github-pr-workflow.md. Do NOT continue with the steps below.

1.0b --feedback Flag Provided

If the user passed --feedback (e.g., /code-forge:review --feedback or /code-forge:review --feedback #123):

→ Feedback Mode — Read and follow skills/review/feedback-workflow.md. Do NOT continue with the steps below.

1.0c --quick Flag Provided

If the user passed --quick, record quick_mode = true. This is orthogonal to the mode (it composes with feature/project mode) and instructs Step 3G to skip the Acceptance Gate entirely — the review runs the static 15-dimension analysis only, with no test execution and no coverage reconciliation. Continue with the normal mode detection below.

1.1 Feature Name Provided

If the user provided a feature name (e.g., /code-forge:review user-auth):

→ Feature Mode — go to Step 2F

1.2 --project Flag Provided

If the user passed --project (e.g., /code-forge:review --project):

→ Project Mode with scope = "full" — go to Step 2P

1.3 No Arguments

If no arguments provided:

1. Scan both {output_dir}/*/state.json and .code-forge/tmp/*/state.json for all features
2. Filter to features with at least one "completed" task
3. Build choice list:

• If completed features exist: include each as an option, plus "Review entire project" as the last option
• If no completed features: go to Project Mode with scope = "changes" automatically

1. If only one option (project review): go to Project Mode with scope = "changes" automatically
2. If multiple options: use AskUserQuestion to let user select

• If user selects "Review entire project": go to Project Mode with scope = "changes"

Step 2F: Feature Mode — Locate Feature

2F.1 Find Feature

1. Look for {output_dir}/{feature_name}/state.json
2. If not found, also check .code-forge/tmp/{feature_name}/state.json
3. If still not found: show error, list available features

2F.2 Load Feature Context

1. Read state.json
2. Read plan.md (for acceptance criteria and architecture)
3. Note completed task count and overall progress

→ Go to Step 3F

Step 2P: Project Mode — Locate Reference

Determine the reference level using a fallback chain.

2P.1 Check for Planning Documents (Level 1: Planning-backed)

Scan {output_dir}/*/plan.md:

• If one or more plan.md files found → planning-backed
• Read all plan.md files and aggregate:
• Acceptance criteria from each feature
• Architecture decisions
• Technology stack
• Read corresponding state.json files for progress context
• Record: reference_level = "planning"
• Record: list of plan file paths and aggregated criteria
• → Go to Step 3P

2P.2 Check for Documentation (Level 2: Docs-backed)

If no planning documents found, scan for upstream documentation:

Search paths (in order):

1. {input_dir}/*.md — feature specs
2. docs/ directory — PRD, SRS, tech-design, test-cases files

Look for files matching patterns:

• **/prd.md, **/srs.md, **/tech-design.md, **/test-cases.md
• **/features/*.md
• Any .md files directly under docs/

If documentation files found → docs-backed:

• Read all found docs
• Extract: requirements, architecture decisions, acceptance criteria, scope definitions
• Record: reference_level = "docs"
• Record: list of doc file paths and extracted criteria
• → Go to Step 3P

2P.3 No Reference (Level 3: Bare)

If neither planning nor docs found → bare:

• Record: reference_level = "bare"
• → Go to Step 3P

Step 3F: Feature Mode — Collect Changes and Review

3F.1 Collect Change Scope

From Commits: Extract all commit hashes from state.json → tasks[].commits:

• Flatten all commit arrays into a single list
• If commits are recorded, use git diff between the earliest and latest commits
• If no commits recorded, fall back to scanning files involved in tasks

From Task Files: Read all tasks/*.md files and collect their "Files Involved" sections:

• Build a complete list of files created/modified by this feature
• Read current state of each file

Summary:

• Total files changed
• Total lines added/removed (from git diff)
• List of all affected files

3F.2 Detect Project Type

Before launching the sub-agent, detect the project type to guide dimension selection:

1. Has frontend? Check for: *.tsx, *.jsx, *.vue, *.svelte, HTML templates, CSS/SCSS files, or frontend framework config (next.config.*, vite.config.*, angular.json)
2. Has backend/service? Check for: server entry points, API route definitions, database models, middleware
3. Language ecosystem: Detect primary language(s) from file extensions and package manifests

Record: project_type = "frontend" | "backend" | "fullstack" | "library" | "cli" | "unknown"

3F.3 Module Grouping

Grouping decides how the scope is split across sub-agents. The split axis matters more than the group count — a partition that cuts through call chains gives every agent a 1-hop view of a multi-hop chain, which raises false negatives (the evidence for a real bug is spread across three agents, so no agent can satisfy Gate 2/Gate 4) and false positives (an agent sees an unguarded parameter and cannot see the caller that already validated it). Group count is a symptom; the axis is the cause.

3F.3.0 Run the partitioner

The deterministic half of grouping — build the graph, measure the cut, pick the axis, apply the size cap, merge to the count cap, decide the guardrail — is implemented as a script. Run it rather than doing the arithmetic by hand; hand-executed partitioning is where group count silently drifts from what the rules intend. Resolve <cf_scripts> once (see Locating the script layer in the configuration step — it lives in the install, not the project), then:

python3 "<cf_scripts>/cf-group.py" <<'JSON'
{
  "files": ["apps/backend/src/api/post.controller.ts", "..."],
  "edges": [["apps/backend/src/api/post.controller.ts", "libraries/.../post.service.ts"]],
  "barrels": ["libraries/nestjs-libraries/src/index.ts"],
  "manifests": ["package.json", "apps/backend/project.json"],
  "architecture_pattern": "Layered API"
}
JSON

edges comes from the impact-zone expansion already run in 3P.1 (project mode) or one grep over the affected files (feature mode). architecture_pattern comes from PA.2.2. It returns grouping_strategy, cut_ratio, groups, group_count, largest_group, groups_merged, path, coverage_ok, and guardrail {triggered, reasons}.

It is a PARTITIONER, not an analyst. It never reads source, resolves imports, or judges findings. These stay yours: identifying which files are cross_cutting (3F.3.3 rule 2) and which are barrels, deciding whether the returned axis matches what you know about the code, and everything downstream of dispatch.

Two results you must act on, not just read:

• coverage_ok: false (script exits 1) — the partition lost files. Do not dispatch; this is a bug, not a trade-off.
• guardrail.triggered: true — stop and present 3F.3.4's options to the user before dispatching anything.

If python3 is unavailable, execute 3F.3.1–3F.3.5 by hand — they are the script's specification, and the thresholds are mirrored in its constants.

3F.3.1 Build the scope dependency graph

Build G — a file-level graph over the affected files. Method-level resolution is not needed here and is not attempted.

1. Reuse the edges already computed. Step 3P.1's impact-zone expansion (project mode) already grepped import/require statements for dependents and dependencies. Do not recompute — carry those edges in. In feature mode, run one grep for import/require statements across the affected files.
2. Treat barrel files as transparent. When A → index.ts → B and index.ts is a pure re-export, record the edge as A → B. Barrel files never become graph nodes and never form their own group — they are cross_cutting (rule 2 below). This also prevents a single barrel from inflating the impact zone.
3. Precision expectations. This graph is a grouping heuristic, not a tier-assignment oracle. Dynamic imports, DI-container wiring, and aliased paths will be missed — that is acceptable, because a mis-grouped callee is still caught by tier-2 expansion at chain-building time. The precision required to assign tier-1 vs tier-2 is much higher and remains the sub-agent's job at read time (see rule 6). Do not conflate the two.
4. Record edge_count and, for any candidate partition P, cut_ratio(P) = cross_group_edges / edge_count. If edge_count == 0 (no intra-scope imports found), treat cut_ratio as 0 and use strategy S1.

3F.3.2 Select the grouping strategy

Take architecture_pattern from PA.2.2 (already produced by the Project Analysis step — do not re-derive it from path strings):

Compute the directory partition's cut_ratio first and let it override the table when it disagrees: a Layered API project whose diff happens to sit entirely inside one layer has a low cut ratio and should use S1. Record grouping_strategy and cut_ratio — both go into the report's methodology note.

S1 — by directory. Files in the same directory form one group. Python: group by top-level package. TypeScript: by src/ subdirectory. This is the historical behavior and stays optimal when modules are genuinely independent.

S2 — by call chain. Compute weakly-connected components of G. Each component becomes one group, even when it spans several directories — a controller → service → repository chain lands in a single agent that can see the whole chain. Attach every size-1 component to the component it has the most edges to; ties break by longest shared path prefix. A file with no edges at all (nothing in scope imports it and it imports nothing in scope — common for config, fixtures, and generated files) has no "most edges to" target: attach it by longest shared path prefix alone. If S2 yields a single component covering >70% of files, fall through to S3.

S3 — do not split. The scope is one semantically indivisible unit; splitting it destroys the evidence chains. Route it to the fast path (3F.4a) with the whole scope when it holds ≤ 12 files — one agent must read every file in full and build call graphs for all of them, and beyond ~12 that context stops being reliable. Above 12 files, S3 has no safe execution: do not split it anyway, trigger the 3F.3.4 guardrail and let the user narrow the scope.

3F.3.3 Common grouping rules (all strategies)

1. Coverage is invariant. Grouping redistributes files across agents; it never drops them. Assert before dispatch: union(all groups' primary_files) == in_diff_files. A violation is a bug in grouping, not an acceptable trade-off.
2. Identify cross-cutting files (shared utilities, base classes, __init__.py, index.ts, exports.ts, types.ts) — mark them cross_cutting: true. Include them in BOTH their group AND the cross-module agent's file list.
3. Group size: soft cap 4 files per group; see 3F.3.4 for when it relaxes to 6. If an S1 group exceeds the cap, split by file role (models / serializers / logic / tests). If an S2 component exceeds the cap, split at the weakest edge (fewest call sites) so the cut lands where the chain is thinnest.
4. Merging never crosses a package boundary. When groups must be merged (3F.3.4), merge greedily upward by shared parent directory, but treat any directory containing package.json / project.json / Cargo.toml / pyproject.toml / go.mod as an impassable wall. Merging libraries/nestjs-libraries/** with libraries/react-shared-libraries/** because both sit under libraries/ would destroy the D5 architecture signal.
5. Record: module_groups = [{group_id, files[], cross_cutting_files[]}], plus grouping_strategy, cut_ratio, groups_merged.
6. Record the complete in_diff_files list (every affected file across all groups, including cross-cutting). Each per-module agent receives it alongside its own primary_files and applies the three-tier rule at chain-building time, from the code it actually reads — if a call target's file is in in_diff_files but not in its primary_files, that callee is tier-2. The 3F.3.1 graph does not pre-assign tiers: barrel re-exports, aliased imports, and dynamic imports make static tier assignment unreliable, and the agent reading the file has strictly better information.

Effect on tier-2. A good axis leaves most call chains inside a group, where tier-1 inlining (uncapped) covers them. High tier-2 pressure — many groups reporting tier2_budget_exceeded: true — is the signal that the axis was wrong. Record it and surface it in the methodology note; a review with grouping_strategy: S1 and widespread budget overflow should have used S2.

3F.3.4 Group-count cap and scope guardrail

The cap is a safety valve, not the primary control — a correct axis usually brings the count down on its own (26 directory groups often collapse to 5–6 call chains under S2).

Group size and group count are independent constraints. Group count is set by the number of directories (S1) or connected components (S2) — it is not files / cap. A 30-file diff spread across 26 directories yields 26 groups of one file each, and no size cap will ever fire on it. Size is controlled by splitting; count is controlled by merging. Run both checks.

Check A — group size (splitting). Soft cap 4 files per group; 6 once the scope reaches 33 files (this band has no upper end — it still applies to an oversized scope the user chose to proceed with). Over the cap, split per 3F.3.3 rule 3.

Check B — group count (merging). Independent of Check A and of the file count. While group_count > 8:

candidates = all pairs of groups that are mergeable:
               - both inside the SAME package boundary (3F.3.3 rule 4), AND
               - sharing a parent directory below that boundary
if candidates is empty:  break        -> guardrail
merge the candidate pair with the smallest combined file count
set groups_merged = true

Merge greedily from the smallest, so the tightly-scoped groups coalesce and the substantial ones stay intact. When the two checks collide, count wins: a merged group may exceed the size cap. Group count bounds agent fan-out and is hard; group size only bounds per-agent context and degrades gracefully. Record the largest resulting group size in the methodology note.

Guardrail. Trigger when either holds after Check B:

• group_count > 8 and no mergeable pair remains (all remaining groups sit in different packages), or
• scope exceeds 48 files

Stop before dispatching and report the actual shape to the user:

Review scope: 71 files across 19 modules (strategy: S2, cut_ratio 0.41)
This would dispatch 21 sub-agents. Options:
  1. Narrow the scope (review one feature or one package at a time)
  2. Run in sequential waves of 8 groups (slower, same coverage)
  3. Proceed anyway

If the user picks option 2 (waves): dispatch the per-module agents in sequential batches of at most 8, each batch in one parallel message, waiting for a batch to finish before starting the next. Coverage and per-agent context are identical to a single dispatch — only wall-clock differs. The cross-module agent (3F.5 / 3P.4) runs once, after every wave completes, since it consumes the merged METHOD_CHAINS and TIER2_DEFERRED from all groups. Do not run a cross-module pass per wave.

Never silently absorb an oversized scope by diluting each agent's context — that trades a visible cost for an invisible quality loss.

3F.3.5 Path decision

• Fast path (3F.4a): fewer than 3 affected files, OR grouping yields only 1 group, OR grouping_strategy == S3 with ≤ 12 files.
• Layered path (3F.4b → 3F.5): ≥ 2 groups AND ≥ 3 affected files.

Rationale: the layered path only pays off when there is real cross-module territory. A 5-file change all inside src/binding/ has no cross-module axis and stays on the fast path — and so does an 8-file tightly-coupled change, which S3 keeps whole rather than cutting into 2–3 blind agents.

3F.4a Fast Path: Single Sub-agent Review (< 3 files, OR 1 group, OR S3 with ≤ 12 files)

Offload to sub-agent to handle the full diff analysis.

Spawn an Agent tool call with:

• subagent_type: "general-purpose"
• description: "Review feature: {feature_name}"

Sub-agent prompt must include:

• Feature name and plan.md file path
• List of all affected files (sub-agent reads them)
• The acceptance criteria from plan.md
• Detected project type
• Reference files to read — EXACTLY these six, no others: references/dimensions-intra.md, references/dimensions-cross.md, references/format-common.md, references/format-single.md, references/suppression-gates.md, references/call-graph-discipline.md. Do NOT read format-per-module.md, format-cross-module.md, or format-acceptance.md — they belong to other agent roles.
• MANDATORY pre-analysis instruction: "Before applying any review dimension, read every affected file in full and build a call graph for every public method / exported function / entry point in those files. Enumerate — for each — the helpers it calls (to leaves within the reviewed scope), the validations it performs, the state mutations it executes, the errors it raises, and its external-input paths (iteration over arguments, subscript into external data, deserialization). Output this as the `METHOD_CHAINS` section per `references/format-single.md`. Only after producing METHOD_CHAINS may you apply dimensions. Do not trust method names, plan claims, or helper-function purity — open and read every callee. See the `references/call-graph-discipline.md` (full protocol including anti-rationalization guard) for the full protocol and anti-rationalization guard."
• MANDATORY mid-analysis instruction (pre-emission scratchpad): "Before writing any dimension block, build a `CANDIDATE_INVENTORY` per `references/format-common.md` §Pre-emission Scratchpad. It is a DROP LEDGER: list ONLY the candidates you decided NOT to emit, each with a fixed-enum `drop_reason` code (free-text is rejected) and a one-line `drop_detail`. Findings you keep are NOT ledgered — write them straight into their dimension blocks. Before ledgering, compare the candidate against `references/suppression-gates.md` §Drop Gallery — if it 'looks like' any ❌ example there, ledger it with the matching code (`extract_helper_under_3_sites`, `documented_known_gap`, `self_admitted_low_value`, `refactor_preference_no_bug`, `pure_symmetry_no_bug`, `defensive_hardening_speculative`, `typo_hypothetical`, `trust_boundary_internal`); if it 'looks like' a ✅ example, keep it and emit it. The orchestrator audits the ledger — no ledgered row may appear in any dimension block; doing so is a bypass attempt and invalidates the response. An empty ledger is valid. Write the ledger BEFORE the dimension blocks, not after."
• MANDATORY post-analysis instruction: *"After the scratchpad, apply the five gates in references/suppression-gates.md as a final check on the KEEP set (Gate 1 Reachability, Gate 2 Trust Boundary, Gate 3 Severity Calibration, Gate 4 Quota Avoidance, Gate 5 Factual Verifiability). The scratchpad should have already filtered most noise; gates 1-5 here catch anything that slipped through. Drop speculative findings whose trigger starts with 'could theoretically' / 'if X ever happens' / 'in case someone'. Determine the project's threat model FIRST — for dev tools / code generators / linters that read the developer's own files, the trust boundary does not include those files. Drop security / defensive-gap findings whose input source is internal/trusted under that model. Accept empty dimensions as a valid result — do NOT fabricate marginal findings to fill a dimension. Any finding asserting a falsifiable claim about the codebase ('zero references', 'never called', 'dead code', 'only used in X', 'duplicates Y', 'N lines exceed Z') MUST paste the actual verification artifact into `evidence` — the full command used (grep/rg/equivalent) AND its matched-line output, OR explicit file:line citations. Paraphrases like 'grep returns only the declaration' are insufficient — the output itself must be visible. Dead-code claims require cross-directory grep (src + tests). Findings missing the artifact are dropped by Gate 5. Every critical and blocker finding MUST include a non-empty evidence field explaining the concrete reachable trigger."*
• Instructions to review across all applicable dimensions (empty dimensions are valid — see Gate 4)
• The severity level definitions (blocker / critical / warning / suggestion — strict, per the SKILL.md severity table)
• Instruction: "For each issue, specify severity, file path, line number/range, what's wrong, and how to fix it. Use the Review Comment Formula: Problem → Why it matters → Suggested fix. When the issue was discovered via the call graph (e.g., a missing validation call, a skipped state mutation, an unguarded external input), reference the relevant METHOD_CHAINS entry in the description. For critical/blocker findings, the `evidence` field MUST show: (a) the concrete input that triggers the failure, (b) the observable wrong behavior, and (c) for D2/defensive-gap findings, the trust-boundary argument per Gate 2."

Review dimensions to apply: Follow Dimension Application Rules. Apply dimensions AGAINST the call graph, not against the surface method body. Route every finding through §Finding Suppression Gate before emission.

Additionally, always check Plan Consistency (feature mode specific):

• All acceptance criteria from plan.md are met
• Architecture matches the design in plan.md
• No unplanned features added (scope creep)
• All planned tasks are implemented

Sub-agent must return the structured format defined in `references/format-single.md` (use the Feature Mode PLAN_CONSISTENCY consistency section).

→ Go to Step 3G (Acceptance Gate), then Step 4F

3F.4b Parallel Per-Module Review (≥ 3 files AND ≥ 2 module groups)

Spawn one sub-agent per module group in a single parallel message (all Agent calls sent together).

For each module group, spawn Agent with:

• subagent_type: "general-purpose"
• description: "Per-module review: {feature_name}/{group_id}"

Sub-agent prompt must include:

• `primary_files` — this module group's own files (the sub-agent reads these in full and produces top-level METHOD_CHAINS entries for their public symbols)
• `in_diff_files` — the complete list of every affected file across ALL module groups in this review. This is the tier-2 eligibility set — when a call's target definition lives in in_diff_files \ primary_files, the agent MUST open that file and inline its top-level body at depth-1 with the X: prefix, up to the tier-2 expansion budget of 8 distinct files.
• Tier-2 expansion budget = 8 distinct files. Rank eligible callees by number of distinct call sites in primary_files (descending; ties → external-input-reaching first, then file path). Expand the top 8. For each callee beyond the budget: mark its chain step [tier-2 DEFERRED: over expansion budget] and add a TIER2_DEFERRED entry. Deferring silently is a protocol violation — an unlisted deferral is indistinguishable from a tier-3 leaf. Set tier2_eligible_count and tier2_budget_exceeded in MODULE_REVIEW_SCOPE regardless of whether the budget was hit.
• Feature name and plan.md acceptance criteria (for context — not a consistency check)
• Detected project type
• Reference files to read — EXACTLY these five, no others: references/dimensions-intra.md, references/format-common.md, references/format-per-module.md, references/suppression-gates.md, references/call-graph-discipline.md (the last one for the three-tier inlining rule). Do NOT read dimensions-cross.md, format-single.md, or format-cross-module.md — D5, D7 and D10-D15 are the cross-module agent's job, and reading them here both wastes context and produces duplicate findings.
• MANDATORY pre-analysis: the three-tier expansion rule from §Call-Graph Discipline (see references/call-graph-discipline.md):
• Tier 1 (same-module private helpers — file in `primary_files`): full recursive inlining
• Tier 2 (cross-module callees — file in `in_diff_files` but NOT in `primary_files`): depth-1 expansion with X:Module.method → prefix, capped at 8 distinct files, ranked by call-site count; overflow → TIER2_DEFERRED
• Tier 3 (everything else — stdlib, third-party, or files in neither list): ext_call leaf, no expansion
• MANDATORY mid-analysis (pre-emission scratchpad): build a CANDIDATE_INVENTORY drop ledger per references/format-common.md §Pre-emission Scratchpad BEFORE writing dimension blocks — ONLY the intra-module candidates (D1, D2, D3, D4, D6, D8, D9) you decided NOT to emit, each with a fixed-enum drop_reason and one-line drop_detail. Kept findings are never ledgered; they go straight into their dimension blocks. Compare each candidate against references/suppression-gates.md §Drop Gallery — ❌ examples → ledger with matching code (e.g. extract_helper_under_3_sites, refactor_preference_no_bug, pure_symmetry_no_bug); ✅ examples → keep and emit. Orchestrator cross-checks ledger ↔ dimensions. Emitting a ledgered finding is a bypass attempt and invalidates the response. An empty ledger is valid.
• MANDATORY post-analysis: after the scratchpad, apply the final pass of Gates 1-5 in references/suppression-gates.md as a redundant check on the KEEP set. Specifically: drop speculative ("could theoretically", "if X ever happens") findings (Gate 1); drop D1/D2 findings whose input source is internal/trusted under this project's threat model (Gate 2); max-severity-warning for design-preference or cross-module-inconsistency findings (Gate 3); empty dimensions are a valid result — do NOT fabricate marginal findings (Gate 4); any finding asserting a falsifiable codebase fact ("zero references", "never called", "dead code", "only used in X", "duplicates Y", "N lines exceed Z") MUST paste the actual verification artifact (full grep/rg command AND its matched-line output, OR explicit file:line citations) into `evidence` — paraphrases are insufficient; dead-code claims require cross-directory grep covering src + tests (Gate 5). Every critical/blocker finding MUST include a non-empty evidence field.
• Intra-module dimensions to apply: D1 (Functional Correctness), D2 (Security), D3 (Resource Management), D4 (Code Quality), D6 (Performance), D8 (Error Handling), D9 (Observability) — applied against the chain INCLUDING tier-2 inlined steps (a D1 defensive gap inside a tier-2 callee IS reported by this agent, subject to Gate 2)
• Do NOT apply: D5, D7, D10-D15 — these are handled in the cross-module pass
• The severity level definitions (blocker / critical / warning / suggestion — per SKILL.md severity table, strictly enforced by Gate 3)
• Return format: Per-Module sub-agent format (see references/format-per-module.md). The output must include primary_files (same as input), tier2_files (the subset of in_diff_files actually opened for tier-2 expansion, ≤8), tier2_eligible_count, tier2_budget_exceeded, TIER2_DEFERRED (entries required whenever the budget was exceeded), and METHOD_CHAINS with top-level entries only for public symbols in primary_files.
• Instruction: "Return ALL issues found in chains rooted at YOUR module's public symbols — including issues discovered via tier-2 inlined steps from cross-module callees — that SURVIVE the §Finding Suppression Gate. When a finding lives in a tier-2 inlined step, set the issue's `file` to the tier-2 callee's file (not your module's file). Do not self-filter or defer cross-module concerns — the cross-module agent handles CONSISTENCY across modules, but defensive gaps visible in your chain are yours to flag even if they live in someone else's file. For every critical/blocker finding, the `evidence` field must show the concrete reachable trigger and observable wrong behavior."

Deduplication note: When agent A (owning module X) tier-2-expands into ModuleY.foo and flags a defensive gap, agent B (owning module Y) will independently tier-1-inline foo as part of its full review and likely flag the same gap. The orchestrator MUST deduplicate in Step 4F (merge step) by (file, line, title).

Wait for all per-module sub-agents to complete before proceeding to 3F.5.

Merge the tier-2 deferrals before dispatching 3F.5. Concatenate every agent's TIER2_DEFERRED entries, deduplicate by (callee, callee_file) — summing call_site_count and unioning call_sites across agents — and sort by reaches_external_input: true first, then call_site_count descending. This merged list goes to the cross-module agent. Also record which module groups reported tier2_budget_exceeded: true; hand that set to the cross-module agent as D5 (Architecture) input — a ≤4-file module group calling into >8 other in-diff files is a coupling signal, not just bookkeeping.

3F.5 Cross-Module Association Review

After all per-module agents complete, spawn one cross-module aggregation sub-agent.

Agent with:

• subagent_type: "general-purpose"
• description: "Cross-module review: {feature_name}"

Sub-agent prompt must include:

• All per-module METHOD_CHAINS outputs verbatim (copy from 3F.4b results)
• The merged `TIER2_DEFERRED` list (built above) + the set of module groups with tier2_budget_exceeded: true. Instruction: "These cross-module callees exceeded a per-module agent's tier-2 expansion budget, so no per-module agent opened them. They are yours — you have the global view and can read the callee files directly. Work them in `TIER2_RECONCILIATION` BEFORE the dimension blocks, prioritizing `reaches_external_input: true` entries. Every handed-over callee needs an entry with status `expanded` / `expanded_no_issue` / `not_expanded` (+ `reason`). A callee with no entry is a coverage hole the orchestrator will surface as a report caveat."
• All per-module findings (to avoid duplicate flagging — cross-module agent adds NEW findings only)
• Cross-cutting files list + the sub-agent reads their full content
• plan.md content and acceptance criteria
• Reference files to read — EXACTLY these five, no others: references/dimensions-cross.md, references/format-common.md, references/format-cross-module.md, references/suppression-gates.md, references/call-graph-discipline.md (the last one needed only to read the inlining notation in the incoming per-module chains — this agent consumes chains, it does not build them). Do NOT read dimensions-intra.md, format-single.md, or format-per-module.md — D1-D4, D6, D8, D9 were already covered by the per-module agents.
• List of all affected files grouped by module (structural map of the feature)
• Detected project type
• The severity level definitions (blocker / critical / warning / suggestion — per SKILL.md severity table, strictly enforced by Gate 3)
• MANDATORY mid-analysis (pre-emission scratchpad): build a CANDIDATE_INVENTORY drop ledger per references/format-common.md §Pre-emission Scratchpad BEFORE writing dimension/consistency blocks — ONLY the cross-module candidates (D5, D7, D10-D15, CROSS_MODULE_CONSISTENCY, SECOND_ORDER_REVIEW) you decided NOT to emit, each with a fixed-enum drop_reason. Kept findings are never ledgered. Compare against references/suppression-gates.md §Drop Gallery: pure-symmetry findings → ledger with pure_symmetry_no_bug; ≥3-site verified consolidation and cross-module defensive-depth inconsistencies with observable wrong behavior → keep and emit. Orchestrator cross-checks ledger ↔ dimensions. An empty ledger is valid.
• MANDATORY post-analysis: after the scratchpad, apply the final pass of Gates 1-5 (§Finding Suppression Gate). Cross-module consistency and second-order findings are subject to Gate 2 (a "missing guard" in module B is only a bug if the underlying input is genuinely external per the project's threat model) and Gate 3 (pure convention divergence without observable wrong behavior is warning at most, not critical). Gate 5 applies to every cross-module/D15 finding that claims "duplicates Y", "parallel implementation", "reimplements" — the evidence MUST cite both locations with file:line, not just name the pattern. Empty sections are valid — do not fabricate findings to fill the cross-module template. Every critical/blocker finding MUST include a non-empty evidence field.

Dimensions to apply (cross-module scope):

• D5 (Architecture & Design) — layer boundary violations, circular deps, coupling across the full module set
• D7 (Test Coverage) — coverage gaps across the full feature scope, test files for each module
• D10–D13 (Standards, Backward Compat, Maintainability, Dependencies)
• D15 (Simplification & Anti-Bloat) — cross-module duplicate detection requires the full picture; per-module agents cannot catch parallel implementations across file boundaries

CROSS_MODULE_CONSISTENCY — apply all five checks (findings subject to §Finding Suppression Gate):

Contract-symmetry pre-flight (MANDATORY for every pattern below). Before flagging any inconsistency, the sub-agent MUST verify the two modules have symmetric contracts: same data-class shape, same lifecycle position, same trust boundary, same caller expectations. Modules that look structurally similar but serve different logical roles (one handles user input, one handles constants; one is public API, one is an internal helper; one is invoked at request time, one at startup) are NOT subject to consistency checks — their differences are intentional. If contract symmetry cannot be demonstrated in one line, drop the finding — do not flag as warning. This pre-flight exists to prevent the common failure: "A and B look similar, A has guard X, B does not, therefore B is buggy" — the assumption is false whenever A and B serve different roles.

1. Coerce/guard pattern: If module A guards entry.get("key", default) on dict external inputs, do all sibling modules with structurally equivalent dict-subscript external inputs follow the same pattern? Flag inconsistency as critical only if the underlying input is genuinely external per Gate 2 AND contract symmetry holds; otherwise warning (pure convention divergence) or drop.
2. Traceback preservation: If module A uses raise X from e or passes exc_info=True in exception logging, are all modules in the diff consistent? Flag inconsistency as warning only when (a) contract symmetry holds and (b) the sub-agent names the observable downside — lost root-cause info when the exception actually fires, with the exception path demonstrably reachable. Pure stylistic divergence (one module uses from e, one doesn't, but both are never logged or surfaced to a debug tool) → drop.
3. Re-export completeness: For every new public symbol introduced in a submodule, verify it appears in the package __init__.py / index.ts / __all__ if the project re-exports its API surface. Flag missing re-exports as warning only when the sub-agent demonstrates at least one external consumer (outside the submodule) that would need the symbol from the top-level package — grep for imports of other symbols at the top level; if nothing imports from the top level for this submodule's public API, re-export is not a convention. New internal symbols that have no external caller → drop (not API surface).
4. Error handling convention: Same error base class hierarchy and chaining approach used across all modules? Flag deviation as warning only when (a) contract symmetry holds and (b) the sub-agent names how the deviation causes a concrete downstream handling failure (e.g., a except ProjectError catch block elsewhere in the codebase will miss the deviating module's errors). Pure class-hierarchy aesthetics with no caller consequence → drop.
5. Defensive coding depth: If module A added input validation guards for a specific data path, are all modules with structurally equivalent data paths at the same validation depth? Flag depth mismatch as critical only when the input is genuinely external per Gate 2 AND contract symmetry holds; otherwise warning or drop.

SECOND_ORDER_REVIEW — active prevention of D-series ("whack-a-mole") bugs:

For each fix pattern visible in the diff (identifiable from per-module METHOD_CHAINS + intra-module findings):

1. Extract the fix pattern (e.g., "coerce non-dict display surface values", "snapshot sys.path before exec_module", "preserve traceback on scan failure", "emit suggested_alias in serializer output")
2. Identify all code paths in OTHER modules in the diff that handle structurally similar data flows
3. Verify the same fix has been applied to each structurally similar path
4. If the fix is missing in any sibling module, emit a critical finding (subject to Gate 2 — if the input is internal/trusted under the project's threat model, DROP the finding entirely; if the input is genuinely external AND contract-symmetry holds between the modules per §CROSS_MODULE_CONSISTENCY pre-flight, keep as `critical`; downgrading to `warning` without a named observable downside is noise and will be dropped by Step 4F validation #4 anyway): "Fix pattern applied in {module_A} was not propagated to {module_B} — structural parity violation. Pattern: {description}. Expected location: {file:line estimate}. Evidence: {concrete reachable trigger showing the gap matters}."

Plan Consistency (always, feature mode):

• All acceptance criteria from plan.md are met across the full combined module set
• Architecture matches the design in plan.md
• No unplanned features added across any module
• All planned tasks are implemented

Return format: Cross-Module sub-agent format (see references/format-cross-module.md)

→ Proceed to Step 3G (Acceptance Gate), then Step 4F with merged results from 3F.4b + 3F.5

Step 3P: Project Mode — Collect Source Code and Review

The primary subject of review is the source code itself. Reference documents (plans, specs) serve only as criteria to check against — the sub-agent must deeply read and analyze the actual implementation.

3P.1 Collect Source Code

Identify and collect project source files for deep code review. The collection strategy depends on scope (set in Step 1):

If `scope = "changes"` (default — no arguments or auto-selected):

1. Identify changed files (primary scope):

• If on a non-main branch: git diff main...HEAD --name-only
• If on main branch with uncommitted changes: git diff HEAD --name-only + git diff --cached --name-only (staged + unstaged)
• If on main branch with no uncommitted changes: git diff HEAD~1 --name-only (last commit)
• Exclude non-source directories: node_modules/, dist/, build/, .git/, vendor/, __pycache__/, the output directory itself

1. Expand to impact zone (1 level): For each changed file, also include:

• Files that import or depend on the changed file (direct dependents — use Grep to find import/require/use statements referencing the changed file)
• Files that the changed file imports from (direct dependencies — read the changed file's import statements)
• Test files corresponding to the changed files (e.g., foo.test.ts for foo.ts)

1. Fallback to full scan: Only if no changed files are found (clean repo, no recent commits), fall through to the scope = "full" strategy below.

If `scope = "full"` (`--project` flag):

1. Use project root markers to find source directories (e.g., src/, lib/, app/, pkg/, or language-specific patterns)
2. Exclude non-source directories: node_modules/, dist/, build/, .git/, vendor/, __pycache__/, the output directory itself
3. Scan all source files
4. If the project is large (>50 source files), prioritize:

• Core modules (entry points, main logic, business logic)
• Test files
• Configuration and infrastructure files

This ranks, it does not truncate. The scope stays whole; the ranking is what the 3F.3.4 guardrail offers the user when they choose to narrow. Do not silently drop the tail here — a scope quietly cut from 90 files to 50 reads in the report as a review that covered everything.

Both modes also collect:

• Package manifests (package.json, Cargo.toml, pyproject.toml, etc.) for dependency review
• Build/CI configuration if present

3P.2 Detect Project Type

Same as Step 3F.2 — detect project_type to guide dimension selection.

3P.3 Module Grouping (Project Mode)

Apply the full 3F.3 protocol — 3F.3.0 (run the partitioner) through 3F.3.5 (path decision) — unchanged. Project-mode deltas only:

• The graph edges are already available. Step 3P.1's impact-zone expansion computed dependents and dependencies for every changed file. Carry those edges directly into 3F.3.1; do not re-grep.
• Fast path (3P.3a): fewer than 3 source files in scope, OR grouping yields only 1 group, OR grouping_strategy == S3 with ≤ 12 files.
• Layered path (3P.3b → 3P.4): ≥ 2 groups AND ≥ 3 source files.
• The guardrail matters more here. scope = "full" (--project) on a monorepo routinely exceeds 48 files. Trigger 3F.3.4's guardrail and let the user narrow the scope rather than dispatching twenty-plus agents.

3P.3a Fast Path: Single Sub-agent Review (< 3 files, OR 1 group, OR S3 with ≤ 12 files)

Same protocol as 3F.4a — one Agent(subagent_type="general-purpose") covering the whole scope, the same six reference files, the same MANDATORY pre-/mid-/post-analysis instructions, the same severity definitions and Review Comment Formula, the same references/format-single.md return format, and the same "apply dimensions against the call graph, route every finding through §Finding Suppression Gate" rule. Project-mode deltas only:

• `description`: "Project code review: {project_name}"
• Scope input — project name + root path + the list of all source files to review (there is no diff here). Add this instruction: "Read every source file. Review the code itself — its logic, structure, correctness, and quality. Reference documents are only criteria for the consistency check, never the subject of review."
• Reference context replaces 3F.4a's plan.md + acceptance criteria. Pass reference_level and its criteria:
• planning → aggregated acceptance criteria (consistency dimension only)
• docs → extracted requirements (consistency dimension only)
• bare → none
• Pre-analysis addendum — append to 3F.4a's instruction: "In bare mode, use the method's name, signature, and public-API promises as the internal consistency check target." (3F.4a's "do not trust plan claims" clause still applies whenever reference_level is planning.)
• Consistency check — instead of 3F.4a's fixed Plan Consistency, branch on reference_level:
• planning → Plan Consistency (criteria met, no scope creep, architecture match)
• docs → Documentation Consistency (requirements implemented, architecture aligned)
• bare → skip; note in the report: "No reference documents found — consistency check skipped."
• Return section — Project Mode CONSISTENCY, not PLAN_CONSISTENCY. Every issue MUST cite specific source files and line numbers/ranges.

→ Go to Step 3G (Acceptance Gate), then Step 4P

3P.3b Parallel Per-Module Review (≥ 3 files AND ≥ 2 module groups)

Same protocol as 3F.4b — spawn one sub-agent per module group in parallel. Each agent receives:

• `primary_files` — its module group's own files (reviewed in full, top-level METHOD_CHAINS entries for their public symbols)
• `in_diff_files` — the complete affected-files list; any call target whose file is in in_diff_files \ primary_files must be tier-2-expanded per §Call-Graph Discipline (see references/call-graph-discipline.md), subject to the same 8-file tier-2 expansion budget as 3F.4b — rank by call-site count, overflow to TIER2_DEFERRED, set tier2_eligible_count / tier2_budget_exceeded
• Three-tier expansion pre-analysis instruction (same as 3F.4b)
• Pre-emission scratchpad mid-analysis instruction (same as 3F.4b) — CANDIDATE_INVENTORY drop ledger listing ONLY the candidates NOT emitted, each with an enum-coded drop_reason, written BEFORE the dimension blocks. Kept findings are never ledgered. Compare against references/suppression-gates.md §Drop Gallery.
• §Finding Suppression Gate post-analysis instruction (same as 3F.4b) — every finding routed through Gates 1-5 before emission; every critical/blocker requires evidence
• Applies D1, D2, D3, D4, D6, D8, D9 against chains INCLUDING tier-2 inlined steps, with findings subject to Gate 2 (trust boundary for D1 defensive-gap and D2 findings)
• Reference files to read — EXACTLY these five, no others (same as 3F.4b): references/dimensions-intra.md, references/format-common.md, references/format-per-module.md, references/suppression-gates.md, references/call-graph-discipline.md
• Returns Per-Module sub-agent format (see references/format-per-module.md)

Wait for all per-module sub-agents to complete before proceeding. Then merge the tier-2 deferrals exactly as in 3F.4b (dedupe by (callee, callee_file), sum call_site_count, sort external-input-reaching first) and carry the merged list plus the over-budget module-group set into 3P.4.

3P.4 Cross-Module Association Review (Project Mode)

Same protocol as 3F.5, with the following adjustments:

• Instead of Plan Consistency, apply the appropriate Consistency check based on reference_level:
• planning-backed → Plan Consistency across the full aggregated method chain set
• docs-backed → Documentation Consistency
• bare → Skip consistency; still apply all five CROSS_MODULE_CONSISTENCY checks and SECOND_ORDER_REVIEW
• All five CROSS_MODULE_CONSISTENCY checks (coerce/guard, traceback, re-export, error convention, defensive depth) — findings subject to §Finding Suppression Gate, especially Gate 2 (a missing guard is only a bug when the input is genuinely external)
• SECOND_ORDER_REVIEW (same as 3F.5) — subject to §Finding Suppression Gate
• D5, D7, D10–D15 — subject to §Finding Suppression Gate Gate 4 (empty dimensions are valid; do not fabricate marginal findings)
• Reference files to read — EXACTLY these five, no others (same as 3F.5): references/dimensions-cross.md, references/format-common.md, references/format-cross-module.md, references/suppression-gates.md, references/call-graph-discipline.md
• The merged `TIER2_DEFERRED` list + over-budget module groups (same handoff as 3F.5) — TIER2_RECONCILIATION is required, worked before the dimension blocks, prioritizing reaches_external_input: true

Return format: Cross-Module sub-agent format (see references/format-cross-module.md)

→ Proceed to Step 3G (Acceptance Gate), then Step 4P with merged results from 3P.3b + 3P.4

Step 3G: Acceptance Gate (both modes — after the dimensional review, before Display)

The dimensional review above checks whether the code that exists is correct. This gate checks the orthogonal question: does every required behavior exist, and is each backed by a test that actually ran and passed? It has the OPPOSITE polarity from the Finding Suppression Gate — see §Acceptance Gate. The five suppression gates and the CANDIDATE_INVENTORY scratchpad DO NOT apply here; an uncovered required behavior is itself the finding and is never dropped for lack of a reachable-trigger or grep artifact.

3G.0 Quick-mode bypass

If quick_mode == true (from Step 1.0c): skip this entire step. Set acceptance_gate = {status: "skipped", reason: "--quick"} and proceed to Step 4F (feature mode) or Step 4P (project mode). The Step 6 summary MUST state the gate was skipped so the user knows the review was static-only.

3G.1 Resolve the authoritative acceptance source

Resolve in order; first found wins. Record acceptance_source.

1. docs/{feature}/srs.md (feature mode) — extract every P0/P1 functional requirement's acceptance criteria + measurable NFRs.
2. docs/{feature}/test-cases.md — extract every P0/P1 test case (TC-ID + title + expected result).
3. Feature mode: plan.md ## Acceptance Criteria. Project mode: the docs-backed criteria already located in Step 2P, or docs/features/*.md ## Acceptance Criteria.
4. None found → bare acceptance: skip reconciliation (3G.3); still run dynamic verification (3G.2). Record acceptance_source = "none" and the note: "No authoritative acceptance source (SRS / test-cases / acceptance criteria) found — coverage reconciliation skipped. Run /spec-forge:srs or /spec-forge:test-cases to enable it."

Build REQUIRED_BEHAVIORS = the P0/P1 items from the first source found, each {id, priority (P0|P1), description, expected_result}.

3G.2 Dynamic verification — run the suite fresh (MANDATORY, never from memory)

Follow the code-forge:verify discipline: IDENTIFY → RUN → READ.

1. Detect the test command (project test framework from the Step "Project Analysis" PA.1; else package.json scripts / pytest / go test ./... / cargo test / mvn test / gradle test / dotnet test / etc.).
2. Run it FRESH via Bash. READ the COMPLETE output — not just the last line.
3. Capture: tests_total, tests_passed, tests_failed, tests_skipped, and the executed test names with pass/fail status (for large suites, capture at least every failing test name plus the passing-test names needed for 3G.3 matching).
4. If no runnable test command exists: record verification = {status: "no_suite"}. No required behavior can be confirmed → every P0 behavior is UNCOVERED → gate BLOCKED (3G.4). Surface: "No runnable test suite found — acceptance cannot be verified."
5. A skipped / todo / pending test does NOT count as passing for any behavior.

3G.3 Coverage reconciliation (skip in bare acceptance)

Dispatch a dedicated acceptance sub-agent — Agent(subagent_type="general-purpose", description="Acceptance reconciliation: {feature}"). The prompt MUST include REQUIRED_BEHAVIORS, the list of test files, and the executed-test names + statuses from 3G.2.

Reference file to read — EXACTLY one: references/format-acceptance.md. This agent reads NO other reference file: not format-common.md, not suppression-gates.md, not call-graph-discipline.md, not any dimensions file. Its polarity is inverted (absence of coverage IS the finding), so the suppression machinery does not apply and loading it would only bias the agent toward dropping real gaps.

The sub-agent maps EACH required behavior to exactly one of:

• covered — a test that (a) exists, (b) executed in 3G.2, (c) passed, and (d) demonstrably exercises this behavior (matched by TC-ID in the test name, OR by behavior/assertion when no TC-ID convention exists). Cite the test name + file:line.
• uncovered — no such test. Absence IS the evidence — do NOT require any further artifact.
• weak — a test exists and passed but only asserts the happy path / "no error thrown" while the behavior requires boundary, error, or negative assertions it lacks. Treat as uncovered for that sub-aspect.

The sub-agent returns ACCEPTANCE_RECONCILIATION per references/format-acceptance.md — a self-contained file. Do NOT hand this agent format-common.md, suppression-gates.md, call-graph-discipline.md, or any dimensions file: it produces no METHOD_CHAINS and no CANDIDATE_INVENTORY, and its findings do not pass through the suppression gates.

3G.4 Compute the gate verdict

1. Convert reconciliation results into findings (these bypass the suppression gates entirely): every uncovered/weak P0 behavior → a blocker titled "Required behavior {id} has no passing test"; every uncovered/weak P1 → a critical; every failed test from 3G.2 → a blocker titled "{test name} is failing".
2. Merge these into the report's blocker_count / critical_count so the existing Step 6 "🚫 Merge blocked" branch fires automatically.
3. ACCEPTANCE_GATE = BLOCKED if (any test failed) OR (any P0 uncovered/weak) OR (verification.status == "no_suite" with a non-empty P0 set). Otherwise PASS. Bare acceptance with a green suite → PASS (unreconciled) with the 3G.1 note.
4. When BLOCKED, set overall merge_readiness = rework_required and list the gate failures at the TOP of Recommendations, above dimensional findings.

Record acceptance_gate = {status, source, required_total, covered, uncovered_p0, uncovered_p1, tests_passed, tests_failed} for the report (Step 4F/4P), state (Step 5F), and summary (Step 6).

→ Proceed to Step 4F (feature mode) or Step 4P (project mode) to display the report.

Step 4F: Feature Mode — Display Report

Review results are displayed in the terminal by default — no file is written. This reflects that reviews are iterative, intermediate checks rather than permanent artifacts.

Orchestrator validation (before display):

Fast path (3F.4a): Verify the single sub-agent's response contains a non-empty METHOD_CHAINS section with at least one entry per public method / exported function in the affected files. If METHOD_CHAINS is missing, empty, or lists fewer public symbols than the affected files contain, reject and re-invoke with an explicit reminder: "Your previous response was missing METHOD_CHAINS or covered only a subset of public symbols. Re-read every affected file and produce the full call graph per §Call-Graph Discipline (see `references/call-graph-discipline.md`) before applying dimensions." Retry at most twice; after the second failure, surface: ⚠ Sub-agent failed to produce full call-graph — findings may miss chain-level bugs. Consider re-running review on a smaller scope.

Layered path (3F.4b + 3F.5):

1. For each per-module agent result, verify METHOD_CHAINS covers all public symbols in that module group's files. Reject and re-invoke any module agent that returned empty or under-covered METHOD_CHAINS (same retry/warning logic as fast path, but scoped per module).
2. Verify the cross-module agent result contains CROSS_MODULE_CONSISTENCY and SECOND_ORDER_REVIEW sections. If either is missing, reject and re-invoke the cross-module agent once.
3. Tier-2 deferral reconciliation (coverage guard). For every callee in the merged TIER2_DEFERRED list handed to the cross-module agent, verify a matching TIER2_RECONCILIATION entry came back. Count the misses as n_tier2_unreconciled. If n_tier2_unreconciled ≥ 1, re-invoke the cross-module agent once with the unreconciled callees listed explicitly. After a second failure, do NOT silently proceed — surface above the findings: ⚠ N cross-module callee(s) exceeded the per-module tier-2 budget and were never expanded by any agent: <callee list>. Defensive gaps on those boundaries are outside this review's coverage. Also verify the inverse: a tier2_files list longer than 8 means an agent ignored its budget — note it, keep the findings (over-expansion costs tokens but does not corrupt results), and do not re-invoke.
4. Merge all findings: Collect issues from all per-module agents + the cross-module agent. Deduplicate by (file, line, title) — if the same finding appears in both a module agent and the cross-module agent, keep the cross-module version (it has more context).
5. Construct a single unified REVIEW_SUMMARY with aggregate counts across all agents.
6. Append a Cross-Module section to the report (see references/report-template.md §Cross-Module section). When any module group reported tier2_budget_exceeded: true, include the over-budget groups and their tier2_eligible_count — a ≤4-file group calling into >8 other in-diff files is D5 (Architecture) signal, and the reader should see it even when the cross-module agent chose not to raise a finding.

Suppression-Gate validation (after merge, before display, both paths):

1. Scratchpad audit (`CANDIDATE_INVENTORY` consistency — REJECT rule, pre-gate). For each sub-agent response, verify the scratchpad and dimension blocks are consistent. This step runs BEFORE gates 1-7 and is the primary defense against the sub-agent bypassing the drop gallery by wording findings cleverly.

0a. Presence: Every sub-agent response MUST include a CANDIDATE_INVENTORY section header. Missing header → reject and re-invoke once with "Your response is missing CANDIDATE_INVENTORY per `references/format-common.md` §Pre-emission Scratchpad. It is a drop ledger: every candidate you decided NOT to emit must be listed with an enum-coded `drop_reason`, and no ledgered row may appear in any dimension block. If you dropped nothing, emit the header with an empty list." After a second failure, treat as automatic fabricating flag in health verdict and surface ⚠ Sub-agent did not produce pre-emission scratchpad — drops not auditable. Note: an empty ledger under a present header is a valid result, not a failure — it asserts that every candidate survived the gates.

0b. Enum compliance: Every drop_reason MUST match one of the DROP enum codes in references/format-common.md §Pre-emission Scratchpad. Free-text reasons (e.g. "not important", "looks minor", "we can defer") are rejected — a row with a non-enum reason is an unaudited drop, so count it as a bypass-attempt signal and surface the row in the health block.

0c. Ledger ↔ dimension cross-check (the critical bypass guard): For every ledger row, verify NO output block — dimension block (D1..D15) OR consistency block (PLAN_CONSISTENCY.unmet_criteria[], CONSISTENCY.unmet_criteria[], CROSS_MODULE_CONSISTENCY.patterns[].issues[], SECOND_ORDER_REVIEW.fix_patterns[].issues[]) — contains an issue with matching (file, line_or_range, title). A match means the sub-agent ledgered the candidate as dropped AND emitted it anyway — a bypass attempt (the ledger written as theater to pass audit). Drop the offending output-block issue and count it as n_scratchpad_bypass. If n_scratchpad_bypass ≥ 2 for a single sub-agent, escalate: reject the response and re-invoke with "Your ledger marks candidate {id} as dropped but the same finding appears in {block}. Either remove the finding from the output blocks OR remove the ledger row — it cannot be both. Further bypass attempts will invalidate the entire response."

0d. Reason-shape sanity: For every ledger row, verify the drop_detail is consistent with the drop_reason code:

• extract_helper_under_3_sites → detail must cite site count ≤ 2 (or reject)
• documented_known_gap → detail must cite where the gap is documented (CLAUDE.md, TODO, etc.)
• self_admitted_low_value → detail must quote the self-downgrade phrase from the finding's description
• trust_boundary_internal → detail must name the input source and argue it's developer-controlled
• unverified_factual_claim → detail must quote the Gate 5 trigger phrase that was unsupported

Rows failing reason-shape sanity are treated as free-text per 0b.

Track count: n_scratchpad_bypass. Surface it in the report health block. A non-zero value materially erodes trust in the whole response.

1. Evidence presence (critical/blocker): For every issue at critical or blocker severity, verify evidence is present and non-empty (more than 10 characters of meaningful content — not "see description" or "TBD"). If any critical/blocker is missing evidence, reject and re-invoke the originating sub-agent with the message: "The following critical/blocker findings are missing required `evidence` per §Finding Suppression Gate: [list]. Either supply concrete reachability evidence (the input that triggers the failure + the observable wrong behavior + the trust-boundary argument for D2) or downgrade / drop the finding." Retry once per agent; after second failure, the orchestrator MUST automatically downgrade those findings to warning and append a [Auto-downgraded: missing evidence] marker to their description.
2. Speculative-phrase scan (ALL severities — DROP, do not downgrade): Scan EVERY finding's description — at blocker, critical, warning, AND suggestion — for the speculative tells (could theoretically, if .* ever, in case someone, potentially might, non-deterministic, might be nicer, smells wrong, feels off, consider .* just in case). DROP each matching finding entirely — do NOT downgrade-and-keep. Track the total as `n_speculative` (break it down by severity for the report summary, but n_speculative is the single total that feeds dropped_total). Rationale: the previous "downgrade one level" policy merely relocated noise from critical/warning into warning/suggestion where no further gate ran; dropping is the only action that actually cleans the report.
3. Trust-boundary check on D2/defensive-gap (critical/blocker): For every critical or blocker in the SECURITY (D2) section or any "missing guard / defensive gap" finding in FUNCTIONAL_CORRECTNESS (D1), verify evidence references a genuinely external input source (network / untrusted user / cross-tenant / third-party API / uploaded file). If evidence describes only an internal/trusted source (project's own files, hard-coded config, type-checked function arguments) and the project type is library / cli / unknown, auto-downgrade to warning with marker [Auto-downgraded: internal trust boundary]. (For frontend / backend / fullstack projects, do NOT auto-downgrade — these often face genuinely untrusted user input and the sub-agent's classification of "internal" deserves more scrutiny than the orchestrator can provide; surface as-is and let the human reviewer decide.)
4. Warning-level observable-downside check (DROP rule): For every warning-severity finding that SURVIVED step 2, verify its description OR evidence explicitly names the observable downside — divergent caller behavior, concrete test failure mode, specific maintenance cost with example, or a missing guard whose input source is genuinely external per Gate 2. Findings that merely report pattern/style divergence ("module A uses X, module B uses Y", "inconsistent with sibling writer") with NO named observable downside are dropped. Track the count as `n_warning_no_downside` and surface it in the report summary. This closes the loophole where the previous validation only scrutinized critical/blocker while warning was a free pass.

4b. Factual-claim verification (DROP rule — applies to ALL severities that survived steps 2/3/4, but most often catches warnings and critical/blockers). Scan every surviving finding's description AND evidence for the Gate 5 trigger phrases: zero references, zero reads, never called, never read, never invoked, no callers, dead code, unreachable, no-op in practice, unused, only used in, only referenced in, only read at, sole consumer, duplicates\s+\w, copy of, redeclares, parallel implementation, reimplements, grep (returns|shows|finds), \d+\s+lines?\s+(exceed|is|are|over|far), exceeds?\s+\d+\s+lines?. When a trigger matches, verify evidence contains a verification artifact:

• A grep / search command line (pattern: grep(-rn|-r|--[A-Za-z])?\s+... OR rg\s+... OR Select-String OR an explicit file-scan description) AND actual matched-line output (at least one line of the form <path>:<line>:<content> OR the explicit string 0 matches / no matches), OR
• Explicit file:line citations (pattern [\w/.-]+:\d+) listing both (for "duplicates X") the original AND the duplicate, or (for "only used in") the definition AND the grep showing no other uses, or (for "N lines") the start:end line range, or
• For "dead code" / "unused" claims: the grep output MUST span at least src/ and tests/ (or the project's test directory equivalent). A dead-code claim whose evidence is only one file or only one directory is dropped — dead-code assertions require cross-project search.

Findings that make a Gate 5 trigger claim without the required verification artifact are dropped entirely (not downgraded — the claim itself is the load-bearing part of the finding; without verification the finding has no substance). Track the count as n_warning_unverified_claim (all severities — the name reflects the dominant source). Rationale: this gate exists specifically to prevent the failure mode where a sub-agent asserts "grep returns only the declaration; zero reads" as summary and the orchestrator has no way to know the grep was never actually run or was run on too narrow a scope. The only defense is to require the output to be visible in the report.

1. Suggestion-level concrete-benefit check (DROP rule): For every suggestion-severity finding that SURVIVED steps 2 and 4b, verify its description names a specific, observable benefit — dead code to delete, non-obvious invariant the comment would clarify, concrete duplication to extract. Findings that merely float a preference ("might be clearer", "could be simpler", "consider refactoring", "for readability") with NO named benefit are dropped — track these as `n_suggestion_no_benefit`. Additionally, scan for the nitpick blocklist — drop the suggestion if its suggestion/description matches any of:

• Pure rename-for-clarity: rename\s+\w+\s+to\s+\w+ (unless description also names a concrete bug-producing ambiguity — past confusion incident, mis-describing name, etc.)
• Style swap without named benefit: consider using \w+ instead of \w+, prefer \w+ over \w+ without a specific downside of the current choice
• Extract-helper against trivial duplication: "extract helper" / "DRY" / "dedup" where the duplicated block is <3 lines OR appears in only 2 sites (single-duplicate extraction almost always adds indirection without a maintenance benefit)
• Formatting / casing: WARNING: vs Warning:, camelCase vs snake_case, whitespace — this is linter/formatter territory, not review territory
• Pure comment-style: "add a comment explaining Y" where Y is described by the code's own name (e.g., "add a comment explaining validateModuleId")
• Binary / packaging / distribution naming: binary-name collisions, npm-scope suggestions, bin script renames — unless the review scope explicitly includes packaging

Track drops from the nitpick blocklist (this sub-check only) as n_suggestion_nitpick — keep it separate from n_suggestion_no_benefit above; dropped_total sums both. Surface both in the report summary.

1. Suggestion budget (DROP rule — applied AFTER step 5 nitpick filter). Compute budget = max(5, ceil(1.5 × warning_count_post_step_4)). If surviving suggestion_count > budget, rank surviving suggestions by (a) whether they have a cited file:line reference, (b) whether they directly map to a dimension with ≥1 critical/warning elsewhere, (c) earlier file paths in alphabetical order as tie-breaker — and drop the tail that exceeds budget. Track drops as n_suggestion_over_budget. Rationale: a review with 2 warnings and 20 suggestions is signaling "I found little substantive but felt obligated to produce bulk" — the reader experience is noise-dominated. Tying suggestion budget to warning count keeps the report's shape matched to the review's actual signal density. Do NOT apply this rule when warning_count_post_step_4 == 0 AND critical_count_post_step_3 == 0 — a review that has nothing else to say may legitimately surface a small set of cleanups (but the budget still caps at 5 in that case).

1. Same-theme consolidation (MERGE rule — applied AFTER step 6 budget): Group surviving suggestions by (file, theme). Theme is derived from a keyword match on title/description:

• renaming — matches rename, misleading name, confusing name
• format_consistency — matches casing, whitespace, formatting, alignment, indent
• error_message_style — matches error message, error string, error prefix, WARNING:|Warning:
• logging_style — matches log level, log prefix, log format
• null_check_style — matches null check, undefined check, ?? vs ||, optional chaining
• iteration_style — matches for loop vs map, .forEach vs for-of, reduce
• import_style — matches import order, barrel import, named vs default import

When a (file, theme) group has ≥3 suggestions, merge them into ONE themed suggestion whose description lists every site (file:line list) and drops the individual entries. Track the count of merged-away entries (original count minus 1 per merged group) as n_suggestion_consolidated. Do NOT merge cross-file themes — a theme crossing files often indicates a legitimate cross-cutting concern that deserves its own warning-level consideration, not consolidation.

Report Health computation (after Suppression-Gate validation, before display):

Compute four health metrics from the merged findings, then derive a single verdict.

Definitions:

• top_pre_downgrade = number of findings that were critical or blocker BEFORE the Suppression-Gate auto-downgrade pass ran (i.e., includes findings that were subsequently downgraded). Track this count separately during the auto-downgrade pass.
• top_post = blocker_count + critical_count after auto-downgrade.
• n_auto_downgrades = n_missing_evidence + n_trust_boundary — only the TWO validation steps that actually downgrade (steps 1 and 3). Speculative-phrase matches are now DROPS, not downgrades, and are tracked in dropped_total instead.
• n_speculative = count of findings dropped by step 2 (speculative-phrase scan, across all severities).
• n_warning_no_downside = count of warnings dropped by step 4 (no named observable downside).
• n_warning_unverified_claim = count of findings dropped by step 4b (factual-claim without verification artifact — most commonly warnings but applies to all severities).
• n_suggestion_no_benefit = count of suggestions dropped by step 5's concrete-benefit sub-check (no named concrete benefit).
• n_suggestion_nitpick = count of suggestions dropped by step 5's nitpick-blocklist sub-check (rename-for-clarity, style swap, trivial-extract-helper, formatting, self-documenting-name comment, packaging-name).
• n_suggestion_over_budget = count of suggestions dropped by step 6 (budget overflow beyond max(5, 1.5 × warning_count_post_step_4)).
• n_suggestion_consolidated = count of suggestions merged away by step 7 (same-(file, theme) groups of ≥3 collapsed to one themed bullet; the merged-away count is original_count − 1 per group).
• n_scratchpad_bypass = count of findings dropped by step 0d (scratchpad marked DROP but dimension block still contained the finding — active bypass attempt).
• dropped_total = n_speculative + n_warning_no_downside + n_warning_unverified_claim + n_suggestion_no_benefit + n_suggestion_nitpick + n_suggestion_over_budget + n_suggestion_consolidated + n_scratchpad_bypass.
• raw_findings_count = total_issues + dropped_total — the sub-agent's raw output count BEFORE any drop.
• Invariants: top_pre_downgrade = top_post + n_auto_downgrades; raw_findings_count = total_issues + dropped_total.

Automatic flag for scratchpad-integrity failures. If n_scratchpad_bypass ≥ 1 OR any sub-agent had to be re-invoked for a missing CANDIDATE_INVENTORY header, raise the fabricating flag unconditionally regardless of the drop-share threshold. A bypass attempt — even a single one — is evidence that the sub-agent is writing the ledger as theater rather than as a genuine drop decision, which means none of its other findings can be trusted without re-verification. (A present but empty ledger is not a failure and never raises this flag.)

Metrics:

1. Finding density = total_issues / max(LOC_reviewed / 100, 1) — issues per 100 LOC reviewed. LOC_reviewed = sum of lines across primary_files (deduplicate when a file appears in multiple module groups; count each file once).
2. Critical share = top_post / max(total_issues, 1) — fraction of findings still at top severity after the gate.
3. Auto-downgrade share = n_auto_downgrades / max(top_pre_downgrade, 1) — fraction of would-be top-severity findings that the gate had to lower.
4. Drop share = dropped_total / max(raw_findings_count, 1) — fraction of raw candidate findings that the gate dropped (from speculative-phrase scan, warning observable-downside check, and suggestion concrete-benefit check combined). raw_findings_count = total_issues + dropped_total; dropped_total is the sum of drops from validation steps 2, 4, and 5.

Per-metric flag rules (each metric independently raises a flag):

The in-between bands (density 1.0–2.0, critical share 5–10%, auto-downgrade share 15–30%, drop share 20–40%) are advisory only — they do NOT raise a flag, but the report header SHOULD show them with a yellow indicator (⚠) so the user can see borderline conditions. The fabricating flag catches sub-agents whose prompt is systematically generating noise the gate then has to remove — if the gate drops >40% of the raw output, the sub-agent is not being careful and the remaining 60% also deserves scrutiny.

Verdict assembly:

• `healthy` — no flags raised
• Otherwise — comma-joined list of raised flags in this order: noisy, inflated, gated, fabricating (e.g., "noisy,fabricating")

Record the verdict and the three numeric metrics in the report header (see references/report-template.md §Report Health) and in state.json review.health (feature mode only — see Step 5F). Persist top_pre_downgrade as well so trend analysis across runs can distinguish "gate caught fewer because there were fewer attempts" from "gate caught fewer because the prompt is now better".

Follow the report template in references/report-template.md (Feature mode variant).

4F.1 Optional: Save to File (--save)

If the user passed --save in the arguments, also write the report to {output_dir}/{feature_name}/review.md. Otherwise, do NOT create the file.

→ Go to Step 5F

Step 4P: Project Mode — Display Report

Orchestrator validation (before display):

Fast path (3P.3a): Verify METHOD_CHAINS covers every public method / exported function in the collected source files. Reject + re-invoke if missing or thin. In project mode the file set can be large; the sub-agent MAY split METHOD_CHAINS into groups-by-file, but total coverage must hit every public symbol. If the sub-agent legitimately cannot cover every symbol within a single response (e.g., 500+ public functions), it MUST explicitly list the un-analyzed symbols in a METHOD_CHAINS_DEFERRED block with reason "scope-too-large" — this surfaces to the user as: ⚠ {N} public symbols not analyzed due to scope — consider narrowing via --project scope=changes or per-feature review. Never silently skip.

Layered path (3P.3b + 3P.4): Apply the same merge and validation logic as Step 4F layered path — verify per-module METHOD_CHAINS coverage, reconcile the merged `TIER2_DEFERRED` list against the cross-module agent's `TIER2_RECONCILIATION` and surface any unreconciled callee as an explicit coverage caveat (Step 4F validation #3), verify cross-module agent produced CROSS_MODULE_CONSISTENCY and SECOND_ORDER_REVIEW sections, merge all findings, deduplicate by (file, line, title), construct unified REVIEW_SUMMARY. Append a Cross-Module section to the report, including any over-budget module groups.

Suppression-Gate validation (after merge, before display, both paths): Apply the same eight checks as Step 4F:

1. Scratchpad audit (`CANDIDATE_INVENTORY` consistency) — verify every sub-agent returned a ledger header, every drop_reason is a fixed-enum code per references/format-common.md §Pre-emission Scratchpad, and no ledgered row has a matching issue in any output block. Drop scratchpad-bypass findings, track n_scratchpad_bypass. See Step 4F for the full protocol. A missing header after retry triggers the automatic fabricating flag; a present-but-empty ledger is valid.
2. Evidence presence (critical/blocker) — every critical/blocker requires non-empty evidence; reject and re-invoke originating agent, auto-downgrade after second failure with [Auto-downgraded: missing evidence] marker.
3. Speculative-phrase scan (ALL severities — DROP) — scan every finding at every severity for the speculative tells (could theoretically / if .* ever / in case someone / potentially might / non-deterministic / might be nicer / smells wrong / feels off). Drop matching findings entirely — do not downgrade-and-keep. Track drops by severity.
4. Trust-boundary check (critical/blocker) — auto-downgrade D2/defensive-gap critical/blocker findings whose evidence describes an internal/trusted source for library / cli / unknown project types, with [Auto-downgraded: internal trust boundary] marker. (Skip auto-downgrade for frontend / backend / fullstack — let humans review those.)
5. Warning-level observable-downside check (DROP) — for every surviving warning, verify description or evidence names the observable downside; pure pattern/style divergence with no named downside is dropped.

4b. Factual-claim verification (DROP — all severities) — when description/evidence contains a Gate 5 trigger phrase (zero references, zero reads, never called, dead code, unused, only used in, only referenced in, duplicates\s+\w, reimplements, parallel implementation, grep (returns|shows|finds), \d+\s+lines?\s+(exceed|is|are)), require evidence to carry a verification artifact (command + output OR file:line citations). Dead-code claims additionally require cross-directory grep (src + tests at minimum). Drop findings without the artifact. Track as n_warning_unverified_claim.

1. Suggestion-level concrete-benefit check (DROP) — for every surviving suggestion, verify a specific, observable benefit is named; preference-only findings ("might be clearer", "consider refactoring") are dropped. Also apply the nitpick blocklist — drop suggestions matching: pure rename-for-clarity, style swap without named downside, extract-helper for <3-line duplication, formatting/casing (linter territory), comment-for-self-documenting-name, binary/packaging naming (out of scope). Track as n_suggestion_nitpick.
2. Suggestion budget (DROP) — budget = max(5, ceil(1.5 × warning_count_post_step_4)); rank surviving suggestions (has file:line > maps to a non-empty dimension > earlier path alphabetically) and drop the tail above budget. Track as n_suggestion_over_budget. Skip when warnings and criticals are both zero and surviving suggestions ≤ 5.
3. Same-theme consolidation (MERGE) — group surviving suggestions by (file, theme); when ≥3 suggestions share a (file, theme), merge into one themed bullet listing every site. Track merged-away entries as n_suggestion_consolidated. Themes: renaming, format_consistency, error_message_style, logging_style, null_check_style, iteration_style, import_style. Do not merge across files.

Surface a single summary line in the report noting the number of auto-downgrades AND the number of drops per severity, so users can spot quota-filling, trust-boundary mistakes, or noise generation without reading the full report.

Report Health computation (after Suppression-Gate validation, before display): Same four-metric computation as Step 4F (finding density, critical share, auto-downgrade share, drop share) and same verdict thresholds (healthy / noisy / inflated / gated / fabricating). Record the verdict and metrics in the report header per references/report-template.md §Report Health. (Project mode does not write to state.json, so the health record only appears in the displayed report.)

Follow the report template in references/report-template.md (Project mode variant).

4P.1 Optional: Save to File (--save)

If the user passed --save in the arguments, also write the report to {output_dir}/project-review.md. Otherwise, do NOT create the file.

→ Go to Step 5P

Step 5F: Feature Mode — Update state.json

1. Read state.json
2. Add or update review field in metadata:

``json { "review": { "date": "ISO timestamp", "rating": "pass_with_notes", "merge_readiness": "fix_required", "total_issues": 12, "blockers": 0, "criticals": 2, "warnings": 6, "suggestions": 4, "acceptance_gate": { "status": "BLOCKED", "source": "test-cases", "required_total": 18, "covered": 15, "uncovered_p0": 2, "uncovered_p1": 1, "tests_passed": 41, "tests_failed": 0 }, "health": { "verdict": "healthy", "finding_density": 1.4, "critical_share": 0.16, "auto_downgrade_share": 0.0, "drop_share": 0.08, "loc_reviewed": 850, "top_pre_downgrade": 2, "top_post": 2, "raw_findings_count": 13, "auto_downgrades": { "missing_evidence": 0, "internal_trust_boundary": 0 }, "drops": { "speculative_phrasing": 1, "warning_no_observable_downside": 0, "warning_unverified_claim": 0, "suggestion_no_concrete_benefit": 0, "suggestion_nitpick": 0, "suggestion_over_budget": 0, "suggestion_consolidated": 0, "orphan_finding": 0, "scratchpad_bypass": 0 } } } } ``

• health.verdict is one of healthy / noisy / inflated / gated / fabricating (or comma-joined when multiple flags apply, e.g. "noisy,fabricating")
• If --save was used, also include "report": "review.md" in the review object
• Persisting health enables trend analysis across runs — a rising auto_downgrade_share signals that the review prompts need reinforcement; a rising drop_share signals the sub-agent is systematically generating noise (speculative / style-only / benefit-less findings) and the prompt needs tightening

1. Update state.json updated timestamp

→ Go to Step 6

Step 5P: Project Mode — No State Update

Project mode does not update any state.json — there is no single feature state to track.

→ Go to Step 6

Step 6: Summary and Next Steps

CRITICAL — Next-step commands are MANDATORY. When the review finds any blocker, critical, or warning issues, you MUST include the /code-forge:fix --review command in the summary output. Never omit it, never paraphrase it, never skip the next-steps block.

6.1 Feature Mode

Display:

Code Review Complete: {feature_name}
 
Rating: {overall_rating}
Merge Readiness: {merge_readiness}
Acceptance Gate: {PASS | BLOCKED | PASS (unreconciled) | skipped (--quick)} — {covered}/{required_total} P0/P1 behaviors covered · tests {tests_passed} passed / {tests_failed} failed{, source: srs | test-cases | plan | none}
Issues: {total_issues} ({blocker_count} blockers, {critical_count} critical, {warning_count} warnings, {suggestion_count} suggestions)
Report Health: {verdict_emoji_concatenated} {verdict} · density {finding_density}/100 LOC · critical share {critical_share_pct}% · auto-downgrades {n_auto_downgrades} · drops {dropped_total}
{If verdict != healthy, append one block-quote line PER raised flag (in order noisy, inflated, gated, fabricating):}
  ⚠ {flag_name}: {hint per §6.3 Verdict Emoji & Hints}
{If --save was used:}
Report saved: {output_dir}/{feature_name}/review.md
 
{If needs_changes (blockers or criticals > 0):}
🚫 Merge blocked — fix these first:
  1. {highest priority blocker/critical with file:line}
  2. {next priority fix}
  ...
  Fix all:    /code-forge:fix --review
  Re-review:  /code-forge:review {feature_name}
 
{If pass_with_notes (warnings > 0, no blockers/criticals):}
⚠ Merge OK with notes — consider fixing:
  1. {top warning}
  2. ...
  Fix all:    /code-forge:fix --review
 
{If pass:}
✅ Ready for next steps:
  /code-forge:status {feature_name}         View final status
  Create a Pull Request
 
Tip: use --save to persist the review report to disk

6.2 Project Mode

Display:

Project Review Complete: {project_name}
 
Rating: {overall_rating}
Merge Readiness: {merge_readiness}
Reference: {planning-backed (N plans) | docs-backed (N documents) | bare}
Acceptance Gate: {PASS | BLOCKED | PASS (unreconciled) | skipped (--quick)} — {covered}/{required_total} P0/P1 behaviors covered · tests {tests_passed} passed / {tests_failed} failed{, source: srs | test-cases | docs | none}
Issues: {total_issues} ({blocker_count} blockers, {critical_count} critical, {warning_count} warnings, {suggestion_count} suggestions)
Report Health: {verdict_emoji_concatenated} {verdict} · density {finding_density}/100 LOC · critical share {critical_share_pct}% · auto-downgrades {n_auto_downgrades} · drops {dropped_total}
{If verdict != healthy, append one block-quote line PER raised flag (in order noisy, inflated, gated, fabricating):}
  ⚠ {flag_name}: {hint per §6.3 Verdict Emoji & Hints}
{If --save was used:}
Report saved: {output_dir}/project-review.md
 
{If needs_changes (blockers or criticals > 0):}
🚫 Issues require attention:
  1. {highest priority blocker/critical with file:line}
  2. {next priority fix}
  ...
  Fix all:    /code-forge:fix --review
  Re-review:  /code-forge:review --project
 
{If pass_with_notes (warnings > 0, no blockers/criticals):}
⚠ Project quality acceptable with notes — consider fixing:
  1. {top warning}
  2. ...
  Fix all:    /code-forge:fix --review
 
{If pass:}
✅ Project quality looks good.
 
Tip: use --save to persist the review report to disk

6.3 Verdict Emoji & Hints

Use this table for the Report Health line (both feature and project mode):

Multi-flag rendering: Concatenate emojis in the order noisy,inflated,gated,fabricating (e.g., 🔊🛑 for noisy,fabricating). Display each flag's hint on its own line below the metrics. Example:

Report Health: 🔊🚧🛑 noisy,gated,fabricating · density 3.1/100 LOC · critical share 6% · auto-downgrades 5 · drops 12
  ⚠ noisy: Density > 2 issues per 100 LOC — likely quota-filling. Re-read findings critically; many may be marginal.
  ⚠ gated: Auto-downgrade share > 30% — gate caught widespread bypass attempts. Sub-agent prompt may need reinforcement.
  ⚠ fabricating: Drop share > 40% — the gate dropped nearly half the raw findings as speculative/style-only/benefit-less. The remaining findings also deserve scrutiny; the sub-agent prompt is systematically generating noise.