--- name: eng-council description: Convene the engineering council — an evidence-grounded review of codebase structure, consistency, layering altitude, subtraction (what should be deleted), and documentation altitude. Modes - whole-codebase sweep (milestone boundary or backfill), refactor review, new-functionality review, and a pre-build consult. Auto-invoke only the consult mode - before introducing a new module/store/modal/service/hook shape, ask the council for the canonical shape and what already exists to reuse. All other modes run on demand. disable-model-invocation: false --- # Engineering Council The structural counterpart to `/council`. The design council guards what the user sees; the engineering council guards the shape of the codebase — structure, consistency, altitude, documentation, and above all **net growth**. `/alignment` reviews a diff in isolation and can't see cross-cutting drift: parallel patterns forming, near-duplicate helpers, modules outgrowing their responsibility, docs accreting a move-by-move log. This skill exists to see that whole picture. **The lens is wider than the diff — on purpose.** `/alignment` stays inside the change and parks anything else as a `// TODO:`. The engineering council is the opposite: it reviews the change _from altitude_, and is expected to lift its eyes to what surrounds it and record improvement opportunities it notices outside the reviewed lines. A review that finds nothing beyond its own diff has stayed too close to the ground. Those observations are **noted, not fixed** (see Beyond-scope notes in the report) — surfacing them is the value; the user triages. **Seats are evidence lenses, not personas.** The design council works because every seat quotes a real corpus. There is no external canon for software structure, so a seat here is defined by _what it measures and lines up_ — different seats run different queries over the codebase. A seat that produces an opinion without evidence (file:line, tool output, or a side-by-side of instances) has not spoken. ## The deletion rules (output format, non-negotiable) LLMs accrete code; aspirational "prefer removal" instructions wash out. So the bias is encoded in the report format instead: 1. **Every finding states its estimated net LOC delta** (e.g. `−120`, `+15`). The report ends with the total if all findings were accepted. 2. **"Remove" is a mandatory report section.** It may be empty, but emptiness must be argued ("knip clean, no single-implementation abstractions found"), not skipped. 3. **Any proposal that adds an abstraction must name ≥ 2 existing call sites** that would use it _today_. No speculative generality. 4. **A refactor that adds a new way to do something already done elsewhere is a finding, not progress** — even if the new way is better, until the old instances are migrated and the old way deleted. ## The seats Run **sequentially, in this order, in this session** — no subagent fan-out. Later seats consume earlier findings (Subtraction needs Consistency's verdict on which variant is canonical before deciding which duplicate dies; Documentation runs last so it can check the docs against what the other seats actually found — a doc describing a shape Consistency says no longer exists is stale). | Seat | Question | Evidence it gathers | | ----------------- | ----------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **Structure** | Is the dependency picture still the documented one? | `npx madge --circular --extensions ts,tsx src` (cycles); `npx madge --json src` for fan-in/fan-out outliers; grep for layer leaks (`docs/architecture/00-overview.md` defines the layers — core imports nothing from app; only infrastructure touches browser APIs). Oversized modules: files ≫ their peers' size doing > 1 job. | | **Consistency** | How many shapes does each _kind_ of thing have? | Line up all instances of a kind (modals, stores, services, hooks, confirm flows, persistence subscribers) side by side. Count distinct shapes; name the canonical one; list divergers. Divergence is only visible in the line-up — never judge an instance alone. | | **Altitude** | Is logic at the layer where it's cheapest to test? | Testable logic stuck in components that belongs in `src/core/` or stores (the testing philosophy in `AGENTS.md` is the canon: core hardest, components lightest). Spec operations outside core. Duplicated derivations that should be a store selector. | | **Subtraction** | What can be deleted? | `npx knip` (dead exports, unused files/deps); `npx jscpd src --min-tokens 50` (duplication); grep for abstractions with a single implementation, re-implementations of an existing utility, props/options/branches no caller exercises, spec'd-then-abandoned remnants. Consumes all prior seats' findings. | | **Documentation** | Do the docs hold a structured general picture, or accrete a move-by-move log? | Doc sizes & growth (`wc -l docs/architecture/*.md`; the `codebase-metrics.md` trend). Grep `docs/` for decision-log narration (`this bit us`, `we chose`/`we decided`, `supersed`, `resolves the former`, `previously`, `council resolution recorded`, one-off measurements) and for per-instance worked examples that re-illustrate a principle already stated generally or restate code. Embedded TS blocks that copy current code (vs. illustrative shape-sketches). Sections > 50 lines (doc-update's own bar). Stale-prone refs: line/file/test counts, positional sub-section cross-refs (`arch 07 §4`) that renumber, one-off timings. **Docs-vs-code drift:** a doc describing a shape the other seats found no longer exists. The bar (from `/doc-update`): docs state _what the design is_ + the standing _why_ at a stable altitude — not how the build arrived there. | Tool notes: `madge`, `knip`, `jscpd` are **not** project dependencies — run via `npx`, treat output as evidence, not verdict (knip false-positives on entry points and dynamic imports; verify a symbol is truly dead before deleting). If a tool fails or is unavailable, the seat still sits — grep is the fallback evidence. ## Modes Pick the mode from what's in front of you; say which mode is running. The two review modes are also executed by a **clean-context subagent** at session wrap-up, after `/doc-update` and the alignment pass (see CLAUDE.md → Session wrap-up protocol). When running as that subagent: you deliberately have no session context — judge the diff against the codebase's own population of shapes, and return the report as your final message so the session agent can relay and arbitrate it. A deliberate-looking divergence whose rationale is recorded nowhere is itself a finding. ### Sweep — whole codebase For: milestone boundaries, the one-off backfill, "how healthy are we?" on demand. **Never on a schedule.** Scope: all of `src/` (+ `styles/` for the Structure seat, `docs/` for the Documentation seat). All five seats. Output: the full report (format below) **plus a metrics snapshot row** appended to `docs/codebase-metrics.md` (create on first sweep): ``` | date | src files | src LOC | core LOC | app LOC | deps | knip dead exports | jscpd dup % | ``` The trend line is the point — accretion becomes visible instead of felt. ### Refactor review For: a landed or in-progress refactor (`git diff` / `git diff --staged`). Scope: changed files **plus their one-hop import neighborhood** (importers and imports) — drift is only visible relative to neighbors. Seats: Consistency and Subtraction always; Structure if imports moved; Documentation if the change touched a doc or makes one stale. Center question: did this refactor _reduce_ the number of shapes, or add variant N+1? (Deletion rule 4 applies with full force.) ### New-functionality review For: a new feature in the diff. Scope: changed files + one-hop neighborhood. Seats: Consistency (does it follow the canonical shape for its kind?), Altitude (is the logic in core/stores, not the component?), Subtraction (does it re-implement an existing utility? does every prop/option/branch have a caller?), Structure if a new module or dependency appeared (a new dependency needs the same justification `/alignment` demands), Documentation if the feature added or should have added docs (and whether what it added stays at altitude, not a move-by-move log). ### Consult — before writing code The cheapest and most preventive mode; the only one that auto-fires. Before building a new instance of a kind (a modal, store, service, hook, persistence path): line up the existing instances, name the canonical shape, list what already exists to reuse, and flag what the new work might make deletable. Two paragraphs, no report scaffolding. ## Report format For sweep and review modes: 1. **Verdict** — two or three sentences: overall shape, the one thing to act on first. 2. **Findings** — per seat (Structure, Consistency, Altitude, Subtraction, Documentation), each finding: `file:line`, the evidence, the proposed fix, net LOC delta. Skip seats with nothing to say in one line, not zero. 3. **Beyond-scope notes** — improvement opportunities the review noticed _outside_ the reviewed change: a shape worth revisiting elsewhere, a doc drifting, an abstraction forming across files the diff didn't touch. Noted with `file:line` and a one-line why, **not fixed here** — the user triages. Expected to be non-empty in a healthy review; an empty one means either a pristine codebase or a council that stayed too close to the diff. 4. **Remove** — the mandatory section (deletion rule 2). 5. **Net delta** — total LOC delta if all findings were accepted. 6. **Rules discovered** — recurring patterns worth making law (see Close the loop). **Fix vs. propose:** mechanical, behavior-preserving removals (dead exports, unused files, unreferenced props) — fix directly, keep `npm run typecheck` + `npm test` green. Trimming decision-log narration from a doc back to the matter-of-fact bar is the documentation equivalent of a behavior-preserving removal — fix directly. Structural findings (merge two shapes, move logic across layers, kill an abstraction, restructure or consolidate a doc) — propose with the evidence; they're the user's call. An in-scope out-of-place observation gets a `// TODO:` breadcrumb at the code site (same rule as `/alignment`); a beyond-scope one goes in the Beyond-scope notes section so it surfaces rather than scattering as TODOs. ## Close the loop A council that re-finds the same drift every sweep has failed. When a finding reveals a _recurring_ rule (not a one-off): capture it into the relevant `docs/architecture/` doc via `/doc-update`, and if it's checkable in a diff, **add it as a numbered check to `/alignment`** so pointwise review enforces it from then on. The council discovers systemic rules; `/alignment` keeps them honest. Sweeps should get quieter over time — that, plus the metrics trend, is how you know it's working.