astrolabe/.claude/skills/alignment/SKILL.md

---
name: alignment
description: Review staged or uncommitted code to ensure quality, test coverage, and alignment with project specifications
disable-model-invocation: true
---

# Code Alignment

Review staged or uncommitted code to ensure quality, test coverage, and alignment with the
project's spec (`docs/spec/`) and architecture playbook (`docs/architecture/`).

This skill is also executed by a **clean-context subagent** at session wrap-up (see
CLAUDE.md → Session wrap-up protocol). When running as that subagent: you deliberately
have no session context — judge the diff against the written contracts only, and return
the summary (rule #15) as your final message so the session agent can relay it. If a
change looks deliberate but its rationale is recorded nowhere, that absence is itself a
finding.

## Scope

Determine the review scope using `git diff` (unstaged) and `git diff --staged` (staged).
Review all changes in scope. If changes span multiple patterns below, apply all relevant sections.

## General Instructions

### Process

1. **Git**: **NEVER** stage (`git add`) or commit (`git commit`) — that is the USER's
   responsibility. If the reviewed changes span multiple independent concerns (a feature + an
   unrelated fix, a refactor + a new capability), suggest splitting them into separate commits
   and mention the logical boundaries.

2. **Verification**: After changes, run `npm run typecheck` and `npm test`; run `npm run build`
   if the change could affect the build. If tests fail, fix the issue if straightforward; ask
   the user only if non-trivial or ambiguous.

3. **Fix directly; don't ask first.** When you find an issue covered by these instructions,
   fix it in place rather than reporting it and waiting. Ask the user only when the fix is
   genuinely ambiguous or several valid approaches exist with real trade-offs. When guidelines
   conflict, prefer in this order: **SOUL.md philosophy > `docs/spec/` behavioral contract >
   `docs/architecture/` patterns > local cleanup**. These instructions are not strictly
   prohibitive — if a guideline has a valid reason to be bypassed, mention it in the summary.

### Code Quality

4. **Code Cleanup**: Remove leftover code, unnecessary defensive programming, and
   over-engineering from iterative development — dead code, try/catch around internal calls
   that can't throw, abstraction layers wrapping a single implementation. Proceed with caution;
   ask if unsure.
   - **Export hygiene**: a symbol is exported only if another module imports it. Symbols used
     only within their module (including `as const` arrays that exist to derive a type) stay
     unexported — `export type` the type, not its source array. Verify with Grep before
     exporting "for future use"; the future caller can add the export.

5. **Styles and UI**: When altering CSS or layout, follow or generalize existing patterns
   (CSS Modules + design tokens in `styles/tokens.css`) rather than writing from scratch. Don't
   fix whitespace/formatting (trailing newlines etc.) — Prettier owns that.
   - **Control primitives & the two-height scale** (arch 09 §4): action buttons are the
     `Button` component, icon-only buttons are `IconButton` — never a freshly styled
     `<button>`. Interactive controls are `var(--control-height)` (32px) or
     `var(--control-height-lg)` (40px); a hardcoded control height (28px, 36px, …) in a
     diff is a finding. **The field look has exactly one home** — the element baseline
     in `styles/base.css` (fill `var(--field)` + bottom border `--border-strong`, no
     box; arch 09 §4): a `border:` or `background:` on an input/textarea in a component
     module is a finding (module classes add only width/padding/font-size); the sole
     sanctioned restatements are the select-like triggers (SelectControl, SortControl).
     A surface that elevates to `--layer-01` sets `--field: var(--field-02)` /
     `--field-hover: var(--field-hover-02)` on its container, mirroring
     `--control-hover-fill`. Call-site classes composed onto a primitive may only do layout
     (flex, margins, reveal) or a documented state accent (outlined-danger, pressed) —
     restyling the primitive's box from a call site is a finding. Borders mark function:
     full `--border-strong` boxes are reserved for segmented controls, secondary
     buttons, drop targets (dashed), and the color-swatch input; fields and triggers
     are underlined, not boxed;
     passive chrome (tags, badges, glyphs) takes `--border`; plain actions are ghost or
     filled; list rows are flat with dividers, not stacked boxes. A shared look travels
     through one of exactly four mechanisms — design tokens, contextual custom
     properties set by surfaces, `base.css` element baselines, React primitives
     (Button/IconButton); introducing a fifth (CSS-module `composes`, utility classes,
     a mixin layer) is a finding. When a recipe migrates to a shared baseline, grep
     for every selector that restated any of its fragments (focus, placeholder,
     border) — a partially deleted restatement is worse than an undeleted one,
     because its higher specificity silently overrides the baseline.

6. **Code Comments**: Comments should not duplicate what the code already says. Remove
   parroting comments. Ensure comments capture non-obvious _why_ — design decisions,
   constraints, gotchas. Flag missing comments where a reader would reasonably ask "why is this
   done this way?" Write them as **matter-of-fact prose** — state what the code _is_ and the
   standing _why_, not the story of how this session arrived at it. Rewrite session-decision
   narration ("this bit us", "we decided", "supersedes the earlier plan", "used to do X") and
   directives-to-future-self ("keep the escape") into a standing property of the code; keep the
   technical fact, drop the resolution framing.

7. **Workarounds**: Flag code that works around a problem rather than solving it (`// HACK`,
   silent catch-and-ignore, feature detection for internal bugs). A justified workaround
   (upstream bug, browser quirk) needs a comment explaining why and a tracking reference; an
   unjustified one should be replaced with a proper fix.

8. **Pre-existing & out-of-scope issues — leave a breadcrumb.** For anything you notice but
   don't fix (pre-existing patterns the new code follows; observations the change exposes but
   that are out of scope), mark it with a `// TODO:` at the relevant code site explaining
   _what_ could be improved and _why_ (1–3 lines), as matter-of-fact prose (rule #6 — no
   session narration). **If an observation is important enough to mention in the summary, it is
   important enough to deserve a `// TODO:` at the code location** — otherwise the next reader
   has no way to recover the context.

