9.0 KiB
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/).
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
-
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. -
Verification: After changes, run
npm run typecheckandnpm test; runnpm run buildif the change could affect the build. If tests fail, fix the issue if straightforward; ask the user only if non-trivial or ambiguous. -
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
-
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.
-
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. -
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?"
-
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. -
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). 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
-
Portable core boundary:
src/core/must stay pure — no browser APIs (window,document,indexedDB,localStorage), no React, no Monaco, novega-embed. Flag any such import. Pure spec logic (detection, profiling, reference resolution, fit transforms, validation, import normalization) belongs insrc/core/and must be unit-tested. Seedocs/architecture/00-overview.mdfor the layering. -
Infrastructure-adapter boundary: Only
src/app/infrastructure/touchesindexedDB,localStorage, orwindow.location. Flag direct access elsewhere — route it through an adapter (docs/architecture/02-persistence.md,04-routing-and-events.md). -
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 previousvega-embedview must be.finalize()d before re-render (no leaks); user-derived field names must be escaped before going intofield:; an invalid/unrenderable spec must fail safe (readable error, no crash), and a blank spec renders nothing. -
Persistence safety: records that may need migration carry a
versionfield; reads apply migrations; destructive actions (delete, revert, reset) confirm; storage failures warn rather than silently lose data. -
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. -
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.
Output
- 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-updateskill 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.
- 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
- Search for usages of modified functions/types across the codebase (Grep).
- Identify call sites (components, stores, services, infrastructure, tests).
- Check exports used by other modules.
- 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 testandnpm 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 |