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

name, description, disable-model-invocation

name	description	disable-model-invocation
alignment	Review staged or uncommitted code to ensure quality, test coverage, and alignment with project specifications	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. A deliberate simplification with a known ceiling — a naive scan that's fine at current sizes, a coarse heuristic, a correct-but-unscalable default — gets a comment naming the ceiling and the upgrade path (// linear scan; index if the library grows large), so it reads as a chosen shortcut, not a missed one; unmarked, it invites a later reviewer to either "fix" it back into complexity or flag the absent rationale.

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. All IndexedDB writes go through db.put — a hand-rolled tx(…, 'readwrite') or raw objectStore.put/delete outside db.ts bypasses the quota normalization that mints StorageQuotaError, silently losing the fail-loud signal (arch 02). A new per-record entity tier's write-through subscriber calls the shared wireEntityWriteThrough helper (orchestration/entity-persistence.ts) instead of re-implementing the prev/next diff loop.

13. Documentation hygiene (the docs are maintained artifacts; keep them at altitude): - 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. - Matter-of-fact, as rule #6 demands of comments: docs state what the design is + the standing why, never the build narration (this bit us, we chose X over Y, resolves the former divergence, council resolution recorded). A decision is recorded by stating its resulting rule, not the story of reaching it. - No stale-prone constructs — three things rot the moment the code moves: - TS code blocks that copy current implementation. A snippet mirroring a real module goes stale on its next rename. An illustrative shape sketch is fine; a copy of specific current code is a finding — replace it with a navigation map (file → role) or the rule it demonstrates. - Positional sub-section cross-refs. Cite a doc by a stable identifier — the file (spec §07, arch 02) or a named section / quoted rule — never a positional sub-section number (arch 07 §4) that renumbers when a section is inserted above it. Applies to code comments too. - Volatile exact counts. Test counts, file/LOC counts, and one-off timings churn daily and read as stale within a week. State the qualitative fact ("seconds of layout on a large dataset"), not the measured number. The only homes for exact figures are docs/exploration/ records (frozen by definition) and docs/codebase-metrics.md (the deliberately-tracked trend).

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. Product claims (landing, About, onboarding, value props) follow arch 10 §10: claim only what we can certify, no absolutes (never/always/fully/everything), no durability the platform doesn't back ("saved", not "permanent"), and state a posture once per surface — reduce uncertain promises, keep the real ones.

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.

16. Editor transform-actions reuse an applier, not an inlined skeleton (src/app/services/spec-transform-actions.ts): a new run* action shaped parse → build(spec) → writeBack (info toast on null) calls the matching shared applier — resolveTarget (scoped), applyArrayEdit (one array), or applyWholeSpecEdit (whole-spec drag/simplify) — never re-inlining the model/parse/writeBack prologue. The family has grown by copy-paste twice (eng-council; arch 08).

Output

17. 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	Project philosophy and core values
AGENTS.md	AI onboarding and project context
docs/spec/	Behavioral contract — what the app does
docs/architecture/	Architecture playbook — how it's built
docs/IMPLEMENTATION-PLAN.md	Milestone sequence and scope