Skill v1.0.0
currentAutomated scan100/100name: 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]" metadata: version: "1.0.0"
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-refactorwith the named target. - As a substitute for code review on a specific PR — open or update the PR with
ghand 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.mdand any per-componentCLAUDE.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 vocabulary the PR-Reviewer uses (Dimension F). | |
| Drift from CLAUDE.md | Stated rules the codebase no longer follows. The rule is right; the code drifted. | |
| ADR drift | Decisions ADRs describe that the code no longer reflects (silently superseded). Either update the ADR or fix the code. | |
| Performance shape | Sync I/O in async handlers; N+1 patterns; in-memory aggregation that should be a query. | |
| Observability gaps | Critical paths without logs / metrics / traces; logs that don't include correlation IDs. |
Step 5 — Score and prioritise
For each finding, assign:
- Severity: S1 (active risk — security, data loss, ongoing pain) / S2 (significant friction) / S3 (technical debt without acute symptom) / S4 (cosmetic, defer).
- Effort: S (small, ≤ half a day) / M (medium, 1–3 days) / L (large, > 3 days; probably needs to split).
- Confidence: High (you've read the code yourself) / Medium (one sub-agent finding, you spot-checked) / Low (sub-agent finding, you haven't verified).
Prioritise by severity / effort ratio, with confidence as a tiebreaker — a High-confidence S2 beats a Low-confidence S1; don't recommend a refactor on a finding you haven't personally read. Do not propose more than 10 findings. Cluster related findings into one if they share a root cause.
Step 6 — Write the report
Path:
- File mode:
docs/architecture-review-{YYYY-MM-DD}.md(a report, not a task — it doesn't entertasks/; each accepted finding becomes a task via/squid-refactor). - gh mode: a single issue, label
squid-architecture-review.
Template:
# Architecture review — {scope}, {YYYY-MM-DD}**Scope:** {whole repo / component / path}**ADRs read:** {N} ({list, e.g. "0001–0007"})**Findings:** {N total — by severity: S1×N, S2×N, S3×N, S4×N}## Summary{2–3 sentences. The headline shape: "Three module cycles between auth/ and core/, all introduced in the last 4 months, blocking the planned auth-extraction work. Tests on the affected modules have grown 2× in setup size. No ADR governs the boundary. Recommend ADR + extraction refactor."}## Findings### F1 — {one-line title} (S{1–4}, effort: {S/M/L}, confidence: {H/M/L})**Where:** `path/to/a.py:42`, `path/to/b.py:117`**Symptom:** {what is concretely wrong, observable}**Root cause hypothesis:** {your best read on why}**Proposed refactor:** {one paragraph; the shape of the fix, not the fix itself}**ADR exposure:** {does an ADR govern this? if yes, cite. If no, an ADR should accompany the refactor.}**Out of scope (explicit):** {what this finding is NOT recommending}### F2 — ...(Repeat for each finding, top to bottom by priority.)## Not recommended{Smells you found but are deliberately NOT proposing. State why — e.g., "F-skip-1: The `utils/strings.py` module has 12 unrelated functions and looks like a junk drawer. Skipping because every function is < 10 lines and the cost of splitting exceeds the reading cost."}## Pre-existing decisions (from ADRs) you should NOT undo{One-bullet-each summary of ADRs that explain *why* something looks weird but is intentional. This protects the next reader from re-litigating.}## Suggested order of operations1.F{best-first} — {why it goes first}2.F{next} — {why}...Pick up a finding with `/squid-refactor F{N}`; run the top 1–3 first, not all at once.
Step 7 — Hand off
Single block:
## Architecture review complete — {scope}**Report:** {path or issue URL}**Findings:** {N} ({severity breakdown})**Top recommendation:** F1 — {title}### Recommended next step`/squid-refactor F1` (or paste the F1 body into a fresh `/squid-refactor` invocation). The first finding is the highest leverage by severity-over-effort.### Open question (if applicable){Anything you found but couldn't conclude on without team input — e.g., "F4 hinges on whether the auth/billing boundary should be a hard-coded contract or a feature-flag-toggled split. Surfacing for team decision before refactoring."}### ADR action{One of:}-"Recommend ADR-{NNNN} to record the resolution of F{X} when its refactor lands."-"No ADRs in `docs/adr/` — recommend bootstrapping with the `adr` spec (`squid-scaffold/specs/adr.md`) before proceeding so this review's findings stay durable."