From afa53cfd181794d6e37982d3bcd83164ae7cb982 Mon Sep 17 00:00:00 2001 From: bartstc Date: Mon, 20 Apr 2026 15:24:12 +0000 Subject: [PATCH 1/4] chore: extend sdd with unit and component tests --- .agents/skills/building-blocks/README.md | 19 +-- .agents/skills/building-blocks/SKILL.md | 9 ++ .agents/skills/building-blocks/metadata.json | 8 +- .../rules/component-story-test.md | 139 ++++++++++++++++++ .../skills/building-blocks/rules/fixture.md | 83 +++++++++++ .../building-blocks/rules/msw-handler.md | 68 +++++++++ .../skills/building-blocks/rules/unit-test.md | 37 +++++ .agents/skills/writing-spec/SKILL.md | 17 ++- docs/spec-driven-development.md | 51 +++++-- docs/spec-template.md | 33 ++++- 10 files changed, 433 insertions(+), 31 deletions(-) create mode 100644 .agents/skills/building-blocks/rules/component-story-test.md create mode 100644 .agents/skills/building-blocks/rules/fixture.md create mode 100644 .agents/skills/building-blocks/rules/msw-handler.md create mode 100644 .agents/skills/building-blocks/rules/unit-test.md diff --git a/.agents/skills/building-blocks/README.md b/.agents/skills/building-blocks/README.md index bab0d06..5b87d25 100644 --- a/.agents/skills/building-blocks/README.md +++ b/.agents/skills/building-blocks/README.md @@ -10,13 +10,14 @@ A structured catalog of typed frontend building blocks, optimized for AI agents. ## Building Block Categories -| Category | Blocks | Description | -| ------------------ | ------ | ------------------------------------------------------------------------------------- | -| Data Fetching | 4 | Query factories, mutations, keys, DTOs | -| State Management | 2 | Zustand stores, React Context providers | -| App Orchestration | 1 | Use case hooks composing mutations + notifications | -| Component Patterns | 7 | Pure components, compound components, forms, pages, HOCs, facade hooks, named effects | -| Data Modeling | 2 | Frontend models, value objects | +| Category | Blocks | Description | +| ------------------- | ------ | ------------------------------------------------------------------------------------- | +| Data Fetching | 4 | Query factories, mutations, keys, DTOs | +| State Management | 2 | Zustand stores, React Context providers | +| App Orchestration | 1 | Use case hooks composing mutations + notifications | +| Component Patterns | 7 | Pure components, compound components, forms, pages, HOCs, facade hooks, named effects | +| Test Infrastructure | 4 | Vitest unit tests, Storybook play-function tests, MSW handlers, test data fixtures | +| Data Modeling | 2 | Frontend models, value objects | ## How It Works @@ -35,8 +36,8 @@ A structured catalog of typed frontend building blocks, optimized for AI agents. ```markdown --- title: Block Display Name -category: Data Fetching | State Management | App Orchestration | Component Patterns | Data Modeling -layer: providers/ | application/ | components/ | models/ | lib/api/ | pages/ +category: Data Fetching | State Management | App Orchestration | Component Patterns | Test Infrastructure | Data Modeling +layer: providers/ | application/ | components/ | models/ | lib/api/ | pages/ | test-lib/handlers/ | test-lib/fixtures/ | co-located composedWith: other-block-1, other-block-2 --- diff --git a/.agents/skills/building-blocks/SKILL.md b/.agents/skills/building-blocks/SKILL.md index c8e610f..79519e1 100644 --- a/.agents/skills/building-blocks/SKILL.md +++ b/.agents/skills/building-blocks/SKILL.md @@ -50,6 +50,15 @@ Read the rule file for each block you are about to implement. The summaries belo | `facade-hook` | `components/` | Private logic extraction for a single component. Defined below the component, not exported. | `rules/facade-hook.md` | | `named-effect` | `components/` | Named function expressions in `useEffect`. Intent visible at a glance. | `rules/named-effect.md` | +### Test Infrastructure + +| Block | Layer | Summary | File | +| ---------------------- | ------------------------- | -------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | +| `unit-test` | co-located `.test.ts(x)` | Vitest test for mechanics — mappers, transformers, value objects, custom hooks with non-trivial logic. | `rules/unit-test.md` | +| `component-story-test` | co-located `.stories.tsx` | Storybook play-function test verifying user-facing behavior. Primary verification layer for anything component-touching. | `rules/component-story-test.md` | +| `msw-handler` | `test-lib/handlers/` | One request handler per endpoint with optional resolver for per-scenario overrides. Shared across tests and stories. | `rules/msw-handler.md` | +| `fixture` | `test-lib/fixtures/` | Deterministic test data factory via `createFixture()`. Supplies `toStructure`, `createPermutation`, `createCollection`. | `rules/fixture.md` | + ### Data Modeling | Block | Layer | Summary | File | diff --git a/.agents/skills/building-blocks/metadata.json b/.agents/skills/building-blocks/metadata.json index ffb9c49..64b4e1d 100644 --- a/.agents/skills/building-blocks/metadata.json +++ b/.agents/skills/building-blocks/metadata.json @@ -1,7 +1,7 @@ { - "version": "1.0.0", + "version": "1.1.0", "date": "April 2026", - "abstract": "Typed catalog of frontend building blocks. Contains 15 blocks across 5 categories (Data Fetching, State Management, App Orchestration, Component Patterns, Data Modeling). Each block defines composition rules, layer constraints, and canonical implementation examples. Designed for AI agents — read the index to route, read the rule file to implement.", + "abstract": "Typed catalog of frontend building blocks. Contains 19 blocks across 6 categories (Data Fetching, State Management, App Orchestration, Component Patterns, Test Infrastructure, Data Modeling). Each block defines composition rules, layer constraints, and canonical implementation examples. Designed for AI agents — read the index to route, read the rule file to implement.", "categories": [ { "name": "Data Fetching", @@ -32,6 +32,10 @@ "named-effect" ] }, + { + "name": "Test Infrastructure", + "blocks": ["unit-test", "component-story-test", "msw-handler", "fixture"] + }, { "name": "Data Modeling", "blocks": ["frontend-model", "value-object"] diff --git a/.agents/skills/building-blocks/rules/component-story-test.md b/.agents/skills/building-blocks/rules/component-story-test.md new file mode 100644 index 0000000..bff007a --- /dev/null +++ b/.agents/skills/building-blocks/rules/component-story-test.md @@ -0,0 +1,139 @@ +--- +title: Component Story Test +category: Testing +layer: co-located `.stories.tsx` +composedWith: fixture, msw-handler +--- + +## Component Story Test + +Storybook story with a `play` function that verifies component behavior through real user interactions — clicks, typing, selection, validation, conditional rendering. One story per meaningful state or flow; the `Default` story is the pure-rendering baseline. + +### Constraints + +- Co-located with the component. `FooForm.tsx` → `FooForm.stories.tsx`. +- `Default: Story = {}` is the pure-rendering baseline — no play function on `Default`. Named stories carry play functions. +- Play function phases are always wrapped in `step("label", async () => { ... })`. Phase labels read as prose ("Validate required fields", "Fill text fields", "Submit form"). +- Query priorities (in order): + 1. `getByRole(role, { name })` — accessible-name queries + 2. `getByLabelText(label)` — form fields + 3. `getByText(text)` — visible content + 4. `getByTestId` — escape hatch only +- Use `canvas` (from `within(canvasElement)`) for elements rendered inside the component. Use `screen` for portaled content (Chakra UI combobox options, toasts, modals). +- `await screen.findBy*` for elements that appear after an interaction. `sleep(N)` only for Chakra UI combobox open/close transitions (typical: `100`–`150` ms). +- Callback props use `action("label")` from `storybook/actions`. Data in args uses fixtures (`ProductFixture.toStructure()`, `ProductFixture.createCollection([...])`) — not inline literals. +- Network-dependent stories declare MSW handlers under `parameters.msw.handlers`. Per-scenario overrides pass a resolver to the handler factory (see `rules/msw-handler.md`). +- New story vs. new step: new story when starting props/state differ; new step within the same story when continuing a single user flow. + +### Example — pure rendering baseline + +Follow the structure of this example (meta at top with `title`, `component`, `decorators`, `parameters`; `satisfies Meta` — not a plain type annotation — to keep inference working for `StoryObj`). + +```tsx +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { withRouter } from "storybook-addon-remix-react-router"; + +import { ProductFixture } from "@/test-lib/fixtures/product-fixture"; +import { generateUuid } from "@/test-lib/generate-uuid"; + +import { ProductsList } from "./ProductsList"; + +const meta = { + title: "modules/Products/ProductsList", + component: ProductsList, + decorators: [withRouter], + parameters: { layout: "centered" }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + args: { + products: ProductFixture.createCollection([ + { id: generateUuid() }, + { id: generateUuid() }, + { id: generateUuid() }, + ]), + }, +}; + +export const WithoutProducts: Story = { + args: { products: [] }, +}; +``` + +### Example — interaction flow with steps + +```tsx +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { userEvent, within, screen, expect } from "storybook/test"; + +import { sleep } from "@/test-lib/storybook/sleep"; + +import { CheckoutForm } from "./CheckoutForm"; + +const meta = { + title: "modules/Carts/CheckoutForm", + component: CheckoutForm, + parameters: { layout: "centered" }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; + +export const Purchasing: Story = { + play: async ({ canvasElement, step }) => { + within(canvasElement); + + await step("Enter credentials", async () => { + await userEvent.type(screen.getByLabelText(/Full Name/i), "John Doe"); + await userEvent.type( + screen.getByLabelText(/Address/i), + "NYC Groove Street" + ); + await userEvent.click(screen.getByRole("combobox")); + await sleep(100); + await userEvent.click(screen.getByRole("option", { name: "PayPal" })); + await sleep(100); + }); + + await expect(screen.getByRole("combobox")).toHaveTextContent("PayPal"); + + await step("Submit form", async () => { + await userEvent.click( + screen.getByRole("button", { name: "Complete Order" }) + ); + }); + + await expect( + await screen.findByText( + "You have successfully purchased all selected products." + ) + ).toBeInTheDocument(); + }, +}; +``` + +### Example — overriding MSW at call site + +```tsx +// In a story's parameters block — simulate a 500 error for this story only +parameters: { + msw: { + handlers: [ + getProductsHandler(() => + HttpResponse.json({ message: "Server error" }, { status: 500 }) + ), + ], + }, +} +``` + +### References + +- `rules/msw-handler.md` — wire mocked endpoints under `parameters.msw.handlers` +- `rules/fixture.md` — deterministic data for `args` +- `rules/unit-test.md` — for pure-logic units (hooks, utilities) that do not require rendering diff --git a/.agents/skills/building-blocks/rules/fixture.md b/.agents/skills/building-blocks/rules/fixture.md new file mode 100644 index 0000000..c5a1fdd --- /dev/null +++ b/.agents/skills/building-blocks/rules/fixture.md @@ -0,0 +1,83 @@ +--- +title: Fixture +category: Testing +layer: test-lib/fixtures/ +composedWith: (none — leaf block) +--- + +## Fixture + +Deterministic test data factory for a domain type. Built via the shared `createFixture(template)` helper, which returns an object with three methods: `toStructure()`, `createPermutation(extend?)`, and `createCollection(extend)`. Used in both unit tests and Storybook story args. + +### Available methods + +- **`toStructure()`** — returns a deep clone of the template. No overrides. Use when the default values are exactly what the test needs. +- **`createPermutation(extend?)`** — returns a single instance with a deep merge applied on top of the template. `extend` can be either a `DeepPartial` object or a function `(template: T) => DeepPartial` (useful when the override depends on a value from the template itself). +- **`createCollection(extend)`** — returns an array. `extend` is either an array of `DeepPartial` (one entry per item) or a function `(template: T) => DeepPartial[]` (again, when the overrides depend on the template). Each item is a deep merge of the template and its entry. + +### Constraints + +- One file per domain type. Filename mirrors the type: `product-fixture.ts` exports `ProductFixture`. +- Templates are deterministic: no `Math.random()`, no `Date.now()`, no `faker`. Use fixed strings, fixed ISO date strings, and `generateUuid()` (from `@/test-lib/generate-uuid`) when a UUID is part of the template. +- Uniqueness across a collection is the caller's responsibility: pass `{ id: generateUuid() }` per item when unique IDs matter. +- Fixtures live in `test-lib/fixtures/` — never in `src/features/`. + +### Example — template definition + +```ts +import { Category } from "@/features/products/models/category"; +import type { Product } from "@/features/products/models/product"; +import { generateUuid } from "@/test-lib/generate-uuid"; + +import { createFixture } from "./create-fixture"; + +export const ProductFixture = createFixture({ + id: generateUuid(), + name: "White Nike Shoes", + category: Category.Clothing, + price: { amount: 129.99, currency: "USD" }, + imageUrl: "https://example.com/white-nike-shoes.jpg", + description: "Lorem ipsum dolor sit amet.", + addedAt: "2025-01-15T10:00:00.000Z", +}); +``` + +### Example — consumer usage + +```ts +// Defaults only +const defaultProduct = ProductFixture.toStructure(); + +// Single instance with an object override +const blackShoes = ProductFixture.createPermutation({ + name: "Black Adidas Shoes", +}); + +// Single instance with a function override (uses the template to derive the override) +const discounted = ProductFixture.createPermutation((template) => ({ + price: { + amount: template.price.amount * 0.5, + currency: template.price.currency, + }, +})); + +// Collection with per-item overrides (unique IDs) +const products = ProductFixture.createCollection([ + { id: generateUuid() }, + { id: generateUuid() }, + { id: generateUuid(), name: "Special Edition" }, +]); + +// Collection with function override +const threeVariants = ProductFixture.createCollection((template) => [ + { id: generateUuid(), name: `${template.name} - S` }, + { id: generateUuid(), name: `${template.name} - M` }, + { id: generateUuid(), name: `${template.name} - L` }, +]); +``` + +### References + +- `rules/msw-handler.md` — fixtures populate default handler responses +- `rules/component-story-test.md` — fixtures populate story `args` +- `rules/unit-test.md` — fixtures replace inline literals in unit tests diff --git a/.agents/skills/building-blocks/rules/msw-handler.md b/.agents/skills/building-blocks/rules/msw-handler.md new file mode 100644 index 0000000..f7eee37 --- /dev/null +++ b/.agents/skills/building-blocks/rules/msw-handler.md @@ -0,0 +1,68 @@ +--- +title: MSW Handler +category: Testing +layer: test-lib/handlers/ +composedWith: fixture +--- + +## MSW Handler + +One request handler per API endpoint. Returns a sensible default response when invoked without arguments; accepts an optional resolver for per-test overrides (error responses, custom payloads, assertions on the request). Shared across unit tests and Storybook stories. + +### Constraints + +- One file per endpoint. Filename mirrors HTTP verb + resource + action: `put-add-to-cart-handler.ts`, `get-products-handler.ts`. +- Export a **single named factory function** — not multiple outcome-specific exports. Per-scenario variation is expressed by passing a resolver at the call site. +- URL uses the shared `host` constant from `@/lib/http` and, for endpoints with query params, the shared `buildUrl` helper — never hand-concatenate URLs. +- Default response body is the minimum valid shape. For collection endpoints, use fixtures with `generateUuid()` IDs — do not inline literal arrays. + +### Example — PUT handler + +```ts +import { http, HttpResponse } from "msw"; + +import { host } from "@/lib/http"; + +import type { PutResolver } from "./resolvers"; + +export const putAddToCartHandler = (resolver?: PutResolver) => + http.put(`${host}/carts/:cartId`, (req) => { + if (resolver) return resolver(req); + + return HttpResponse.json({}); + }); +``` + +### Example — GET handler with query params and fixtures + +```ts +import { http, HttpResponse } from "msw"; + +import { buildUrl } from "@/lib/build-url"; +import { host } from "@/lib/http"; +import { ProductFixture } from "@/test-lib/fixtures/product-fixture"; +import { generateUuid } from "@/test-lib/generate-uuid"; + +import type { GetResolver } from "./resolvers"; + +export const getProductsHandler = (resolver?: GetResolver) => + http.get( + `${host}/${buildUrl("products", { limit: 10, sort: "asc" })}`, + (req) => { + if (resolver) return resolver(req); + + return HttpResponse.json({ + products: ProductFixture.createCollection([ + { id: generateUuid() }, + { id: generateUuid() }, + ]), + meta: { limit: 10, sort: "asc", total: 2 }, + }); + } + ); +``` + +### References + +- `rules/fixture.md` — deterministic response payloads +- `rules/component-story-test.md` — handler usage in stories, including per-scenario resolver overrides diff --git a/.agents/skills/building-blocks/rules/unit-test.md b/.agents/skills/building-blocks/rules/unit-test.md new file mode 100644 index 0000000..143d19f --- /dev/null +++ b/.agents/skills/building-blocks/rules/unit-test.md @@ -0,0 +1,37 @@ +--- +title: Unit Test +category: Testing +layer: co-located `.test.ts` / `.test.tsx` +composedWith: fixture +--- + +## Unit Test + +Vitest test for a pure-logic unit — utility, hook, store, value object, or model transformation. Verifies behavior, not implementation detail. One test file per source unit, co-located. + +### Constraints + +- Co-located with source. `foo.ts` → `foo.test.ts`. `use-bar.ts` → `use-bar.test.ts`. +- One `describe` block per unit. Name matches the exported symbol under test. +- One `it` per distinct behavior. Do not write redundant cases that other tests already cover or that the type system already prevents. +- Assert on observable behavior (return values, DOM, side effects via mocks). Do not assert internal state that isn't exposed. +- Use fixtures for any domain object — do not inline literals when a fixture exists. + +### Example + +```ts +import { describe, it, expect } from "vitest"; + +import { add } from "./add"; + +describe("add", () => { + it("should return the sum of two numbers", () => { + expect(add(2, 3)).toBe(5); + }); +}); +``` + +### References + +- `rules/component-story-test.md` — for component-level behavior (prefer this over unit tests for rendered components) +- `rules/fixture.md` — test data factory used inside unit tests diff --git a/.agents/skills/writing-spec/SKILL.md b/.agents/skills/writing-spec/SKILL.md index af90692..4cdbe92 100644 --- a/.agents/skills/writing-spec/SKILL.md +++ b/.agents/skills/writing-spec/SKILL.md @@ -46,7 +46,7 @@ Collaborate on sections 4-6 of the template. ### Phase 3 — Spec (Sequencing) -Fill sections 7-10 of the template. +Fill sections 7-11 of the template. 1. Break work into a **Task Breakdown** — ordered, independently testable tasks. Each task: - References building blocks from section 4 if any are involved @@ -54,18 +54,21 @@ Fill sections 7-10 of the template. - Includes target file paths - Is marked `[P]` (parallelizable) or `[S]` (sequential) 2. Draft **Error & Edge Cases** using GIVEN/WHEN/THEN — cover failure modes (including fetch errors for data-fetching components), boundary conditions, concurrency -3. Add **Open Questions** for anything unresolved that blocks a specific task -4. Present for review +3. Draft the **Test Plan** (section 9 of the template) — but only if the spec introduces new testable behavior. Skip with a one-line notice when the changes are presentation-only, pure refactors, renames, or any change that leaves observable behavior identical. Map every R# to a layer (`storybook` or `vitest`) and file. Multiple R#s may point at the same File / Story — that's fine and self-documenting. +4. Add **Open Questions** for anything unresolved that blocks a specific task +5. Present for review ### Phase 4 — Review & Finalize 1. Run a **structured self-audit** and present findings to the developer (don't silently verify — show the results): - **Coverage matrix**: for each requirement ID, list which task(s) implement it. Flag any requirement with zero tasks + - **Test coverage completeness**: when Section 9 (Test Plan) is present, verify every R# from section 2 appears in the table with a layer (`storybook` / `vitest`) and file. Flag any missing R#. When Section 9 is skipped, verify the skip notice is present and accurately reflects the change (presentation-only, pure refactor, rename, or other change with no new testable behavior) - **Orphan tasks**: flag any task that doesn't trace back to a requirement ID - **EARS compliance**: flag any requirement missing WHEN/THE SYSTEM SHALL or using vague language ("handle properly", "work correctly") + - **Test infrastructure traceability**: tasks that produce `msw-handler` or `fixture` blocks are exempt from R# traceability. They must instead trace to at least one other task that uses them. Flag any `msw-handler`/`fixture` task with no consumer task - **Boundary specificity**: flag any boundary item (✅/⚠️/🚫) that references a vague category instead of a file path or module name - - **Building block references**: flag any block in Section 4 that doesn't exist in the building-blocks catalog - - **Line count**: report total. If >150, identify which section to compress or extract + - **Building block references**: flag any block in Section 4 OR in Test Plan-generating tasks that doesn't exist in the building-blocks catalog (including Test Infrastructure blocks: `unit-test`, `component-story-test`, `msw-handler`, `fixture`) + - **Line count**: report total. If >130 and ≤150, surface to the developer: "This spec is at N lines (approaching the 150 ceiling). Before we finalize, is there a natural seam where this could split into two specs?" If >150, identify which section to compress or extract, or split the feature 2. Fix any issues found in step 1 before proceeding 3. Set status to `review` in the Meta table 4. Present the audit results and the final spec for developer sign-off @@ -78,6 +81,7 @@ Fill sections 7-10 of the template. - ALWAYS use EARS notation for requirements and GIVEN/WHEN/THEN for edge cases - ALWAYS assign stable IDs to requirements (R1, R2, …) — tasks reference these for traceability - ALWAYS include the three-tier boundary system (✅ / ⚠️ / 🚫) with specific paths +- ALWAYS produce a Test Plan (section 9) when section 4 contains any behavior-bearing block AND the spec introduces new testable behavior. When the spec introduces no new testable behavior — presentation-only changes, pure refactors, renames, file moves, or any change that leaves observable behavior identical — explicitly write the one-line skip notice explaining why. Never silently omit the section ### What the agent MUST NOT do @@ -86,6 +90,8 @@ Fill sections 7-10 of the template. - NEVER skip a review gate — each phase needs explicit developer approval - NEVER conflate spec layers: requirements constrain intent, design constrains approach, tasks constrain sequencing. Keep them separate - NEVER add boilerplate boundaries — every item in ✅/⚠️/🚫 must be reachable during implementation of this specific feature +- NEVER invent Layer values in the Test Plan outside `storybook` and `vitest`. If a requirement genuinely doesn't fit either, stop and ask the developer — it may mean the requirement is ill-formed +- NEVER map edge cases (GIVEN/WHEN/THEN bullets in section 8) as separate rows in the Test Plan. They are facets of their parent R# and are covered transitively ### Prefer @@ -154,6 +160,7 @@ The spec is ready for implementation when: - [ ] Every task traces to ≥1 requirement ID - [ ] Building blocks reference name + type only, no implementation details - [ ] Boundaries use specific file paths, not vague categories +- [ ] Test Plan (section 9) is present when the spec introduces new testable behavior, or the skip notice is present and explains why (presentation-only, refactor, rename, etc.) - [ ] Open questions are either resolved or explicitly block named tasks - [ ] Developer has approved the final spec (status set to `approved`) - [ ] Spec is under 150 lines diff --git a/docs/spec-driven-development.md b/docs/spec-driven-development.md index 5c3dfb6..0e44323 100644 --- a/docs/spec-driven-development.md +++ b/docs/spec-driven-development.md @@ -154,7 +154,15 @@ THEN redirect to dashboard. If you skip this section, expect the agent to guess — and guess wrong. -### Sections 9–11 +### Section 9: Test Plan (required when the spec introduces new testable behavior) + +Required when the spec introduces new testable behavior. Skipped with a one-line notice explaining why when the changes don't produce new behavior — presentation-only specs, pure refactors, renames, file moves, or any change that leaves observable behavior identical. + +When present, maps every R# to a `Layer` (`storybook` or `vitest`) and a file path (or story name). Scope: Vitest unit tests and Storybook component tests. E2E tests are not part of the spec. + +The Test Plan enforces behavior-covered-not-requirement-mapped: multiple R#s may point at the same test file when they exercise the same code path. Duplicate File / Story entries are self-documenting; no explicit sharing note is needed. This matches the existing testing philosophy, which prohibits artificial test duplication. + +### Sections 10–12 - **Acceptance Criteria** (optional) — high-level "done" checklist, often redundant if requirements are precise. - **Open Questions** (optional) — unresolved decisions blocking specific tasks. @@ -180,18 +188,20 @@ The agent proposes a Building Blocks Diff and, for non-trivial features, two pla ### Phase 3 — Spec (Sequencing) -Tasks are broken down, traced to requirements, and ordered. Error & edge cases are documented in GIVEN/WHEN/THEN. Open questions are captured. +Tasks are broken down, traced to requirements, and ordered. Error & edge cases are documented in GIVEN/WHEN/THEN. The Test Plan is drafted when the spec introduces new testable behavior, or skipped with a one-line notice otherwise (presentation-only, refactor, rename, etc.). Open questions are captured. ### Phase 4 — Review & Finalize The agent runs a structured self-audit and presents findings: - **Coverage matrix** — each requirement ID mapped to implementing tasks. Flags requirements with zero tasks. +- **Test coverage completeness** — every R# appears in the Test Plan with a layer + file, or the skip notice is present and explains why the spec introduces no new testable behavior. - **Orphan tasks** — tasks that don't trace to any requirement. - **EARS compliance** — flags requirements missing WHEN/SHALL or using vague language. +- **Test infrastructure traceability** — `msw-handler`/`fixture` tasks trace to consumer tasks rather than R#s. - **Boundary specificity** — flags boundary items referencing vague categories instead of file paths. -- **Building block references** — flags blocks not found in the catalog. -- **Line count** — reports total; identifies bloated sections if >150 lines. +- **Building block references** — flags blocks not found in the catalog (including Test Infrastructure blocks). +- **Line count** — reports total. If >130, surfaces a prompt to consider splitting. If >150, identifies bloated sections or recommends splitting the feature. Issues are fixed before presenting the final spec for developer sign-off. @@ -210,13 +220,14 @@ The building blocks catalog lives in `skills/building-blocks/`. It uses progress ### Block categories -| Category | Blocks | What they cover | -| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------ | -| Data Fetching | `mutation-hook`, `query-options-factory`, `query-keys-factory`, `dto-model` | Server reads/writes, cache keys, API response types | -| State Management | `store`, `provider` | Zustand stores, React Context dependency injection | -| App Orchestration | `use-case-hook` | Feature-level operations composing mutations + notifications | -| Component Patterns | `notification-hook`, `pure-component`, `compound-component`, `form`, `hoc`, `error-boundary`, `page`, `facade-hook`, `named-effect` | UI components, hooks, and composition patterns | -| Data Modeling | `frontend-model`, `value-object` | Domain types, value-based logic grouping | +| Category | Blocks | What they cover | +| ------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- | +| Data Fetching | `mutation-hook`, `query-options-factory`, `query-keys-factory`, `dto-model` | Server reads/writes, cache keys, API response types | +| State Management | `store`, `provider` | Zustand stores, React Context dependency injection | +| App Orchestration | `use-case-hook` | Feature-level operations composing mutations + notifications | +| Component Patterns | `notification-hook`, `pure-component`, `compound-component`, `form`, `hoc`, `error-boundary`, `page`, `facade-hook`, `named-effect` | UI components, hooks, and composition patterns | +| Test Infrastructure | `unit-test`, `component-story-test`, `msw-handler`, `fixture` | Vitest unit tests, Storybook play-function tests, API mocks, test data factories | +| Data Modeling | `frontend-model`, `value-object` | Domain types, value-based logic grouping | ### How specs reference building blocks @@ -251,6 +262,22 @@ The spec system works within the project's feature slice architecture documented --- +## Test scope + +The spec Test Plan covers Vitest unit tests and Storybook component tests. E2E tests are handled separately outside the spec system. This keeps specs focused on feature-local test concerns. + +The project's testing philosophy allocates layers as follows: + +- **Storybook play-function tests** are the primary verification layer for feature behavior. Anything user-facing — including mutation hooks used by a component — is verified transitively via the component's play function. +- **Vitest unit tests** are reserved for mechanics: mappers, transformers, value objects, and custom hooks with non-trivial logic. +- **No tests** for fixtures, MSW handlers, or trivial glue. + +Within this scope, coverage is behavior-covered, not requirement-mapped. Multiple requirements may share one test when they exercise the same code path. + +The Test Plan governs test _authoring_ — declaring upfront which tests will exist. The "Verification Before Done" principle in `CLAUDE.md` governs test _execution_ at task-completion time. They are complementary, not duplicate. + +--- + ## Self-improvement loop After every implementation, the agent captures lessons learned in `specs/lessons.md` — patterns that caused rework, misunderstandings, or repeated mistakes. This file is reviewed at the start of each session so the same mistake doesn't happen twice. @@ -289,7 +316,7 @@ These are common failure modes the system is designed to prevent: - Specs live in `specs/NNN-feature-name/spec.md`. - Check the `Status` field in the Meta table. - The task breakdown in Section 7 shows what's done and what remains. -- Open questions in Section 10 may block specific tasks. +- Open questions in Section 11 may block specific tasks. **Adding a new building block:** diff --git a/docs/spec-template.md b/docs/spec-template.md index 29c56ec..1b84284 100644 --- a/docs/spec-template.md +++ b/docs/spec-template.md @@ -124,11 +124,38 @@ - GIVEN the API is unreachable, WHEN the user submits login, THEN display a network error with a retry button. --> -## 9. Acceptance Criteria +## 9. Test Plan + + + + + + + + + +| ID | Layer | File / Story | +| --- | --------- | ---------------------------------------------------- | +| R1 | storybook | CheckoutForm.stories.tsx → Purchasing | +| R2 | storybook | CheckoutForm.stories.tsx → Purchasing | +| R3 | vitest | src/features/products/models/price-formatter.test.ts | + +## 10. Acceptance Criteria -## 10. Open Questions +## 11. Open Questions @@ -137,6 +164,6 @@ - [ ] Q2: Rate limiting strategy for login attempts — server-side only or client-side throttle too? (blocks task 2) --> -## 11. References +## 12. References From 9f690cbe4a9b95aa4bc6b2ec2a750b72f4bca496 Mon Sep 17 00:00:00 2001 From: bartstc Date: Mon, 20 Apr 2026 16:11:14 +0000 Subject: [PATCH 2/4] chore: update --- .agents/skills/building-blocks/README.md | 2 +- .../skills/building-blocks/rules/fixture.md | 2 +- .agents/skills/writing-spec/SKILL.md | 19 ++++--- docs/spec-driven-development.md | 22 ++++---- docs/spec-template.md | 50 ++++++++++--------- server/src/db/db.json | 43 ++++++---------- 6 files changed, 64 insertions(+), 74 deletions(-) diff --git a/.agents/skills/building-blocks/README.md b/.agents/skills/building-blocks/README.md index 5b87d25..4e28a7a 100644 --- a/.agents/skills/building-blocks/README.md +++ b/.agents/skills/building-blocks/README.md @@ -37,7 +37,7 @@ A structured catalog of typed frontend building blocks, optimized for AI agents. --- title: Block Display Name category: Data Fetching | State Management | App Orchestration | Component Patterns | Test Infrastructure | Data Modeling -layer: providers/ | application/ | components/ | models/ | lib/api/ | pages/ | test-lib/handlers/ | test-lib/fixtures/ | co-located +layer: providers/ | application/ | components/ | models/ | pages/ | lib/api/ | test-lib/handlers/ | test-lib/fixtures/ | co-located composedWith: other-block-1, other-block-2 --- diff --git a/.agents/skills/building-blocks/rules/fixture.md b/.agents/skills/building-blocks/rules/fixture.md index c5a1fdd..b67be0b 100644 --- a/.agents/skills/building-blocks/rules/fixture.md +++ b/.agents/skills/building-blocks/rules/fixture.md @@ -2,7 +2,7 @@ title: Fixture category: Testing layer: test-lib/fixtures/ -composedWith: (none — leaf block) +composedWith: - --- ## Fixture diff --git a/.agents/skills/writing-spec/SKILL.md b/.agents/skills/writing-spec/SKILL.md index 4cdbe92..138cb44 100644 --- a/.agents/skills/writing-spec/SKILL.md +++ b/.agents/skills/writing-spec/SKILL.md @@ -34,7 +34,7 @@ Collaborate with the developer to fill sections 1-3 of the template. Collaborate on sections 4-6 of the template. -1. Propose a **Building Blocks Diff** — list every block that is ADDED, MODIFIED, or DELETED. Use the project's building block taxonomy from `.agents/skills/building-blocks/SKILL.md`. Reference by **name and type only** — do not define internals. Implementation details belong in coding standards and per-type skills, not specs. For changes that don't map to a typed building block, use the target file path + a short description instead. +1. Propose a **Building Blocks Diff** — list every block that is ADDED, MODIFIED, or DELETED. Use the project's building block taxonomy from `.agents/skills/building-blocks/SKILL.md`. Reference by **name and type only** — do not define internals. Implementation details belong in coding standards and per-type skills, not specs. For changes that don't map to a typed building block, use the target file path + a short description instead. Test building blocks `unit-test` and `component-story-test` are NOT listed here — they are implied by the Test Plan in section 9. `msw-handler` and `fixture` ARE listed here when added or modified. 2. For non-trivial features, propose **two plausible designs** with tradeoffs. Let the developer choose. Capture the winner and rationale in **Design Decisions** 3. Draft the **Boundaries** section using the three-tier system: - ✅ **Always** — proceed without asking (e.g., create files in the feature directory) @@ -50,11 +50,11 @@ Fill sections 7-11 of the template. 1. Break work into a **Task Breakdown** — ordered, independently testable tasks. Each task: - References building blocks from section 4 if any are involved - - Traces to requirement IDs (R1, R2, …) + - Traces to requirement IDs (R1, R2, …), or marks support infrastructure tasks as "support task for R#, R#" when producing `msw-handler` or `fixture` blocks - Includes target file paths - Is marked `[P]` (parallelizable) or `[S]` (sequential) 2. Draft **Error & Edge Cases** using GIVEN/WHEN/THEN — cover failure modes (including fetch errors for data-fetching components), boundary conditions, concurrency -3. Draft the **Test Plan** (section 9 of the template) — but only if the spec introduces new testable behavior. Skip with a one-line notice when the changes are presentation-only, pure refactors, renames, or any change that leaves observable behavior identical. Map every R# to a layer (`storybook` or `vitest`) and file. Multiple R#s may point at the same File / Story — that's fine and self-documenting. +3. Draft the **Test Plan** (section 9 of the template) — but only if the spec introduces new testable behavior. Skip with a one-line notice when the changes are presentation-only, pure refactors, renames, or any change that leaves observable behavior identical. Map every R# to a layer (`storybook` or `vitest`) and a test — use the bare file name of the tested module or component (e.g., `CheckoutForm`, `priceFormatter`), not a full file path. Multiple R#s may point at the same test — that's fine and self-documenting. 4. Add **Open Questions** for anything unresolved that blocks a specific task 5. Present for review @@ -62,12 +62,12 @@ Fill sections 7-11 of the template. 1. Run a **structured self-audit** and present findings to the developer (don't silently verify — show the results): - **Coverage matrix**: for each requirement ID, list which task(s) implement it. Flag any requirement with zero tasks - - **Test coverage completeness**: when Section 9 (Test Plan) is present, verify every R# from section 2 appears in the table with a layer (`storybook` / `vitest`) and file. Flag any missing R#. When Section 9 is skipped, verify the skip notice is present and accurately reflects the change (presentation-only, pure refactor, rename, or other change with no new testable behavior) + - **Test coverage completeness**: when Section 9 (Test Plan) is present, verify every R# from section 2 appears in the table with a layer (`storybook` / `vitest`) and a test. Flag any missing R#. When Section 9 is skipped, verify the skip notice is present and accurately reflects the change (presentation-only, pure refactor, rename, or other change with no new testable behavior) - **Orphan tasks**: flag any task that doesn't trace back to a requirement ID - **EARS compliance**: flag any requirement missing WHEN/THE SYSTEM SHALL or using vague language ("handle properly", "work correctly") - **Test infrastructure traceability**: tasks that produce `msw-handler` or `fixture` blocks are exempt from R# traceability. They must instead trace to at least one other task that uses them. Flag any `msw-handler`/`fixture` task with no consumer task - **Boundary specificity**: flag any boundary item (✅/⚠️/🚫) that references a vague category instead of a file path or module name - - **Building block references**: flag any block in Section 4 OR in Test Plan-generating tasks that doesn't exist in the building-blocks catalog (including Test Infrastructure blocks: `unit-test`, `component-story-test`, `msw-handler`, `fixture`) + - **Building block references**: flag any block in Section 4 that doesn't exist in the building-blocks catalog. Flag any `unit-test` or `component-story-test` incorrectly listed in Section 4 (these belong in the Test Plan, not the Building Blocks Diff) - **Line count**: report total. If >130 and ≤150, surface to the developer: "This spec is at N lines (approaching the 150 ceiling). Before we finalize, is there a natural seam where this could split into two specs?" If >150, identify which section to compress or extract, or split the feature 2. Fix any issues found in step 1 before proceeding 3. Set status to `review` in the Meta table @@ -81,7 +81,6 @@ Fill sections 7-11 of the template. - ALWAYS use EARS notation for requirements and GIVEN/WHEN/THEN for edge cases - ALWAYS assign stable IDs to requirements (R1, R2, …) — tasks reference these for traceability - ALWAYS include the three-tier boundary system (✅ / ⚠️ / 🚫) with specific paths -- ALWAYS produce a Test Plan (section 9) when section 4 contains any behavior-bearing block AND the spec introduces new testable behavior. When the spec introduces no new testable behavior — presentation-only changes, pure refactors, renames, file moves, or any change that leaves observable behavior identical — explicitly write the one-line skip notice explaining why. Never silently omit the section ### What the agent MUST NOT do @@ -92,6 +91,7 @@ Fill sections 7-11 of the template. - NEVER add boilerplate boundaries — every item in ✅/⚠️/🚫 must be reachable during implementation of this specific feature - NEVER invent Layer values in the Test Plan outside `storybook` and `vitest`. If a requirement genuinely doesn't fit either, stop and ask the developer — it may mean the requirement is ill-formed - NEVER map edge cases (GIVEN/WHEN/THEN bullets in section 8) as separate rows in the Test Plan. They are facets of their parent R# and are covered transitively +- NEVER list `unit-test` or `component-story-test` blocks in the Building Blocks Diff (section 4). They are implied by the Test Plan (section 9) and ride along with the component/module they verify. `msw-handler` and `fixture` ARE listed in section 4 when added or modified ### Prefer @@ -115,8 +115,8 @@ Fill sections 7-11 of the template. ```markdown ### Added -- `loginMutation` (mutation) — handles POST /auth/login -- `LoginForm` (component) — email/password form with validation +- `loginMutation` (mutation-hook) — handles POST /auth/login +- `LoginForm` (pure-component) — email/password form with validation ``` ### ❌ Wrong: Building block with implementation details @@ -157,10 +157,9 @@ The spec is ready for implementation when: - [ ] Every section marked REQUIRED in the template is filled - [ ] Every requirement has a stable ID and uses EARS notation -- [ ] Every task traces to ≥1 requirement ID +- [ ] Every task traces to ≥1 requirement ID (or is marked as support infrastructure tracing to a consumer task) - [ ] Building blocks reference name + type only, no implementation details - [ ] Boundaries use specific file paths, not vague categories -- [ ] Test Plan (section 9) is present when the spec introduces new testable behavior, or the skip notice is present and explains why (presentation-only, refactor, rename, etc.) - [ ] Open questions are either resolved or explicitly block named tasks - [ ] Developer has approved the final spec (status set to `approved`) - [ ] Spec is under 150 lines diff --git a/docs/spec-driven-development.md b/docs/spec-driven-development.md index 0e44323..5d7d0b2 100644 --- a/docs/spec-driven-development.md +++ b/docs/spec-driven-development.md @@ -110,15 +110,17 @@ Example: ```markdown ### Added -- `loginMutation` (mutation) — handles POST /auth/login -- `LoginForm` (component) — form with email/password fields +- `loginMutation` (mutation-hook) — handles POST /auth/login +- `LoginForm` (pure-component) — form with email/password fields ### Modified -- `AppRouter` (route) — add /login route +- `AppRouter` (page) — add /login route - `authStore` (store) — add `isAuthenticated` derived state ``` +**Test building block exception.** `unit-test` and `component-story-test` are NOT listed in the Building Blocks Diff — they are implied by the Test Plan in section 9 and ride along with the component or module they verify. `msw-handler` and `fixture`, by contrast, ARE listed here when added or modified, because they are reusable infrastructure shared across multiple tests. + ### Section 5: Design Decisions (required for non-trivial features) Key choices with brief rationale — what you chose, what you rejected, and why. Kept short (2–4 decisions max). If you need more, the feature should be split. @@ -142,6 +144,8 @@ Ordered list of independently testable tasks. Each task: - Includes target file paths - Is marked `[P]` (parallelizable) or `[S]` (sequential) +Support-infrastructure tasks producing `msw-handler` or `fixture` blocks trace to consumer tasks instead of R#s — mark them with "(support task for R#, R#)". + ### Section 8: Error & Edge Cases (optional but recommended) Uses **GIVEN/WHEN/THEN** format (from BDD) for precision: @@ -158,9 +162,7 @@ If you skip this section, expect the agent to guess — and guess wrong. Required when the spec introduces new testable behavior. Skipped with a one-line notice explaining why when the changes don't produce new behavior — presentation-only specs, pure refactors, renames, file moves, or any change that leaves observable behavior identical. -When present, maps every R# to a `Layer` (`storybook` or `vitest`) and a file path (or story name). Scope: Vitest unit tests and Storybook component tests. E2E tests are not part of the spec. - -The Test Plan enforces behavior-covered-not-requirement-mapped: multiple R#s may point at the same test file when they exercise the same code path. Duplicate File / Story entries are self-documenting; no explicit sharing note is needed. This matches the existing testing philosophy, which prohibits artificial test duplication. +When present, maps every R# to a `Layer` (`storybook` or `vitest`) and a test. The `Test` column is the bare file name of the tested module or component (e.g., `CheckoutForm`, `priceFormatter`) — not a full file path. The layer column already identifies whether the test is a `.stories.tsx` or a `.test.ts`. Scope: Vitest unit tests and Storybook component tests. E2E tests are not part of the spec. ### Sections 10–12 @@ -195,12 +197,12 @@ Tasks are broken down, traced to requirements, and ordered. Error & edge cases a The agent runs a structured self-audit and presents findings: - **Coverage matrix** — each requirement ID mapped to implementing tasks. Flags requirements with zero tasks. -- **Test coverage completeness** — every R# appears in the Test Plan with a layer + file, or the skip notice is present and explains why the spec introduces no new testable behavior. +- **Test coverage completeness** — every R# appears in the Test Plan with a layer and a test, or the skip notice is present and explains why the spec introduces no new testable behavior. - **Orphan tasks** — tasks that don't trace to any requirement. - **EARS compliance** — flags requirements missing WHEN/SHALL or using vague language. - **Test infrastructure traceability** — `msw-handler`/`fixture` tasks trace to consumer tasks rather than R#s. - **Boundary specificity** — flags boundary items referencing vague categories instead of file paths. -- **Building block references** — flags blocks not found in the catalog (including Test Infrastructure blocks). +- **Building block references** — flags blocks not found in the catalog, and flags `unit-test`/`component-story-test` incorrectly listed in the Building Blocks Diff. - **Line count** — reports total. If >130, surfaces a prompt to consider splitting. If >150, identifies bloated sections or recommends splitting the feature. Issues are fixed before presenting the final spec for developer sign-off. @@ -239,6 +241,8 @@ In the spec's Section 4 (Building Blocks Diff), each entry references a block by The agent then reads `rules/mutation-hook.md` to understand the implementation pattern — error handling conventions, cache invalidation approach, return tuple shape, etc. +**Test block exception:** `unit-test` and `component-story-test` live in the catalog for authoring reference but do NOT appear in the Building Blocks Diff. They are implied by the Test Plan (section 9). `msw-handler` and `fixture` DO appear in the Diff when added or modified — they are reusable infrastructure, not per-test authoring patterns. + --- ## Architecture integration @@ -272,8 +276,6 @@ The project's testing philosophy allocates layers as follows: - **Vitest unit tests** are reserved for mechanics: mappers, transformers, value objects, and custom hooks with non-trivial logic. - **No tests** for fixtures, MSW handlers, or trivial glue. -Within this scope, coverage is behavior-covered, not requirement-mapped. Multiple requirements may share one test when they exercise the same code path. - The Test Plan governs test _authoring_ — declaring upfront which tests will exist. The "Verification Before Done" principle in `CLAUDE.md` governs test _execution_ at task-completion time. They are complementary, not duplicate. --- diff --git a/docs/spec-template.md b/docs/spec-template.md index 1b84284..9c77a9a 100644 --- a/docs/spec-template.md +++ b/docs/spec-template.md @@ -37,21 +37,24 @@ - + + + ### Added ### Modified @@ -59,7 +62,7 @@ ### Deleted @@ -107,12 +110,14 @@ + + ## 8. Error & Edge Cases @@ -126,30 +131,27 @@ ## 9. Test Plan - - - +Test column: bare file name of the tested module or component (e.g., `CheckoutForm`, `priceFormatter`) — not a full file path. The layer column already identifies whether the test is a `.stories.tsx` or a `.test.ts`. --> -| ID | Layer | File / Story | -| --- | --------- | ---------------------------------------------------- | -| R1 | storybook | CheckoutForm.stories.tsx → Purchasing | -| R2 | storybook | CheckoutForm.stories.tsx → Purchasing | -| R3 | vitest | src/features/products/models/price-formatter.test.ts | +| ID | Layer | Test | +| --- | --------- | -------------- | +| R1 | storybook | CheckoutForm | +| R2 | storybook | CheckoutForm | +| R3 | vitest | priceFormatter | ## 10. Acceptance Criteria diff --git a/server/src/db/db.json b/server/src/db/db.json index 11e5171..3c0295e 100644 --- a/server/src/db/db.json +++ b/server/src/db/db.json @@ -336,29 +336,29 @@ { "id": "4f968992-1aab-49c9-8913-09405915c1c0", "rating": { - "rate": 3.9, - "count": 120 + "rate": 3.89, + "count": 121 }, "addedAt": "2025-01-15T10:00:00.000Z", - "updatedAt": null + "updatedAt": "2026-04-19T08:33:34.589Z" }, { "id": "4f968992-1aab-49c9-8913-09405915c1c1", "rating": { "rate": 4.1, - "count": 259 + "count": 263 }, "addedAt": "2025-01-15T10:00:00.000Z", - "updatedAt": null + "updatedAt": "2026-04-18T17:33:02.848Z" }, { "id": "4f968992-1aab-49c9-8913-09405915c1c2", "rating": { "rate": 4.7, - "count": 500 + "count": 501 }, "addedAt": "2025-01-15T10:00:00.000Z", - "updatedAt": null + "updatedAt": "2026-04-19T07:25:04.323Z" }, { "id": "4f968992-1aab-49c9-8913-09405915c1c3", @@ -372,20 +372,20 @@ { "id": "4f968992-1aab-49c9-8913-09405915c1c4", "rating": { - "rate": 4.6, - "count": 400 + "rate": 4.59, + "count": 401 }, "addedAt": "2025-01-15T10:00:00.000Z", - "updatedAt": null + "updatedAt": "2026-04-19T07:25:31.383Z" }, { "id": "4f968992-1aab-49c9-8913-09405915c1c5", "rating": { - "rate": 3.9, - "count": 70 + "rate": 3.91, + "count": 73 }, "addedAt": "2025-01-15T10:00:00.000Z", - "updatedAt": null + "updatedAt": "2026-04-17T22:24:00.838Z" }, { "id": "4f968992-1aab-49c9-8913-09405915c1c6", @@ -518,21 +518,8 @@ { "id": "00000000-0000-0000-0000-000000000001", "userId": 1, - "date": "2026-04-14T14:13:41.156Z", - "products": [ - { - "productId": "4f968992-1aab-49c9-8913-09405915c1c8", - "quantity": 1 - }, - { - "productId": "4f968992-1aab-49c9-8913-09405915c1c9", - "quantity": 1 - }, - { - "productId": "4f968992-1aab-49c9-8913-09405915c1c1", - "quantity": 1 - } - ] + "date": "2026-04-19T08:33:55.629Z", + "products": [] }, { "id": "00000000-0000-0000-0000-000000000002", From b1b0412d78188b08397f5bdecd64526cc61e3a82 Mon Sep 17 00:00:00 2001 From: bartstc Date: Mon, 20 Apr 2026 16:40:15 +0000 Subject: [PATCH 3/4] chore: update --- .agents/skills/writing-spec/SKILL.md | 34 +++++++++++-------- docs/spec-driven-development.md | 51 ++++++++++++++++------------ docs/spec-template.md | 45 +++++++++++------------- 3 files changed, 68 insertions(+), 62 deletions(-) diff --git a/.agents/skills/writing-spec/SKILL.md b/.agents/skills/writing-spec/SKILL.md index 138cb44..53b8638 100644 --- a/.agents/skills/writing-spec/SKILL.md +++ b/.agents/skills/writing-spec/SKILL.md @@ -34,7 +34,7 @@ Collaborate with the developer to fill sections 1-3 of the template. Collaborate on sections 4-6 of the template. -1. Propose a **Building Blocks Diff** — list every block that is ADDED, MODIFIED, or DELETED. Use the project's building block taxonomy from `.agents/skills/building-blocks/SKILL.md`. Reference by **name and type only** — do not define internals. Implementation details belong in coding standards and per-type skills, not specs. For changes that don't map to a typed building block, use the target file path + a short description instead. Test building blocks `unit-test` and `component-story-test` are NOT listed here — they are implied by the Test Plan in section 9. `msw-handler` and `fixture` ARE listed here when added or modified. +1. Propose a **Building Blocks Diff** — list every block that is ADDED, MODIFIED, or DELETED, including test blocks (`unit-test`, `component-story-test`, `msw-handler`, `fixture`). Use the project's building block taxonomy from `.agents/skills/building-blocks/SKILL.md`. Reference by **name and type only** — do not define internals. Implementation details belong in coding standards and per-type skills, not specs. For changes that don't map to a typed building block, use the target file path + a short description instead. 2. For non-trivial features, propose **two plausible designs** with tradeoffs. Let the developer choose. Capture the winner and rationale in **Design Decisions** 3. Draft the **Boundaries** section using the three-tier system: - ✅ **Always** — proceed without asking (e.g., create files in the feature directory) @@ -48,26 +48,30 @@ Collaborate on sections 4-6 of the template. Fill sections 7-11 of the template. -1. Break work into a **Task Breakdown** — ordered, independently testable tasks. Each task: - - References building blocks from section 4 if any are involved - - Traces to requirement IDs (R1, R2, …), or marks support infrastructure tasks as "support task for R#, R#" when producing `msw-handler` or `fixture` blocks +1. Break production work into a **Task Breakdown** (section 7) — ordered, independently testable production tasks. Each task: + - References production building blocks from section 4 + - Traces to requirement IDs (R1, R2, …) - Includes target file paths - Is marked `[P]` (parallelizable) or `[S]` (sequential) -2. Draft **Error & Edge Cases** using GIVEN/WHEN/THEN — cover failure modes (including fetch errors for data-fetching components), boundary conditions, concurrency -3. Draft the **Test Plan** (section 9 of the template) — but only if the spec introduces new testable behavior. Skip with a one-line notice when the changes are presentation-only, pure refactors, renames, or any change that leaves observable behavior identical. Map every R# to a layer (`storybook` or `vitest`) and a test — use the bare file name of the tested module or component (e.g., `CheckoutForm`, `priceFormatter`), not a full file path. Multiple R#s may point at the same test — that's fine and self-documenting. +2. Draft **Error & Edge Cases** (section 8) using GIVEN/WHEN/THEN — cover failure modes (including fetch errors for data-fetching components), boundary conditions, concurrency +3. Break test work into a **Test Breakdown** (section 9) — same format as Task Breakdown. Skip the whole section with a one-line notice when the spec introduces no new testable behavior (presentation-only, pure refactors, renames, or any change that leaves observable behavior identical). Each test task: + - References a `component-story-test`, `unit-test`, `msw-handler`, or `fixture` block from section 4 + - Traces to R#s (for `component-story-test` and `unit-test` tasks), or to consumer test tasks (for `msw-handler` and `fixture` support tasks — mark as "support task for test N") + - Includes the target file path + - Is marked `[P]` or `[S]` 4. Add **Open Questions** for anything unresolved that blocks a specific task 5. Present for review ### Phase 4 — Review & Finalize 1. Run a **structured self-audit** and present findings to the developer (don't silently verify — show the results): - - **Coverage matrix**: for each requirement ID, list which task(s) implement it. Flag any requirement with zero tasks - - **Test coverage completeness**: when Section 9 (Test Plan) is present, verify every R# from section 2 appears in the table with a layer (`storybook` / `vitest`) and a test. Flag any missing R#. When Section 9 is skipped, verify the skip notice is present and accurately reflects the change (presentation-only, pure refactor, rename, or other change with no new testable behavior) - - **Orphan tasks**: flag any task that doesn't trace back to a requirement ID + - **Coverage matrix**: for each requirement ID, list which production task(s) from section 7 implement it AND which test task(s) from section 9 verify it. Flag any requirement missing a production task OR (when Section 9 is present) a test task + - **Test coverage completeness**: when Section 9 (Test Breakdown) is present, verify every R# from section 2 appears on at least one test task. Flag any missing R#. When Section 9 is skipped, verify the skip notice is present and accurately reflects the change (presentation-only, pure refactor, rename, or other change with no new testable behavior) + - **Orphan tasks**: flag any task in section 7 that doesn't trace to a requirement ID. Flag any test task in section 9 that is neither traced to an R# nor marked as a support task for another test - **EARS compliance**: flag any requirement missing WHEN/THE SYSTEM SHALL or using vague language ("handle properly", "work correctly") - - **Test infrastructure traceability**: tasks that produce `msw-handler` or `fixture` blocks are exempt from R# traceability. They must instead trace to at least one other task that uses them. Flag any `msw-handler`/`fixture` task with no consumer task + - **Test infrastructure traceability**: tasks that produce `msw-handler` or `fixture` blocks live in section 9 as support tasks. They must trace to at least one other test task that uses them. Flag any `msw-handler`/`fixture` task with no consumer test task - **Boundary specificity**: flag any boundary item (✅/⚠️/🚫) that references a vague category instead of a file path or module name - - **Building block references**: flag any block in Section 4 that doesn't exist in the building-blocks catalog. Flag any `unit-test` or `component-story-test` incorrectly listed in Section 4 (these belong in the Test Plan, not the Building Blocks Diff) + - **Building block references**: flag any block in Section 4 that doesn't exist in the building-blocks catalog (including Test Infrastructure blocks: `unit-test`, `component-story-test`, `msw-handler`, `fixture`) - **Line count**: report total. If >130 and ≤150, surface to the developer: "This spec is at N lines (approaching the 150 ceiling). Before we finalize, is there a natural seam where this could split into two specs?" If >150, identify which section to compress or extract, or split the feature 2. Fix any issues found in step 1 before proceeding 3. Set status to `review` in the Meta table @@ -89,9 +93,8 @@ Fill sections 7-11 of the template. - NEVER skip a review gate — each phase needs explicit developer approval - NEVER conflate spec layers: requirements constrain intent, design constrains approach, tasks constrain sequencing. Keep them separate - NEVER add boilerplate boundaries — every item in ✅/⚠️/🚫 must be reachable during implementation of this specific feature -- NEVER invent Layer values in the Test Plan outside `storybook` and `vitest`. If a requirement genuinely doesn't fit either, stop and ask the developer — it may mean the requirement is ill-formed -- NEVER map edge cases (GIVEN/WHEN/THEN bullets in section 8) as separate rows in the Test Plan. They are facets of their parent R# and are covered transitively -- NEVER list `unit-test` or `component-story-test` blocks in the Building Blocks Diff (section 4). They are implied by the Test Plan (section 9) and ride along with the component/module they verify. `msw-handler` and `fixture` ARE listed in section 4 when added or modified +- NEVER map edge cases (GIVEN/WHEN/THEN bullets in section 8) as separate test tasks in the Test Breakdown. They are facets of their parent R# and are covered transitively by the test that verifies that R# +- NEVER put test tasks in the Task Breakdown (section 7) or production tasks in the Test Breakdown (section 9). The two sections are separate by design ### Prefer @@ -117,6 +120,7 @@ Fill sections 7-11 of the template. - `loginMutation` (mutation-hook) — handles POST /auth/login - `LoginForm` (pure-component) — email/password form with validation +- `LoginForm` (component-story-test) — verifies submission and validation ``` ### ❌ Wrong: Building block with implementation details @@ -157,7 +161,7 @@ The spec is ready for implementation when: - [ ] Every section marked REQUIRED in the template is filled - [ ] Every requirement has a stable ID and uses EARS notation -- [ ] Every task traces to ≥1 requirement ID (or is marked as support infrastructure tracing to a consumer task) +- [ ] Every production task traces to ≥1 requirement ID; every test task either traces to ≥1 requirement ID or is marked as a support task for another test task - [ ] Building blocks reference name + type only, no implementation details - [ ] Boundaries use specific file paths, not vague categories - [ ] Open questions are either resolved or explicitly block named tasks diff --git a/docs/spec-driven-development.md b/docs/spec-driven-development.md index 5d7d0b2..be37e0c 100644 --- a/docs/spec-driven-development.md +++ b/docs/spec-driven-development.md @@ -103,7 +103,7 @@ Explicit list of what the feature will NOT do. Prevents scope creep and stops th ### Section 4: Building Blocks Diff (required) -The core of the spec. Lists every building block that is **added**, **modified**, or **deleted** — referenced by name and type from the building blocks catalog. No implementation details. The agent reads the relevant building block rule file to understand the pattern. +The core of the spec. Lists every building block that is **added**, **modified**, or **deleted** — referenced by name and type from the building blocks catalog. No implementation details. Test blocks (`unit-test`, `component-story-test`, `msw-handler`, `fixture`) are listed here uniformly alongside production blocks; there is no carve-out. Example: @@ -112,6 +112,8 @@ Example: - `loginMutation` (mutation-hook) — handles POST /auth/login - `LoginForm` (pure-component) — form with email/password fields +- `LoginForm` (component-story-test) — verifies submission and validation +- `userFixture` (fixture) — deterministic user test data ### Modified @@ -119,8 +121,6 @@ Example: - `authStore` (store) — add `isAuthenticated` derived state ``` -**Test building block exception.** `unit-test` and `component-story-test` are NOT listed in the Building Blocks Diff — they are implied by the Test Plan in section 9 and ride along with the component or module they verify. `msw-handler` and `fixture`, by contrast, ARE listed here when added or modified, because they are reusable infrastructure shared across multiple tests. - ### Section 5: Design Decisions (required for non-trivial features) Key choices with brief rationale — what you chose, what you rejected, and why. Kept short (2–4 decisions max). If you need more, the feature should be split. @@ -137,14 +137,14 @@ This pattern comes from [GitHub's analysis of 2,500+ agent configuration files]( ### Section 7: Task Breakdown (required) -Ordered list of independently testable tasks. Each task: +Ordered list of independently testable **production** tasks. Each task: -- References building blocks from Section 4 +- References production building blocks from Section 4 - Traces to requirement IDs (R1, R2, …) - Includes target file paths - Is marked `[P]` (parallelizable) or `[S]` (sequential) -Support-infrastructure tasks producing `msw-handler` or `fixture` blocks trace to consumer tasks instead of R#s — mark them with "(support task for R#, R#)". +Test tasks live in Section 9 (Test Breakdown), not here. ### Section 8: Error & Edge Cases (optional but recommended) @@ -158,11 +158,18 @@ THEN redirect to dashboard. If you skip this section, expect the agent to guess — and guess wrong. -### Section 9: Test Plan (required when the spec introduces new testable behavior) +### Section 9: Test Breakdown (required when the spec introduces new testable behavior) + +Mirrors Section 7's format for test tasks. Required when the spec introduces new testable behavior. Skipped with a one-line notice when the changes don't produce new behavior — presentation-only specs, pure refactors, renames, file moves, or any change that leaves observable behavior identical. + +When present, each test task: -Required when the spec introduces new testable behavior. Skipped with a one-line notice explaining why when the changes don't produce new behavior — presentation-only specs, pure refactors, renames, file moves, or any change that leaves observable behavior identical. +- References a `component-story-test`, `unit-test`, `msw-handler`, or `fixture` block from Section 4 +- Traces to R#s (for `component-story-test` and `unit-test` tasks), or to consumer test tasks (for `msw-handler` and `fixture` support tasks — marked "support task for test N") +- Includes the target file path +- Is marked `[P]` or `[S]` -When present, maps every R# to a `Layer` (`storybook` or `vitest`) and a test. The `Test` column is the bare file name of the tested module or component (e.g., `CheckoutForm`, `priceFormatter`) — not a full file path. The layer column already identifies whether the test is a `.stories.tsx` or a `.test.ts`. Scope: Vitest unit tests and Storybook component tests. E2E tests are not part of the spec. +Scope: Vitest unit tests and Storybook component tests. E2E tests are not part of the spec. ### Sections 10–12 @@ -190,19 +197,19 @@ The agent proposes a Building Blocks Diff and, for non-trivial features, two pla ### Phase 3 — Spec (Sequencing) -Tasks are broken down, traced to requirements, and ordered. Error & edge cases are documented in GIVEN/WHEN/THEN. The Test Plan is drafted when the spec introduces new testable behavior, or skipped with a one-line notice otherwise (presentation-only, refactor, rename, etc.). Open questions are captured. +Tasks are broken down across two sections: Task Breakdown (production tasks, section 7) and Test Breakdown (test tasks, section 9). Error & edge cases are documented in GIVEN/WHEN/THEN in section 8. Test Breakdown is skipped with a one-line notice when the spec introduces no new testable behavior. Open questions are captured. ### Phase 4 — Review & Finalize The agent runs a structured self-audit and presents findings: -- **Coverage matrix** — each requirement ID mapped to implementing tasks. Flags requirements with zero tasks. -- **Test coverage completeness** — every R# appears in the Test Plan with a layer and a test, or the skip notice is present and explains why the spec introduces no new testable behavior. -- **Orphan tasks** — tasks that don't trace to any requirement. +- **Coverage matrix** — each requirement ID mapped to implementing production task(s) in section 7 AND verifying test task(s) in section 9. Flags requirements missing either. +- **Test coverage completeness** — every R# appears on at least one test task in Test Breakdown, or the skip notice is present and explains why the spec introduces no new testable behavior. +- **Orphan tasks** — production tasks in section 7 that don't trace to any requirement, and test tasks in section 9 that are neither traced to an R# nor marked as a support task. - **EARS compliance** — flags requirements missing WHEN/SHALL or using vague language. -- **Test infrastructure traceability** — `msw-handler`/`fixture` tasks trace to consumer tasks rather than R#s. +- **Test infrastructure traceability** — `msw-handler`/`fixture` tasks in section 9 must trace to a consumer test task within the same section. - **Boundary specificity** — flags boundary items referencing vague categories instead of file paths. -- **Building block references** — flags blocks not found in the catalog, and flags `unit-test`/`component-story-test` incorrectly listed in the Building Blocks Diff. +- **Building block references** — flags blocks not found in the catalog (including Test Infrastructure blocks). - **Line count** — reports total. If >130, surfaces a prompt to consider splitting. If >150, identifies bloated sections or recommends splitting the feature. Issues are fixed before presenting the final spec for developer sign-off. @@ -211,7 +218,7 @@ Issues are fixed before presenting the final spec for developer sign-off. ## Building blocks -Building blocks are typed, named patterns that form the project's architectural vocabulary. When a spec says `loginMutation (mutation)`, that name maps to a specific pattern with defined constraints, layer placement, and a canonical code example. +Building blocks are typed, named patterns that form the project's architectural vocabulary. When a spec says `loginMutation (mutation-hook)`, that name maps to a specific pattern with defined constraints, layer placement, and a canonical code example. ### The catalog @@ -231,6 +238,8 @@ The building blocks catalog lives in `skills/building-blocks/`. It uses progress | Test Infrastructure | `unit-test`, `component-story-test`, `msw-handler`, `fixture` | Vitest unit tests, Storybook play-function tests, API mocks, test data factories | | Data Modeling | `frontend-model`, `value-object` | Domain types, value-based logic grouping | +All categories work the same way: blocks are referenced in Section 4 (Building Blocks Diff) when added, modified, or deleted, and produced by tasks in Section 7 (production) or Section 9 (tests). + ### How specs reference building blocks In the spec's Section 4 (Building Blocks Diff), each entry references a block by name and type: @@ -241,8 +250,6 @@ In the spec's Section 4 (Building Blocks Diff), each entry references a block by The agent then reads `rules/mutation-hook.md` to understand the implementation pattern — error handling conventions, cache invalidation approach, return tuple shape, etc. -**Test block exception:** `unit-test` and `component-story-test` live in the catalog for authoring reference but do NOT appear in the Building Blocks Diff. They are implied by the Test Plan (section 9). `msw-handler` and `fixture` DO appear in the Diff when added or modified — they are reusable infrastructure, not per-test authoring patterns. - --- ## Architecture integration @@ -268,15 +275,15 @@ The spec system works within the project's feature slice architecture documented ## Test scope -The spec Test Plan covers Vitest unit tests and Storybook component tests. E2E tests are handled separately outside the spec system. This keeps specs focused on feature-local test concerns. +The Test Breakdown covers Vitest unit tests and Storybook component tests. E2E tests are handled separately outside the spec system. This keeps specs focused on feature-local test concerns. The project's testing philosophy allocates layers as follows: - **Storybook play-function tests** are the primary verification layer for feature behavior. Anything user-facing — including mutation hooks used by a component — is verified transitively via the component's play function. - **Vitest unit tests** are reserved for mechanics: mappers, transformers, value objects, and custom hooks with non-trivial logic. -- **No tests** for fixtures, MSW handlers, or trivial glue. +- **No tests** for fixtures, MSW handlers, or trivial glue — these are support infrastructure, not the thing being verified. -The Test Plan governs test _authoring_ — declaring upfront which tests will exist. The "Verification Before Done" principle in `CLAUDE.md` governs test _execution_ at task-completion time. They are complementary, not duplicate. +The Test Breakdown governs test _authoring_ — declaring upfront which tests will exist. The "Verification Before Done" principle in `CLAUDE.md` governs test _execution_ at task-completion time. They are complementary, not duplicate. --- @@ -317,7 +324,7 @@ These are common failure modes the system is designed to prevent: - Specs live in `specs/NNN-feature-name/spec.md`. - Check the `Status` field in the Meta table. -- The task breakdown in Section 7 shows what's done and what remains. +- The task breakdown in Section 7 shows what production work remains; Section 9 shows what test work remains. - Open questions in Section 11 may block specific tasks. **Adding a new building block:** diff --git a/docs/spec-template.md b/docs/spec-template.md index 9c77a9a..0695e07 100644 --- a/docs/spec-template.md +++ b/docs/spec-template.md @@ -35,17 +35,16 @@ ## 4. Building Blocks Diff - + - - - + ### Added + - - ## 8. Error & Edge Cases @@ -129,29 +125,28 @@ - GIVEN the API is unreachable, WHEN the user submits login, THEN display a network error with a retry button. --> -## 9. Test Plan +## 9. Test Breakdown - - +Scope: Vitest unit tests and Storybook component tests. E2E tests are not part of the spec. --> - + -| ID | Layer | Test | -| --- | --------- | -------------- | -| R1 | storybook | CheckoutForm | -| R2 | storybook | CheckoutForm | -| R3 | vitest | priceFormatter | + ## 10. Acceptance Criteria @@ -159,7 +154,7 @@ Test column: bare file name of the tested module or component (e.g., `CheckoutFo ## 11. Open Questions - +