feat(dfm): map features to directions, and plan the ways up

.claude/skills/check/SKILL.md

@@ -9,8 +9,11 @@ Run checks from the repository root. This is a pnpm 10/Turborepo workspace;
 do not substitute `npm`, `npx`, or a bare `tsc` command.
 
 1. Inspect `git status --short` first and leave unrelated work untouched.
-2. Run `pnpm check`. It is the normal fast gate and runs the workspace build,
- type generation/type-checking, and unit tests.
+2. Run `pnpm check`. It is the normal fast gate: `check-style`, `lint`, the
+ workspace build, type generation/type-checking, and unit tests, in that
+ order. `pnpm lint` also enforces the layering, so a boundary violation
+ fails here rather than in review; `pnpm lint --fix` settles the
+ formatting-shaped rules.
 3. Run `pnpm audit` as a separate dependency-health check. Report an audit
  failure as a finding; do not change dependencies unless the user asks.
 4. Run `pnpm test:e2e` when the user requests full verification, the change

.claude/skills/review-code/SKILL.md

@@ -10,31 +10,69 @@ report objective, actionable findings ordered by severity. Do not make fixes.
 
 Use the repository's real gates where useful:
 
+- `pnpm lint` for the style rules and the layering.
 - `pnpm check-types` for workspace type-checking.
 - `pnpm test` for unit tests.
 - `pnpm audit` for dependency health.
 - `pnpm test:e2e` when the review includes browser behavior or a user-facing
  flow. Do not run the Docker build unless the reviewed change affects it.
 
-There is no configured lint or coverage script. State either as unavailable
-rather than substituting a different tool or treating its absence as a passing
-result.
+There is no configured coverage script. State that as unavailable rather than
+substituting a different tool or treating its absence as a passing result.
 
-Review against this architecture:
+## What is already proven, and what is left for you
 
-- Browser code in `apps/dfm/app/` calls only app-owned `/api/*` endpoints.
-- `apps/dfm/server/` is the only layer that handles the Toolpath SDK or API
- key. Validate client input and upstream responses at this boundary.
-- API keys and raw presigned/artifact URLs must not be logged, persisted, or
- exposed to the browser. CAD uploads should stay direct to the presigned URL.
+`pnpm lint` fails on any of these, so do not spend the review restating them.
+Run it, and if it is clean, say so and move on:
+
+- browser code under `app/` importing `server/`, by alias or by relative path;
+- a runtime (non-type) Toolpath SDK import outside `apps/dfm/server`;
+- `app/shared/` importing any other layer;
+- the style rules listed in the AGENTS.md table.
+
+The review is for what no check can see:
+
+- Browser code calls only app-owned `/api/*` endpoints, and the server validates
+ client input and upstream responses at that boundary.
+- API keys and raw presigned/artifact URLs are not logged, persisted, or handed
+ to the browser. CAD uploads stay direct to the presigned URL.
 - Part analysis and feature enrichment are asynchronous jobs; route behavior
- must correctly handle queued, running, succeeded, and failed states.
-- Put pure client-domain logic in `app/shared/` and cover it with focused
- tests. Keep React components and Hono handlers as thin integration layers.
+ handles queued, running, succeeded, and failed correctly.
+- Pure client-domain logic sits in `app/shared/` with focused tests, and React
+ components and Hono handlers stay thin.
+
+## Blast radius
+
+For the change under review, ask what a plausible next requirement in the same
+area would cost. If a single change of intent forces edits across several
+modules, the finding is the coupling, not the edit — name the specific import or
+shared shape that causes the ripple, and where the seam should be.
+
+Some files already amplify that cost. Get the current set rather than trusting a
+list — a typed table goes stale within a few commits:
+
+```sh
+find apps/dfm/app -name '*.ts' -o -name '*.tsx' | grep -v '\.test\.' \
+ | xargs wc -l | sort -rn | sed -n '2,8p'
+```
+
+`part-inspector.tsx`, `map-features.tsx`, `metrics.ts`, `rules.ts`,
+`best-reading.ts`, and `rule-editor.tsx` have held the top of that list; check
+whether the diff moved any of them. Size alone is not a finding. Growth in one
+of them, in a change that had a seam available, is.
+
+## Guides and sensors
+
+When a finding is a rule that a command could prove rather than a judgment about
+this diff, say so and name the check that would prove it — a rule in
+`eslint.config.js`, a case in `scripts/check-style.mjs`, or a test. A convention
+repeated in review is a convention that will be violated again.
+
+## Reporting
 
 Also flag demonstrated duplication, missing error handling, unbounded work,
 large response payloads, excessive rerenders, and unnecessary client-bundle