### Architecture & Project-Specific Checks

9. **Portable core boundary**: `src/core/` must stay pure — no browser APIs (`window`,
   `document`, `indexedDB`, `localStorage`), no React, no Monaco, no `vega-embed`. Flag any such
   import. Pure spec logic (detection, profiling, reference resolution, fit transforms,
   validation, import normalization) belongs in `src/core/` and must be unit-tested. See
   `docs/architecture/00-overview.md` for the layering.

10. **Infrastructure-adapter boundary**: Only `src/app/infrastructure/` touches `indexedDB`,
    `localStorage`, or `window.location`. Flag direct access elsewhere — route it through an
    adapter (`docs/architecture/02-persistence.md`, `04-routing-and-events.md`).

11. **Rendering safety** (`docs/architecture/05-rendering-theming-preview.md`): the
    reference-resolution/fit-mode transform must run on a **copy** of the spec — never mutate the
    stored spec; a previous `vega-embed` view must be `.finalize()`d before re-render (no leaks);
    user-derived field names must be escaped before going into `field:`; an invalid/unrenderable
    spec must fail safe (readable error, no crash), and a blank spec renders nothing.

12. **Persistence safety**: records that may need migration carry a `version` field; reads
    apply migrations; destructive actions (delete, revert, reset) confirm; storage failures
    warn rather than silently lose data.

13. **Self-containment**: documentation and comments must not add pointers that require an
    external repository to follow. Knowledge gets captured locally (`docs/spec/`,
    `docs/architecture/`), not linked out.

14. **User-facing copy**: keep user-visible strings centralized and written for users (sentence
    case, active voice, no "please", no exclamation marks in errors). If/when an i18n layer
    exists, route strings through it instead of hardcoding.

