astrolabe/.claude/skills/eng-council/SKILL.md

---
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 whether it needs to exist and the laziest rung that meets the need (stdlib/native/already-shipped dep before new code), then 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. Two questions, in
order, before a line is written.

**Does it need to exist — and at what rung?** Stop at the first rung that holds: (1) the
need is speculative → say so and skip it (YAGNI); (2) stdlib or a language built-in does
it; (3) a native platform feature covers it — `<input type="date">` over a picker lib, CSS
over JS, an IndexedDB/DB constraint over app code; (4) an already-shipped dependency solves
it — Monaco, vega/vega-lite, Zustand are already in the bundle, so never add a dependency,
or hand-roll, for what one of them or a few lines covers (a new dependency owes the
justification `/alignment` demands). Only past the rungs is fresh structure earned. Lazy is
less code, not a flimsier algorithm: between two equal-size options take the one correct on
edge cases, and never simplify away input validation at trust boundaries, error handling
that prevents data loss, or accessibility.

**If it must be built — what shape?** Line up the existing instances of the kind (modal,
store, service, hook, persistence path), name the canonical shape, list what to reuse, and
flag what the new work might make deletable. An abstraction is earned only by ≥ 2 call
sites that would use it today (deletion rule 3).

No report scaffolding — these two answers are the output.

## 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.