ehoandClaude Sonnet 5
b6bd7eea5a
Add three dependency-cruiser rules that forbid upward edges inside libs/shared/src/ui: atoms-compose-nothing-above, molecules-below-organisms, and design-system-not-layout. RD-27 made the atomic ladder expressible by moving components into atoms/molecules/organisms folders; these rules make it enforced, so an upward import now fails dep:check instead of shipping. No pathNot exemption for specs and stories: measured, zero upward edges exist anywhere in libs/shared/src/ui today, in production code, specs, and stories alike, so the exemption PLAN proposed has nothing to protect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
329 lines
20 KiB
Markdown
329 lines
20 KiB
Markdown
# 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 | | done |
|
|
| RD-03 | `overzicht` context: page + 2 nav sections, boundary edge, admin-links token | 02 | yes | done |
|
|
| RD-04 | Story titles to `Domein/<Context>/<Name>`; add the missing stories | 03 | yes | done |
|
|
| RD-05 | `createStore` gains the effect map + specs | 02 | | done |
|
|
| RD-06 | **Bug fix:** 2 single-step forms to the effect map + retry affordance | 05 | yes | done |
|
|
| RD-07 | Add `Primary` to the 3 wizard machines + specs | 05 | | done |
|
|
| RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | done |
|
|
| RD-09 | Teach the effect map: ARCHITECTURE §2d + fp-tea (2 docs, no generator) | 08 | | done |
|
|
| RD-10 | `WizardStatus` to a payload-carrying `WizardPhase` | 08 | yes | done |
|
|
| RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | done |
|
|
| RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | done |
|
|
| RD-13 | Same for org-template, folding `pendingPublish` in | 12 | | done |
|
|
| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | done |
|
|
| RD-15 | Remove 22 abandoned agent worktrees (4.7 GB) | 01 | | done |
|
|
| RD-16 | ~~`parseDashboardView` returns `BigProfile`~~ — DROPPED, see PLAN.md 2.2 | 01 | | n/a |
|
|
| RD-17 | `successOf`/`successOr` sweep — 10 sites, 8 files | 01 | | done |
|
|
| RD-18 | Ticket-reference sweep, frontend — 181 refs, 100 files | 01 | yes | done |
|
|
| RD-19 | Ticket-reference sweep, backend — 370 refs, 86 files | 01 | | done |
|
|
| RD-20 | `wizard-errors.ts` + spec, adopted by all 3 wizards | 02 | | done |
|
|
| RD-21 | `rich-text-dom.ts` helpers + spec cases | 02 | yes | done |
|
|
| RD-22 | `intake-wizard` to 3 step components | 08, 20 | yes | done |
|
|
| RD-23 | `registratie-wizard` to 3 steps + the upload-controller move | 08, 20 | yes | done |
|
|
| RD-24 | `concepts.page` to 6 sections + `concept-card` + globals + code tokens | 02 | yes | done |
|
|
| RD-25 | `org-template-editor` to `sample-letter.ts` + labels + 2 children | 02 | yes | done |
|
|
| RD-26 | `letter-canvas`: inline the labels + `letter-line`; keep one disable | 02 | yes | done |
|
|
| RD-27 | **The layer move:** 33 `git mv` + 28 specifiers + 8 MDX imports | 21 | yes | done |
|
|
| RD-28 | Layer-tag fixes + the `libs/beheer` title rule | 27 | yes | done |
|
|
| RD-29 | The 3 atomic-ladder rules in dependency-cruiser | 27 | | done |
|
|
| 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 |
|
|
| RD-36 | `ui/dashboard/` → `ui/overzicht-secties/` + 2 stale `dashboard.page` paths | 04 | yes | 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.
|
|
|
|
**RD-36 is last by number, not by dependency.** It needs only RD-04, it is 8 `git mv`s plus
|
|
four import lines, and it collides with nothing else in the table — RD-27's move is confined to
|
|
`libs/shared/src/ui/`. Pull it forward into any short session. It is numbered last only because
|
|
it was added after RD-04 shipped.
|
|
|
|
**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 <n>
|
|
|
|
## 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.
|
|
4. **Run every acceptance command against the tree before you hand the ticket over.** A
|
|
command that cannot pass is worse than no command: the agent either wastes a cycle or,
|
|
worse, "fixes" correct code to satisfy it. Seven real misses so far, all in tickets written
|
|
by the supervisor:
|
|
- RD-06 grepped only `runIfSubmitting`, missing that one wizard spells it `runIfIndienen`.
|
|
- RD-08 grepped bare `onPrimary\|onRetry`, which can never return nothing — an unrelated
|
|
`uploadCtl.onRetry` exists in `upload-controller.ts`.
|
|
- RD-08 said "no machine changes" while also requiring a repo-wide grep to come back
|
|
clean, which forced comment edits in three machines. The two instructions contradicted
|
|
each other. RD-23 repeated it exactly: its Decisions block mandated the line
|
|
`dispatch: (msg) => this.uploadMsg.emit(msg)` — the upload controller's own property name —
|
|
while its acceptance demanded zero occurrences of `dispatch` in the step files. **Grep the
|
|
text your own mandated snippet contains, and you have written a check that cannot pass.**
|
|
Anchor on what you actually forbid: here, `this.dispatch` or `store.dispatch`, not the bare
|
|
word. RD-25 made it a third time, and the clearest one: its decision 4 said "the parent
|
|
keeps seven outputs" while decision 5, four lines below, said all eleven still exist and
|
|
are re-emitted from children. Seven was the count of outputs the parent _raises_; eleven is
|
|
the count it _declares_. The acceptance line copied the wrong one, and satisfying it would
|
|
have broken `org-template.page.ts`, which binds all eleven.
|
|
|
|
**The pattern in all three: a decision describes a design in one vocabulary, and the
|
|
acceptance line counts something else that happens to share a word.** Before writing a
|
|
number, say out loud what the command counts — declarations, call sites, or matching lines
|
|
— and check that the decisions use that same meaning.
|
|
|
|
RD-27 then did it a fourth time, in the very next ticket written after that rule was
|
|
recorded. Its decision 3 said the `@shared/ui/` occurrence count "must not change" at 200,
|
|
while decision 4, nine lines below, mandated converting five relative imports into
|
|
`@shared/ui/` aliases — five new occurrences by construction. The answer is 205.
|
|
**When two decisions both touch the thing you are counting, add them up before you write
|
|
the number.** Naming a habit does not retire it.
|
|
|
|
- RD-09 grepped `docs/ apps/ libs/ .claude/`, which also matched this backlog's own ticket
|
|
files (they name the deleted method as the history of `done` work) and 22 gitignored
|
|
abandoned worktrees. Satisfying it literally would have corrupted completed-ticket
|
|
history.
|
|
- RD-11 asserted `git grep "machineRemoteData\|LoadLifecycle"` returns nothing, but the
|
|
replacement it mandates is named **`fromLoadLifecycle`** — which contains the old name as
|
|
a substring. The check can never pass. **When the new name contains the old one, anchor
|
|
on word boundaries**: `git grep -w machineRemoteData` and
|
|
`git grep -nE "(^|[^a-zA-Z])LoadLifecycle\b"`.
|
|
|
|
- RD-19 asserted the backend sweep must reach **0** references, after RD-18 had established
|
|
that a reference inside a path to a document that still exists is a pointer, not
|
|
provenance, and exempted one. The supervisor did not run that check against `backend/`.
|
|
Two such paths existed. The target of 0 gave the agent no way to keep them, so it
|
|
described the two documents in prose instead — losing a path that resolved. Fixed
|
|
afterwards; the target is 2. **When a ticket repeats an earlier sweep, re-run the earlier
|
|
sweep's exemption check against the new scope.** An exemption is a property of the
|
|
content, not of the directory that happened to hold it first.
|
|
|
|
- RD-20 required `gen:behaviour-spec` in its own Steps list, then asserted
|
|
`git grep -l "toWizardErrors" -- apps libs | wc -l` would be **5**: the helper, its spec
|
|
and the three call sites. The honest answer is 6. The generator publishes every `describe`
|
|
title, so naming the spec after the function it tests — which is house style — puts the
|
|
name into `behaviour-spec.mdx` as well. The agent correctly refused to rename the describe
|
|
block to satisfy the number. **A name that appears in a spec title also appears in the
|
|
generated document.** Count that file, or use `>=` as RD-17 did.
|
|
|
|
Four habits that prevent all five:
|
|
|
|
- **Use `git grep`, not `grep -r`.** It searches tracked files only, so untracked and
|
|
gitignored paths never pollute the result. Measured on this repo: `grep -r` finds 132
|
|
hits under `.claude/`, `git grep` finds 0. This matters most for RD-17, RD-18 and RD-19,
|
|
which are repo-wide sweeps.
|
|
- **Anchor on a declaration** (`^ onRetry\(\)`), not on a name that may legitimately
|
|
appear elsewhere.
|
|
- **`git grep -c` counts matching LINES, not occurrences.** RD-14 asserted
|
|
`git grep -c "'Idle'\|'Saving'\|'Saved'\|'Error'"` would be `>= 4`, but all four tags
|
|
live on one line of a single-line type declaration, so the honest answer is `1`. The
|
|
agent correctly refused to reformat the type across four lines to satisfy the number.
|
|
When you want occurrences, use `grep -o … | wc -l`; when a line count is what you mean,
|
|
say so. **A symbol you import and then use is two lines, never one.** RD-23 repeated the
|
|
mistake in the other direction, asserting `git grep -c "createUploadController"` would be
|
|
1 in the file that both imports and calls it. The only way to reach 1 is an import alias
|
|
that exists solely to satisfy the check.
|
|
- **Scope every acceptance command to the ticket's Files list, never to a parent
|
|
directory.** This is the habit most often broken, including by the supervisor in RD-12:
|
|
the check `git grep "ActionState" -- apps/ssp/src/app/brief` cannot pass, because
|
|
`org-template.store.ts` lives in that directory and is deliberately out of scope until
|
|
RD-13. Name the files. If a command reaches a file the ticket says not to touch, one of
|
|
the two is wrong.
|
|
- **Prefer a number over a prohibition for anything that must not change.** "Do not rename
|
|
`BriefStatus`" invites reasoning around it; "this count must still be 54, and if it moves,
|
|
revert rather than adjust the number" does not. RD-11 renamed tags across 19 files with a
|
|
wire contract in the same file — on the same line in one place — and the count held.
|
|
|
|
## Troubleshooting
|
|
|
|
**`dotnet test` fails with `SQLite Error 1: 'no such table: <X>'`.** Stale, gitignored
|
|
`bigregister.db` artifacts from an old build. Found during RD-11, where a 0-byte file dated
|
|
months earlier failed 6 backend tests on an otherwise clean tree. Delete all three and re-run:
|
|
|
|
```bash
|
|
rm -f backend/bigregister.db backend/src/BigRegister.Api/bigregister.db \
|
|
backend/tests/BigRegister.Tests/bin/Debug/net10.0/bigregister.db
|
|
```
|
|
|
|
These are build artifacts, not fixtures — removing them is always safe.
|
|
|
|
A second cause reaches the same error: **two agents running `dotnet test` at the same time in
|
|
one checkout.** They share those files, so one run truncates the other's database. RD-17 hit
|
|
this. The fix is the same three deletions, but the cause is concurrency — see the next entry.
|
|
|
|
**`dotnet test` reports one failure in `OpenZaakIntegrationTests`.** That test carries
|
|
`[Trait("Category", "Integration")]` and needs the live OpenZaak harness. Both `ci.yml` and
|
|
`scripts/ci-local.sh` filter it out. A bare `dotnet test` does not. Run `npm run ci` — a bare
|
|
`dotnet test` is the wrong command, not a red build.
|
|
|
|
**An unexplained dirty tree may be a running agent, not an interrupted session.** `/clear`
|
|
ends the supervisor's context. It does **not** stop the `developer` agent that session
|
|
spawned. That agent keeps writing to the same branch. In RD-17 a fresh supervisor read the
|
|
half-finished tree as an interrupted session, wrote a "continue from here" ticket, and
|
|
dispatched a second agent onto the work the first was still doing. Both agents then ran the
|
|
backend tests together, which produced the stale-database failure above.
|
|
|
|
Nothing was lost, because the second agent committed first and the first agent checked before
|
|
committing rather than committing blindly. That was luck. Before you dispatch, confirm no
|
|
agent is live:
|
|
|
|
```bash
|
|
git status --short # modified files nobody in THIS session touched = suspect
|
|
git log --oneline -1 # did an agent already commit the ticket?
|
|
```
|
|
|
|
If the tree is dirty and this session did not dirty it, find the agent before you write a
|
|
ticket around the evidence it leaves.
|
|
|
|
**The mirror-image mistake, made during RD-24: the supervisor became the second writer.** A
|
|
task notification fires every time an agent stops with **no live children — including a pause
|
|
mid-task**. RD-24's agent paused, said it was waiting on a check, and notified. The supervisor
|
|
read that as an abandoned task, took over the working tree, ran the gate and edited the ticket
|
|
`Status:` — while the agent was still running. The agent then resumed, correctly detected a
|
|
second process writing its tree, and refused to commit.
|
|
|
|
Nothing was lost, because the agent stopped instead of committing. Two rules follow:
|
|
|
|
- **A notification is not proof the work is finished.** A finished agent hands back a report
|
|
with acceptance numbers. "I am waiting for X" is a pause.
|
|
- **Before taking over an agent's tree, confirm the agent is gone**: `ps aux | grep "npm run"`,
|
|
and check whether its own result has arrived. Taking over is a real option — RD-24 was
|
|
finished that way — but it must be a decision, not an assumption.
|