mirror of
https://github.com/olehomelchenko/astrolabe.git
synced 2026-08-08 02:02:33 +00:00
200 lines
9.0 KiB
Markdown
200 lines
9.0 KiB
Markdown
---
|
||
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/`).
|
||
|
||
## 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.
|
||
|
||
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.
|
||
|
||
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?"
|
||
|
||
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). **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.
|
||
|
||
### Output
|
||
|
||
15. **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.
|
||
- 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 |
|