15. **Chart-builder guidance reasons over role, not raw type** (`src/core/chart-builder.ts`):
    a `builderWarnings` rule (or any measure/dimension decision) must ask the post-transform
    **role** via the shared predicates (`isMeasureMapping`, `isReorderableCategory`) — never
    test `effectiveType(m) === 'quantitative'` directly for measure-ness. `bin` makes a field a
    discretized _dimension_ (mirrors Vega-Lite's `isDiscrete`); `aggregate` makes it a _measure_.
    Reasoning over raw type is what made a histogram trip the two-measures→scatter nudge
    (eng-council 2026-06-13; arch 10 §5). A new taste-heuristic warning should also be
    high-precision: prefer structural/data-driven hints; lean on the intent front door + smart
    defaults for positive guidance rather than enumerating bad combinations.

### Output

16. **Summary**: respond with a summary of changes — choices made due to these instructions,
    choices where multiple approaches existed, and non-obvious architectural assumptions the
    user should know but might not spot in the diff. If the summary mentions an observation you
    chose not to fix (rule #8), confirm a `// TODO:` breadcrumb was placed at the code site.

---

## Pattern A: New Functionality

### Testing

- Unit tests for new `src/core/` logic (test the core hardest).
- Lighter component/interaction tests for new UI.
- Tests pass before proceeding.

### Documentation

Update relevant docs if the feature is significant:

- **`docs/spec/`** — if product behavior changed (this is a contract; change deliberately).
- **`docs/architecture/`** — if a new pattern, navigation map, or decision rule emerged.
- **`docs/IMPLEMENTATION-PLAN.md`** — mark milestone progress.
  Use the `/doc-update` skill for session-discovered gaps. The list is not exclusive.

### Dependencies

If `package.json` changed:

- Flag each new dependency; explain what it does and why it's needed.
- Every package imported directly in `src/` must be declared in `dependencies` — never rely
  on a transitive install (it can vanish or drift on any lockfile churn). Declaring a package
  the bundle already carries adds no weight.
- Could a small custom implementation avoid it? Note the trade-off.
- Prefer dependencies that solve genuinely hard problems (parsing, rendering) over those that
  save boilerplate.

### Alignment Check

- **SOUL.md** — philosophy (must not violate without good reason).
- **`docs/spec/`** — behavioral contract.
- **`docs/architecture/`** — the relevant pattern doc.

---

## Pattern B: Bug Fixes

### Testing

- Add a regression test that reproduces the bug and verifies the fix.
- Interaction test if the bug affected UI behavior.

### Documentation

Usually not required unless the bug revealed incorrect docs, or the fix changes documented
(spec) behavior.

### Alignment Check

- **SOUL.md** philosophy; **`docs/spec/`** behavioral contract; **`docs/architecture/`** patterns.

---

## Pattern C: Refactoring

### Impact Analysis

1. **Search for usages** of modified functions/types across the codebase (Grep).
2. **Identify call sites** (components, stores, services, infrastructure, tests).
3. **Check exports** used by other modules.
4. **Review dependencies** — what the code depends on and what depends on it.

### Testing

- Update existing tests to the new structure; verify all call sites.
- Run `npm test` and `npm run typecheck`.

### Documentation

Update `docs/architecture/` if a pattern, module responsibility, or navigation map changed.
Update JSDoc/inline comments if signatures or behavior changed.

### Alignment Check

- **SOUL.md** (simplicity, no parallel systems); **`docs/architecture/`** (consistent with the
  documented patterns); **`docs/spec/`** (behavior unchanged unless intended).

### Common Refactoring Checks

- Function signatures → all call sites updated.
- Type definitions → search type usages.
- Imports → correct after file moves.
- Stores → all consumers verified.
- Component props → all usages checked.
- Constants/enums → all references updated.

---

## Reference Documents

| Document                                                            | Purpose                                   |
| ------------------------------------------------------------------- | ----------------------------------------- |
| [SOUL.md](../../../SOUL.md)                                         | Project philosophy and core values        |
| [AGENTS.md](../../../AGENTS.md)                                     | AI onboarding and project context         |
| [docs/spec/](../../../docs/spec/)                                   | Behavioral contract — _what_ the app does |
| [docs/architecture/](../../../docs/architecture/00-overview.md)     | Architecture playbook — _how_ it's built  |
| [docs/IMPLEMENTATION-PLAN.md](../../../docs/IMPLEMENTATION-PLAN.md) | Milestone sequence and scope              |