architecture-review-method

name	architecture-review-method
description	Use when reviewing a diff for cross-file coherence, coupling, or abstraction-boundary regressions — the architecture lens. Supplies precise vocabulary (depth/seam/locality), the deletion test, the adapter-count seam rule, and a code-graph-first caller-impact recipe with a graph-free fallback. Do not use for writing code, style policing, or other reviewers' lenses.

Architecture review method

Your lens is whole-repo coherence: coupling, abstraction-boundary violations, and maintainability regressions a diff introduces. This skill gives you the vocabulary to name them precisely and a method to trace them — not a new verdict format (keep your agent's verdict convention).

Vocabulary — use these terms exactly in findings

Don't drift into "component", "service", "API", or "boundary". Consistent language is the point.

• Module — anything with an interface and an implementation: a function, class, package, or tier-spanning slice. (Avoid: unit, component, service.)
• Interface — everything a caller must know to use the module correctly: the type signature, plus invariants, ordering constraints, error modes, required config, performance characteristics. Not just the signature. (Avoid: API, signature — too narrow.)
• Implementation — the code inside the module.
• Depth — leverage at the interface: how much behavior a caller (or test) exercises per unit of interface they must learn. Deep = a lot of behavior behind a small interface. Shallow = the interface is nearly as complex as the implementation.
• Seam (Michael Feathers) — a place you can alter behavior without editing in that place; the location where an interface lives. Use this, not "boundary" (overloaded with DDD's bounded context).
• Adapter — a concrete thing that satisfies an interface at a seam (describes the role it fills, not what's inside).
• Leverage — what callers get from depth: more capability per unit of interface learned.
• Locality — what maintainers get from depth: change, bugs, knowledge, and verification concentrated in one place. Fix once, fixed everywhere.

Principles

• The deletion test. Imagine deleting the module. If complexity vanishes, it was a pass-through (shallow). If complexity reappears across N callers, it was earning its keep. Apply this to anything a diff makes you suspect is shallow indirection.
• The interface is the test surface. Callers and tests cross the same seam. If a change forces tests to reach past the interface (asserting on internal state), the module is the wrong shape.
• One adapter = a hypothetical seam. Two adapters = a real one. A seam with a single implementation is just indirection. Flag a diff that adds a port/abstraction with only one concrete implementation, and flag a diff that hard-couples logic across what should be a real seam (two adapters justified — typically production + test).

Is a seam even warranted? (dependency categories)

Classify a dependency before recommending a port — this turns the adapter-count rule into a concrete Fix:. Concrete dependency examples for this stack are part of the {{STACK_DESCRIPTION}} scan; the categories:

• In-process (pure compute, in-memory state) — merge the modules and test through the new interface directly; no port.
• Local-substitutable (a dependency with an in-memory or embedded stand-in) — the seam is internal; test with the stand-in in the suite, no port at the module's external interface. A diff that adds a public port here is needless indirection — say so.
• Remote but owned (an internal service across the network) — define a port; a live adapter for prod, an in-memory adapter for tests. Logic stays in one deep module.
• True external (third-party services you don't control) — inject a port; tests use a mock adapter.

Method on a diff

Do the read per your eager instructions (fresh full-repo+diff pass, your largest-in-panel context). This skill only adds what to look for through the vocabulary above — friction the diff introduces or worsens:

• Understanding one concept now requires bouncing between many small modules.
• A module made shallow — its interface nearly as complex as its implementation.
• Pure functions extracted only for testability, where the real bug lives in how they're called (no locality).
• Tightly-coupled modules leaking across their seams. Known recurring coupling shapes for this repo (filled by the retro loop as they're found):

• Code the diff leaves untested or hard to test through its current interface.

Apply the deletion test, the dependency-category check, and the adapter-count rule to each suspect.

Caller-impact: code-graph tool first, grep fallback

Prefer your seat's code-graph tool ({{CODE_GRAPH_TOOL}}) for out-of-diff caller-impact: its impact/context/detect-changes equivalents for callers, callers+callees+flows, and the diff's affected scope. FALLBACK when the code-graph tool is unavailable or unreliable on your runtime: trace with a repo-wide grep to enumerate call sites outside the diff, check each for drift (arity, param/return types, ordering/invariant, error modes), and resolve each new/changed import path to catch "referenced-but-missing module" gaps. Either way your accuracy guards hold: tag VERIFIED vs INFERRED, and a graph hit is INFERRED until you read the cited code.

Findings

Render each finding in the squad canonical finding grammar:

[SEVERITY] (confidence: N/10) {file}:{line} — {coupling/seam/locality defect, named in the vocabulary above + why it's wrong} | Fix: {the deepening or decoupling that closes it}

Confidence follows the evidence: a VERIFIED finding you read end-to-end rates higher; an INFERRED one is capped lower. Emit your verdict and end-marker per your agent's own VERDICT FORMAT (already in your instructions) — do not restate the verdict token here, and do not adopt a different skill's verdict spelling. Strictly read-only: never edit files, write CONTEXT.md/ADRs, run a PoC against a live service, push, or merge.