diff --git a/docs/project/readable-codebase/PLAN.md b/docs/project/readable-codebase/PLAN.md new file mode 100644 index 0000000..5cf650a --- /dev/null +++ b/docs/project/readable-codebase/PLAN.md @@ -0,0 +1,895 @@ +# Make the rest of the codebase as readable as the dashboard + +> This is the **living** design record for the arc, committed so that the repository holds +> the complete state and a fresh session needs nothing outside it. Corrections found while +> executing are folded in and marked. See [`README.md`](README.md) for the ticket ledger and +> the session protocol. + +## Context + +The dashboard refactor (branch `refactor/readable-dashboard`, 3 commits, pushed) cut +`dashboard.page.ts` from 340 lines to 42 by splitting it into six per-concern section +components. It was built as a **reference implementation**: prove the pattern on one screen, +then hold the rest of the app to the budget it establishes. + +This plan applies that result to the rest of the codebase. It is grounded in fresh +measurement, not in the original plan's estimates — several of which turned out to be stale +(see "Corrections" at the end). + +Two findings reframe the work: + +1. **Pages are already thin.** 17 pages, median 88 lines; only the sanctioned showcase + teaching page exceeds 250. The "page → sections" split is essentially done. The remaining + bulk sits one layer down, in **organisms**. +2. **The worst problem is not size, it is a silent-failure idiom.** `runIfSubmitting` is + copy-pasted into 5 components under 2 names and must be called by hand right after + `dispatch`. Forgetting it fails silently. That is a correctness risk, not a cosmetic one. + +**Sequencing matters, and the phase numbers are not quite the running order:** + +1. **Phase 3a first** — the `max-lines` rule plus `reportUnusedDisableDirectives: 'error'`. It + is one small commit, it lands green today, and it stops every later phase from adding a new + oversized file. Cheap insurance bought before the spending starts. +2. **Then Phase 0** (put the dashboard in its right context) — small, self-contained, and it + settles where the section files live before anything else edits them. +3. **Then Phase 1** (idioms) — it _deletes_ code that the Phase 3 splits would otherwise have + to carry: the copy-pasted submit plumbing, the `WizardStatus` switch, several `computed`s. + Split first and you split code you are about to delete. +4. **Then Phase 3b–3i** (the splits), **Phase 2** (mechanical sweeps, independent — fit + anywhere), **Phase 4** (layer move), **Phase 5** (docs) last, so it documents the end state + once rather than tracking each step. + +--- + +## Execution: how this survives a restart and runs on Sonnet agents + +This document is a **design record**. It is not executable as-is: a fresh Sonnet agent has +none of the conversation that produced it, and nothing here records progress. So the first +ticket converts it into the artifact this repo already uses for exactly this. + +### Reuse the existing protocol, do not invent one + +`docs/project/backlog/README.md` is a proven mechanism — 75 work packages driven to `done` +through it, and its own notes say the tickets are _"self-contained (each WP file carries its +own current-state handoff) and sized for a fresh Sonnet session."_ Copy it wholesale: + +- **One ticket file per commit**, using that README's existing template (Status / Why / Read + first / Decisions (pre-made, don't relitigate) / Files / Steps / Acceptance criteria / + Verification / Out of scope / Risks). +- **`Status: todo | in-progress | done`** inside each ticket file. +- **A README with an Order table** carrying every ticket, its dependencies and its status. +- **A runnable GREEN one-liner** as the global definition of done. + +New home: **`docs/project/readable-codebase/`**, prefix **`RD-NN`**. It must not extend +`docs/project/backlog/`, because Phase 5 archives that directory — a finished arc gets +archived, a new arc gets its own folder. `RD-` also avoids collision with the existing +`WP-`/`RB-` prefixes, which matters because Phase 2 greps for those. + +### The one property that makes a restart safe + +**Each ticket updates its own `Status:` line and the README row in the _same commit_ as its +code.** Never in a follow-up commit. That makes `git log` and the ledger impossible to +desync: whatever is committed is done, whatever is not is not. + +Recovery for a fresh session with zero context is three commands: + +```bash +git log --oneline -8 +grep -rn '^Status:' docs/project/readable-codebase/RD-*.md | grep -v done # next work +npm run ci # is HEAD green? +``` + +### Making each ticket Sonnet-executable + +An agent reads _its own ticket_, not this whole document. So each ticket must be +self-contained. Ticket files are written **just in time by the supervisor**, immediately +before delegating, and land in that ticket's own commit — not all 35 up front, which would be +speculative. Three rules when writing one: + +1. **Copy the decision, never a pointer to it.** The `Decisions (pre-made, don't relitigate)` + block carries the verdict from this plan verbatim. No agent re-derives "effect map vs full + Elm" — that is settled here, with reasons, and re-opening it wastes an Opus-shaped judgment + on a Sonnet-shaped task. +2. **Inline the traps that apply to _that_ ticket.** The Risks section below is global; an + agent will not read it. The `Seed` exemption belongs in RD-05's Decisions block, the + longest-key-first sed order in RD-27's, the parameterised-`$localize` rule in RD-25's and + RD-26's. A trap left only in a global list is a trap that fires. +3. **State acceptance as a command, not a sentence.** `npm run lint` exits non-zero, or the + file is under 250 rule-lines, or `npx eslint --report-unused-disable-directives` is clean. + "Lands ~230 lines" is a design estimate and is not checkable; do not put it in Acceptance. + +### GREEN for this arc + +```bash +npm run ci +``` + +Plus, for any ticket touching a story, an `.mdx`, or `libs/shared/src/ui/**`: + +```bash +npm run ci --full # the only thing that builds Storybook and catches a broken MDX import +``` + +Phase 4's move commit **must** run `--full`. So must anything in Phase 3 that moves a template. + +### The agent loop + +One supervisor session drives it; one `developer` agent (Sonnet) executes each ticket. +Per iteration: + +1. Read the README Order table. Pick the first `todo` whose dependencies are all `done`. +2. Spawn **one** `developer` agent with a fixed prompt: _"Read CLAUDE.md, then + `docs/project/readable-codebase/README.md`, then `RD-NN.md` and its Read-first list. + Execute it. End GREEN. Update the ticket's Status and the README row in the same commit. + Do not start another ticket."_ +3. Verify with a `task-runner` agent (Haiku): `npm run ci`, `git log -1 --stat`, and that the + `Status:` line now reads `done`. +4. Green → next iteration. Red → stop and surface. Never mark a ticket done on an agent's + word alone; the check is the exit code. + +For unattended running, `/loop` with that iteration as its prompt works — the ledger is the +state, so a loop that dies mid-arc resumes from the ledger with nothing lost. + +### Sequential by default — and why parallel is worse here + +Every ticket must end `npm run ci` green **on the branch**, and three properties make +concurrent writes to one branch actively hostile: + +- `libs/shared/docs/behaviour-spec.mdx` and `showcase/snippets.generated.ts` are regenerated + and **drift-checked** by CI. Two agents both regenerating conflict by construction. +- Every ticket writes the same README Order row table — contention on literally every + iteration. +- The file sets overlap heavily: the three wizards appear in RD-07, RD-08, RD-20, RD-22 and + RD-23. + +**Where parallelism does pay:** genuinely disjoint tickets, in separate git worktrees +(`isolation: "worktree"` on the Agent tool), merged deliberately. Good candidates: the ticket +sweep (RD-18/RD-19 — 186 files, semantically touching nothing), and the Phase 5 doc tickets. +Cap it at two at a time. + +Evidence for caution: `.claude/worktrees/` currently holds **22 abandoned agent checkouts at +4.7 GB** (RD-09 deletes them). Parallel worktree agents have been used in this repo before and +left the debris behind. Use them deliberately, and clean up. + +### The ticket table (RD-01 materialises this verbatim as the README Order table) + +Each row becomes one `RD-NN-.md` and one commit. "Deps" must all be `done` before a +ticket is picked. The phase sections below this table are the source for each ticket's +`Decisions` block. + +| ID | Ticket | Deps | Source | `--full`? | +| ----- | ------------------------------------------------------------------------------------ | ---------- | --------- | --------- | +| RD-01 | Scaffold `docs/project/readable-codebase/` — README + all RD files | — | Execution | | +| RD-02 | `max-lines` rule + `reportUnusedDisableDirectives: 'error'` + 7 disables | 01 | 3a | | +| RD-03 | `overzicht` context: page + 2 nav sections, dep-cruiser edge, `HEADER_ADMIN_LINKS` | 02 | 0 | yes | +| RD-04 | Story titles → `Domein//`; add stories only where >1 state | 03 | 0 | yes | +| RD-05 | `createStore` gains the effect map + 5 specs | 02 | 1a | | +| RD-06 | **Bug fix:** 2 single-step forms → effect map + retry affordance | 05 | 1a | yes | +| RD-07 | Add `Primary` to the 3 wizard machines + specs | 05 | 1a | | +| RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | 1a | yes | +| RD-09 | **Docs + generator:** `plop-templates/form-machine.hbs`, ARCHITECTURE, fp-tea, skill | 08 | 1c | | +| RD-10 | `WizardStatus` → `WizardPhase` (payload-carrying) | 08 | 1b#4 | yes | +| RD-11 | Fold the projection into `remote-data.ts`; PascalCase the 3 machines | 01 | 1b#3 | | +| RD-12 | `ActionState` → `action` on `BriefState.Loaded` | 11 | 1b#2a | | +| RD-13 | Same for org-template, folding `pendingPublish` in | 12 | 1b#2a,#6 | | +| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | 1b#2b | | +| RD-15 | Delete `.claude/worktrees/` (22 checkouts, 4.7 GB) | 01 | 2.1 | | +| RD-16 | `parseDashboardView` returns `BigProfile`; delete `DashboardView` | 01 | 2.2 | | +| RD-17 | `successOf`/`successOr` sweep — 10 sites, 8 files | 01 | 2.3 | | +| RD-18 | Ticket sweep, frontend — 181 refs / 100 files | 01 | 2.4 | | +| RD-19 | Ticket sweep, backend — 370 refs / 86 files | 01 | 2.4 | | +| RD-20 | `wizard-errors.ts` + spec, adopted by all 3 wizards | 02 | 3c | | +| RD-21 | `rich-text-dom.ts` helpers + spec cases | 02 | 3h | yes | +| RD-22 | `intake-wizard` → 3 step components | 08, 20 | 3c | yes | +| RD-23 | `registratie-wizard` → 3 steps + upload-controller move | 08, 20 | 3c | yes | +| RD-24 | `concepts.page` → 6 sections + `concept-card` + 2 globals + `--app-code-*` | 02 | 3g | yes | +| RD-25 | `org-template-editor` → `sample-letter.ts` + labels + 2 children | 02 | 3e, 3f | yes | +| RD-26 | `letter-canvas` → labels + `letter-line`; keep its disable | 02 | 3d, 3e | yes | +| RD-27 | **The layer move:** 33 `git mv` + 28 specifiers + 8 MDX imports | 21 | 4a | yes | +| RD-28 | Layer-tag fixes (`async` missing, `breadcrumb` Chrome) + beheer doc rule | 27 | 4a, 4b | | +| RD-29 | The 3 ladder rules in `.dependency-cruiser.base.js` | 27 | 4c | | +| RD-30 | Archive `backlog/` + `refactor-backlog-setup/` (16,300 lines) + archive README | 01 | 5.1 | | +| RD-31 | `ARCHITECTURE.md` §6a — symbols not lines, 2 dead paths, new section names | 03, 08, 16 | 5.2 | | +| RD-32 | `fp-tea-atomic-design.md` — 11 broken paths + the broken anchor | 27 | 5.3 | | +| RD-33 | CLAUDE.md + `atomic-design.mdx` + `ui-component` skill | 03, 27, 29 | 5.4-5 | yes | +| RD-34 | _(optional)_ `NO_SUBORGS`/`NO_TABLES` → `RemoteData.Empty` | 11 | 1b | | +| RD-35 | _(optional, last, alone)_ upload `type:` → `tag:` | 27 | 1b#5 | | + +Recommended running order is the ID order; it already respects every dependency. RD-15 through +RD-19 are independent of everything and can be pulled forward whenever a short session needs +filling — RD-15 in particular makes every later search faster and should go early. + +Two ordering traps the table encodes but an agent should be told outright: + +- **RD-01 must precede RD-30**, because RD-01 copies its ticket template _out of_ the very + directory RD-30 archives. +- **Four tickets edit the same two doc files in different sections** — RD-09 rewrites the + submit-idiom teaching (`ARCHITECTURE.md` §2d area, `fp-tea` §338-350), while RD-31 rewrites + `ARCHITECTURE.md` §6a and RD-32 fixes `fp-tea`'s paths. Sequential is fine; never run these + two pairs in parallel worktrees. + +--- + +## Phase 0 — Put the dashboard in the right context + +The dashboard is the portal home, but it lives inside `registratie`, a context that +`.dependency-cruiser.ssp.js` declares as `registratie: []` — permitted to import no other +context. Three concrete symptoms: + +- Its six sections span four concerns: registratie data (3), aanvragen (1), cross-context + action links to `/herregistratie` `/intake` `/brief` `/concepts` (1), admin links to + `/beheer/*` (1). +- The cross-context coupling is **invisible to the linter**, because + `wat-wilt-u-doen.section.ts` links by route _string_. `dep:check` passes and gives false + assurance on exactly this file. +- `beheer-links.section.ts:6` imports `ADMIN_LINKS` from `../../../shell/nav.config` — a + context reaching into the app frame. `app.config.ts:72` **already** provides that same + constant to the shared site header through the `HEADER_ADMIN_LINKS` token. + +### Steps + +1. Scaffold a context with `npm run gen:context` (`plop context`) named **`overzicht`** + (Dutch, per CLAUDE.md: domain contexts are Dutch; the page's own title is + "Mijn overzicht"). +2. Move the **page** and the two **portal-level navigation** sections into it: + - `overzicht/ui/overzicht.page.ts` (was `registratie/ui/dashboard.page.ts`) + - `overzicht/ui/wat-wilt-u-doen.section.ts` + - `overzicht/ui/beheer-links.section.ts` +3. **Leave the four data sections in `registratie/ui/dashboard/`** — `mijn-aanvragen`, + `wat-moet-ik-regelen`, `mijn-registratie`, `specialismen` render registratie data and + belong beside their store. This separation is only possible _because_ of the split; it is + the refactor's first real payoff. +4. Declare the edge in `.dependency-cruiser.ssp.js`: `overzicht: ['registratie']` — the + second sanctioned cross-feature edge, mirroring `herregistratie: ['registratie']`. +5. `beheer-links.section.ts` **injects `HEADER_ADMIN_LINKS`** instead of importing + `shell/nav.config`, removing the context→shell reach-in. +6. Consider giving `wat-wilt-u-doen`'s action list the same treatment — a token beside + `NAV_ITEMS`/`ADMIN_LINKS` in `shell/nav.config`. Route strings on a landing page are + legitimate, but the list is app-frame copy, not registratie's. +7. Update `app.routes.ts:19` to `loadComponent: () => import('@overzicht/ui/overzicht.page')`, + and add the `@overzicht/*` alias to `apps/ssp/tsconfig.json`. + +**Keep the `/dashboard` route** — it is user-visible and in the e2e specs. Renaming it to +`/overzicht` is a separate, optional change needing a redirect. + +Also settle the two deviations the refactor left behind: + +- **Story titles — fix.** They are `Domein/Registratie/Dashboard/`; CLAUDE.md specifies + `Domein//`, "full stop", and all 41 other story files comply. Retitle to + `Domein/Registratie/` for the four that stay and `Domein/Overzicht/` for those + that move. Safe: only `layers.mdx` deep-links a story id, and not one of these three. +- **8 imports — accept, do not fix.** CLAUDE.md has no import-count rule; ≤6 was a proxy + metric, and one `import` per rendered section is exactly right. The only way down is a + `DASHBOARD_SECTIONS` const spread into `imports:`, which trades a self-documenting array for + an indirection and adds a barrel-shaped thing to a repo that deliberately has none. +- Three of six sections have no story (`beheer-links`, `wat-moet-ik-regelen`, + `wat-wilt-u-doen`). Add one only where the section has more than one visual state. + +--- + +## Phase 1 — One submit idiom, and two state encodings instead of six + +> **This phase fixes two live bugs.** It is not only hygiene. + +### 1a. Submit + +`runIfSubmitting` is a `private async` method duplicated across 5 components — named +`runIfIndienen` in the registratie wizard — invoked at 8 call sites, always as: + +```ts +this.dispatch({ tag: 'Submit' }); // the reducer decides +this.runIfSubmitting(); // then re-read state() and re-check the tag it hoped for +``` + +- `behandeling/ui/besluit-form/besluit-form.component.ts:129` +- `registratie/ui/change-request-form/change-request-form.component.ts:175` +- `herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts:275` +- `herregistratie/ui/intake-wizard/intake-wizard.component.ts:386` +- `registratie/ui/registratie-wizard/registratie-wizard.component.ts:634` + +#### The two bugs (fix these regardless of the rest) + +`besluit-form.component.ts` and `change-request-form.component.ts` never dispatch `Retry`, +although `besluit.machine.ts:64` and `change-request.machine.ts:50` both export it. On a +failed submit, `Failed` falls into the `@else` branch that renders the form, but `editing()` +is `null` there, so `besluit()`/`toelichting()` return `''` — **the user sees an emptied +form**. The still-enabled submit button dispatches `Submit`, which the reducer no-ops from +`Failed`. Unrecoverable dead end, reachable from any failed submit, in both components. + +Other defects to close in the same pass: + +- Step-boundary logic lives in the component (`s.step < 3 ? Next : Submit`), beside the + reducer's own exported `next`/`submit`. +- The two herregistratie wizards interleave optimistic store calls the reducer knows nothing + about (`herregistratie-wizard.component.ts:277,282,285`; + `intake-wizard.component.ts:388,396,399`). `registratie-wizard` has no optimistic flag. + +#### Design: `createStore` gains an effect map + +Constraint: **`reduce` stays pure** (CLAUDE.md). The fix lands in the `dispatch` wrapper in +`libs/shared/src/application/store.ts` — inside the one sanctioned wiring idiom, not beside +it. + +```ts +export type StoreEffects = Model extends { tag: string } + ? { + [K in Model['tag']]?: ( + state: Extract, + store: Store, + ) => unknown; + } + : never; + +export function createStore( + init: Model, + update: (model: Model, msg: Msg) => Model, + effects?: StoreEffects, +): Store; +``` + +Four load-bearing choices: + +- **A conditional type, not a `Model extends {tag}` constraint** — the constraint would break + `store.spec.ts:8`'s `createStore(0, (n, m) => n + m)`. With the conditional, `Model = number` + resolves to `never`, so effects are a compile error there and omitting them stays legal. +- **Keys are `Model['tag']`** — a renamed or typo'd state tag becomes a compile error. +- **The narrowed state is argument one** — this is what deletes the + `const s = this.state(); if (s.tag !== 'Submitting') return;` preamble at all 8 sites. The + body cannot run in the wrong state, so it cannot re-guess. +- **The store is argument two** — the effect needs `dispatch`, but `createStore(...)` runs in a + field initializer before `this.store` exists. + +**The trigger rule is where the design lives.** Run `effects[next.tag]` when **both**: + +1. `prev.tag !== next.tag` — the store _entered_ the tag. A `Submit` that fails validation is + `Editing → Editing`: no fire. A second `Submit` while `Submitting` is a reducer no-op: no + fire, so **double-submit protection falls out of the rule**. `Retry` is `Failed → +Submitting`: fires, so `onRetry` needs no special case. +2. **the msg tag is not `Seed`.** This single line is what stops the five `Submitting` + Storybook stories from firing real HTTP (see Risks), and stops `draftSync.onResume` → + `Seed` from re-submitting a resumed draft. + +Implementation note for whoever writes it: capture `prev`/`next` inside the `model.update(...)` +callback and invoke the effect **after** `update` returns. Do not read `model()` inside +`dispatch` — the comment at `store.ts:24-28` explains the livelock, and +`store.spec.ts:16-30` exists because that bug already happened once. + +**Effect bodies do not move.** They stay as private component methods, registered in the map; +the `begin*`/`confirm*`/`rollback*` calls stay inside them. The effect slot _is_ the sanctioned +place for effects, so "side effects stay out of the reducer" holds unchanged. Per-site diff: +delete two guard lines, take the narrowed state, register one map entry. + +```ts +private store = createStore(initial, reduce, { + Submitting: (s, store) => this.submitBesluit(s, store), +}); +``` + +**`onPrimary` → a `Primary` msg** on the 3 wizard machines: +`primary(s) = isLastStep(s) ? submit(s) : next(s)`, composed from each machine's own already- +exported `next`/`submit`/`currentStep`. Keep `Next` and `Submit` in the unions (templates, +specs and the showcase use them). Then `onPrimary()` is one dispatch, `onRetry()` is one +dispatch, and the shell's outputs map 1:1 onto messages — which is what +`.claude/skills/form-machine/SKILL.md:74-78` already claims. + +**Retry affordance:** reuse the existing id `@@wizard.opnieuwProberen` with byte-identical +source text. It already has an English target in **both** `messages.en.xlf` files, so no new +translation is needed. + +#### Rejected alternatives (recorded, not re-litigated) + +- **An Angular `effect()` watching state** — reject. `store.spec.ts:16-30` exists because this + exact pattern livelocked the app. Worse, a signal effect is a latest-value notification, not + an event stream: two dispatches in one tick coalesce, so a transient `Submitting` can be + observed as never having happened — a silently dropped submit, the very failure being fixed. +- **Full Elm `reduce -> [state, Cmd]`** — the honest end state and the only option with + statically exhaustive effect coverage, but disproportionate here: 9 machines, 9 specs, every + dispatch site. And `domain/` may not import Angular (lint-enforced), so a `Cmd` cannot carry + `BigProfileStore.beginHerregistratie` — it becomes a symbolic tag plus a `switch` in the + component, which is the guard being deleted, relocated. Keep as the documented upgrade path. + +### 1b. Six encodings of "in flight / ok / failed" → two + +Two survive: **`RemoteData`** for "data I fetched", and **the machine's own state union** +for "where this thing is". Everything else either folds into one of those or is an honest +exception with a reason. + +| # | Encoding | Where | Disposition | +| --- | ------------------------------------- | ----------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `RemoteData` | `application/remote-data.ts` (91 lines), 19 files | **Keep**, and absorb #3 so one file is where a lifecycle becomes async state | +| 2a | `ActionState` | `application/action-state.ts`, 2 producers | **Delete** — both stores collapse it to `busy` + `lastError` byte-identically; zero consumers keep the union. Becomes an `action` field on each machine's `Loaded` variant | +| 2b | `SaveState` | same file | **Keep** — it has **2 genuine 4-way consumers** (`brief.page.ts:149-160`, `org-template.page.ts:102-113`). Move it to `debounced-save.ts`, beside its only producer, then delete `action-state.ts` | +| 3 | `LoadLifecycle` + `machineRemoteData` | `application/machine-remote-data.ts` (24 lines), 3 identical call sites | **Relocate, not delete** — fold into `remote-data.ts` as `fromLoadLifecycle`, keyed PascalCase, beside the existing `fromResource` | +| 4 | `WizardStatus` | `layout/wizard-shell/wizard-shell.component.ts:19` | **Replace with a payload-carrying `WizardPhase`** — the 3 switches drop the error payload, which then travels as a second `errorMessage` input | +| 5 | `UploadStatus` | `domain/upload.machine.ts:11-18` | **Keep** — it _is_ encoding #2 for a sub-machine, and its payloads (`progressPct`, `documentId`, `reason`) are consumed by 3 UI components. Only the `type:` dialect is off; optional, last, alone | +| 6 | 5 ad-hoc flags | see below | **1 folds, 4 keep** | + +**Why #3 relocates rather than deletes:** the mapping has to exist somewhere, because +`` takes `RemoteData`. Deleting the module re-inlines a 6-line switch in 3 stores — +recreating the duplication WP-31 removed. Relocating still wins the whole prize: the lowercase +constraint dies, the dialect drift resolves, and the survivor sits beside `fromResource` as +what it actually is — **a `RemoteData` constructor, not a sixth encoding**. + +**The causal chain that makes the dialect fix free:** `brief`, `org-template` and +`stamdata-editor` are the only three state unions in the repo with lowercase tags, their Msg +tags are PascalCase in the same file, and the _only_ thing pinning them is +`machineRemoteData`'s structural `S extends LoadLifecycle`. Relocate the projection with +PascalCase keys and the drift resolves itself — no separate renaming pass. + +**#4 — the shell genuinely needs the payload.** Replace `WizardStatus` + `errorMessage` with +one input: `{ tag:'Editing' } | { tag:'Submitting' } | { tag:'Submitted' } | { tag:'Failed'; +message: string }`. The 3 mapping `computed`s stay — `Answering` and registratie's Dutch +`Invullen/Indienen/Ingediend/Mislukt` are not the shell's vocabulary — but now carry the +message, so the 3 duplicated `errorMessage` computeds fold in and the second input disappears. +`@switch` cannot narrow, so read `Failed` via the existing `whenTag` (`kernel/fp.ts:27`). +`wizard-shell.stories.ts` must change in the same commit. + +**#6 — one folds, four keep.** Only `org-template.store.ts:59` `pendingPublish` is a genuine +illegal-state pair (`pendingPublish && busy` is representable and meaningless) — it becomes a +fourth `action` variant. The other four are **correctly modelled as they are**: +`big-profile.store.ts:61` `pending` is a lone boolean the dashboard reads _after_ the wizard is +destroyed, so it cannot be derived from the machine; and `aanvragen.store.ts:28`, +`admin-cases.store.ts:26`, `feature-flags.page.ts:96` are one-shot **action** errors sitting +beside a successfully-loaded list. Folding those into the list's `RemoteData` would make the +error _replace_ the list, since `Failure` carries no value — precisely the RB-20 behaviour +those comments exist to prevent. Add a comment saying so, and leave them. + +**A win this reveals:** `org-template.store.ts:29,129` (`NO_SUBORGS`) and `stamdata.store.ts` +(`NO_TABLES`) dispatch `LoadFailed` for what is semantically **`Empty`**. Once the projection +is explicit, a distinct state → `RemoteData.Empty` is a few lines, and `` already +renders it via `emptyText`. Optional, own commit, needs one new `$localize` id. + +### 1c. Commit order + +Ticket mapping: **A1=RD-05, A2=RD-06, A3=RD-07, A4=RD-08, A5=RD-09, B4=RD-10, B3=RD-11, +B2a=RD-12, B2b=RD-13, B2c=RD-14, B6=RD-34, B5=RD-35.** + +Submit first — it settles the final shape of the 5 components, and 1b#4 touches 3 of them. + +| # | Commit | Notes | +| ----- | ---------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | +| A1 | `createStore` gains the effect map + specs | No call sites change. Behaviour-neutral, so risk is isolated to one commit | +| A2 | Migrate the 2 single-step forms; **add the retry affordance** | Closes both bugs. Reuses `@@wizard.opnieuwProberen`, no new xlf target | +| A3 | Add `Primary` to the 3 wizard machines + specs | Pure domain. `check:seam` greps `BESLUIT_TAGS` and `SCHOLING_THRESHOLD_DEFAULT` — keep both greppable | +| A4 | Migrate the 3 wizards to the effect map + `Primary` | `runIfSubmitting`/`runIfIndienen` become registered effects | +| A5 | **Docs and the generator, same diff** | See below — non-optional | +| B4 | `WizardStatus` → `WizardPhase` | Drops the `errorMessage` input; update `wizard-shell.stories.ts` | +| B3 | Fold the projection into `remote-data.ts`; PascalCase the 3 machines | Widest mechanical commit; 77 lowercase literals | +| B2a/b | `ActionState` → `action` on `Loaded` (brief, then org-template + `pendingPublish`) | Must follow B3 or the same tags get renamed twice | +| B2c | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | Type-only | +| B6 | _(optional)_ `NO_SUBORGS`/`NO_TABLES` → `Empty` | Needs a new `$localize` id per app that renders it | +| B5 | _(optional, last, alone)_ upload `type:` → `tag:` | ~20 files; see the sed hazard in Risks | + +**A5 is not optional.** `docs/reference/architecture/ARCHITECTURE.md:308-322` and `:574`, and +`docs/reference/fp-tea-atomic-design.md:338-350`, both teach `runIfSubmitting` verbatim — and +`plop-templates/form-machine.hbs` **generates it**, as does +`.claude/skills/form-machine/SKILL.md:65-78`. If the generator is not updated, the next form +scaffolded in this repo recreates the bug. Update all five in the same commit as A4. + +**Scope discipline:** A1 + A2 alone deliver "dispatching cannot silently skip the effect" and +fix both bugs. If the budget shrinks, stop after A5; B3 and B2 are hygiene, not correctness. + +--- + +## Phase 2 — Mechanical sweeps (no behaviour change) + +1. **Remove `.claude/worktrees/`** — 22 abandoned `agent-` checkouts, **4.7 GB**, + gitignored (`.gitignore:64`). They are why unqualified repo-wide `grep`/`find` return ~23× + inflated counts, which taxes every future search by a human or an agent. + + **Correction made while executing RD-01:** these are **live registered git worktrees**, not + orphaned directories. Each has a `worktree-agent-` branch carrying real RB-xx commits. + So `rm -rf` is wrong — it leaves 22 broken worktree registrations behind. Use + `git worktree remove` per worktree, then delete each branch, then `git worktree prune`. + + Verified during RD-01: the commits are already reachable from `main` (spot-checked + `95bb773`, `80de261`, `dfc6c41`, `ce95294` with `git merge-base --is-ancestor`), because + `637d500` merged the whole RB-01..RB-33 arc. **RD-15 must re-verify all 22 before removing + any** — check every branch tip is an ancestor of `main`, and stop if one is not. + +2. **Finish Step 2's name collapse** (committed as done, but did not land): + `parseDashboardView` still returns `DashboardView` + (`registratie/infrastructure/dashboard-view.adapter.ts:119`), and + `big-profile.store.ts` pays twice — line 35 computes `RemoteData`, + line 45 re-maps to `RemoteData`. Make the parse return `BigProfile` + directly and delete the intermediate. One payload, one name. +3. **`successOf` / `successOr` sweep** — 10 inline unwraps remain in 8 files. They do not all + want the same helper: + - `undefined` fallback → existing `successOf`: `beoordeling.page.ts:78` + - `[]` / `null` fallback → add `successOr(rd, fallback)`: `werkvoorraad.page.ts:61`, + `admin-cases.page.ts:83`, `audit.page.ts:99`, `feature-flags.store.ts:27`, + `registratie-wizard.component.ts:509`, `mijn-aanvragen.section.ts:93` + - boolean predicates → leave as-is: `access.store.ts:36`, `feature-flags.store.ts:51` + - `big-profile.store.ts:57` hand-rolls `map` — **use the existing `map`** from + `remote-data.ts` +4. **Ticket-comment sweep, frontend and backend** (user-selected scope): **551 refs across + 186 files** — 181 in `apps`+`libs` (100 files), 370 in `backend/` (86 files: 70 `.cs`, + plus `Dockerfile`, 4 `.sh`, 4 `.yml`, 2 `.md`). Strip `WP-`/`RB-` and keep the surrounding + sentence; git blame holds the provenance. **Keep all 90 `ADR-000x` refs** across 68 files — + those point at documents that exist. Note `CD-` appears nowhere in the repo. + +--- + +## Phase 3 — The guard first, then 7 splits + +### 3a. The guard goes FIRST, not last + +Putting the rule ahead of the splits stops the refactor itself from adding a new 300-line +file. In `eslint.config.mjs` (which today has no per-folder rules at all): + +```js +{ + files: ['{apps,libs}/**/*.{page,component,section,step}.ts'], + rules: { 'max-lines': ['error', { max: 250, skipBlankLines: true, skipComments: true }] }, +}, +``` + +Two corrections to the approved plan's Step 8, both load-bearing: + +- **The glob must include `section` and `step`.** `*.{page,component}.ts` does **not** match + `*.section.ts` — the file kind the dashboard refactor invented, and the kind Phase 3 creates + most of, would escape the guard entirely. +- **Add `linterOptions: { reportUnusedDisableDirectives: 'error' }`** in the same commit. + ESLint 9 only _warns_ by default and `npm run lint` does not fail on warnings. At `error`, + **every split commit is forced to delete its own `eslint-disable` or lint fails** — the + exemption list cannot rot into permanent debt. The repo has zero disables outside the + generated `api-client.ts` and is clean under this flag today, so it lands green. + +### 3b. Seven offenders, not nine + +Measured with the real rule (`skipBlankLines`, `skipComments`), not `wc -l`: + +| `wc -l` | rule | File | Axis | +| ------- | ------- | ------------------------------------ | ---------------------------------------------- | +| 644 | **574** | `registratie-wizard.component.ts` | **per step** | +| 496 | **472** | `showcase/concepts.page.ts` | **per teaching section** | +| 463 | **414** | `letter-canvas.component.ts` | **labels + one extraction, then stays exempt** | +| 406 | **368** | `intake-wizard.component.ts` | **per step** | +| 353 | **329** | `org-template-editor.component.ts` | **per output cluster** | +| 293 | **253** | `rich-text-editor.component.ts` | over by 3 — move 2 helpers | +| 288 | **252** | `herregistratie-wizard.component.ts` | over by 2 — one shared helper | +| 267 | 232 | `behandel-scherm.component.ts` | **already compliant — leave alone** | +| 262 | 236 | `stamdata-table-editor.component.ts` | **already compliant — leave alone** | + +### 3c. The step contract already exists — do not invent one + +For the wizards, the pattern to copy is **not** the dashboard. It is +`registratie/ui/address-fields/address-fields.component.ts`, which this very wizard already +composes and whose header comment _is_ the contract, verbatim: _"Pure & presentational — +values in via `value`, errors in via `errors`, every keystroke out via `fieldChange`. No +store, no services, no internal state; the container owns the Model and decides what a change +means."_ Two containers already reuse it. + +So: **inputs down, one narrow output up, `dispatch` never passed down.** + +- `registratie-wizard` → `adres.step.ts` (~90), `beroep.step.ts` (~130), `controle.step.ts` + (~110); parent lands ~230. `RegistratieLookupStore` is `providedIn: 'root'`, so the beroep + step injects the same instance and owns its own `` over the DUO lookup — this is + the one place the dashboard's axis _does_ apply. **Moving the upload controller is what gets + the parent under 250**: `createUploadController` takes a `dispatch` callback, so the step + creates its own with `dispatch: (msg) => this.uploadMsg.emit(msg)` — one output, not five. + The BRP prefill `effect` **stays in the parent**, because it writes to the machine. +- `intake-wizard` → `buitenland.step.ts`, `werk.step.ts`, `review.step.ts`; parent ~200. + `scholingZichtbaar` is **not** an input — each step takes the threshold and calls the pure + `lageUren(answers, threshold)` itself ("derive, don't store"). +- `herregistratie-wizard` is over by **two lines**. Do not split its steps for symmetry — its + whole template is ~100 lines. Extract one shared pure helper instead: + `layout/wizard-shell/wizard-errors.ts` with `toWizardErrors()` + spec, beside the existing + `naarStapLabel` that lives there for exactly this reason. Removes ~6 lines from all three + wizards. **Do not** touch the three `shellStatus` switches — the tags genuinely differ per + machine, and an exhaustive switch is the house style. +- `wizard-shell` (205 lines) **already provides the whole frame** — stepper, error summary, + `
`, navigation, submitting/submitted/failed, a11y focus. The steps slot into its + existing default ``. **Nothing new in `libs/shared`.** + +**Corollary: give the new steps no stories.** Each wizard's existing story already mounts +every step by seeding the machine. The dashboard got this right too — 3 stories for 6 +sections, only where there was async state to show. + +### 3d. `letter-canvas` — a misapplied rule, not a split + +20 of its 28 `input()`s are pure `$localize` labels and **no caller overrides a single one** +across all 4 call sites. The CLAUDE.md rule they were built for — _"Shared/English components +must not hardcode Dutch — expose copy as `input()`s"_ — governs `libs/shared`, **not** a Dutch +domain component in `brief/ui/`. Inline them as `i18n="@@id"` in the template; same id, same +source text means **zero `messages.en.xlf` edits**. + +Do **not** collapse them into a config object or an injection token. +`HEADER_NAV_ITEMS`/`DEBUG_PANEL` exist because two apps genuinely differ; here nothing +differs, so a token adds a provider and an indirection to solve a problem nobody has. + +One extraction earns its keep: `letter-line.component.ts` (~70 lines out) — the `#line` +template plus the sample/diff helpers. It replaces six 4-line `ngTemplateOutlet` incantations +with three one-line tags and is the only part with logic worth a spec. + +That leaves ~329: 77 lines of CSS and 204 lines of _one letter_. Splitting it into +letterhead/body/signature/footer makes "what does the letter look like" a five-file question +for no behavioural seam. **Keep one `/* eslint-disable max-lines */`** with an honest reason. +It becomes the only disable in the repo, and `reportUnusedDisableDirectives` keeps it honest. + +### 3e. The `$localize` boundary that governs 3d and 3f + +**Plain messages move to the template; parameterised ones stay in TS.** The xlf stores +interpolations as ``; moving such a message into a +template renames the placeholder to `INTERPOLATION` and **breaks the translation merge**, so +`ng build --localize` fails. Only 4 messages are affected: `orgTemplate.margins`, +`orgTemplate.invalid`, `orgTemplate.publish.impact`, `wizard.naarStap`. + +### 3f. `org-template-editor` — split by output cluster + +The 11 `output()`s are the tell; each child takes one mutation family: + +- `SAMPLE_LETTER_BRIEF` (44 lines) → `brief/domain/sample-letter.ts`. It is a **dead export** + (used only in its own file) and it is production content, not a fixture — so it must **not** + go near `brief.testing.ts`, or dependency-cruiser's `no-testing-in-production` rule fails. +- 11 of 13 label inputs → template `i18n` (they are declared `protected`, so they were never + bindable — constants wearing `input()` ceremony). The two parameterised ones stay per 3e. +- `logo-upload.component.ts` (~34 out) and `version-history.component.ts` (~18 out). +- Parent drops 11 outputs → 5 and lands ~222. No exemption. + +### 3g. `concepts.page.ts` — split by section, but decompose the CSS by owner + +A per-section split does **not** fix the 142-line `styles:` block, because Angular scopes +styles per component: the page's `.card` cannot style a child's DOM. So: + +- `.section` → **delete**, use the existing global `.app-section`. +- `.lead`, `.cols` → two new globals beside `.app-text-subtle`/`.app-stack` in + `libs/shared/styles.scss`, whose own comment says it exists to centralise these idioms. +- `.card`, `.tag*`, `.note`, `pre` (~70 lines) → owned once by `concept-card.component.ts`, + used ~11 times, which also deletes ~11 copies of the card boilerplate. + +**Watch the colour guard.** `scripts/check-tokens.sh` greps only `--include='*.component.ts'` +— which is why this page currently gets away with `#1e2430`, `#fff`, `#e5e5e5`. Moving that +CSS into a `*.component.ts` brings it under the guard **for the first time**, so in the same +commit: drop the `var(--rhc-x, #hex)` fallbacks (all 16 tokens are defined) and add +`--app-code-bg/-fg/-keyword/-string/-comment` for the `pre` palette, following the existing +`--app-devpanel-*` precedent added for this exact reason. + +### 3h. `rich-text-editor` — over by three + +`rich-text-dom.ts` already exists beside it with its own spec, so the seam is built. Move the +selection/range surgery out of `deleteAdjacentChip`/`insert` into it. Cheapest of the seven, +and it converts two untested imperative-DOM branches into spec cases. + +### 3i. Order + +1. the rule + `reportUnusedDisableDirectives` + 7 disables +2. `wizard-errors.ts` + spec, adopted by all three wizards → delete that disable +3. `rich-text-dom` helpers + spec → delete that disable +4. `intake-wizard` → 3 steps +5. `registratie-wizard` → 3 steps + the upload-controller move +6. `concepts.page` → 6 sections + `concept-card` + 2 globals + `--app-code-*` tokens +7. `org-template-editor` → `sample-letter.ts` + labels + 2 children +8. `letter-canvas` → labels + `letter-line`; **keep** its disable, rewrite the reason + +Steps 2–8 are independent; only 2 must precede 4 and 5. + +--- + +## Phase 4 — Make the folder equal the layer, then enforce the ladder + +### 4a. Move (`libs/shared/src/ui/` only) + +``` +ui/atoms/ 12 flat + upload/{delivery-channel-toggle,document-chip,file-input, + upload-progress-bar,upload-status-icon} (17) +ui/molecules/ 13 flat + upload/single-upload (14) +ui/organisms/ upload/{document-category,document-upload} (2) +``` + +`upload/` **splits by layer but keeps its feature subfolder inside each layer**. It satisfies +decision #2 literally, costs the same 6 relative-import rewrites as a flat split, keeps a +genuinely cohesive group together, and makes "5 atoms + 1 molecule + 2 organisms" visible in +the tree instead of hidden in story titles. No barrel — the repo has none and does not need +one. + +**`layout/` does not move.** CLAUDE.md §5 _explicitly_ enumerates `libs/shared/layout` +components getting Atoms…Templates buckets, so `layout/` is sanctioned to hold several layers; +its organisms are chrome only its own templates compose. Instead, fix the two mislabels: +`async.component.ts` is missing its `/** Molecule: */` tag, and `breadcrumb.component.ts` says +`/** Chrome: */` where its title says Molecules. + +**`libs/beheer/src/ui/` — fix the doc, not the code.** Its story title is +`Domein/Beheer/Stamdata Table Editor` while CLAUDE.md §5 and `layers.mdx` say +`Design System/…`. The code is right: `libs/beheer` _is_ a bounded context that lives in +`libs/` only to be shared by two apps. Amend the two doc lines. That dissolves the +"two taxonomies" oddity and beheer correctly needs no layer folders. + +**Mechanics, one commit for all of `ui/`:** 33 whole-directory `git mv`s break zero relative +imports except the 6 inside `upload/`; then rewrite the 28 distinct specifier strings +(179 occurrences, 59 files) **longest-key-first**, so `upload//` is processed before any +bare `upload/`. Confirmed: no edits to `angular.json`, `eslint.config.mjs`, `plopfile.mjs`, +`e2e/`, or any tsconfig; both Storybook globs are recursive; dependency-cruiser's +`ui-not-infrastructure` pattern still matches a nested path; and `check-tokens.sh`'s CIBG-GAP +check keys on the **directory basename**, which a parent-folder move preserves. + +Verify with `npm run typecheck` (4 tsconfigs — catches every missed specifier), `dep:check`, +`test`, then **`npm run ci --full`**. `git diff --stat -M` should show only renames plus +one-line import edits. + +### 4b. Keep all 78 layer-tag comments + +Reversal of my earlier claim. The tag prefixes a real one-line description +(`/** Atom: thin wrapper over CIBG .btn — typed variant API. */`); deleting the word leaves +the sentence and buys nothing. And only the 32 in `ui/` are made redundant by folders — the 25 +organisms and 9 pages in `apps/**/ui/` have no layer folder and a title that deliberately +omits the layer, so there the comment is the **sole** carrier. A three-way redundancy that has +never once disagreed is cheap documentation. + +### 4c. The ladder rules are the real prize + +The folder move is what makes this _expressible_; this is what makes it _enforced_. Today +nothing stops an atom importing an organism. Add to `.dependency-cruiser.base.js`: + +- `atoms-compose-nothing-above`: `ui/atoms/` → `ui/(molecules|organisms)/` forbidden +- `molecules-below-organisms`: `ui/molecules/` → `ui/organisms/` forbidden +- `design-system-not-layout`: `ui/` → `layout/` forbidden + +Two details that matter: **forbid upward only, never "atoms are leaves"** — same-layer edges +are legitimate and exist today (`masked-value → button`, `review-section → data-block`, +`task-list → choice-link`); and `pathNot` must exempt `\.(spec|stories)\.ts$`, because +`async.stories.ts` composes `skeleton` and a story may legitimately reach for context. + +Zero upward edges exist today, so all three land green immediately. Dependency-cruiser rather +than ESLint: it is where every other boundary rule lives and it emits the architecture graph. + +--- + +## Phase 5 — Fix the docs that describe this flow + +1. **Archive the finished backlog.** `git mv docs/project/backlog` and + `docs/project/refactor-backlog-setup` under `docs/project/archive/`. Verified: **all 74 + WP files are `Status: done`**; the two trees are 6,982 + 9,318 = **16,300 of the docs + tree's 20,317 lines**. Add a ~15-line `archive/README.md`: this is historical, git holds + the rest. +2. **`ARCHITECTURE.md` §6a** ("The request lifecycle today", line 542) is the best onboarding + artifact in the repo and has rotted: + - 15 `L` line citations, now wrong — `Program.cs` L80 lands on a `// WP-60:` comment + about client timeouts, not `/dashboard-view`; L120 lands mid-expression. **Cite symbols, + not lines.** + - Two cited paths do not exist: `src/environments/environment.ts` (pre-monorepo) and + `proxy.conf.json`. + - Its read-walkthrough shows `` on the dashboard page; + after the split that markup lives in `mijn-registratie.section.ts`. + - It states the boundary yields `RemoteData` — wrong once Phase 2.2 lands. +3. **`docs/reference/fp-tea-atomic-design.md`** — 11 pre-monorepo `src/app/…` paths, all + broken; one (`submit-herregistratie.ts`) points at a deleted file; and a broken anchor at + line 427 (`#1-the-big-picture-three-contexts-four-layers` vs the actual + "two apps, cross-app libraries"). +4. Update CLAUDE.md for the new `overzicht` context, the `max-lines` budget, the `libs/beheer` + title rule (4a), and the step-component contract (3c). +5. `libs/shared/docs/atomic-design.mdx` gains the step contract and the layer table; also fix + its stale claim that `eslint.config.mjs` enforces the layer rules — dependency-cruiser + does. Fix the stale `src/app/shared/ui/...` paths in `.claude/skills/ui-component/SKILL.md`. + +--- + +## Risks + +1. **The Storybook trap (highest).** All 5 components mount their `Submitting`/`Indienen` state + via `Seed` in stories that use a real `provideHttpClient()` with **no request mocking** + (`besluit-form.stories.ts:33`, `herregistratie-wizard.stories.ts:70`, + `intake-wizard.stories.ts:38`, `change-request-form.stories.ts:34`, + `registratie-wizard.stories.ts:88`). Without the `Seed` exemption in the trigger rule they + fire real network calls, flip to `Failed`, and red the `storybook-a11y` job + (`npm run ci --full`). Verifying those 5 stories still show a spinner **is** the exemption's + acceptance test. +2. **`dispatch` becomes effectful, and two dispatch sites sit inside Angular `effect()`s** — + `intake-wizard.component.ts:363` (`SetPolicy`) and `registratie-wizard.component.ts:~596` + (`PrefillAdres`). Both land on an unchanged tag, so nothing fires, and both are already + `untracked`. **Never key an effect on the editing tag** — that is the livelock. +3. **The `type:` → `tag:` sed hazard (B5).** `type` is also a legitimate _field_ name in that + neighbourhood — `rejectReason(cat, { type: file.type, sizeMb })` — and + `FileRejected.reason` has the literal value `'type'`. A blind rename breaks upload + validation silently. File-by-file with the type-checker, or defer. +4. **The ticket sweep is 186 files, including `backend/`'s `Dockerfile`, 4 `.sh` and 4 `.yml`.** + Do not blanket-`sed`. A `WP-`/`RB-` token could appear in a string that matters (seed data, + a test name, a migration id) rather than a comment. Review the diff per file group, and + keep all 90 `ADR-000x` refs. +5. **CI drift gates bite mechanically.** `scripts/ci-local.sh:33-34` diffs + `showcase/snippets.generated.ts` (fed by `// #region showcase:` markers — `remote-data.ts:30` + carries `showcase:fold`, `intake.machine.ts:59` carries `showcase:steps`) and + `libs/shared/docs/behaviour-spec.mdx`. Run `gen:snippets` / `gen:behaviour-spec` in the + **same commit** as any change that moves a region or a spec title. +6. **`action` inside `Loaded` narrows where "busy" can exist.** Re-check every current + `actionState.set({tag:'Busy'})` site for reachability from a non-loaded state. Note a + `BriefLoaded` reload then resets `action` to `Idle`, which is a behaviour _improvement_: a + stale error can no longer outlive a reload. +7. **Highest risk in Phase 4: a broken MDX story import passes `npm run ci` and fails CI.** + `libs/shared/docs/*.mdx` has **8 relative story imports** across `a11y.mdx`, + `atomic-design.mdx`, `remote-data.mdx` and `fp-in-ui.mdx` of the form + `../src/ui//.stories`. These break on the move, and only `build-storybook` + catches them — which is **not** in the default `npm run ci`, only `--full`. Fix all 8 in the + move commit and run `npm run ci --full` before pushing it. +8. **`check-tokens.sh` has blind spots in both directions.** It greps only + `--include='*.component.ts'`. Moving CSS from a `*.page.ts` into a `*.component.ts` newly + _exposes_ it (see 3g); moving CSS to a `.styles.ts` or `.scss` newly _hides_ it. Decide + deliberately, and widen the glob in the same commit if you move CSS out. +9. **`styles: [importedConst]` is unproven in this repo** — 47 of 47 components use inline + literals and only 2 `.scss` files exist. If the `letter-canvas` style extraction is + attempted, prove it with `ng build` first; the fallback is the single `eslint-disable`. +10. **Behaviour drift while moving 200+ template lines.** Move template text byte-identically + and let `git diff -M` prove it. The wizards' specs cover `reduce`, not the markup, so the + markup's only guards are review and the axe run in `ci --full`. +11. **A step component reaching for the store.** Passing `dispatch` down is tempting and would + let a step dispatch `Submit`. Use outputs, per `address-fields`. If overridden, type the + input as `Extract` so an illegal dispatch is + unrepresentable. + +## Verification + +Per commit: + +1. `npm run ci` — lint (incl. the new `max-lines`), typecheck, `dep:check`, format, tokens, + both apps' tests, `ng build --localize` (catches a missing `messages.en.xlf` target for + any moved `$localize` string), audit, backend `dotnet test`, API-client drift. +2. `cd backend && dotnet test` for anything touching the wire. + +End to end, after Phase 0 and Phase 3: + +3. `npm start`, open `http://localhost:4200/dashboard`. All six sections render as before. +4. `?scenario=loading`, `=empty`, `=error`, `=slow` — each section shows its **own** state, + not one page-wide spinner. +5. `?role=admin` — the Beheer section appears; without it, absent. +6. Resume and cancel a concept aanvraag — optimistic update and error path both work. +7. Walk each wizard end to end after Phase 1: submit, retry after a failure, and the + step-boundary transitions. +8. `npm run storybook` and `npm run storybook:behandelportal` — moved and new stories render, + a11y addon clean. + +--- + +## Corrections to the original plan's claims + +Measured against the current tree, not assumed: + +- **Step 2 did not fully land** — the `DashboardViewDto → DashboardView → BigProfile` chain is + intact (Phase 2.2). +- **7 files exceed 250 lines, not 8.** The plan counted by `wc -l`; the rule as specified uses + `skipBlankLines` + `skipComments`. `concepts.page.ts` (472) was missing from its list, but + `behandel-scherm` (232) and `stamdata-table-editor` (236) were on it and already pass. +- **551 ticket refs across 186 files**, not 478/170. **`CD-` does not exist** anywhere. +- **Only 2 of the 4 named adapters make no HTTP call.** `letter-preview.adapter.ts` and + `reveal-bignummer.adapter.ts` both `fetch` for real (deliberately hand-written, not the + generated client). Only `MedewerkerAdapter` and `DigidAdapter` are pure stand-ins, and both + already carry a `// ponytail: fake …` label. Renaming those two is cosmetic — low priority. +- **`registratie-wizard`'s Dutch tags (`Invullen`/`Indienen`/`Ingediend`/`Mislukt`) are + correct**, not drift — CLAUDE.md requires Dutch domain contexts. Do not "fix" them. +- **Storybook titles and header comments agree in all but two cases** across the 41 story + files, so the folder=layer move is mechanical, not a taxonomy debate. The two: + `async.component.ts` has no tag at all, and `breadcrumb.component.ts` says `/** Chrome: */`. + +And four corrections to claims made **earlier in this same investigation**, caught by reading +the consumers and the rule semantics rather than the definitions: + +- **"~62 of the 77 layer-tag comments become deletable" — withdrawn.** Keep all 78 (the count + was also one short). The tag prefixes a real description, so deleting the word leaves the + sentence; and only the 32 in `libs/shared/src/ui/` are made redundant by folders. Elsewhere + the comment is the sole carrier of the layer. +- **`libs/shared/src/ui/` holds 17 atoms, not 16.** + +- **`SaveState` is not redundant.** It has 2 genuine consumers that keep all four cases + (`brief.page.ts:149-160`, `org-template.page.ts:102-113`). Only `ActionState` collapses, so + `action-state.ts` must be **split, not deleted**. +- **"Five ad-hoc boolean/nullable-string pairs" was overstated.** Only + `org-template.store.ts:59` is a genuine illegal-state pair. Three of the five + (`aanvragen.store.ts:28`, `admin-cases.store.ts:26`, `feature-flags.page.ts:96`) are a lone + `signal` with **no boolean partner** — an action error beside a loaded list, + on a different axis from the fetch, correctly modelled as it stands. + +## Deliberately out of scope + +- Renaming the `/dashboard` route to `/overzicht` (needs a redirect; user-visible). +- Splitting `libs/shared/src/layout/` by layer (see 4a for why not). Cost if you disagree: 10 + distinct specifiers, 34 occurrences, 28 files. +- Splitting `herregistratie-wizard`'s three steps for symmetry with the other two wizards. It + is over budget by two lines and its template is ~100 lines. Worth noting as a consistency + follow-up, not as work. +- `behandel-scherm.component.ts` and `stamdata-table-editor.component.ts` — both already pass + the rule. Leave them alone. +- The 7 non-component files over 250 lines (`brief.adapter.ts` 408, `upload.machine.spec.ts` + 364, `brief.store.spec.ts` 357, …). The glob deliberately does not reach them. +- A `scripts/check-layers.sh` asserting folder == tag == story title. Cheap (~15 lines, + modelled on the CIBG-GAP check) but it only catches doc typos. +- The admin `Case`/`Zaak` vocabulary rename — a separate read model. +- NgRx, real auth, runtime DTO validation on every endpoint (CLAUDE.md "out of scope"). diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md new file mode 100644 index 0000000..1cee96b --- /dev/null +++ b/docs/project/readable-codebase/README.md @@ -0,0 +1,180 @@ +# Readable codebase — apply the dashboard pattern to the rest of the app + +The dashboard refactor cut `dashboard.page.ts` from 340 lines to 42. It was built as a +**reference implementation**: prove the pattern on one screen, then hold the rest of the app +to the budget it establishes. + +This arc applies that result. The full design record, with every measurement and every +rejected alternative, is [`PLAN.md`](PLAN.md). Each ticket below is one commit. + +Two findings shape the work: + +1. **Pages are already thin** (17 pages, median 88 lines). The remaining bulk sits one layer + down, in organisms. +2. **The worst problem is not size.** `runIfSubmitting` is copy-pasted into 5 components and + must be called by hand after `dispatch`. Forgetting it fails silently. RD-06 fixes two + user-facing bugs that follow from it. + +## Session protocol + +- **One ticket per session.** Read `CLAUDE.md`, this README, the ticket file, and the + ticket's "Read first" list — then execute. Do not start the next ticket in the same + session. +- The **Decisions** block in each ticket is pre-made. Do not relitigate it. `PLAN.md` records + why, including the alternatives that were rejected and the reasons. +- **Ticket files are written just in time, not all 35 up front.** Writing one means choosing + which decisions and which traps from `PLAN.md` belong in it, so the **supervisor** writes + the ticket file (an Opus-shaped job) immediately before delegating it. The file lands in + that ticket's own commit. Generating all 35 in advance would be speculative — later tickets + are better written once the earlier ones have taught us something. +- **Match the model to the step** (see CLAUDE.md, "Model routing for agent delegation"). + Executing a ticket is written for the `developer` agent (Sonnet). Read-only checks go to + `task-runner` (Haiku). Escalate to `planner` (Opus) only if a Decisions block turns out to + be wrong — in which case stop, and fix `PLAN.md` first. +- A ticket ends **GREEN**, with its acceptance criteria checked, its `Status:` set to `done`, + **and its README row updated — all in the same commit as the code.** Never in a follow-up + commit. This is what makes a restart safe: whatever is committed is done, and whatever is + not is not. +- **`Status: done` carries no commit hash**, because a commit cannot contain its own hash. The + commit is recoverable when you need it: + `git log --oneline --diff-filter=A -- docs/project/readable-codebase/RD-NN-*.md`. +- No ticket leaves a check disabled without an inline reason **and** a reference to the + ticket that removes it. + +## GREEN (global definition of done) + +```bash +npm run ci +``` + +For any ticket whose "`--full`?" column says yes — it touches a story, an `.mdx`, or +`libs/shared/src/ui/**` — additionally: + +```bash +npm run ci --full +``` + +`npm run ci` does **not** build Storybook. Only `--full` does, and a broken `.mdx` story +import is invisible until it runs. RD-27 in particular must not be pushed without it. + +## Recovery after a restart + +A fresh session with no context needs three commands: + +```bash +git log --oneline -8 +grep -rn '^Status:' docs/project/readable-codebase/RD-*.md | grep -v done # next work +npm run ci # is HEAD green? +``` + +Then read `PLAN.md` for the design record, and the first `todo` ticket for the work. + +## The agent loop + +One supervisor session drives it; one `developer` agent executes each ticket: + +1. Read the Order table. Pick the first `todo` whose dependencies are all `done`. +2. Spawn **one** `developer` agent: _"Read `CLAUDE.md`, then + `docs/project/readable-codebase/README.md`, then `RD-NN.md` and its Read-first list. + Execute it. End GREEN. Update the ticket Status and the README row in the same commit as + the code. Do not start another ticket."_ +3. Verify with `task-runner`: `npm run ci`, `git log -1 --stat`, and that `Status:` now reads + `done`. Never mark a ticket done on an agent's report alone — the check is the exit code. +4. Green: next iteration. Red: stop and surface it. + +**Run tickets sequentially.** Three properties make concurrent writes to one branch hostile: +`behaviour-spec.mdx` and `snippets.generated.ts` are regenerated and drift-checked, so two +agents regenerating conflict by construction; every ticket writes this README's Order table; +and the file sets overlap (the three wizards appear in RD-06, RD-07, RD-22 and RD-23). + +Parallel work pays only for genuinely disjoint tickets, in separate git worktrees, merged +deliberately — RD-18/RD-19 (the ticket sweep) and the Phase 5 doc tickets qualify. Cap at +two. Note that RD-15 exists because 22 abandoned agent worktrees are still on disk. + +## Order + +| ID | Ticket | Deps | `--full`? | Status | +| ----- | ---------------------------------------------------------------------------- | ---------- | --------- | ------ | +| RD-01 | Scaffold this backlog: README, PLAN, ticket template | — | | done | +| RD-02 | `max-lines` rule + `reportUnusedDisableDirectives` + 7 disables | 01 | | todo | +| RD-03 | `overzicht` context: page + 2 nav sections, boundary edge, admin-links token | 02 | yes | todo | +| RD-04 | Story titles to `Domein//`; add the missing stories | 03 | yes | todo | +| RD-05 | `createStore` gains the effect map + specs | 02 | | todo | +| RD-06 | **Bug fix:** 2 single-step forms to the effect map + retry affordance | 05 | yes | todo | +| RD-07 | Add `Primary` to the 3 wizard machines + specs | 05 | | todo | +| RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | todo | +| RD-09 | **Docs + generator:** `form-machine.hbs`, ARCHITECTURE, fp-tea, skill | 08 | | todo | +| RD-10 | `WizardStatus` to a payload-carrying `WizardPhase` | 08 | yes | todo | +| RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | todo | +| RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | todo | +| RD-13 | Same for org-template, folding `pendingPublish` in | 12 | | todo | +| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | todo | +| RD-15 | Remove 22 abandoned agent worktrees (4.7 GB) | 01 | | todo | +| RD-16 | `parseDashboardView` returns `BigProfile`; delete `DashboardView` | 01 | | todo | +| RD-17 | `successOf`/`successOr` sweep — 10 sites, 8 files | 01 | | todo | +| RD-18 | Ticket-reference sweep, frontend — 181 refs, 100 files | 01 | | todo | +| RD-19 | Ticket-reference sweep, backend — 370 refs, 86 files | 01 | | todo | +| RD-20 | `wizard-errors.ts` + spec, adopted by all 3 wizards | 02 | | todo | +| RD-21 | `rich-text-dom.ts` helpers + spec cases | 02 | yes | todo | +| RD-22 | `intake-wizard` to 3 step components | 08, 20 | yes | todo | +| RD-23 | `registratie-wizard` to 3 steps + the upload-controller move | 08, 20 | yes | todo | +| RD-24 | `concepts.page` to 6 sections + `concept-card` + globals + code tokens | 02 | yes | todo | +| RD-25 | `org-template-editor` to `sample-letter.ts` + labels + 2 children | 02 | yes | todo | +| RD-26 | `letter-canvas`: inline the labels + `letter-line`; keep one disable | 02 | yes | todo | +| RD-27 | **The layer move:** 33 `git mv` + 28 specifiers + 8 MDX imports | 21 | yes | todo | +| RD-28 | Layer-tag fixes + the `libs/beheer` title rule | 27 | | todo | +| RD-29 | The 3 atomic-ladder rules in dependency-cruiser | 27 | | todo | +| RD-30 | Archive the finished backlogs (16,300 lines) + an archive README | 01 | | todo | +| RD-31 | `ARCHITECTURE.md` section 6a: symbols not lines, 2 dead paths, new names | 03, 08, 16 | | todo | +| RD-32 | `fp-tea-atomic-design.md`: 11 broken paths + the broken anchor | 27 | | todo | +| RD-33 | CLAUDE.md + `atomic-design.mdx` + the `ui-component` skill | 03, 27, 29 | yes | todo | +| RD-34 | _(optional)_ `NO_SUBORGS`/`NO_TABLES` become `RemoteData.Empty` | 11 | | todo | +| RD-35 | _(optional, last, alone)_ upload `type:` discriminant to `tag:` | 27 | | todo | + +The ID order already respects every dependency, so it is the recommended running order. + +**Independent tickets.** RD-15 through RD-19 depend only on RD-01. Pull them forward to fill +a short session. Take RD-15 early: it makes every later repository search faster. + +**Two ordering traps the table encodes.** RD-01 must precede RD-30, because RD-01 copies its +ticket template out of the directory that RD-30 archives. And four tickets edit the same two +documents in different sections — RD-09 rewrites the submit-idiom teaching, while RD-31 and +RD-32 fix section 6a and the stale paths. Sequential is fine. Never put those pairs in +parallel worktrees. + +## Ticket template + +```markdown +# RD-NN — Title + +Status: todo | in-progress | done +Source: PLAN.md section + +## Why + +## Read first + +## Decisions (pre-made, don't relitigate) + +## Files + +## Steps + +## Acceptance criteria + +## Verification + +## Out of scope + +## Risks +``` + +Three rules when you write a ticket file, because the agent reads its ticket and not +`PLAN.md`: + +1. **Copy the decision, never a pointer to it.** The verdict goes in the Decisions block, + verbatim. +2. **Inline the traps that apply to that ticket.** A trap recorded only in `PLAN.md`'s global + Risks section is a trap that fires. +3. **State acceptance as a command, not a sentence.** "Lands about 230 lines" is a design + estimate and nothing can check it. `npm run lint` has an exit code.