mirror of
https://github.com/olehomelchenko/astrolabe.git
synced 2026-08-08 10:12:34 +00:00
119 lines
8.3 KiB
Markdown
119 lines
8.3 KiB
Markdown
---
|
||
name: eng-council
|
||
description: Convene the engineering council — an evidence-grounded review of codebase structure, consistency, layering altitude, and subtraction (what should be deleted). 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, 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. This skill exists to see that whole picture.
|
||
|
||
**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).
|
||
|
||
| 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. |
|
||
|
||
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.
|
||
|
||
### 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). All
|
||
four 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. 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).
|
||
|
||
### 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, each finding: `file:line`, the evidence, the proposed fix,
|
||
net LOC delta. Skip seats with nothing to say in one line, not zero.
|
||
3. **Remove** — the mandatory section (deletion rule 2).
|
||
4. **Net delta** — total LOC delta if all findings were accepted.
|
||
5. **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.
|
||
Structural findings (merge two shapes, move logic across layers, kill an abstraction) —
|
||
propose with the evidence; they're the user's call. Out-of-scope observations get a
|
||
`// TODO:` breadcrumb at the code site, same rule as `/alignment`.
|
||
|
||
## 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.
|