ehoandClaude Opus 5
d9c56b0c24
scripts/ci-local.sh gated its two storybook + axe steps on
`[[ "${1:-}" == "--full" ]]`, so the flag had to arrive as a positional
argument. CLAUDE.md documents `npm run ci --full`, but npm parses that
flag itself and exports `npm_config_full=true` instead of passing it
through. Proven with `npm run env --full`.
The documented command therefore skipped both steps and still printed
"local CI passed" — a gate that reported success without running. Anyone
following CLAUDE.md has been skipping storybook-a11y.
Accept either form. Fixing the script makes every existing document
correct, instead of changing four of them and re-teaching the reader.
Found while verifying RD-06, which adds stories and so depends on those
steps actually running.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
906 lines
60 KiB
Markdown
906 lines
60 KiB
Markdown
# 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-<slug>.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/<Context>/<Name>`; 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/<Name>`; CLAUDE.md specifies
|
||
`Domein/<Context>/<Name>`, "full stop", and all 41 other story files comply. Retitle to
|
||
`Domein/Registratie/<Name>` for the four that stay and `Domein/Overzicht/<Name>` 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, Msg> = Model extends { tag: string }
|
||
? {
|
||
[K in Model['tag']]?: (
|
||
state: Extract<Model, { tag: K }>,
|
||
store: Store<Model, Msg>,
|
||
) => unknown;
|
||
}
|
||
: never;
|
||
|
||
export function createStore<Model, Msg>(
|
||
init: Model,
|
||
update: (model: Model, msg: Msg) => Model,
|
||
effects?: StoreEffects<Model, Msg>,
|
||
): Store<Model, Msg>;
|
||
```
|
||
|
||
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<BesluitState, BesluitMsg>(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<E,T>`** 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<E,T>` | `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
|
||
`<app-async>` 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 `<app-async>` 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-<hex>` 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-<hex>` 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<Err, DashboardView>`,
|
||
line 45 re-maps to `RemoteData<Err, BigProfile>`. 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 `<app-async>` 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,
|
||
`<form>`, navigation, submitting/submitted/failed, a11y focus. The steps slot into its
|
||
existing default `<ng-content>`. **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 `<x id="min" equiv-text="MARGIN_MIN_MM"/>`; 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/<sub>/` 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<n>` 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 `<app-async [data]="store.profile()">` on the dashboard page;
|
||
after the split that markup lives in `mijn-registratie.section.ts`.
|
||
- It states the boundary yields `RemoteData<DashboardView>` — 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/<name>/<name>.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<RegistratieMsg, {tag: 'SetField' | …}>` 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: */`.
|
||
|
||
**A live CI-gate defect, found while executing RD-06 (now fixed).**
|
||
`scripts/ci-local.sh` gated its storybook + axe steps on `[[ "${1:-}" == "--full" ]]`, but
|
||
CLAUDE.md documents `npm run ci --full` — and npm parses that flag itself, exporting
|
||
`npm_config_full=true` instead of passing `--full` through as `$1`. Proven with
|
||
`npm run env --full`. So the documented command **skipped both steps and still printed
|
||
"local CI passed"**: a gate reporting success without running. The script now accepts either
|
||
form, which makes every existing doc correct rather than requiring them all to change. This
|
||
matters directly for RD-27, whose highest risk is a broken `.mdx` story import that **only**
|
||
`build-storybook` catches.
|
||
|
||
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<string | null>` 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").
|