-growth. Do not make speculative refactor suggestions. For every finding give
-the exact path, evidence, impact, and a concrete remediation direction; note
-when a safe refactor needs tests first. End with a prioritized plan only when
-findings justify one.
+growth. Do not make speculative refactor suggestions. For every finding give the
+exact path, evidence, impact, and a concrete remediation direction; note when a
+safe refactor needs tests first. End with a prioritized plan only when findings
+justify one.

.claude/skills/review-testing/SKILL.md

@@ -18,10 +18,21 @@ counting test files. Use this repository's actual test layout and commands:
  and builds the production Docker image on pull requests and `main` pushes.
 
 Run the narrowest relevant command(s), plus `pnpm check-types` when type errors
-could hide test failures. Coverage tooling is not configured; report that fact
-plainly but do not install it or treat a numeric target as the goal.
+could hide test failures, and `pnpm lint` — it is the structural test layer, and
+it proves the layering that no unit test asserts. Coverage tooling is not
+configured; report that fact plainly but do not install it or treat a numeric
+target as the goal.
 
-Prioritize missing behavior at these seams:
+## Where a test belongs
+
+`apps/dfm/docs/README.md` § Testing settles this, its reasons are in
+`docs/directions-replay.md`, and the `setup-testing` skill restates it for
+writing. Read one of those rather than working from instinct, and audit against
+it: a click-on-part test outside `tests/on-the-part.spec.ts`, or any test built
+on captured Engine JSON, is a finding on placement alone regardless of what it
+asserts.
+
+## Behavioral seams to prioritize
 
 - session-cookie and API-key handling, including redaction;
 - Zod validation and app-owned API response contracts;
@@ -30,9 +41,28 @@ Prioritize missing behavior at these seams:
 - pure rule, scoring, selection, measurement, and report transformations;
 - representative UI interactions and the end-to-end part-analysis flow.
 
+## Sensor coverage
+
+Separately from missing tests, report which behaviors and which AGENTS.md rules
+have no automated proof at all — nothing in Vitest, Playwright, `eslint.config.js`,
+or `scripts/check-style.mjs` that would fail if they were broken. Naming that set
+is itself the finding, because those are the rules a long session drifts off
+first. The AGENTS.md style table marks its own unproven rules as judgment; check
+whether that list is still honest, in both directions — a rule marked judgment
+that now has a sensor is as stale as one claiming a sensor it lacks.
+
+Two tests are sensors rather than coverage, and are audited as rules:
+
+- `app/styles.test.ts` — cascade-layer ordering and light/dark role parity.
+- `app/kit-usage.test.ts` — a ratchet holding raw `<button>` at or below a
+ budget. If the count has fallen well under it and the budget was not lowered,
+ that is a finding: the ground is not being kept.
+
+## Reporting
+
 Flag weak assertions, implementation-mirroring tests, over-mocking inside a
 single layer, missing error/edge-state cases, focused or skipped tests, and CI
-steps that can hide a failure. Do not recommend tests for static presentation
-or framework bootstrap that has no behavior. Give each finding a path, the
-missing behavior, and the most suitable test level (unit, route, or E2E), then
-provide a short priority-ordered plan if action is warranted.
+steps that can hide a failure. Do not recommend tests for static presentation or
+framework bootstrap that has no behavior. Give each finding a path, the missing
+behavior, and the most suitable test level (unit, component, route, or E2E),
+then provide a short priority-ordered plan if action is warranted.

.claude/skills/setup-testing/SKILL.md

@@ -9,24 +9,71 @@ This repository already has Vitest, React Testing Library, Playwright, a
 workspace test command, and CI. Do not replace the configuration or install a
 second test framework.
 
