/squid-architecture-review

name: squid-architecture-review
description: >-
  Periodic architectural sweep — reads existing ADRs, maps modules/dependencies/layering, and
  reports up to 10 prioritised findings shaped as refactor proposals `/squid-refactor` can consume
  directly.
disable-model-invocation: true
argument-hint: "[scope-path or component name; default = whole repo]"

Architecture review — periodic structural audit

The team's day-to-day pipeline (`/squid-implement-task`, `/squid-implement-night`) operates at task grain. Long-horizon codebase health — drift, unintended coupling, dead modules, layering violations — accumulates between tasks and never gets addressed unless someone deliberately looks. This skill is that deliberate look.

The output is **not a single PR**. It's a prioritised backlog of refactor proposals, each shaped so `/squid-refactor` can pick one up and turn it into a Tasks Plan.

You are the **auditor** — you delegate exploration to sub-agents, you read the target repo's `docs/adr/` to avoid re-proposing settled questions, and you produce a written report. You do NOT write code, do NOT start refactors, and do NOT decide priority for the team — you propose, the human prioritises.

`$ARGUMENTS`:

• Empty → audit the whole repo.
• A path (`packages/backend/`) → audit only that subtree.
• A component name (`backend`, `frontend-web`) → audit only that component.

When NOT to use

• During active feature delivery — review when the team has bandwidth to act on findings, not as theatre.
• On a codebase < 6 months old. There isn't yet enough crystallised pattern to review against.
• When the team has clear architectural anxieties already named — go straight to `/squid-refactor` with the named target.
• As a substitute for code review on a specific PR — open or update the PR with `gh` and review it there instead.
• On infra-only repos (Terraform, CI tooling). The shape doesn't fit; this skill assumes application code with modules, layers, dependencies.

Step 1 — Frame the scope

Resolve `$ARGUMENTS`. If empty, ask the user one question via `AskUserQuestion`:

> Whole repo, one component, or a specific subtree?

Echo the resolved scope back as a one-line confirmation. Don't block.

Step 2 — Read prior decisions

Read the repo's `docs/adr/` end-to-end if it exists. Build a mental model of:

• What decisions are **Accepted** and still in force.
• What decisions are **Superseded** (and by what).
• What constraints (technical, business, team-shape) the ADRs cite.

Never re-propose what an ADR already settled. If ADRs don't exist, the hand-off's ADR action (Step 7) says so.

Also read, if present:

• `docs/glossary.md` (spec: `squid-scaffold/specs/ubiquitous-language.md`) — to know whether the team already names concepts consistently.
• The root `CLAUDE.md` and any per-component `CLAUDE.md` — for stated rules.

Step 3 — Map the current architecture

Spawn 2–3 Explore agents in parallel:

Agent(
  subagent_type="Explore",
  prompt="""Architecture mapping pass on {scope}.

  Produce four artefacts:
  (1) Module / package list with one-line responsibility per module (file:line for the canonical entry point).
  (2) Dependency graph — who imports whom. Flag any cycles. Flag any "junk drawer" modules imported by everyone.
  (3) Layering — name the de-facto layers (e.g., handlers → services → repositories → models) and list any layer-skipping imports.
  (4) Public surface — what's exported / consumed by other components or external callers. Mark anything intended-internal that's leaked.

  Be exhaustive on the dependency graph; missing an edge produces a wrong recommendation. Report as four sections."""
)

Agent(
  subagent_type="Explore",
  prompt="""Hot-spot scan on {scope}.

  Find:
  (1) Files with > 500 lines.
  (2) Files / modules touched by > 30% of commits in the last 6 months — `git log --since='6 months ago' --name-only | sort | uniq -c | sort -rn | head -20`.
  (3) Tests with > 100 setup lines or > 5 fixtures — usually a smell that the system-under-test is too coupled.
  (4) `# TODO` / `# FIXME` / `# HACK` markers — count and cluster.
  (5) Any `# type: ignore` / `eslint-disable` / `nolint` clusters — places where the team is fighting their own static analysis.

  Report as five sections with file:line."""
)

When agents return, **read the top-3 most-implicated files yourself**. Don't rely on summaries on the load-bearing modules — you need firsthand familiarity to call findings credibly.

Step 4 — Identify smells

Walk these dimensions. For each, form 0 or more findings. Don't manufacture findings to pad the report — empty dimensions are fine and signal health; say so explicitly in the report ("Layer violations: none observed").

| Dimension | What to look for | |---|---| | **Cycles** | Import cycles between modules / packages. Always a smell. | | **Layer violations** | Lower layers (e.g., models) importing higher layers (e.g., HTTP handlers). | | **God modules** | One module that everything imports; usually means responsibilities accreted. | | **Junk drawers** | `utils/`, `common/`, `helpers/` modules with > 10 unrelated functions. | | **Hidden coupling** | Two modules that should be independent but share a hidden contract (a shape, a magic string, an env var). | | **Test smells** | High setup-to-assert ratio; brittle tests on private internals; flaky tests. Usually rooted in a structural problem in the SUT. | | **API leakage** | Internal types reaching public surface; public surface that should be private (e.g., a route that no caller uses). | | **Dead code** | Unused exports, unreachable branches, modules with zero importers. | | **Over-engineering** | Abstraction the code doesn't earn: a single-implementation interface / factory / wrapper that only forwards, a layer with one caller, config nobody sets, hand-rolled logic the stdlib ships, or a dependency duplicating a platform/framework feature. Tag each with the same `delete`/`stdlib`/`native`/`yagni`/`shrink`