-Choose the smallest test level that protects the requested behavior:
-
-- For deterministic contracts, transformations, rules, scoring, selection, or
- presentation helpers, add a colocated test in `apps/dfm/app/shared/`.
-- For client API behavior or interactive components, use Vitest and Testing
- Library in the owning `app/` area.
-- For Hono validation, session, proxy, upload, SSE, or artifact behavior, add
- a focused `apps/dfm/server/**/*.test.ts` test with network boundaries mocked.
-- For a complete browser journey or integration between client and server, add
- a Playwright spec in `apps/dfm/tests/` only when a unit/route test cannot
- prove it.
+## Where a test goes
+
+`apps/dfm/docs/README.md` § Testing is the source for this, and its reasons are
+in `docs/directions-replay.md`. These four rules are not preferences, and
+`review-code`/`review-testing` will flag a test that breaks them:
+
+- **Pure logic goes in `app/shared/*.test.ts`.** That is the bulk of the value
+ and the cheapest place to add coverage. Prefer moving logic here over testing
+ it through a component.
+- **Component tests work, including for components importing `@toolpath/ui`.**
+ `app/components/face-list.test.tsx` is the proof. They are the cheapest
+ coverage for anything list-shaped.
+- **Anything that begins with a click on the part goes in
+ `tests/on-the-part.spec.ts`**, against `tests/cube-fixture.ts` — the only
+ fixture that mounts geometry. Every hand-built report sets `hasMeshGlb: false`,
+ so none of that stack is reachable elsewhere, and three logged bugs reached
+ users because nothing could catch them. Putting a click-on-part test anywhere
+ else does not test it.
+- **Never capture a real part's report and check it in.** Build reports by hand
+ with `tests/part-fixture.ts`; a foreign report tests another codebase's
+ normalization too. The single exception is the vendored viewer cube that
+ `tests/cube-fixture.ts` reads — geometry cannot be written out by hand, and
+ `@toolpath/viewer` publishes `dist` only, so there is nothing to import. Do
+ not add a second exception without the same kind of reason.
+
+Otherwise choose the smallest level that protects the behavior:
+
+- client API behavior or interactive components — Vitest and Testing Library in
+ the owning `app/` area;
+- Hono validation, session, proxy, upload, SSE, or artifact behavior — a focused
+ `apps/dfm/server/**/*.test.ts` with network boundaries mocked;
+- a complete browser journey — a Playwright spec in `apps/dfm/tests/`, only when
+ a unit or route test cannot prove it.
+
+## A rule may want a sensor instead of a test
+
+Some behavior is not reachable by rendering anything. Four existing tests read
+source files directly rather than exercising a component, and a new
+repository-wide invariant usually belongs alongside them:
+
+- `app/styles.test.ts` reads `app/styles.css` for cascade-layer ordering and
+ light/dark role parity.
+- `app/kit-usage.test.ts` ratchets raw `<button>` against the `@toolpath/ui`
+ kit. When a migration lands, lower its `BUDGET` in the same change.
+- `app/shared/reported-regions.test.ts` holds the allowlist of files that may
+ read `regionIdxs` rather than asking the plan what a reading cuts. Adding a
+ path to it is a claim; put the reason beside the use.
+- `app/shared/redaction.test.ts` builds its fixture from the installed SDK's own
+ `PartResponse` declaration, so a URL field added upstream cannot slip through
+ `toPublicInspectionReport` unnoticed. A failure there is an SDK handing out a
+ URL nobody has decided about yet.
+
+If the behavior is really a convention rather than a case, the sensor may belong
+in `eslint.config.js` or `scripts/check-style.mjs` instead. AGENTS.md § Code
+Styling says which sensor owns which rule.
+
+## Writing them
 
 Keep tests behavior-focused: assert specific outputs, request/response
 contracts, errors, and state transitions. At Toolpath boundaries, never use a
 real API key, a live API call, or a real presigned URL; use realistic redacted
 fixtures and test that sensitive upstream fields cannot reach the browser.
 
 Run the closest test first (`pnpm --filter @toolpath/dfm test` for Vitest or
-`pnpm test:e2e` for Playwright), then run `pnpm check` when the change affects
-multiple layers. CI already runs the quality gate, E2E suite, and Docker build;
-do not modify its workflow unless the user's request changes a gate.
+`pnpm test:e2e` for Playwright), then `pnpm check` when the change affects
+multiple layers — it runs `pnpm lint`, which proves the layering a new test file
+must also respect. CI already runs the quality gate, E2E suite, and Docker
+build; do not modify its workflow unless the user's request changes a gate.

.claude/skills/status/SKILL.md

@@ -15,6 +15,9 @@ unstaged/untracked work. Name only a few files; summarize larger groups.
 
 Then identify only evidence-backed open items:
 
+- `pnpm lint` when the diff touches `apps/dfm/`. It is the cheapest gate and the
+ only one that proves the layering, so a boundary break is worth catching at a
+ checkpoint rather than at review;
 - test results from the most relevant existing command, if it is safe and
  proportionate to run (`pnpm test` for unit work; `pnpm test:e2e` for an E2E
  change);

.github/workflows/ci.yml

@@ -20,6 +20,10 @@ jobs:
  node-version: 24.18.0
  cache: pnpm
  - run: pnpm install --frozen-lockfile
+ # The Husky pre-commit hook formats staged files, but it does not run on a
+ # rebase, an amend with --no-verify, or an edit made through the web UI.
+ # Without this step nothing in CI proves the formatting rule at all.
+ - run: pnpm format:check
  - run: pnpm check
  - run: pnpm --filter @toolpath/dfm exec playwright install --with-deps chromium
  - run: pnpm test:e2e

.prettierignore

@@ -2,3 +2,8 @@ apps/*/build/
 apps/*/.react-router/
 apps/*/test-results/
 apps/*/playwright-report/
+
+# pnpm writes this file in its own compact style. Formatting it makes every
+# install rewrite it back, so an unrelated `pnpm install` lands thousands of
+# lines of churn.
+pnpm-lock.yaml