From 664a43bf2d05df38c258337b35a4bad227614933 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Wed, 26 Aug 2026 16:44:32 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20refactoring-backlog=20workspace=20?= =?UTF-8?q?=E2=80=94=20baseline=20+=203=20Phase=201=20agents?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs the multi-agent refactoring-backlog pipeline in docs/project/ refactor-backlog-setup/ up to and including three of the seven Phase 1 agents. 00-baseline.md establishes the metrics every later agent must cite, using only tooling already in the repo (vitest lcov, coverlet cobertura, ESLint's core `complexity` rule at threshold 0 for a full distribution, depcruise --metrics). Duplication and C# complexity had no tooling, so tools/baseline-scan.mjs adds a deterministic ~200-line text scan rather than a new dependency; the approximations are labelled as such. Headline: FE 75.1% line coverage but only over the 98 of 220 source files a spec loads; BE 97.6% line / 79.6% branch; 0 layering violations; 7.1% duplication; 25 of 2085 TS functions over CC 10. Then 02-testability, 04-cqrs-light and 06-adr-conformance (27 findings). 01/03/05 were skipped deliberately — the baseline shows little for them to find; 07 (BIO2) and 08 (consolidation) are still open. Each agent corrected a baseline observation of mine, and in every case the error was in something derived rather than measured: - BL-007 counted ~13 adapter "mutations" from the `runSubmit` helper name; 5 of those call sites are reads. It also missed 3 real mutations that reach the raw ApiClient and never return a Result. - BL-002 diagnosed the 100%-duplicated auth folders as ADR-0002's divergence prediction failing. It never had a chance to fail: §3's `Principal` union was never built. - BL-004 named libs/shared/domain and libs/beheer/contracts as coverage gaps; both are pure type declarations where 0% is unimprovable. All three corrections are recorded inline in 00-baseline.md §10, so agent 08 does not inherit the bad numbers. .prettierignore excludes the agent prompt directories — reflowing their markdown would edit the prompt text itself. Co-Authored-By: Claude Opus 5 --- .prettierignore | 4 + docs/project/refactor-backlog-setup/README.md | 63 ++ .../agents/00-baseline.prompt.md | 22 + .../agents/01-readability.prompt.md | 15 + .../agents/02-testability.prompt.md | 14 + .../agents/03-ddd-hexagonal.prompt.md | 16 + .../agents/04-cqrs-light.prompt.md | 15 + .../agents/05-bdd.prompt.md | 14 + .../agents/06-adr-conformance.prompt.md | 16 + .../agents/07-bio2-compliance.prompt.md | 24 + .../agents/08-consolidation.prompt.md | 35 + .../agents/09-implementation.prompt.md | 33 + .../agents/_persistence-protocol.md | 25 + .../refactor-backlog/00-baseline.md | 547 +++++++++++++++ .../refactor-backlog/01-readability.md | 9 + .../refactor-backlog/02-testability.md | 641 ++++++++++++++++++ .../refactor-backlog/03-ddd-hexagonal.md | 9 + .../refactor-backlog/04-cqrs-light.md | 543 +++++++++++++++ .../refactor-backlog/05-bdd.md | 9 + .../refactor-backlog/06-adr-conformance.md | 582 ++++++++++++++++ .../refactor-backlog/07-bio2-compliance.md | 9 + .../refactor-backlog/99-backlog.md | 0 .../refactor-backlog/_status.md | 13 + .../final-prompts/00-baseline.prompt.md | 46 ++ .../final-prompts/01-readability.prompt.md | 39 ++ .../final-prompts/02-testability.prompt.md | 38 ++ .../final-prompts/03-ddd-hexagonal.prompt.md | 40 ++ .../final-prompts/04-cqrs-light.prompt.md | 39 ++ .../final-prompts/05-bdd.prompt.md | 38 ++ .../06-adr-conformance.prompt.md | 40 ++ .../07-bio2-compliance.prompt.md | 48 ++ .../final-prompts/08-consolidation.prompt.md | 59 ++ .../final-prompts/09-implementation.prompt.md | 57 ++ .../implementation/adr-c-006.md | 58 ++ .../refactor-backlog/tools/baseline-scan.mjs | 255 +++++++ docs/project/refactor-backlog-setup/setup.sh | 73 ++ 36 files changed, 3488 insertions(+) create mode 100644 docs/project/refactor-backlog-setup/README.md create mode 100644 docs/project/refactor-backlog-setup/agents/00-baseline.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/01-readability.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/02-testability.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/03-ddd-hexagonal.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/04-cqrs-light.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/05-bdd.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/06-adr-conformance.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/07-bio2-compliance.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/08-consolidation.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/09-implementation.prompt.md create mode 100644 docs/project/refactor-backlog-setup/agents/_persistence-protocol.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/00-baseline.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/01-readability.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/02-testability.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/03-ddd-hexagonal.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/04-cqrs-light.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/05-bdd.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/06-adr-conformance.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/_status.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/00-baseline.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/01-readability.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/02-testability.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/03-ddd-hexagonal.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/04-cqrs-light.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/05-bdd.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/06-adr-conformance.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/07-bio2-compliance.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/08-consolidation.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/09-implementation.prompt.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-006.md create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/tools/baseline-scan.mjs create mode 100755 docs/project/refactor-backlog-setup/setup.sh diff --git a/.prettierignore b/.prettierignore index deed558..2e7e30e 100644 --- a/.prettierignore +++ b/.prettierignore @@ -21,3 +21,7 @@ plop-templates/ # Backend is formatted by `dotnet format`, not prettier backend/ + +# Agent prompts — their exact wording is the input, reflowing markdown edits the prompt +docs/project/refactor-backlog-setup/agents/ +docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/ diff --git a/docs/project/refactor-backlog-setup/README.md b/docs/project/refactor-backlog-setup/README.md new file mode 100644 index 0000000..e2b432a --- /dev/null +++ b/docs/project/refactor-backlog-setup/README.md @@ -0,0 +1,63 @@ +# Refactoring backlog — automated setup + +## What's in this package + +``` +refactor-backlog-setup/ + setup.sh ← run this once, from the root of the target repo + agents/ ← source prompts (edit these if you need to tweak + scope/wording before running setup.sh) + _persistence-protocol.md + 00-baseline.prompt.md + 01-readability.prompt.md + 02-testability.prompt.md + 03-ddd-hexagonal.prompt.md + 04-cqrs-light.prompt.md + 05-bdd.prompt.md + 06-adr-conformance.prompt.md + 07-bio2-compliance.prompt.md + 08-consolidation.prompt.md + 09-implementation.prompt.md (template — one TICKET-ID per Phase 3 dispatch) +``` + +## Usage + +1. Copy this `refactor-backlog-setup/` folder into the root of the target repo + (or reference it via a relative path). +2. Edit anything in `agents/` if scope/exclusions need repo-specific detail + (e.g. exact module paths, ADR folder location) — the prompts currently use + the defaults agreed in the design conversation. +3. Run: + ``` + bash refactor-backlog-setup/setup.sh + ``` + This creates `./refactor-backlog/` with: + - `_status.md` initialized, all agents `not_started` + - `00-baseline.md` through `07-bio2-compliance.md` initialized with headers + - `99-backlog.md` empty, ready for Consolidation + - `implementation/` folder for Phase 3 notes + - `final-prompts/` — every agent prompt with the persistence protocol + already merged in. **These are the exact prompts to dispatch — no manual + copy-paste needed.** + +## Dispatch order + +1. Dispatch `final-prompts/00-baseline.prompt.md` (Opus). Wait for + `_status.md` → baseline: complete. +2. Dispatch the 7 Phase 1 prompts in parallel (Opus): `01` through `07`. + Each checks its own dependency in `_status.md` before starting. +3. Once all 7 show `complete`, dispatch `final-prompts/08-consolidation.prompt.md` + (Opus). It writes `99-backlog.md` and halts for human approval — check the + file for any `ADR-fix` or BIO2-flagged tickets before proceeding. +4. For each approved ticket, copy `final-prompts/09-implementation.prompt.md`, + fill in `TICKET-ID:`, dispatch (Sonnet). Run tickets in parallel within a CD + batch, sequential across batches, per the `Depends on` column in + `99-backlog.md`. + +## Re-running / resuming + +Safe to re-run `setup.sh` only on a fresh workspace — it does not check for an +existing `./refactor-backlog/` and will overwrite `_status.md` and the phase +output files. If a run is already in progress, don't re-run `setup.sh`; just +re-dispatch the relevant `final-prompts/*.prompt.md` — each agent reads +`_status.md` and its own output file first and resumes from where it left off. diff --git a/docs/project/refactor-backlog-setup/agents/00-baseline.prompt.md b/docs/project/refactor-backlog-setup/agents/00-baseline.prompt.md new file mode 100644 index 0000000..73ea9c2 --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/00-baseline.prompt.md @@ -0,0 +1,22 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/00-baseline.md +DEPENDS ON: none + +--- + +[Insert contents of _persistence-protocol.md here] + +ROLE: Metrics Baseline Agent + +Before any refactoring suggestions, establish a baseline for the scoped codebase: +- Test coverage (line/branch) per module, .NET and Angular separately. +- Cyclomatic complexity per method/function (flag >10). +- Duplication percentage (tool-based, e.g. jscpd/SonarQube if configured). +- Dependency graph / layering violations (existing static analysis if present). +- Count and location of existing CQRS-light and hexagonal architecture patterns + already in use (so later agents compare against actual current state, not + assumed absence). + +Output: a metrics table per module, plus a short list of modules ranked +worst-to-best on each metric. This file is fixed input to every Phase 1 agent — +no agent may propose a change without citing which baseline metric it improves. diff --git a/docs/project/refactor-backlog-setup/agents/01-readability.prompt.md b/docs/project/refactor-backlog-setup/agents/01-readability.prompt.md new file mode 100644 index 0000000..cc9f23d --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/01-readability.prompt.md @@ -0,0 +1,15 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/01-readability.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: Readability Agent + +Junior = fluency in language constructs, not domain knowledge. Assume familiarity +with generics, async/await, LINQ, DI, RxJS operators, TS type system — do NOT flag +idiomatic use of these as "unreadable". Flag only: unclear naming, methods/components +exceeding [N] lines, nesting >3 levels, magic values, misleading types, missing guard +clauses. Cite baseline complexity score per finding. diff --git a/docs/project/refactor-backlog-setup/agents/02-testability.prompt.md b/docs/project/refactor-backlog-setup/agents/02-testability.prompt.md new file mode 100644 index 0000000..8d381cf --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/02-testability.prompt.md @@ -0,0 +1,14 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/02-testability.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: Testability Agent + +Flag constructs that block unit testing without excessive mocking: static/singleton +dependencies, hidden I/O, constructors doing work, mixed pure/impure logic. Cite +baseline coverage gap per finding. Propose the minimal seam needed (interface +extraction, pure function split) — not a rewrite. diff --git a/docs/project/refactor-backlog-setup/agents/03-ddd-hexagonal.prompt.md b/docs/project/refactor-backlog-setup/agents/03-ddd-hexagonal.prompt.md new file mode 100644 index 0000000..7e050f7 --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/03-ddd-hexagonal.prompt.md @@ -0,0 +1,16 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/03-ddd-hexagonal.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: DDD/Hexagonal Agent + +Target architecture: hexagonal (ports/adapters), already partially present in the +codebase per baseline.md — treat that as the pattern to extend, not reinvent. Do NOT +introduce hexagonal structure into modules where it is absent; only propose closing +gaps where the pattern is already started. Flag anemic domain models, domain logic +leaked into controllers/services/components, primitive obsession, missing ubiquitous +language. diff --git a/docs/project/refactor-backlog-setup/agents/04-cqrs-light.prompt.md b/docs/project/refactor-backlog-setup/agents/04-cqrs-light.prompt.md new file mode 100644 index 0000000..850d4ac --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/04-cqrs-light.prompt.md @@ -0,0 +1,15 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/04-cqrs-light.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: CQRS-light Agent + +Target: command/query separation at the application-service level (not event +sourcing or separate read models unless already present per baseline.md). Only +extend existing CQRS-light patterns — do not introduce the pattern into modules +where it's absent. Identify handlers/services mixing reads and writes within +modules that already show the pattern elsewhere. diff --git a/docs/project/refactor-backlog-setup/agents/05-bdd.prompt.md b/docs/project/refactor-backlog-setup/agents/05-bdd.prompt.md new file mode 100644 index 0000000..67859a7 --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/05-bdd.prompt.md @@ -0,0 +1,14 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/05-bdd.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: BDD Agent + +Check whether existing tests express behavior in domain/business language mapped +to acceptance criteria, or only technical steps. Flag test names/structure gaps. +Do not propose new BDD tooling if none is present — flag as a separate structural +item instead, not a per-module ticket. diff --git a/docs/project/refactor-backlog-setup/agents/06-adr-conformance.prompt.md b/docs/project/refactor-backlog-setup/agents/06-adr-conformance.prompt.md new file mode 100644 index 0000000..13e43e8 --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/06-adr-conformance.prompt.md @@ -0,0 +1,16 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/06-adr-conformance.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: ADR-Conformance Agent + +Read all ADRs/docs in the repo. Compare code against each. Two outcomes per +deviation: +(a) code violates a correct ADR → refactoring ticket, cite ADR. +(b) ADR itself appears outdated/wrong given current code or constraints → propose + an ADR amendment as a separate ticket type ("ADR-fix"), with rationale — not + a code ticket. diff --git a/docs/project/refactor-backlog-setup/agents/07-bio2-compliance.prompt.md b/docs/project/refactor-backlog-setup/agents/07-bio2-compliance.prompt.md new file mode 100644 index 0000000..1068022 --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/07-bio2-compliance.prompt.md @@ -0,0 +1,24 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/07-bio2-compliance.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +[Insert contents of _persistence-protocol.md here] + +AGENT: BIO2/Compliance Agent + +No explicit control list supplied — using the following BIO2/ISO 27002:2022 +controls, selected for privacy and security relevance. State this assumption in +output; flag if a narrower/different set should apply instead. + +- Access control (9.1, 9.2, 9.4): authorization checks, RBAC, least privilege. +- Logging & monitoring (8.15, 8.16): audit trails, esp. BIG-register/DUO data access. +- Data classification & handling (5.12, 5.13): BSN, health data, AVG-sensitive fields. +- Cryptography (8.24): encryption at rest/in transit. +- Secure development (8.25, 8.28, 8.29): secure coding, review, security testing gates. +- Change control (8.32): deployment register / change approval exceptions. +- Input validation (8.26): boundary validation on public-facing forms/APIs. + +Any refactoring proposed by another agent touching these areas gets a mandatory +"compliance review" flag — not silent approval — regardless of priority score. diff --git a/docs/project/refactor-backlog-setup/agents/08-consolidation.prompt.md b/docs/project/refactor-backlog-setup/agents/08-consolidation.prompt.md new file mode 100644 index 0000000..1319d8e --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/08-consolidation.prompt.md @@ -0,0 +1,35 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/99-backlog.md +DEPENDS ON: 01-readability.md through 07-bio2-compliance.md (all complete) + +--- + +[Insert contents of _persistence-protocol.md here — file-level: re-run only if a +Phase 1 file changed since last run] + +ROLE: Consolidation & CD-Sequencing Agent + +Input: all Phase 1 files (01–07) + 00-baseline.md. + +1. Deduplicate overlapping findings across agents — merge into one ticket, list all + contributing reasons/agents. +2. Score priority: + P1 = violates a correct ADR, blocks testability, or is a BIO2 compliance risk. + P2 = significant maintainability cost, moderate effort. + P3 = low urgency. +3. Sequence for continuous delivery: every ticket must be independently deployable + without a big-bang release. Reject/split any ticket that can't ship alone — + decompose into a dependency chain of smaller tickets. +4. Any ticket touched by the BIO2 agent requires compliance sign-off before merge, + regardless of priority score — mark explicitly. +5. Output final table: + +| ID | Module | Category | Description | Baseline metric improved | Effort (S/M/L) | +Risk | Priority | CD batch # | Depends on | Status | + +6. Separately list "ADR-fix" tickets — require human/architect approval before any + dependent code ticket proceeds. + +HALT CONDITION: after writing 99-backlog.md, stop and report to the human for +approval before any Implementation Agent (Phase 3) starts — even if no tickets +carry a compliance or ADR-fix flag. diff --git a/docs/project/refactor-backlog-setup/agents/09-implementation.prompt.md b/docs/project/refactor-backlog-setup/agents/09-implementation.prompt.md new file mode 100644 index 0000000..ae584ec --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/09-implementation.prompt.md @@ -0,0 +1,33 @@ +MODEL: Sonnet +OUTPUT: status update in /refactor-backlog/99-backlog.md + + /refactor-backlog/implementation/[ticket-id].md +DEPENDS ON: ticket status = not_started, no unresolved compliance/ADR-fix flag, + all tickets in "Depends on" column = implemented or needs_review + +--- + +[Insert contents of _persistence-protocol.md here — scoped to one ticket, not a +module list] + +AGENT: Implementation Agent + +Input: one ticket from 99-backlog.md (fill in TICKET-ID below), the Phase 1 +file(s) that produced it, and 00-baseline.md. + +TICKET-ID: [fill in before dispatching this agent] + +Scope discipline: +- Implement exactly the change described in the ticket. No scope expansion, no + incidental fixes. +- If the ticket is ambiguous or underspecified for implementation, do not guess — + write a blocker note to the ticket's status and stop. +- Do not modify architecture/pattern decisions (hexagonal boundaries, CQRS-light + structure) — those are Opus-level design calls already made in the ticket. If + implementation reveals the design call was wrong, flag back to Consolidation + rather than deciding unilaterally. +- Tickets touching a BIO2-flagged item are blocked until human compliance + sign-off is recorded in the ticket status — do not implement first and flag + after. + +Update ticket status in 99-backlog.md: not_started → in_progress → implemented +→ needs_review. Append implementation notes to implementation/[ticket-id].md. diff --git a/docs/project/refactor-backlog-setup/agents/_persistence-protocol.md b/docs/project/refactor-backlog-setup/agents/_persistence-protocol.md new file mode 100644 index 0000000..37183a3 --- /dev/null +++ b/docs/project/refactor-backlog-setup/agents/_persistence-protocol.md @@ -0,0 +1,25 @@ +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/00-baseline.md b/docs/project/refactor-backlog-setup/refactor-backlog/00-baseline.md new file mode 100644 index 0000000..c71617c --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/00-baseline.md @@ -0,0 +1,547 @@ +## Scope: apps/ssp (6 contexts), apps/behandelportal (3 contexts), libs/shared (11 layers), libs/beheer (5 layers), backend/src/BigRegister.Api (6 folders), backend/tests + +## Status: complete + +## Last updated: 2026-08-26 + +## Depends on: none + +## --- + +# 00 — Metrics Baseline + +Fixed input to every Phase 1 agent (01–07). **No agent may propose a change without +citing a `BL-###` observation or a metric row from this file.** + +--- + +## 1. Method, and what to trust + +Everything was measured with tooling already in the repo. No dependency was added, no +config file edited. + +| Metric | Command | Trust | +| ------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------ | +| FE coverage | `npm run test:coverage` → `coverage/{ssp,behandelportal,shared,beheer}/lcov.info` | exact | +| FE spec reach | derived: source files present in any lcov vs. files on disk | exact | +| BE coverage | `cd backend && dotnet test BigRegister.slnx --filter "Category!=Integration" --collect:"XPlat Code Coverage"` | exact | +| TS complexity + fn length | `npx eslint apps libs --rule '{"complexity":["warn",0],"max-lines-per-function":["warn",{"max":0,"skipBlankLines":true,"skipComments":true}]}' -f json` — threshold 0 makes ESLint report _every_ function with its score, giving a full distribution rather than only violations | exact (ESLint core rule, no plugin) | +| C# complexity | `node docs/project/refactor-backlog-setup/refactor-backlog/tools/baseline-scan.mjs --complexity` | **file CC exact-ish; per-method CC approximate** | +| Duplication | `node …/tools/baseline-scan.mjs --dup` | **approximate** | +| Layering violations | `npm run dep:check` | exact | +| Coupling / instability | `npx depcruise apps//src libs --config .dependency-cruiser..js --metrics --output-type metrics` | exact | + +`tools/baseline-scan.mjs` is new and lives in this workspace so the numbers reproduce. +It is a crude text scan: line-window hashing for duplication (not token-based like +jscpd), regex method detection for C# (does not understand expression-bodied members or +nested lambdas). Deterministic, zero-install, good enough to answer "did this move". +Upgrade path if a ticket ever needs more precision: jscpd, and a Roslyn analyzer. + +**Excluded as generated** (counted once, then excluded everywhere): +`libs/shared/src/infrastructure/api-client.ts` (2372 lines, NSwag) and +`backend/.../Data/Migrations/**` (17 files, 2690 lines, EF Core). + +**LCOV aggregation rule.** The four FE project runs overlap — `libs/shared` files appear +in `ssp`'s lcov as well as `shared`'s. Each file is attributed to its owning module and +the **best** result across the four runs is taken; a shared file exercised by an app's +specs is genuinely covered. + +**Protocol deviation, stated for the record.** The measurement runs are repo-global (one +coverage run, one depcruise run) and cannot be executed module-by-module; only the +write-up is per module. The resume contract still holds — the file is the recovery state. + +--- + +## 2. Size inventory + +| Unit | src .ts/.cs | src lines | spec files | spec lines | stories | +| ------------------------------ | ----------: | ---------: | ---------: | ---------: | ------: | +| apps/ssp | 92 | 10 415 | 39 | 3 564 | 21 | +| apps/behandelportal | 29 | 1 309 | 7 | 396 | 4 | +| libs/shared (excl. generated) | 86 | 5 194 | 21 | 1 226 | 40 | +| libs/beheer | 13 | 1 146 | 4 | 269 | 1 | +| **Frontend total** | **220** | **18 064** | **71** | **5 455** | **66** | +| backend/src (excl. Migrations) | 54 | 4 989 | — | — | — | +| backend/tests | 39 | — | 39 | 4 253 | — | +| e2e | 5 | 332 | — | — | — | + +Backend source by folder: `Program.cs` 940 · `Data` (excl. Migrations) 1 697 · `Domain` +991 · `Zgw` 710 · `Contracts` 329 · `Stamdata` 322. Zero `.html` template files exist +anywhere in `apps/`/`libs/` — every Angular template is inline. + +--- + +## 3. Coverage + +### 3a. Frontend — line/branch coverage of files a spec actually reaches + +| Module | Files | Lines | Line % | Branches | Branch % | +| -------------------------- | -----: | --------: | --------: | --------: | --------: | +| ssp/auth | 4 | 35 | 42.9% | 26 | 46.2% | +| bhp/auth | 4 | 35 | 42.9% | 26 | 46.2% | +| libs/shared/upload | 2 | 125 | 52.0% | 118 | 50.0% | +| libs/beheer/application | 1 | 70 | 65.7% | 37 | 40.5% | +| libs/beheer/infrastructure | 1 | 37 | 67.6% | 43 | 60.5% | +| ssp/herregistratie | 5 | 302 | 70.9% | 286 | 67.8% | +| libs/shared/layout | 2 | 80 | 72.5% | 36 | 55.6% | +| libs/shared/ui | 13 | 279 | 72.8% | 230 | 75.2% | +| ssp/brief | 11 | 486 | 75.3% | 461 | 68.8% | +| ssp/registratie | 21 | 430 | 80.0% | 471 | 77.3% | +| libs/shared/application | 8 | 76 | 80.3% | 50 | 70.0% | +| bhp/behandeling | 5 | 107 | 91.6% | 146 | 81.5% | +| libs/shared/infrastructure | 9 | 75 | 94.7% | 84 | 81.0% | +| libs/shared/kernel | 5 | 28 | 96.4% | 20 | 90.0% | +| libs/beheer/domain | 2 | 54 | 98.1% | 64 | 75.0% | +| ssp/showcase | 1 | 11 | 100.0% | 4 | 100.0% | +| libs/shared/testing | 3 | 9 | 100.0% | 2 | 50.0% | +| **TOTAL** | **98** | **2 240** | **75.1%** | **2 104** | **70.6%** | + +### 3b. Frontend — spec _reach_ (the number that matters) + +75.1% is coverage **of the 98 files a spec imports**. It is not coverage of the codebase. +122 of 220 source files are never loaded by any Vitest run at all. + +| Module | Source files | Reached | Never reached | % reached | +| ---------------------------------------- | -----------: | ------: | ------------: | --------: | +| ssp/root, ssp/shell, bhp/root, bhp/shell | 12 | 0 | 12 | 0% | +| libs/shared/domain | 3 | 0 | 3 | 0% | +| libs/beheer/ui | 4 | 0 | 4 | 0% | +| libs/beheer/contracts | 1 | 0 | 1 | 0% | +| libs/shared/layout | 11 | 2 | 9 | 18% | +| bhp/behandeling | 16 | 5 | 11 | 31% | +| ssp/showcase | 3 | 1 | 2 | 33% | +| libs/shared/ui | 34 | 13 | 21 | 38% | +| ssp/brief | 26 | 11 | 15 | 42% | +| bhp/auth | 8 | 4 | 4 | 50% | +| libs/shared/upload | 4 | 2 | 2 | 50% | +| ssp/registratie | 41 | 21 | 20 | 51% | +| ssp/herregistratie | 9 | 5 | 4 | 56% | +| ssp/auth | 6 | 4 | 2 | 67% | +| libs/beheer/domain | 3 | 2 | 1 | 67% | +| libs/shared/application | 11 | 8 | 3 | 73% | +| libs/shared/infrastructure | 11 | 9 | 2 | 82% | +| libs/shared/kernel | 5 | 5 | 0 | 100% | +| **TOTAL** | **220** | **98** | **122** | **45%** | + +Most of the 122 are `ui/` components, which CLAUDE.md §5 deliberately exercises through +**Storybook + the a11y addon (66 stories)**, not Vitest. That is a house decision, not a +gap — see BL-004 before filing anything against it. + +### 3c. Backend + +| Module | Files | Lines | Line % | Branches | Branch % | +| ------------------ | -----: | --------: | --------: | --------: | --------: | +| backend/Domain | 17 | 730 | 94.2% | 356 | 82.0% | +| backend/Stamdata | 10 | 252 | 96.8% | 120 | 71.7% | +| backend/Program.cs | 1 | 1 160 | 97.4% | 316 | 84.8% | +| backend/Contracts | 2 | 334 | 97.6% | 40 | 65.0% | +| backend/Zgw | 8 | 624 | 98.1% | 124 | 85.5% | +| backend/Data | 13 | 1 852 | 99.0% | 400 | 75.5% | +| **TOTAL** | **51** | **4 952** | **97.6%** | **1 356** | **79.6%** | + +241 tests, all green. Every backend source file is reached. Branch coverage is the weak +axis, not line coverage — `Contracts` 65.0%, `Stamdata` 71.7%, `Data` 75.5%. + +--- + +## 4. Complexity + +### 4a. TypeScript (exact — ESLint core `complexity`) + +Distribution over 2 085 source functions (specs/stories excluded): +**p50 1 · p75 2 · p90 3 · p95 5 · p99 12 · max 27.** 25 functions exceed CC 10 (1.2%). + +Function length over 1 149 functions: **p50 3 · p75 7 · p90 13 · p95 20 · p99 34 · +max 143.** Only 2 functions exceed 75 lines. + +| Module | Fns | max CC | p90 CC | CC>10 | max fn lines | fn>75 | +| -------------------------------------------------------- | --: | -----: | -----: | ----: | -----------: | ----: | +| libs/shared/upload | 90 | 27 | 4 | 1 | 109 | 1 | +| ssp/registratie | 361 | 23 | 4 | 7 | 143 | 1 | +| ssp/herregistratie | 144 | 19 | 3 | 4 | 31 | 0 | +| libs/shared/infrastructure | 32 | 19 | 6 | 1 | 33 | 0 | +| ssp/brief | 608 | 17 | 3 | 6 | 73 | 0 | +| bhp/behandeling | 85 | 16 | 5 | 3 | 28 | 0 | +| libs/beheer/domain | 22 | 11 | 7 | 1 | 24 | 0 | +| libs/beheer/infrastructure | 13 | 11 | 7 | 1 | 25 | 0 | +| libs/shared/ui | 285 | 11 | 3 | 1 | 37 | 0 | +| libs/beheer/ui | 100 | 10 | 1 | 0 | 8 | 0 | +| libs/shared/application | 61 | 7 | 3 | 0 | 30 | 0 | +| bhp/auth · ssp/auth · ssp/shell | 63 | 5 | 3 | 0 | 13 | 0 | +| libs/beheer/application | 42 | 4 | 3 | 0 | 15 | 0 | +| libs/shared/kernel | 18 | 4 | 4 | 0 | 13 | 0 | +| libs/shared/layout | 75 | 4 | 2 | 0 | 29 | 0 | +| ssp/showcase · ssp/root · bhp/root · libs/shared/testing | 86 | 3 | 1 | 0 | 15 | 0 | + +**Read BL-001 before filing any complexity ticket.** 23 of the 25 CC>10 functions are +reducers (9), `parse*` trust boundaries (10), or `validate*` (4) — all three are +mandated house idioms. + +The 25, in full: + +| CC | Function | Location | Kind | +| --: | ------------------------ | -------------------------------------------------------------------------------- | --------- | +| 27 | `reduceUpload` | libs/shared/src/upload/upload.machine.ts:131 | reducer | +| 23 | `validateStep` | apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts:117 | validate | +| 20 | `reduce` | apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts:321 | reducer | +| 19 | `validateStep` | apps/ssp/src/app/herregistratie/domain/intake.machine.ts:97 | validate | +| 19 | `parseAanvraagStatus` | apps/ssp/src/app/registratie/infrastructure/applications.adapter.ts:72 | parse | +| 19 | `fetch` | libs/shared/src/infrastructure/api-client.provider.ts:49 | **other** | +| 17 | `parseOrgTemplate` | apps/ssp/src/app/brief/infrastructure/brief.adapter.ts:331 | parse | +| 17 | `parseDuoLookup` | apps/ssp/src/app/registratie/infrastructure/duo.adapter.ts:46 | parse | +| 16 | `parseBeoordelingStatus` | apps/behandelportal/src/app/behandeling/infrastructure/beoordeling.adapter.ts:31 | parse | +| 16 | `parseStatus` | apps/ssp/src/app/brief/infrastructure/brief.adapter.ts:202 | parse | +| 16 | `validateAll` | apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts:166 | validate | +| 15 | `parseBeoordelingView` | apps/behandelportal/src/app/behandeling/infrastructure/beoordeling.adapter.ts:68 | parse | +| 15 | `validateAll` | apps/ssp/src/app/herregistratie/domain/intake.machine.ts:135 | validate | +| 14 | `reduce` | apps/behandelportal/src/app/behandeling/domain/besluit.machine.ts:70 | reducer | +| 14 | `reduce` | apps/ssp/src/app/brief/domain/org-template.machine.ts:57 | reducer | +| 14 | `reduce` | apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts:161 | reducer | +| 14 | `reduce` | apps/ssp/src/app/herregistratie/domain/intake.machine.ts:222 | reducer | +| 14 | `reduce` | apps/ssp/src/app/registratie/domain/change-request.machine.ts:57 | reducer | +| 13 | `reduce` | apps/ssp/src/app/brief/domain/brief.machine.ts:188 | reducer | +| 12 | `parsePassage` | apps/ssp/src/app/brief/infrastructure/brief.adapter.ts:235 | parse | +| 12 | `parseBrief` | apps/ssp/src/app/brief/infrastructure/brief.adapter.ts:277 | parse | +| 12 | `parseDashboardView` | apps/ssp/src/app/registratie/infrastructure/dashboard-view.adapter.ts:50 | parse | +| 11 | `reduce` | libs/beheer/src/domain/stamdata-editor.machine.ts:31 | reducer | +| 11 | `parseAuditEntries` | libs/beheer/src/infrastructure/audit.adapter.ts:21 | parse | +| 11 | `collect` | libs/shared/src/ui/rich-text-editor/rich-text-dom.ts:130 | **other** | + +Functions over 75 lines — the entire population: +`createDraftSync` 143 lines (apps/ssp/src/app/registratie/application/draft-sync.ts:50), +`reduceUpload` 109 lines (libs/shared/src/upload/upload.machine.ts:131). + +### 4b. C# (approximate) + +| Module | Files | Σ file CC | max file CC | Methods | max method CC | p90 | CC>10 | +| ------------------ | ----: | --------: | ----------: | ------: | ------------: | --: | ----: | +| backend/Program.cs | 1 | **78** | **78** | 13 | 12 | 7 | 1 | +| backend/Data | 23 | 101 | 27 | 77 | 16 | 4 | 1 | +| backend/tests | 39 | 121 | 26 | 230 | 20 | 2 | 1 | +| backend/Domain | 18 | 71 | 21 | 36 | 8 | 6 | 0 | +| backend/Stamdata | 10 | 34 | 21 | 11 | 10 | 6 | 0 | +| backend/Zgw | 8 | 39 | 11 | 20 | 8 | 4 | 0 | +| backend/Contracts | 2 | 4 | 3 | 6 | 3 | 3 | 0 | + +Method-length distribution (n=393): p50 9 · p90 22 · p99 90 · max 293. + +Highest-CC files: `Program.cs` 78 · `Data/ApplicationStore.cs` 27 · +`tests/OpenZaakZaakSourceTests.cs` 26 · `Domain/Letters/LetterHtml.cs` 21 · +`Stamdata/StamdataTable.cs` 21 · `Data/BriefStore.cs` 17. + +Methods over CC 10: `ToDto` (Data/BriefStore.cs:34, CC 16), +`LogBrief` (Program.cs:889, CC 12), and one 293-line test method +(`CreateZaak_posts_zaak_status_and_rol_and_maps_the_result_back`, CC 20). + +--- + +## 5. Duplication (approximate — 6-line normalized window) + +Repo-wide: **1 755 of 24 701 significant lines duplicated = 7.1%.** + +| Module | Sig. lines | Duplicated | % | +| --------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------: | ---------: | ---------: | +| ssp/auth | 211 | 211 | **100.0%** | +| bhp/auth | 243 | 211 | **86.8%** | +| bhp/root | 90 | 68 | 75.6% | +| ssp/root | 122 | 68 | 55.7% | +| bhp/shell | 21 | 8 | 38.1% | +| ssp/herregistratie | 1 536 | 121 | 7.9% | +| ssp/brief | 5 015 | 393 | 7.8% | +| backend/tests | 2 538 | 197 | 7.8% | +| backend/Data | 1 068 | 82 | 7.7% | +| libs/shared/infrastructure | 510 | 36 | 7.1% | +| libs/beheer/domain | 250 | 17 | 6.8% | +| libs/beheer/ui | 509 | 34 | 6.7% | +| libs/beheer/application | 185 | 12 | 6.5% | +| bhp/behandeling | 954 | 55 | 5.8% | +| ssp/shell | 194 | 8 | 4.1% | +| ssp/registratie | 3 798 | 144 | 3.8% | +| libs/shared/ui | 2 604 | 58 | 2.2% | +| backend/Zgw | 327 | 6 | 1.8% | +| ssp/showcase | 487 | 8 | 1.6% | +| backend/Program.cs | 510 | 6 | 1.2% | +| libs/shared/application | 566 | 6 | 1.1% | +| libs/shared/layout | 869 | 6 | 0.7% | +| backend/Contracts, backend/Domain, backend/Stamdata, libs/shared/{domain,kernel,upload,testing,environments}, libs/beheer/{contracts,infrastructure}, e2e | 1 894 | 0 | 0.0% | + +Top clone pairs: + +| Windows | Pair | +| ------: | ------------------------------------------------------------------------------------------------ | +| 39 | `bhp/auth/application/session.store.ts` ↔ `ssp/auth/application/session.store.ts` | +| 36 | `bhp/auth/auth.guard.spec.ts` ↔ `ssp/auth/auth.guard.spec.ts` | +| 35 | `bhp/auth/ui/login-form/…` ↔ `ssp/auth/ui/login-form/…` | +| 25 | `bhp/app.config.ts` ↔ `ssp/app.config.ts` | +| 23 | `bhp/auth/ui/login.page.ts` ↔ `ssp/auth/ui/login.page.ts` | +| 21 | `bhp/auth/auth.guard.ts` ↔ `ssp/auth/auth.guard.ts` | +| 13 | `libs/shared/…/role.interceptor.spec.ts` ↔ `…/subject.interceptor.spec.ts` | +| 11 | `brief/ui/letter-canvas.stories.ts` ↔ `brief/ui/letter-composer.stories.ts` | +| 9 | `ssp/brief/ui/org-template.page.ts` ↔ `libs/beheer/src/ui/stamdata.page.ts` | +| 8 | `bhp/behandeling/domain/besluit.machine.ts` ↔ `ssp/registratie/domain/change-request.machine.ts` | +| 8 | `tests/OpenZaakZaakSourceTests.cs` ↔ `tests/ZgwDivergenceTests.cs` | +| 7 | `bhp/…/besluit-form.component.ts` ↔ `ssp/…/change-request-form.component.ts` | +| 7 | `ssp/…/intake-wizard.component.ts` ↔ `ssp/…/registratie-wizard.component.ts` | + +**The auth duplication is a deliberate decision, not an accident** — see BL-002 before +proposing to merge it. + +--- + +## 6. Layering and coupling + +`npm run dep:check`: **0 violations** (223 modules, 584 dependencies cruised) across 11 +`severity: error` rules — `shared-no-features`, `beheer-no-features`, `shared-no-beheer`, +`-no-other-app`, per-context `--scope`, `domain-is-pure`, +`contracts-import-nothing`, `ui-not-infrastructure`, `apiclient-infrastructure-only`, +`no-testing-in-production`, `no-circular`. + +Instability I = Ce/(Ca+Ce). Low I = stable base, high I = volatile leaf. Measured in the +ssp cruise (behandelportal's figures for `libs/*` differ only in Ca, same shape): + +| Folder | N | Ca | Ce | I | +| --------------------------------------- | --: | --: | --: | -----: | +| apps/ssp/src/app/brief | 46 | 2 | 143 | 99% | +| apps/ssp/src/app/herregistratie | 18 | 4 | 80 | 95% | +| apps/ssp/src/app/showcase | 4 | 1 | 17 | 94% | +| apps/ssp/src/app/registratie | 67 | 19 | 181 | 91% | +| apps/behandelportal/src/app/behandeling | 24 | 2 | 62 | 97% | +| apps/behandelportal/src/app/auth | 11 | 4 | 25 | 86% | +| apps/ssp/src/app/auth | 9 | 5 | 24 | 83% | +| libs/beheer/src/ui | 5 | 3 | 27 | 90% | +| libs/beheer/src/application | 3 | 2 | 16 | 89% | +| libs/beheer/src/infrastructure | 3 | 3 | 11 | 79% | +| libs/shared/src/layout | 19 | 22 | 52 | 70% | +| libs/shared/src/ui | 68 | 128 | 111 | 46% | +| libs/shared/src/infrastructure | 19 | 43 | 25 | 37% | +| libs/shared/src/application | 18 | 55 | 30 | 35% | +| libs/shared/src/upload | 5 | 30 | 13 | 30% | +| libs/beheer/src/domain | 5 | 8 | 3 | 27% | +| libs/shared/src/testing | 4 | 17 | 2 | 11% | +| libs/shared/src/kernel | 9 | 71 | 4 | **5%** | +| libs/shared/src/domain | 3 | 11 | 0 | 0% | +| libs/beheer/src/contracts | 1 | 0 | 0 | 0% | + +This is textbook: `kernel`/`domain` are the stable base (I ≤ 5%), feature contexts are +volatile leaves (I ≥ 83%), nothing depends on them. **The frontend dependency structure +is not a problem area** — do not spend tickets here. + +**The backend has no equivalent enforcement at all.** `Domain/` purity (verified: zero +`Microsoft.EntityFrameworkCore` / `Microsoft.AspNetCore` imports) holds by convention and +code review only. See BL-006. + +--- + +## 7. Pattern inventory — what already exists + +Agents 03 (DDD/hexagonal) and 04 (CQRS-light) may only **extend** what is listed here. +They may not introduce either pattern into a module where it is absent. + +### Backend + +| Pattern | State | +| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Ports with 2+ implementations | `IZaakSource` (`Data/LocalZaakSource.cs` ↔ `Zgw/OpenZaakZaakSource.cs`), `IDocumentSource` (`Data/LocalDocumentSource.cs` ↔ `Zgw/OpenZaakDocumentSource.cs`) — config-switched on `Zgw:Enabled` at `Program.cs:57-84`, ADR-0005 | +| Single-impl interface | `IIdentityProvider` → `StubIdentityProvider` | +| **Not behind any port** | 7 static, non-DI stores: `ApplicationStore`, `DocumentStore`, `BriefStore`, `OrgTemplateStore`, `FeatureFlagStore`, `AuthzAuditStore`, `IdempotencyStore`. No `AddDbContext`; each opens a short-lived context via `Db.Create()` under its own lock. Deliberate, documented in `Data/Db.cs` and `Program.cs:40-45` | +| Domain purity | `Domain/` is EF-free and ASP-free (verified). Rules are static classes of pure functions with co-located tests in `tests/Domain/`: `SubmissionRules`, `DocumentRules`, `IntakePolicy`, `BeoordelingRules`, `DiplomaRules`, `HerregistratieRule`, `LetterHtml`, `OrgTemplateRules`, `Authz`, `FeatureFlags`. `Domain/Applications/Aanvraag.cs` is a C# tagged union (`Concept`/`Submitted`/`Decided`) | +| CQRS-light | **Partial.** `Contracts/Dtos.cs` holds 65 records split by direction (`*Request` in, `*Dto`/`*Response` out). Read/write split exists as _comment banners_ inside a single **940-line `Program.cs`** carrying all 48 endpoint mappings. No handler types, no mediator, no `Features/` folders. Cross-cutting behaviour is factored into local helpers (`Submit`, `StamdataAdmin`, `CasesAdmin`, `Beoordelen`, `OrgAdmin`, `FlagsAdmin`) — authorization/idempotency wrappers, not handlers | +| Mapping | `Contracts/Mappers.cs` (`.ToDto()`, `.ToDetailDto()`), `Data/AanvraagMapper.cs` | +| ZGW anti-corruption layer | Fully built: `Zgw/{OpenZaakZaakSource,OpenZaakDocumentSource,ZgwHttpClient,ZgwTokenProvider,ZgwZaakMapper,ZgwOptions,ZgwDiagnosticHandler}.cs`, 5 test files. ADR-0005 | + +### Frontend + +| Pattern | Count | Notes | +| ------------------------------------- | ----: | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Command factories (write side) | 3 | `registratie/application/submit-change-request.ts`, `behandeling/application/submit-besluit.ts`, `registratie/application/draft-sync.ts` | +| Mutations living _inline in adapters_ | ~13 | `brief.adapter.ts` (save/submit/approve/reject/send/reset), `org-template.adapter.ts` (save/publish/rollback), `stamdata.adapter.ts` — they call `runSubmit` directly instead of going through a command factory | +| Infrastructure adapters (read side) | 20 | +1 outside an `infrastructure/` folder: `libs/shared/src/upload/upload.adapter.ts` | +| `parse*` trust boundaries | 30 | 24 in adapters, 6 in value objects / kernel | +| Application stores | 15 | all `providedIn: 'root'` | +| Elm-style machines | 9 | 8 under a `domain/` folder; outlier `libs/shared/src/upload/upload.machine.ts` | +| Explicit port | 1 | `SessionPort` + `SESSION_PORT` token (`libs/shared/src/application/session.port.ts`) | +| Config-seam tokens | 3 | `DEBUG_PANEL`, `HEADER_NAV_ITEMS`, `HEADER_ADMIN_LINKS` | +| `contracts/` DTO files | 4 | most adapters consume NSwag-generated types directly instead | + +Shared application kit (`libs/shared/src/application/`): `remote-data.ts`, `store.ts`, +`submit.ts`, `action-state.ts`, `debounced-save.ts`, `history.ts`, +`machine-remote-data.ts`, `pending-saves.ts`. + +### ADRs on record + +`0001` BFF-lite + decision DTOs (Accepted) · `0002` user groups as actors, not bounded +contexts (was **Proposed** when measured; promoted to **Accepted** 2026-08-26 by ADR-C-005, +with §3's `Principal` omission recorded in the ADR as known debt) · `0003` CIBG Huisstijl +(Accepted) · `0004` +stamdata as code (Accepted) · `0005` OpenZaak behind the BFF — the explicit +ports-and-adapters ADR (Accepted) · `0006` test data through the production door +(Accepted). + +--- + +## 8. Rankings, worst to best + +**Spec reach (FE):** ssp/root · ssp/shell · bhp/root · bhp/shell · libs/shared/domain · +libs/beheer/ui · libs/beheer/contracts (all 0%) → libs/shared/layout 18% → +bhp/behandeling 31% → libs/shared/ui 38% → ssp/brief 42% → … → libs/shared/kernel 100%. + +**Line coverage (FE, of reached files):** ssp/auth · bhp/auth 42.9% → +libs/shared/upload 52.0% → libs/beheer/application 65.7% → … → ssp/showcase 100%. + +**Branch coverage (BE):** Contracts 65.0% → Stamdata 71.7% → Data 75.5% → Domain 82.0% → +Program.cs 84.8% → Zgw 85.5%. + +**Duplication:** ssp/auth 100% → bhp/auth 86.8% → bhp/root 75.6% → ssp/root 55.7% → +bhp/shell 38.1% → ssp/herregistratie 7.9% → … → 12 modules at 0%. + +**Complexity (TS, CC>10 count):** ssp/registratie 7 → ssp/brief 6 → ssp/herregistratie 4 +→ bhp/behandeling 3 → 5 modules with 1 → the rest 0. + +**Complexity (C#, file CC):** Program.cs 78 → Data 101 across 23 files (max 27) → +Domain 71 (max 21) → Zgw 39 → Stamdata 34 → Contracts 4. + +**Coupling:** nothing to rank — 0 violations, healthy instability gradient (§6). + +--- + +## 9. Thresholds Phase 1 must use + +Derived from the measured distributions above, not invented. + +| Threshold | Value | Basis | +| ---------------------------------------- | ----------------------------------------- | -------------------------------------------------------------------- | +| Cyclomatic complexity | **> 10** | fixed by the agent prompt; TS p99 is 12, so this catches the top ~1% | +| **Agent 01's `[N] lines` — TS function** | **> 40** | TS fn-length p99 is 34; only 4 functions exceed 50 | +| Agent 01's `[N] lines` — C# method | **> 60** | C# method p99 is 90, p90 is 22 | +| Agent 01's `[N] lines` — component/file | **> 400** | 6 TS files and 1 C# file exceed it | +| Nesting depth | **> 3** | fixed by the agent prompt | +| Duplication worth a ticket | **> 10% of a module's significant lines** | repo mean is 7.1% | + +Largest files (the >400 population): `backend/Program.cs` 940 · +`registratie/ui/registratie-wizard.component.ts` 645 · `showcase/concepts.page.ts` 496 · +`brief/ui/letter-canvas.component.ts` 463 · `brief/infrastructure/brief.adapter.ts` 437 · +`herregistratie/ui/intake-wizard.component.ts` 406. + +--- + +## 10. Baseline observations (cite these) + +Stable IDs. A Phase 1 finding must reference one of these or a metric row above. + +**BL-001 — 23 of 25 TS functions over CC 10 are mandated house idioms.** +Reducers (9), `parse*` trust boundaries (10) and `validate*` (4) are switch/guard-dense +by construction: CLAUDE.md §3 requires a tagged-union reducer over booleans, and ADR-0001 +requires a hand-written `parse*` at every wire boundary. High CC there is the design +working, not decaying. Only two CC>10 functions are outside the idiom: +`api-client.provider.ts:49 fetch` (CC 19) and `rich-text-dom.ts:130 collect` (CC 11). +_Any complexity ticket against a `reduce*`/`parse*`/`validate*` must argue why this case +differs — a bare CC number is not sufficient grounds._ + +**BL-002 — `ssp/auth` is 100% duplicated against `bhp/auth`, deliberately.** +211 of 211 significant lines; `session.store.ts`, `login-form.component.ts`, `login.page.ts` +and `auth.guard.ts` are near-identical. CLAUDE.md §1 and ADR-0002 state auth is _not_ +shared because Zorgverlener and Medewerker are different `Principal` variants expected to +diverge. **The divergence has not happened yet.** This is legitimately in scope for +agent 06 (ADR conformance) as either an `ADR-fix` — the prediction has not held over two +phases of work — or a "still waiting" note. It is _not_ a straightforward +extract-to-shared refactor; that would contradict an accepted ADR. + +> **Sharpened 2026-08-26 by agent 06 (ADR-C-004/005) — verified.** "The prediction has +> not held" is the wrong diagnosis. ADR-0002 §3's concrete deliverable, `Session → +Principal`, was **never built**: `grep -rn "Principal" apps libs` returns exactly one +> hit, a comment in `libs/shared/src/infrastructure/role.ts:8`, and no type. `diff -rq` +> over the two auth folders shows **zero** content differences — 9 of 11 files identical, +> the only delta being two extra files in behandelportal. `behandelportal`'s Behandelaar +> still carries a `bsn` and `login.page.ts:31` logs a backoffice user in through DigiD. +> The rule was not falsified, it was untested; the divergence that did occur went through +> an orthogonal side door (`medewerker.interceptor.ts`) that never touches `Session`. +> Amending ADR-0002 would ratify the omission. Use agent 06's sequencing instead. + +**BL-003 — `Program.cs` is the single largest complexity concentration in the repo.** +940 lines, 48 endpoint mappings, file CC 78 (next-highest file: 27), read/write separated +only by comment banner. 97.4% line / 84.8% branch covered, so it is well-tested, not +fragile — this is a structure finding, not a correctness one. It is the one place where +agent 04's CQRS-light and agent 03's vertical-slice thinking both have real purchase, +_and_ the one place where "extend the existing pattern, don't introduce one" is hardest to +honour: there is no `Features/` folder to extend. + +**BL-004 — 122 of 220 FE source files (55%) are never loaded by any Vitest run.** +Overwhelmingly `ui/` components, which CLAUDE.md §5 covers via 66 Storybook stories with +the a11y addon instead. Agent 02 must distinguish _"untested"_ from _"tested through +Storybook"_ before filing; the genuine gaps are non-`ui/` files with no spec — +`libs/shared/domain` (3 files, 0%), `libs/beheer/contracts`, and the app root/shell files. + +> **Corrected 2026-08-26 by agent 02 — verified.** The two named "genuine gaps" are false +> positives. `libs/shared/src/domain` is 30 lines across 3 files — interfaces and type +> aliases plus a single string const — and `libs/beheer/src/contracts/stamdata.dto.ts` is +> 30 lines of DTO shapes. Neither contains an executable statement, so 0% is correct and +> unimprovable. Close both rows rather than ticketing them. Only the app root/shell files +> remain as candidates from this observation. + +**BL-005 — backend branch coverage lags line coverage by 18 points** (97.6% vs 79.6%). +Weakest: `Contracts` 65.0%, `Stamdata` 71.7%, `Data` 75.5%. Line coverage is near-total, +so the missing tests are edge-case branches, not whole units. + +**BL-006 — the backend has zero automated architecture enforcement.** +The frontend has 11 dependency-cruiser rules at `severity: error`, 0 violations, running +in CI. The backend has none: one assembly, no `Domain.csproj` boundary, no +NetArchTest/ArchUnitNET. `Domain/` purity currently holds by convention. Any agent-03 +proposal that depends on the backend's layering staying clean should note that nothing +enforces it. + +**BL-007 — the FE write side is inconsistently placed.** +3 command factories vs ~13 call sites invoking `runSubmit` directly inside adapters +(`brief.adapter.ts`, `org-template.adapter.ts`, `stamdata.adapter.ts`). CLAUDE.md §3 makes +the command factory the idiom. This is agent 04's clearest extend-an-existing-pattern +target — the pattern exists, it is just not applied uniformly. + +> **Corrected 2026-08-26 by agent 04 (CQ-003/CQ-005) — verified.** The "~13 mutations" +> count was derived from the `runSubmit` helper name and is wrong: 5 of those call sites +> are **reads**, not writes. `libs/beheer/src/infrastructure/stamdata.adapter.ts` exposes +> only `list()` and `load()` and its own docstring says "Both endpoints are reads … There +> is no write method", yet both call `runSubmit`. Same at `brief.adapter.ts:56` and +> `org-template.adapter.ts:39,51`. `runSubmit` is the write fold — it mints the +> Idempotency-Key — so the name, not the code, produced the miscount. Conversely agent 04 +> found **3 mutations this baseline missed entirely**: `ApplicationsStore.cancel`, +> `AdminCasesStore.delete`, `FeatureFlagStore.set` reach the raw `ApiClient` and never +> return a `Result`. Use agent 04's inventory, not this count. + +**BL-008 — `coverageExclude` does not exclude the generated API client.** +`libs/shared/src/infrastructure/api-client.ts` is listed in `coverageExclude` in all four +`angular.json` test targets, yet appears in all four `lcov.info` files (987 lines at ~7%), +dragging the reported `libs/shared/infrastructure` figure from 94.7% down to 6.9%. This +file excludes it manually. Small, real, and cheap to fix. + +**BL-009 — no coverage threshold is enforced anywhere.** +Neither `angular.json` nor CI sets a minimum; `npm run test:coverage` runs in CI and the +output is discarded. There is no ratchet, so no ticket can be verified as "improved +coverage" by CI alone — verify against the numbers in this file. + +**BL-010 — `libs/shared/src/upload/` sits outside the layer convention.** +`upload.machine.ts` (CC 27, 109 lines — the highest in the repo) lives in its own +top-level folder rather than under `domain/`, and `upload.adapter.ts` outside +`infrastructure/`. It is carved out by name in the `apiclient-infrastructure-only` +dependency-cruiser rule, i.e. the exception is already encoded rather than resolved. + +**BL-011 — the FE test suite is flaky under parallel load.** +`npm run test:coverage` failed on the first run with two `[vitest-pool] Timeout waiting +for worker to respond` errors in `libs/shared`, then passed 21/21 when re-run serially. +Not a code defect; relevant to any ticket whose acceptance is "CI green". + +--- + +## 11. Reproducing this file + +```bash +npm run test:coverage # if flaky, re-run the failing project alone +cd backend && dotnet test BigRegister.slnx --filter "Category!=Integration" \ + --collect:"XPlat Code Coverage" +npx eslint apps libs --rule '{"complexity":["warn",0],"max-lines-per-function":["warn",{"max":0,"skipBlankLines":true,"skipComments":true}]}' -f json +node docs/project/refactor-backlog-setup/refactor-backlog/tools/baseline-scan.mjs +npm run dep:check +npx depcruise apps/ssp/src libs --config .dependency-cruiser.ssp.js --metrics --output-type metrics +npx depcruise apps/behandelportal/src libs --config .dependency-cruiser.behandelportal.js --metrics --output-type metrics +``` + +The `baseline-scan.mjs` figures are deterministic and must reproduce exactly. Coverage +figures move with the tests. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/01-readability.md b/docs/project/refactor-backlog-setup/refactor-backlog/01-readability.md new file mode 100644 index 0000000..37451f0 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/01-readability.md @@ -0,0 +1,9 @@ +## Scope: [to be filled by agent] + +## Status: not_started + +## Last updated: - + +## Depends on: [see agent prompt] + +## --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/02-testability.md b/docs/project/refactor-backlog-setup/refactor-backlog/02-testability.md new file mode 100644 index 0000000..b4a6b18 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/02-testability.md @@ -0,0 +1,641 @@ +## Scope: apps/ssp (auth, registratie, herregistratie, brief, showcase+shell+root), apps/behandelportal (auth, behandeling, shell+root), libs/shared (domain, application, infrastructure, ui, layout, kernel, upload, testing), libs/beheer, backend (Program.cs, Domain, Data, Zgw, Contracts, Stamdata, tests) + +## Status: complete + +## Last updated: 2026-08-26 + +## Depends on: 00-baseline.md + +## --- + +# 02 — Testability + +What blocks a **unit** test — static/singleton dependencies, hidden I/O, work in +constructors/field initializers, pure logic entangled with impure. Every finding cites a +`BL-###` or a metric row from `00-baseline.md`. Seams proposed are extractions, never +rewrites. + +## How this file reads the baseline + +Three filters were applied before anything was written down, and they killed more +candidates than they kept: + +1. **BL-004's carve-out is honoured.** No finding is filed against a `ui/` (or + `layout/` component) file for lacking a Vitest spec. 66 Storybook stories + the a11y + addon are the house strategy (CLAUDE.md §5), not a gap. +2. **"Untested" ≠ "untestable".** Several of the worst-covered non-`ui/` files are + perfectly injectable and simply have no spec (`submit-besluit.ts`, + `Contracts/Mappers.cs`, `breadcrumb-trail.ts`). Those are noted in their module + section but not filed as testability findings — there is no seam to add. Whoever + owns coverage should pick them up. +3. **Already-covered code needs a positive argument.** Backend line coverage is 97.6% + (BL-005); a "this is untestable" claim there has to point at a branch the current + test shape genuinely cannot reach. Two do (TE-007, TE-008); one points at the cost + of how it is reached (TE-009). + +**Two baseline items are closed as false gaps** — see `libs/shared/domain` and +`libs/beheer/contracts` below. BL-004 names both as "genuine gaps"; on inspection +neither contains an executable statement. + +**Deliberate decisions engaged with, not overridden:** the 7 static backend stores +(documented in `Data/Db.cs`) are left alone — TE-009 extracts rules _out_ of one of +them without touching its shape. BL-002's auth duplication is respected — TE-004 lands +the same seam twice rather than proposing a shared extraction. + +--- + +## apps/ssp — auth + +**TE-001 — `SessionStore.restore()` reads `localStorage` inline, so its shape guard cannot be unit-tested** + +- Module / file:line — `apps/ssp/src/app/auth/application/session.store.ts:12-21` +- **What blocks unit testing.** `restore()` is module-private and calls + `localStorage.getItem(STORAGE_KEY)` itself, then does the parse + shape validation in + the same function. It is invoked from a field initializer + (`private _session = signal(restore())`, L37), so the storage read + happens the instant the singleton is constructed. A spec cannot feed it a raw string; + it must stub the `localStorage` global before the injector builds the store. The + logic being guarded is not incidental — the comments mark it G1 (never persist the + BSN) and G2 (validate the shape before trusting it), i.e. a trust boundary, and + CLAUDE.md §5 mandates a spec for boundary `parse*` adapters. +- **Baseline citation.** §3a: `ssp/auth` 42.9% line / 46.2% branch — **jointly the worst + line coverage in the frontend table** (§8 ranking). Per-file lcov for this file: + **LH 2 / LF 20 (10.0% line), BRH 3 / BRF 13 (23.1% branch)** — 4 of the module's 6 + files are spec-reached (§3b, 67%), yet this one barely executes. +- **Minimal seam.** Split the pure half out and move it next to the type it produces: + `export function parseStoredSession(raw: string | null): Session | null` in + `auth/domain/session.ts` — which **already has a spec file** + (`auth/domain/session.spec.ts`) and is pure TS, so no new test scaffolding is needed. + `restore()` collapses to `parseStoredSession(localStorage.getItem(STORAGE_KEY))`. Three + test cases (absent, non-JSON, wrong shape) cover the guard. +- **Effort S.** Independently shippable in one deploy — pure move, no call-site change + outside the file. +- **Note on BL-002.** `bhp/auth` carries the identical function; the seam lands **twice**, + once per app. That is correct, not duplication to fix — ADR-0002 / CLAUDE.md §1 make + `auth` deliberately unshared, and BL-002 flags any extract-to-shared here as + contradicting an accepted ADR. Agent 06 owns whether that prediction still holds. + +## apps/ssp — registratie + +**No findings.** + +The module's shape is the reason. Every `parse*` in its six adapters is exported and +directly spec'd (`applications`, `big-register`, `brp`, `dashboard-view`, `duo` all have +`.spec.ts` files); the machines are pure `domain/` units with specs; the five value +objects each have one. + +`createDraftSync` deserves an explicit acquittal: at 143 lines it is the longest function +in the repo (§4a, "Functions over 75 lines — the entire population") and it owns a +`setTimeout` debounce, a `Router` navigation and an in-flight-create race guard. It is +nevertheless **the best-seamed effectful unit in the frontend** — deps arrive through an +explicit `DraftSyncDeps` object (`draft-sync.ts:24-33`), `Router`/`ActivatedRoute` are +`inject(..., { optional: true })` so it is inert without them, and `enabled()` exists +specifically so stories and tests can neutralize it (L31-32). It has a spec. Its length +is agent 01's call, not a testability defect. + +`BigProfileStore` creates two `resource()`s in field initializers (constructor-time I/O), +which is normally a blocker — but the store is pure glue over `parseDashboardView` +(exported, spec'd) and `map`/`fromResource` (spec'd), so there is no untested decision +hiding behind the construction. §3a: 80.0% line / 77.3% branch, §3b 51% reach with the +20 unreached files being 11 `ui/` components (BL-004) and 3 pure-type `contracts/` files. + +## apps/ssp — herregistratie + +**No findings.** §3a 70.9% / 67.8%, §3b 56% reach. The four unreached files are the +`ui/` pages and wizard organisms (BL-004) plus `intake-policy.store.ts`, a thin +`resource()` wrapper over the exported-and-spec'd `parseIntakePolicy`. Both machines are +pure, Angular-free and carry four spec files between them, including an acceptance spec. +`intake.testing.ts` gives the wizard specs a fixture builder — the seam already exists. + +## apps/ssp — brief + +**TE-002 — `RevealBigNummerAdapter` hides a trust boundary inside a global-`fetch` method** + +- Module / file:line — `apps/ssp/src/app/brief/infrastructure/reveal-bignummer.adapter.ts:32-41` +- **What blocks unit testing.** The shape validation of the response body — the code's + own comment calls it a "Trust boundary" — is written inline inside `async reveal()`, + after an `await fetch(...)` on the **global** `fetch` (L24). There is no injected transport. To assert that a + `{ bigNummer: 42 }` response is rejected, a spec must stub `globalThis.fetch`; the + boundary itself is not callable. Every other `parse*` in the repo is exported (30 of + them, §7) — this one is the outlier, and it guards a PII reveal (PRD-0002 §5c). The + same shape recurs in the sibling hand-written-`fetch` adapters: + `letter-preview.adapter.ts:56-62` (`errorMessage`) and + `org-template.adapter.ts:82-89` (the proefbrief error mapping) — both un-exported, + both unreachable without a `fetch` stub. +- **Baseline citation.** §3b: `ssp/brief` **42% spec reach** (11 of 26 files) — the + lowest of any non-zero context outside `bhp/behandeling`; §3a 75.3% line / **68.8% + branch**. All three `fetch` adapters are among the 15 unreached files, and none of + them is a `ui/` component, so BL-004's Storybook carve-out does not cover them. +- **Minimal seam.** Export the pure half as a named boundary, matching the file's 30 + siblings: `export function parseRevealed(body: unknown): Result` — + five lines moved verbatim out of the method, which becomes + `return res.ok ? parseRevealed(await res.json().catch(() => null)) : err(...)`. Same + move for `errorMessage` in the other two adapters (already separate functions; they + only need `export` + a spec). No transport abstraction, no `HttpClient` migration — + the hand-written `fetch` stays, and the documented reasons for it + (`.ExcludeFromDescription()`, per-request headers) are untouched. +- **Effort S.** Independently shippable. + +## apps/ssp — showcase, shell, root + +**No findings.** + +§3b lists `ssp/root` + `ssp/shell` (with the behandelportal equivalents) at **0% reach, +12 files**. That is the correct number for what these files are: `main.ts`, +`app.config.ts`, `app.routes.ts`, `app.ts` and `shell/nav.config.ts` are composition +roots and static data — a spec asserting a provider array restates it. `debug-state` +is a dev-only devtool with a Storybook story. + +One honest note, no ticket: `shell/debug-state/mask.ts::redactProfile` is a pure, +Angular-free PII-redaction function (it maps a `BigProfile` to a redacted shape) with no +spec and **no blocker** — it is directly callable today. Its dependencies +(`maskTail`, `REDACTED`) are in `libs/shared/kernel/pii.ts`, which is spec'd at 96.4%. +Missing test, not blocked test. + +`showcase` is 100% line coverage on its one reached file (§3a); `snippets.generated.ts` +is generated and `concepts.page.ts` is a teaching page. + +## apps/behandelportal — auth + +**TE-001 applies here identically** — `apps/behandelportal/src/app/auth/application/session.store.ts:12-21`, +same `restore()`, same `localStorage` read in a field initializer, same §3a row +(`bhp/auth` 42.9% / 46.2%). Fix it in `apps/behandelportal/src/app/auth/domain/session.ts`, +which also already has a spec. Counted once as TE-001; it is two commits, or one commit +touching two apps. + +No additional findings. `medewerker.ts:17-22` reads `window.location.search` + +`sessionStorage` directly, but it is a five-line dev-only role stand-in with the same +shape as `libs/shared/infrastructure/role.ts` — whose twin `subject.ts` is spec'd, so +the pattern is demonstrably testable as written. + +## apps/behandelportal — behandeling + +**No findings.** + +§3b 31% reach (5 of 16) looks alarming and is not: the 11 unreached are 5 `ui/` files +(BL-004), the three `resource()`-wrapper stores, and the two command factories. §3a +records **91.6% line / 81.5% branch** on what is reached — the highest line coverage of +any frontend module in the table. + +Explicitly not a testability finding: `application/submit-besluit.ts` is +**structurally identical** to `apps/ssp/src/app/registratie/application/submit-change-request.ts`, +which has a spec (`submit-change-request.spec.ts`). Same `inject()` + `runSubmit` +factory, same signature shape. It is not blocked by anything; it is a missing spec whose +template already exists in the repo. Both adapters' `parse*` functions are exported and +spec'd (`beoordeling.adapter.spec.ts`, `werkvoorraad.adapter.spec.ts`). + +## apps/behandelportal — shell, root + +**No findings.** Same composition-root reasoning as `ssp/shell + root` above. + +## libs/shared — domain + +**No findings — BL-004's "genuine gap" is a false positive here, and can be closed.** + +§3b lists `libs/shared/domain` at **0% reached, 3 files**, and BL-004 names it first +among "the genuine gaps are non-`ui/` files with no spec". Reading all three files +(30 lines total): `capability.ts` is a 9-member string-literal union, `role.ts` is a +3-member union, `feature-flag.ts` is one `interface` plus one exported string constant. +**There is no executable statement in the folder.** 0% is the correct and unimprovable +number; the types are checked by `tsc` and their runtime counterparts are validated in +`parseMe` (spec'd, 94.7% infrastructure coverage). No ticket should be written against +this row. + +## libs/shared — application + +**No findings.** + +§3a 80.3% / 70.0%, §3b 73% reach (8 of 11). The three unreached are `session.port.ts` +(an `InjectionToken` + interface — a declaration, nothing to run), `feature-flags.store.ts` +and `access.store.ts`. The seam kit itself (`remote-data`, `store`, `submit`, +`history`, `pending-saves`, `machine-remote-data`, `debounced-save`) is fully spec'd — +this is the folder that makes the rest of the frontend testable. + +Noted without a ticket: `AccessStore.can()` (`access.store.ts:34-37`) is a +deny-by-default security gate whose decision reduces to +`rd.tag === 'Success' && rd.value.includes(capability)` over a `resource()` created in a +field initializer. The decision is two lines; the substance it guards +(`parseMe`, where a real silent-deny bug shipped — see the WP-66 regression test in +`me.adapter.spec.ts:26-31`) is already exported and thoroughly spec'd. Extracting a pure +`canFrom(rd, cap)` would be honest but buys close to nothing. Filing it would be volume, +not quality. + +## libs/shared — infrastructure + +**No findings.** + +§3a 94.7% line / 81.0% branch, §3b 82% reach — the second-best module in the repo. +BL-001 singles out `api-client.provider.ts:49 fetch` (CC 19) as one of only two CC>10 +functions outside the mandated idioms, so it is worth stating why it is _not_ a +testability finding: `httpClientFetch(http: HttpClient)` takes its dependency as an +ordinary function parameter (L47) rather than injecting it, and it has a spec +(`api-client.provider.spec.ts`). Its complexity is agent 01's call. The module-level +mutable `pendingIdempotencyKey` (L21) is self-clearing in a `finally` (L25), so it does +not leak between tests. + +## libs/shared — ui + +**No findings — BL-004 governs.** 34 files, 13 reached; the unreached 21 are components +covered by the Storybook + a11y strategy CLAUDE.md §5 mandates. The one non-component +module in the folder, `rich-text-editor/rich-text-dom.ts` (home of `collect`, CC 11 — +the other non-idiom CC>10 function per BL-001), **is** spec'd +(`rich-text-dom.spec.ts`). The layer is doing what the house rules ask. + +## libs/shared — layout + +**No findings.** + +§3b 18% reach (2 of 11) is the lowest non-zero row, but 8 of the 9 unreached are +components (`shell`, `page-shell`, `site-header`, `site-footer`, `breadcrumb`, +`language-switcher`, `wizard-shell`) — BL-004 applies to `layout/` exactly as to `ui/`, +since CLAUDE.md §5 titles both under `Design System/`. + +Two non-component files, neither ticketed: + +- `breadcrumb/breadcrumb-trail.ts::trailFor` is a pure exported function with a subtle + parent-walk and a `delete trail[last].link` mutation, and has no spec. **No blocker** — + it is directly callable, and its sibling `language-switcher/locale-links.ts` is the + spec'd proof. Missing test, not blocked test. +- `route-focus.ts` is a 20-line `ENVIRONMENT_INITIALIZER` wrapping a `Router` subscription + and `afterNextRender`. Genuinely awkward to unit-test, but it is a11y wiring with no + branch worth asserting; a seam here would cost more than it returns. + +## libs/shared — kernel + +**No findings.** §3a 96.4% line / 90.0% branch, §3b **100% reach** — the best module in +the repo, and (§6) the most-depended-on at I = 5% with Ca 71. Pure functions, all spec'd. +This is the reference standard the other findings point back at. + +## libs/shared — upload + +Three findings. This module carries the frontend's weakest testability profile, and +BL-010 already flags it as sitting outside the layer convention. + +**TE-003 — `UploadShellService` declares a port, then injects the concrete class instead** + +- Module / file:line — `libs/shared/src/upload/upload-shell.service.ts:12-24` and `:35` +- **What blocks unit testing.** The file defines `export interface UploadTransport` and + documents it as _the_ swap seam ("swapping it in touches only this interface", L10-11). + It then binds it as + `private transport: UploadTransport = inject(KeepaliveTransport)` (L35) — the + **concrete class**, which is `@Injectable` but **not exported** (L18). A spec that + wants a fake transport cannot reference the class to override its provider, and cannot + provide against the interface (interfaces are not DI tokens). Result: every one of + `upload()`, `delete()`, `cancel()` and `pollReturning()` — the code that translates + transport outcomes into `UploadMsg`s — is reachable only through a real + `XMLHttpRequest`. The port exists on paper and does nothing. +- **Baseline citation.** §3a: `libs/shared/upload` 52.0% line / 50.0% branch — the + worst line coverage of any module except the two `auth` rows. §3b: 50% reach, and + per the lcov file list the **two unreached files are `upload-shell.service.ts` and + `upload-controller.ts`** — neither is a `ui/` component, so this is exactly the + non-`ui/` gap BL-004 says is genuine. +- **Minimal seam.** Add the token the repo already uses elsewhere: + `export const UPLOAD_TRANSPORT = new InjectionToken('UPLOAD_TRANSPORT', +{ providedIn: 'root', factory: () => inject(KeepaliveTransport) })`, then + `inject(UPLOAD_TRANSPORT)` on L35. This **extends an existing pattern** — §7 records + exactly one explicit port in the frontend, `SessionPort` + `SESSION_PORT` + (`libs/shared/src/application/session.port.ts`), with the same interface-plus-token + shape. Runtime behaviour is byte-identical; the default factory returns the same + instance. +- **Effort S.** Independently shippable in one deploy. + +**TE-004 — `createUploadController` performs injection, DOM subscription and an `effect()` at call time** + +- Module / file:line — `libs/shared/src/upload/upload-controller.ts:23-46`, policy at `:62-74` +- **What blocks unit testing.** Calling the factory does four irreversible things before + returning: three `inject()` calls (L24-25, L46), an `effect()` registration (L31), and + `window.addEventListener('focus', onFocus)` (L45). It must therefore run inside a + `TestBed` injection context with `UploadAdapter`, `UploadShellService` and `DestroyRef` + all satisfied — and `UploadShellService` is itself un-fakeable per TE-003, so the + mocking cost compounds. What is trapped behind that cost is real policy: + `onFileSelected` (L62-74) decides per file whether to emit `FileRejected` with reason + `'multiple'`, `FileRejected` with a `rejectReason` result, or to start an upload — a + decision over `(categories, categoryId, files)` with no I/O in it. +- **Baseline citation.** §3a `libs/shared/upload` 52.0% / 50.0%; §3b 50% reach with this + file among the two unreached. §4a additionally records the module's `max CC 27` and the + repo's only two >75-line functions include `reduceUpload` (109 lines) — the reducer this + controller feeds. The reducer is spec'd (`upload.machine.spec.ts`); the code choosing + _which_ messages reach it is not. +- **Minimal seam.** Pure-function split into the file that is already the tested unit: + add `export function planFileSelection(state: UploadState, categoryId: string, files: +{ name: string; type: string; size: number }[]): UploadMsg[]` to `upload.machine.ts`, + moving L62-73 verbatim. `rejectReason` — the predicate it calls — is already exported + from that file and already spec'd, so the move is downhill. The controller keeps the + `crypto.randomUUID()` + `files.set()` + `shell.upload()` side effects and just executes + the plan. No change to the controller's public surface or to the organism that calls it. +- **Effort S.** Independently shippable. + +**TE-005 — `UploadAdapter.xhrUpload` buries response interpretation inside an `XMLHttpRequest` closure** + +- Module / file:line — `libs/shared/src/upload/upload.adapter.ts:113-157`, helpers at `:169-199` +- **What blocks unit testing.** The method constructs `new XMLHttpRequest()` directly + (L118) — no transport parameter, no injected factory — and attaches four listeners + whose bodies contain the actual decisions: 2xx-vs-not (L131), `JSON.parse` of the body + with a fallback (L132-136), ProblemDetails mapping via the un-exported `parseError` + (L193-199), and abort-vs-error disambiguation (L142-144). None of it can be reached + without stubbing the XHR global. Compounding it, the method also branches on + `currentScenario()` at L115 and returns a `setTimeout`-driven dev simulator + (`simulateUpload`, L169-192), so a dev-only fake and the production transport share + one entry point. +- **Baseline citation.** Per-file lcov: **LH 5 / LF 64 (7.8% line), BRH 3 / BRF 57 + (5.3% branch)**. The file is counted as "reached" in §3b only because another spec + imports it — essentially nothing in it executes. It is the single largest contributor + to the module's 52.0% / 50.0% row in §3a. +- **Minimal seam.** Extract the interpretation, not the transport: + `export function uploadOutcome(status: number, responseText: string): Result` containing L131-139's logic plus `parseError`. The listener + becomes a two-line dispatch into it. Optionally (same ticket, still small) move the + `currentScenario()` branch from L115 up into `KeepaliveTransport.send()` — the seam + TE-003 makes usable — so `xhrUpload` is transport only. Do **not** abstract + `XMLHttpRequest`: the file documents why XHR is required (progress events + + cancellation, which `fetch` cannot give) and that reason still holds. +- **Effort S** for `uploadOutcome` alone, **M** if the scenario branch moves too. + Independently shippable; sequence it after TE-003 if both are taken. + +## libs/shared — testing + +**No findings.** §3a 100% line coverage. `given()` (`machine.ts`) and the `RemoteData` +constructors (`remote-data.ts`) are the DSL the domain specs are built on, and the +`no-testing-in-production` dependency-cruiser rule (§6) keeps them out of shipped code. +The gap this folder does _not_ yet cover is a `resource()`-shaped fake — which is why +`AccessStore`/`BigProfileStore` stay unreached — but adding one is a test-infrastructure +task, not a source-code seam, and no metric row demands it. + +## libs/beheer + +**TE-006 — blob-to-browser handoff is inlined in three application-layer commands** + +- Module / file:line — `libs/beheer/src/application/stamdata.store.ts:137-147`; + also `apps/ssp/src/app/brief/application/brief.store.ts:230` and + `apps/ssp/src/app/brief/application/org-template.store.ts:217` +- **What blocks unit testing.** Each of the three commands ends in raw DOM/browser API + calls that jsdom cannot meaningfully execute: `StamdataStore.download()` does + `URL.createObjectURL` → `document.createElement('a')` → `a.click()` → + `URL.revokeObjectURL`; `BriefStore.previewLetter()` and + `OrgTemplateStore.proefbrief()` both do `window.open(URL.createObjectURL(blob), +'_blank')`. Because the call is the **last statement**, the entire success path of each + command is unassertable — a spec can only exercise the early-return/failure branches. + `brief.store.spec.ts` demonstrates this exactly: it tests `previewLetter`'s failure + case (which returns at the `!r.ok` guard) and cannot test the success case. In + `download()` the untestable tail sits directly behind a two-clause guard + (`if (!s || !this.canDownload()) return;`, L139), so the guard's true-branch is + permanently dark. +- **Baseline citation.** §3a: `libs/beheer/application` **40.5% branch — the worst + branch coverage of any frontend module in the table**, and its 65.7% line figure is + third-worst. Per-file lcov confirms `stamdata.store.ts` _is_ that row: LH 46 / LF 70, + **BRH 15 / BRF 37**. On the brief side, §3a `ssp/brief` is 68.8% branch and + `brief.store.ts` measures **BRH 32 / BRF 64 — exactly 50%**. +- **Minimal seam.** One small injectable in `libs/shared/src/application`, mirroring the + `SESSION_PORT` token shape already in that folder: + `export const BLOB_PRESENTER = new InjectionToken<{ open(b: Blob): void; download(b: +Blob, filename: string): void }>('BLOB_PRESENTER', { providedIn: 'root', factory: () => +realBlobPresenter })`. The three commands each lose 1-4 lines of DOM code and gain one + method call; specs provide a recording fake and finally assert the success paths + (including `toJson(...)`'s output actually reaching the file, which today is only + tested one level down in `beheer/domain`). The content-producing logic stays exactly + where it is. +- **Effort S** (one token + three one-line edits) — **M** including the specs the seam + unlocks. Independently shippable; the three call sites can also land separately. + +**Other beheer layers — no findings.** + +- `libs/beheer/contracts` — §3b lists it at **0% reached, 1 file**, and BL-004 names it + as a genuine gap. It is not: `stamdata.dto.ts` is 30 lines of `interface` and `type` + declarations with **zero executable statements** and, by design, zero imports (it is + the wire seam). Like `libs/shared/domain`, this row should be closed rather than + ticketed. +- `libs/beheer/ui` — 0% reach, 4 files, all components → BL-004 / Storybook. +- `libs/beheer/domain` — 98.1% line, spec'd machine and rules. Nothing blocked. +- `libs/beheer/infrastructure` — `parseStamdataTable`/`parseColumn`/`parseRows` all + exported and spec'd; the 60.5% branch figure is unexercised defensive arms in an + otherwise open unit. + +--- + +## backend/Program.cs + +**No findings.** + +§3c: 97.4% line / 84.8% branch, the second-best branch figure on the backend. Ten +endpoint bodies read the wall clock inline (`DateTimeOffset.UtcNow` at L282, L286, L301, +L390, L394, L426, L435, L449, L482, L931; `DateOnly.FromDateTime(DateTime.Today)` at +L138), which would normally be a finding — but **every rule and mapper they hand it to +already takes the instant as a parameter**: `HerregistratieRule.Evaluate(reg, today)`, +`ToDetailDto(a, now)`, `ToStatusDto(a, now)`, `ListCases(now)`, +`ApplicationStore.RecordBesluit(..., now)`. The clock-dependent _decisions_ are all +testable at any date; only the endpoint wiring is pinned to now, and that wiring is what +`EndpointTests`/`AdminCasesTests` legitimately cover through the host. Injecting +`TimeProvider` into 48 minimal-API lambdas would be a rewrite, not a seam, and no metric +row asks for it. + +BL-003 (940 lines, file CC 78, read/write split by comment banner) is a **structure** +finding, explicitly, and belongs to agents 03/04. + +## backend/Domain + +**TE-007 — `LetterHtml.ResolveAuto` reads the wall clock although `Render` is already given the instant** + +- Module / file:line — `backend/src/BigRegister.Api/Domain/Letters/LetterHtml.cs:138` + (resolver) vs. `:27` (signature) and `:49` (the correct usage) +- **What blocks unit testing.** `Render(BriefEntity brief, OrgTemplateDto template, +string at, bool watermark)` already accepts the letter's instant, and uses it properly + for the letterhead: `sb.Append(Enc(FormatDatumNl(at)))` at L49. But the body's + `datum` placeholder resolves through `ResolveAuto`, which ignores `at` and calls + `FormatDatumNl(DateTimeOffset.UtcNow.ToString("o"))` (L138). `ResolveAuto` is + `private static`, reached only via `RenderNode` ← `RenderParagraphs` ← `Render`, so a + test has no way to pin the value: it can only assert "whatever today is". The gap is + visible in the existing test file — `LetterHtmlTests.cs:26` declares a + `new PlaceholderDefDto("datum", "Datum", true)` in the fixture and **no assertion + anywhere in the file checks what it renders to**. This is a pure `Domain/` rule class + reaching for ambient state, which is precisely the purity §7 credits the folder with. +- **Baseline citation.** §3c: `backend/Domain` **82.0% branch** — the third-weakest + branch axis (BL-005). §4b: `Domain/Letters/LetterHtml.cs` file **CC 21**, the + third-highest-CC file in the backend after `Program.cs` (78) and + `Data/ApplicationStore.cs` (27). +- **Minimal seam.** Thread the parameter that already exists: `ResolveAuto(string key, +string label, string at)` → `"datum" => FormatDatumNl(at)`, passing `at` down through + `RenderParagraphs`/`RenderNode` (both private, both already in `Render`'s call chain + with `at` in scope). Two signature changes, one expression change, zero public API + change, zero call-site change. Then assert the rendered `datum` against a fixed + expected string in `LetterHtmlTests`. +- **Secondary benefit, stated conservatively.** This is not a shipped bug today — every + caller (`Program.cs:697`, `:708`, `BriefStore.cs:120`) passes `Now()` at render time, + so the two dates coincide. It becomes one the moment `Render` is called with a + historical `at` (re-rendering an archive, back-dating a letter), at which point the + letterhead and the body would disagree within a single document. +- **Effort S.** Independently shippable. + +No other Domain findings. §7's claim holds under inspection: `SubmissionRules`, +`DocumentRules`, `IntakePolicy`, `BeoordelingRules`, `DiplomaRules`, +`HerregistratieRule`, `OrgTemplateRules`, `Authz` and `FeatureFlags` are static classes of +pure functions with a matching file in `tests/Domain/`, and the clock-dependent ones take +their instant as an argument. That is the correct shape. + +## backend/Data + +**TE-008 — brief state-transition and authorization rules live inside DB-opening, lock-held store methods** + +- Module / file:line — `backend/src/BigRegister.Api/Data/BriefStore.cs`, five guard + clusters: `:72-76` (Save), `:88-90` (Submit), `:111-112` (Send), `:162-164` + (Approve/Reject shared path), plus the `RequiredFilled(e)` predicate +- **What blocks unit testing.** Each guard is a pure decision over + `(status tag, actor role, entity completeness)` — e.g. `Save` returns `Forbidden` if + `!isDrafter`, `Conflict` unless the status is `draft` or `rejected`, and reopens a + `rejected` letter to `draft`; `Submit` additionally requires `RequiredFilled`. But each + sits **inside** a method that has already done `lock (_gate) { using var db = +Db.Create(); ... }`, so exercising any of them requires a booted host and a real SQLite + file. There is no `BriefRules` class: `Domain/Letters/` contains only `LetterHtml.cs` + and `OrgTemplateRules.cs`. The pattern is visibly **half-applied** — `Authz.CanActOn` + at L163 _is_ a pure `Domain/` call, sitting one line away from three guards that are not. + The `Save` guard's own comment says it "mirrors the FE reducer", i.e. it is business + logic with a known pure counterpart on the other side of the wire. +- **Baseline citation.** §3c: `backend/Data` **75.5% branch** — named in BL-005 as one + of the three weak branch axes, against 99.0% line coverage (the exact signature of + "every unit is entered, edge branches are not"). §4b: `Data/BriefStore.cs` file + **CC 17**, and its `ToDto` at **CC 16** is the highest-CC non-`Program.cs` method in + the backend. §5: `backend/Data` 7.7% duplication. +- **Minimal seam.** Add `Domain/Letters/BriefRules.cs` with pure statics — + `CanSave(BriefStatusDto status, bool isDrafter) → Outcome`, + `StatusAfterSave(BriefStatusDto) → BriefStatusDto`, + `CanSubmit(status, isDrafter, bool requiredFilled) → Outcome`, `CanSend(status)`, + `CanDecide(status, Principal, drafterId)` — and have each store method call one. + The store keeps its lock, its `Db.Create()`, its static shape and its signature; only + the `if` cascade moves. This **extends the pattern §7 already records** for + `SubmissionRules` / `BeoordelingRules` / `OrgTemplateRules` / `DocumentRules`, and adds + a `tests/Domain/BriefRuleTests.cs` alongside the seven that exist. +- **Explicitly NOT proposed: changing the static-store shape.** `Data/Db.cs:6-12` + documents the static, non-DI store decision, and + `tests/TestWebApplicationFactory.cs:1-12` states the position outright — "Serializing + test classes is the fix, not a redesign of the stores for a test-only concern." + TE-008 respects that completely: it is orthogonal, and works _because_ the rules never + needed the DbContext in the first place. +- **The cost this seam actually pays down.** Because `Db.ConnectionString` is one static + field, that same file carries + `[assembly: CollectionBehavior(DisableTestParallelization = true)]` — **all 241 backend + tests run serially, process-wide**, and every brief-rule assertion currently pays a + host boot + SQLite file for a decision that is a pure function of two enums. Each rule + moved out of `BriefStore` moves a test out of the serialized integration lane into the + free-running unit lane. That is the argument for the seam; it is not an argument for + touching the stores. +- **Effort M** (five extractions + one new test file). Independently shippable, and + splittable one method at a time if preferred. + +Two smaller Data notes, neither ticketed: `IdempotencyStore` is the only store that is +purely in-memory with no `Reset()` and no TTL, so its dictionary survives +`TestWebApplicationFactory` disposal and is shared by every test class in the process — +harmless today only because `IdempotencyTests.cs:24` keys on `Guid.NewGuid()`. And +`DocumentStore.cs:54` / `AuthzAuditStore.cs:35` stamp `DateTimeOffset.UtcNow` inline +while `ApplicationStore.RecordBesluit` correctly takes `now` — an inconsistency, but +neither audit timestamp is asserted on, so no metric supports a ticket. + +## backend/Zgw + +**No findings.** §3c 98.1% line / **85.5% branch — the strongest branch figure on the +backend**, and §5 records 1.8% duplication. This is the module that was built as +ports-and-adapters from the start (ADR-0005): `IZaakSource`/`IDocumentSource` each have +two implementations (§7), `ZgwHttpClient` takes an injected `HttpClient`, and +`tests/ZgwStubHandler.cs` provides the transport fake — five test files ride on it. It +is the backend's worked example of the seam TE-003 asks the upload module for. + +## backend/Contracts + +**No testability findings — but state the gap accurately.** + +§3c records `backend/Contracts` at **65.0% branch, the worst branch figure in the repo** +(BL-005 names it first). It is nonetheless not a testability finding: `Mappers.cs` is 79 +lines of pure `static` extension methods over records, with the clock already injected +where it matters (`ToStatusDto(this Aanvraag a, DateTimeOffset now)` at `:52`, +`ToSummaryDto(..., now)` at `:68`, `ToDetailDto(..., now)` at `:76`), and `Dtos.cs` is +250 lines of `record` declarations. §4b confirms the shape: file CC 4, max method CC 3, +the lowest complexity of any backend folder. **Nothing blocks a unit test here.** + +What is missing is a test _file_: `backend/tests/` has `Domain/`, `Acceptance/` and +`Builders/` folders but no `Contracts/`, so all 65% is incidental coverage picked up +through endpoint tests. That is a coverage ticket for whoever owns coverage, requiring +zero source change — and per BL-009 there is no ratchet, so it would have to be verified +against §3c's numbers by hand. + +## backend/Stamdata + +**TE-009 — `Professions.ByProgram` freezes its valid-time filter at type-load from `DateTime.Today`** + +- Module / file:line — `backend/src/BigRegister.Api/Stamdata/Professions.cs:25-27` +- **What blocks unit testing.** `ByProgram` is a `static readonly IReadOnlyDictionary` + whose initializer runs `Mappings.Where(m => StamdataFile.ActiveOn(m.GeldigVan, +m.GeldigTot, DateOnly.FromDateTime(DateTime.Today)))`. Two compounding problems: the + peildatum is the ambient wall clock, **and** the result is computed once per process at + type-load and then immutable. A test cannot ask "which mappings are active on + 2030-01-01" — not by arranging state, not by ordering, not at all. The temporal + behaviour of the one business-tunable table that has a validity window is therefore + unreachable. The file's own comment concedes the consequence: it "preserves the + pre-valid-time behaviour exactly **while the file's rows are all current**" — i.e. the + `ActiveOn` call is presently a constant-true filter, so both of its interesting + branches (not-yet-valid, expired) are dead in every run. +- **Baseline citation.** §3c: `backend/Stamdata` 96.8% line but **71.7% branch** — + named in BL-005 as the second-weakest branch axis, an exact 25-point line/branch split. + §4b: `Stamdata/StamdataTable.cs` file CC 21, joint-third-highest in the backend. + This is the rare backend case where "untestable" is defensible against 97.6% line + coverage: the lines run, the branches provably cannot. +- **Minimal seam.** Add the parameterized overload and define the existing field in terms + of it: + `public static IReadOnlyDictionary ByProgramOn(DateOnly on) => Mappings.Where(m => StamdataFile.ActiveOn(m.GeldigVan, m.GeldigTot, on)).ToDictionary(...);` + then `public static readonly IReadOnlyDictionary ByProgram = ByProgramOn(DateOnly.FromDateTime(DateTime.Today));`. + **Zero call-site changes** — `DiplomaRules.ProfessionFor` and `All()` keep using + `ByProgram`. `StamdataValidationTests` gains the ability to assert both validity-window + branches against authored future/expired rows. +- **This extends an existing pattern in the same folder.** `StamdataTable.cs:63` already + does exactly this — `Temporal ? Rows().Where(r => ActiveOn(r, on)).ToArray() : Rows()`, + with `on` as a parameter — and `StamdataFile.ActiveOn(van, tot, on)` (`:36`) is already + clock-free. `Professions.cs` is the one caller that swallows the parameter. +- **Effort S.** Independently shippable; additive only. + +## backend/tests + +**No findings.** 39 files, 4 253 lines, 241 green tests, every backend source file +reached (§3c). The suite already carries the fixtures a unit lane needs +(`Builders/AanvraagBuilder.cs`, `ZgwStubHandler.cs`, `TestWebApplicationFactory` with +per-class throwaway SQLite files). + +The one structural observation is not a defect to fix here: +`[assembly: CollectionBehavior(DisableTestParallelization = true)]` +(`TestWebApplicationFactory.cs:12`) serializes the entire suite because +`Db.ConnectionString` is a process-global. The repo reached that decision deliberately +and documented the race it prevents. Rather than reopen it, TE-008 and TE-009 reduce how +much _needs_ to run in that serialized lane. Note also §4b's outlier in this folder: a +**293-line test method at CC 20** +(`CreateZaak_posts_zaak_status_and_rol_and_maps_the_result_back`, +`OpenZaakZaakSourceTests.cs`) — a test-readability item for agent 01, not a testability +seam. + +--- + +## Summary + +| ID | Title | Module | Blocker | Baseline | Effort | 1 deploy | +| ------ | -------------------------------------------------------------- | ----------------------- | ------------------------------------------------------------------ | ------------------------------------------------------------ | ------ | -------- | +| TE-001 | `SessionStore.restore()` reads `localStorage` inline | ssp/auth + bhp/auth | hidden I/O in a field initializer; guard is module-private | §3a 42.9%/46.2% (worst line); file LH 2/20, BRH 3/13 | S ×2 | yes | +| TE-002 | Trust boundary hidden inside a global-`fetch` method | ssp/brief | un-exported shape validation behind `await fetch` | §3b 42% reach; §3a 68.8% branch | S | yes | +| TE-003 | `UploadTransport` port declared, concrete class injected | libs/shared/upload | `inject(KeepaliveTransport)`; class not exported | §3a 52.0%/50.0%; file unreached (§3b, non-`ui/`) | S | yes | +| TE-004 | `createUploadController` injects + binds `window` at call time | libs/shared/upload | 3× `inject()`, `effect()`, `addEventListener` before returning | §3a 52.0%/50.0%; file unreached (§3b, non-`ui/`) | S | yes | +| TE-005 | `xhrUpload` interprets responses inside an XHR closure | libs/shared/upload | `new XMLHttpRequest()` hard-coded; dev simulator shares the method | file LH 5/64 (7.8%), BRH 3/57 (5.3%) | S–M | yes | +| TE-006 | Blob-to-browser handoff inlined in 3 commands | libs/beheer + ssp/brief | `window.open` / `a.click()` as the last statement of each command | §3a beheer/application 40.5% branch (worst); brief.store 50% | S–M | yes | +| TE-007 | `LetterHtml` resolves `datum` from `UtcNow`, not from `at` | backend/Domain | ambient clock in a private resolver inside a pure rule class | §3c Domain 82.0% branch; §4b file CC 21 | S | yes | +| TE-008 | Brief transition rules live inside DB-opening store methods | backend/Data | 5 pure guards behind `lock` + `Db.Create()` | §3c Data 75.5% branch (BL-005); §4b CC 17, `ToDto` CC 16 | M | yes | +| TE-009 | `Professions.ByProgram` freezes valid-time at type-load | backend/Stamdata | `static readonly` + `DateTime.Today`; both branches unreachable | §3c Stamdata 71.7% branch (BL-005) | S | yes | + +**Modules with no findings:** ssp/registratie · ssp/herregistratie · ssp/showcase+shell+root · +bhp/behandeling · bhp/shell+root · libs/shared/{domain, application, infrastructure, ui, +layout, kernel, testing} · libs/beheer/{domain, infrastructure, ui, contracts} · +backend/Program.cs · backend/Zgw · backend/Contracts · backend/tests. + +**Baseline rows recommended for closure as false gaps:** `libs/shared/domain` (0% reach, +3 files) and `libs/beheer/contracts` (0% reach, 1 file) — both named in BL-004 as genuine +gaps; both contain only type declarations and no executable statement. + +**Cross-references, not owned here:** BL-001 complexity (agent 01) · BL-002 auth +duplication (agent 06) · BL-003 `Program.cs` structure (agents 03/04) · BL-006 backend +architecture enforcement (agent 03) · BL-007 write-side placement (agent 04) · BL-008 +`coverageExclude` · BL-009 no coverage ratchet — which means none of the findings above +can be verified as "improved" by CI alone; verify against `00-baseline.md`'s numbers · +BL-010 `libs/shared/upload` layer placement (TE-003/004/005 all land inside that +carve-out and do not resolve it) · BL-011 suite flakiness under parallel load. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/03-ddd-hexagonal.md b/docs/project/refactor-backlog-setup/refactor-backlog/03-ddd-hexagonal.md new file mode 100644 index 0000000..37451f0 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/03-ddd-hexagonal.md @@ -0,0 +1,9 @@ +## Scope: [to be filled by agent] + +## Status: not_started + +## Last updated: - + +## Depends on: [see agent prompt] + +## --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/04-cqrs-light.md b/docs/project/refactor-backlog-setup/refactor-backlog/04-cqrs-light.md new file mode 100644 index 0000000..b775777 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/04-cqrs-light.md @@ -0,0 +1,543 @@ +## Scope: apps/ssp (auth, registratie, herregistratie, brief, showcase+shell+root), apps/behandelportal (auth, behandeling, shell+root), libs/shared (per layer), libs/beheer, backend (Program.cs, Domain, Data, Zgw, Contracts, Stamdata) + +## Status: complete + +## Last updated: 2026-08-26 + +## Depends on: 00-baseline.md + +## --- + +# 04 — CQRS-light: command/query separation at the application-service level + +**Mandate reminder, applied literally.** This agent may only _extend_ CQRS-light where +baseline §7 records it already exists. It may not introduce it. Every finding below names +the concrete existing artifact it extends. Three things the baseline flagged as tempting +are therefore **not** filed as tickets — they are in "Out of mandate (pattern absent)" at +the end. + +**What "the pattern" concretely is in this repo** (from §7 + CLAUDE.md §3, so later +sections can just point at it): + +| Side | The existing artifact | +| ----------- | ------------------------------------------------------------------------------------------------------------------------------------------- | +| FE query | `resource({ loader })` in an `infrastructure/*.adapter.ts` + a `parse*` boundary + a `providedIn:'root'` read store exposing `RemoteData` | +| FE command | `application/submit-*.ts` command factory → `runSubmit(fn, fallback)` → `Result` → the caller dispatches a Msg | +| FE fold | `libs/shared/src/application/submit.ts` — the single try/catch + ProblemDetails → error-string fold, **and** the Idempotency-Key mint point | +| BE | `Contracts/Dtos.cs` direction split (`*Request` in / `*Dto`+`*Response` out); read/write comment banners in `Program.cs` | +| BE read-mdl | `ToDetailDto(now)` / `ToDto(now)` — the read side _projects_ status from timestamps rather than the write side storing it | + +**The cleanest module in the repo is `bhp/behandeling`**, and it is worth naming up front +because three findings below propose making another module look like it: it splits its +query adapter (`beoordeling.adapter.ts` `get`, `werkvoorraad.adapter.ts` `list`) from its +command adapter (`besluit.adapter.ts` `besluit`) into **separate files**, wraps only the +command in a command factory (`application/submit-besluit.ts`), and keeps the read store +(`beoordeling.store.ts`, `werkvoorraad.store.ts`) write-free. Its backend counterpart does +the same: `Program.cs:441` "read side only" banner, `Program.cs:464` the write banner. That +is the target shape, and it is already in the tree. + +--- + +## apps/ssp — auth + +**No findings.** + +`SessionStore.login` (`apps/ssp/src/app/auth/application/session.store.ts:58`) is a write +and `session`/`isAuthenticated` are reads, but they share one three-field aggregate with no +network read path at all — `DigidAdapter.authenticate` is the only I/O and it is a command. +There is nothing to separate. §7 lists no command factory or read adapter in this context to +extend. BL-002 is agent 06's, not this agent's. + +--- + +## apps/ssp — registratie + +### CQ-001 — `createDraftSync` is registered as a command factory but owns three query paths + +- **Module / file:line** — `apps/ssp/src/app/registratie/application/draft-sync.ts:50-236` + (queries at `:141 load`, `:160 findConcept`, `:179 resume`; commands at `:63 ensureId`, + `:100 flush`, `:213 submit`, `:221 reset`) +- **Extends** — the command-factory idiom itself. §7 counts `draft-sync.ts` as one of the + repo's **3 command factories**, alongside `submit-change-request.ts` (18 lines, one + command, zero reads) and `submit-besluit.ts` (16 lines, one command, zero reads). This + finding asks the third member of that set to look like the other two. +- **Baseline citation** — §7 Frontend, "Command factories (write side) | 3"; §4a metric + row **`createDraftSync` 143 lines** (the largest function in the codebase, tied to + `reduceUpload`'s 109 only in the two-member `fn>75` population); §9 threshold "TS + function > 40 lines". +- **The mixing, concretely** — the factory returns four members. `resume()` is pure query + orchestration: read `?aanvraag`, `adapter.detail(linked)`, else `findConcept()` → + `adapter.list()` → `parseApplications`. `submit()`/`reset()`/the debounce `effect` are + writes. They are entangled through three pieces of shared mutable closure state — `id`, + `ensuring`, and `resumeGate` (`:56-61`) — where `resumeGate` exists _only_ so the write + path (`ensureId`) can wait for the read path (`resume`) to finish. That coupling is + genuine and load-bearing, which is exactly why it is worth naming rather than leaving as + an unexplained 143-line function. +- **Proposed change, minimal** — extract the read half into + `application/find-concept.ts`: `findConcept(adapter, type)` and `loadConcept(adapter, id)` + as free functions taking the adapter (no `inject`, so they get a direct spec — + `draft-sync.spec.ts` already exists and would shrink). `createDraftSync` keeps `resumeGate` + and the write path and calls them. This is a move, not a redesign; the closure state stays + where it is. +- **Effort** — M. Independently shippable in one deploy (no wire change, no DTO change). + +### CQ-002 — two read stores perform writes that bypass the `runSubmit` fold + +- **Module / file:line** — `apps/ssp/src/app/registratie/application/applications.store.ts:54-64` + (`cancel`) and `apps/ssp/src/app/registratie/application/admin-cases.store.ts:46-56` + (`delete`); the adapter methods are + `infrastructure/applications.adapter.ts:60 cancel` and `:41 deleteAny`. +- **Extends** — `runSubmit` + `SUBMIT_FAILED` (`libs/shared/src/application/submit.ts:15,28`), + the fold that all 16 other mutations in the repo pass through, and the + `createSubmitChangeRequest` command factory that lives _in this same folder_ + (`registratie/application/submit-change-request.ts`) and does exactly this for the other + registratie write. +- **Baseline citation** — **BL-007** ("the FE write side is inconsistently placed"); + §7 Frontend, "Command factories | 3" vs "Mutations living inline in adapters | ~13". + Note these two are a _fourteenth and fifteenth_ case BL-007 did not enumerate: they are + worse than the ~13, because those at least reach `runSubmit` inside the adapter — these + reach the raw `ApiClient` and never produce a `Result` at all. +- **The mixing, concretely** — both stores own a `RemoteData` read signal _and_ a write, and + the write's failure path is `catch { this.state.set(before); }` — a bare rollback with no + error channel. A failed cancel makes the row silently reappear with no message, no + `ActionState`, no ProblemDetails `detail`. `BriefStore`/`OrgTemplateStore` in the sibling + context both hold an `ActionState` + `lastError` for exactly this. The bare + `adapter.cancel()` also means the `Idempotency-Key` on the wire is a fresh UUID minted per + HTTP attempt by `api-client.provider.ts:58`, not the per-logical-submit key `runSubmit` + promises at `submit.ts:11-13` — that invariant's docstring is currently false for these + two calls (harmless today: `Program.cs` only honours the header inside the `Submit` helper, + see CQ-005's note). +- **Proposed change, minimal** — route both through `runSubmit` and surface the error. + Two options, pick one and apply to both stores identically: + (a) smallest — `const r = await runSubmit(() => this.adapter.cancel(id), SUBMIT_FAILED); +if (!r.ok) { this.state.set(before); this.error.set(r.error); }` plus one `error` signal; + (b) fuller — a `application/cancel-application.ts` command factory mirroring + `submit-change-request.ts`, which the store injects. (a) is enough to close the finding. +- **Effort** — S. Independently shippable; (a) touches 2 files plus a UI line each to render + the error. + +### Not filed — `applications.adapter.ts` mixes 3 reads and 5 writes in one file + +`infrastructure/applications.adapter.ts:31-66` holds `list`/`listAll`/`detail` next to +`create`/`syncDraft`/`cancel`/`deleteAny`/`submit`, where `bhp/behandeling` splits the +equivalent into `beoordeling.adapter.ts` + `besluit.adapter.ts`. The split would be the +structural enabler for CQ-002(b). On its own, though, it moves 8 thin one-line +`this.client.x()` wrappers between files and changes nothing observable — file placement is +agent 03's axis, not a read/write-mixing defect. Noted here so it is a deliberate omission +rather than a miss; fold it into CQ-002 if that ticket takes option (b). + +### Not filed — `BigProfileStore` + +`application/big-profile.store.ts` is the reference implementation of the split and needs no +change: reads are two resources projected through `parseDashboardView`, and the only +write-adjacent members are the `beginHerregistratie`/`confirmHerregistratie`/ +`rollbackHerregistratie` invalidation hooks (`:65-74`) — the store never performs the write +itself, `createDraftSync.submit` does. ADR-0001's "Out of scope" section already records the +optimistic-flag race; that is a correctness note, not a CQRS-light one. + +--- + +## apps/ssp — herregistratie + +**No findings.** + +`IntakePolicyStore` (`application/intake-policy.store.ts`) is a pure query facade over one +`resource()`. The context has no write of its own — its submit is `createDraftSync.submit`, +owned by `registratie` and covered by CQ-001. `intake.machine.ts` / `herregistratie.machine.ts` +are reducers; §3's "side effects stay out of the reducer" is honoured (verified: no `inject`, +no adapter import in either). + +--- + +## apps/ssp — brief + +### CQ-003 — `runSubmit` (the write-side fold, incl. the Idempotency-Key mint) is used for reads + +- **Module / file:line** — `apps/ssp/src/app/brief/infrastructure/brief.adapter.ts:56` + (`load` → `briefGET`), `apps/ssp/src/app/brief/infrastructure/org-template.adapter.ts:39` + (`list` → `orgTemplates`) and `:51` (`load` → `orgTemplateGET`). Same defect in + libs/beheer — see that section; one ticket should fix all five call sites. +- **Extends** — the two halves already present in `libs/shared/src/application/submit.ts`: + the ProblemDetails→`Result` **fold** (which reads legitimately want) and the + **`withIdempotencyKey` wrapper** (which is write-only by construction). The read idiom it + should join is the one the other 6 read adapters use — `resource({ loader })` + + `parse*` — or, for these imperative reads, the fold alone. +- **Baseline citation** — **BL-007**; §7 Frontend "Infrastructure adapters (read side) | 20" + and "Mutations living inline in adapters | ~13" — these three GETs are counted in the + wrong column of that inventory, because the code cannot tell them apart from the writes + beside them. +- **The mixing, concretely** — `submit.ts:11-13` states `runSubmit` is "the one place a + logical submit's Idempotency-Key is minted — once per `runSubmit` call". Each of these + reads therefore mints a UUID and assigns the module-level `pendingIdempotencyKey` + (`api-client.provider.ts:21-26`) for the duration of a GET. It is inert today: the header + is attached only when `method !== 'GET'` (`api-client.provider.ts:58`). But + `api-client.provider.ts:15-19` explicitly documents that the module-level variable "holds + up because every submit command calls its adapter synchronously (no await before reaching + this file)". Reads have no such discipline — `BriefStore.load()` and + `OrgTemplateStore.load()` are awaited across `await`s — so routing reads through the write + helper quietly widens the assumption that comment relies on. Reading `brief.adapter.ts:55-92` + it is also simply impossible to see which of the seven methods are commands: all seven are + `await runSubmit(...) → parseBriefView`. +- **Proposed change, minimal** — split `submit.ts` in place, no new concept: + `runResult(fn, fallback)` = the existing try/catch + `problemDetail` fold; + `runSubmit(fn, fallback)` = `runResult(() => withIdempotencyKey(crypto.randomUUID(), fn), fallback)`. + Point the 5 reads at `runResult`. Zero behaviour change, and afterwards the keyword at each + call site states the side. `submit.spec.ts` already exists and covers the fold. +- **Effort** — S. Independently shippable in one deploy; 1 shared file + 3 adapters (+2 in + libs/beheer). + +### Not filed — `BriefStore` and `OrgTemplateStore` own both the read path and the write commands + +`application/brief.store.ts` holds `load()` alongside `save`/`submit`/`approve`/`reject`/ +`send`/`resetDemo`/`revealBigNummer`; `application/org-template.store.ts` holds +`load`/`selectSubOrg` alongside `flushSave`/`confirmPublish`/`rollback`. BL-007 points at +these as "mutations living inline in adapters", and it is tempting to file them. + +They are deliberately not filed, and the reason matters for whoever reads this next. +CLAUDE.md §3 defines a command as "does the HTTP, then dispatches a message describing the +outcome" — and that is precisely what `BriefStore.transition()` (`:248-259`) and +`OrgTemplateStore.confirmPublish()` (`:175-189`) do: `ActionState → Busy`, cancel the +debounce, call the adapter, then `store.dispatch(...)` or `actionState.set(Failed)`. The +reducer stays pure. These stores _are_ the command layer; they are not a store that +accidentally grew writes. Both also implement the write→invalidate-read handoff correctly +(`confirmPublish` reloads via `selectSubOrg`, mirroring `BigProfileStore.confirmHerregistratie`'s +`viewRes.reload()`). Extracting six `createSubmitX()` factories out of `BriefStore` would move +code without changing which layer performs which effect. **The real defect in these two files +is CQ-003, and that is filed.** + +--- + +## apps/ssp — showcase, shell, root + +**No findings.** `showcase/concepts.page.ts`, `app.ts`, `app.config.ts`, `app.routes.ts` +contain no application services, no adapter calls and no state writes — routing, providers +and a teaching page. §7 lists no pattern here to extend. (Their 0% spec reach in §3b is +agent 02's; their `bhp/root` duplication in §5 is agent 01's.) + +--- + +## apps/behandelportal — auth + +**No findings.** Identical to ssp/auth by BL-002; the same reasoning applies. + +--- + +## apps/behandelportal — behandeling + +**No findings — this module is the reference implementation.** + +Stated positively so later phases do not "clean it up" into something worse: +`werkvoorraad.adapter.ts` (`list`) and `beoordeling.adapter.ts` (`get`) are query-only files; +`besluit.adapter.ts` (`besluit`) is a command-only file; `submit-besluit.ts` is the command +factory; `werkvoorraad.store.ts` and `beoordeling.store.ts` contain no writes at all — not +even a rollback. `besluit-form.component.ts:88` holds the command +(`private submit = createSubmitBesluit()`), never the adapter. §3a records the highest FE +line coverage of any feature context here (91.6%), which is consistent with the split: the +read stores are trivially testable because nothing writes through them. + +--- + +## apps/behandelportal — shell, root + +**No findings.** Same as ssp/shell+root. + +--- + +## libs/shared — application + +### CQ-004 — `FeatureFlagStore.set` writes without the fold and drops the error entirely + +- **Module / file:line** — `libs/shared/src/application/feature-flags.store.ts:53-59`; + adapter at `libs/shared/src/infrastructure/feature-flags.adapter.ts:18`; caller at + `libs/beheer/src/ui/feature-flags.page.ts:93`. +- **Extends** — `runSubmit`/`SUBMIT_FAILED` (`libs/shared/src/application/submit.ts`), which + lives in this very folder — the store sits three files away from the fold it skips. +- **Baseline citation** — **BL-007**; §7 Frontend "Mutations living inline in adapters | ~13" + (this is another case not in BL-007's enumeration, which named only `brief.adapter.ts`, + `org-template.adapter.ts` and `stamdata.adapter.ts`). +- **The mixing, concretely** — the store owns the read (`load`, `flags`, `all`, `enabled`) + and the admin write. The write is `try { await this.adapter.set(...) } finally { await this.load() }` + — **no `catch`**. The rejection propagates out of `set()`; the caller is + `void this.store.set(key, enabled)` (`feature-flags.page.ts:93`), so a failed toggle + becomes an unhandled promise rejection. The admin sees the switch flick back after the + `finally`'s reload with no explanation and no ProblemDetails `detail`, on a write gated by + `flags:manage` that is exactly the kind an operator needs confirmation of. Every other + write in the repo that goes through `runSubmit` gets `problemDetail(e, fallback)`. +- **Proposed change, minimal** — `const r = await runSubmit(() => this.adapter.set(key, enabled), SUBMIT_FAILED); +await this.load(); if (!r.ok) this.error.set(r.error);` with one `error` signal rendered by + `feature-flags.page.ts`. Keep the reload unconditional (it is the read-side invalidation and + is correct). +- **Effort** — S. Independently shippable; 2 files. + +### Not filed — the rest of the layer + +`access.store.ts` (query-only over `/me`), `remote-data.ts`, `store.ts`, `action-state.ts`, +`debounced-save.ts`, `history.ts`, `machine-remote-data.ts`, `pending-saves.ts`, +`session.port.ts` are the read/state kit itself, not services. `submit.ts` is the subject of +CQ-003 rather than a finding of its own. + +--- + +## libs/shared — infrastructure + +**No findings.** `me.adapter.ts` and `feature-flags.adapter.ts` are correctly +direction-labelled (`list` vs `set`); `api-client.provider.ts` is the single HTTP seam and +already gates the Idempotency-Key on `method !== 'GET'`, i.e. the _transport_ layer honours +the command/query split that CQ-003 shows the _application_ layer blurring. The CC-19 `fetch` +there is BL-001's "outside the idiom" case and belongs to agent 01. + +--- + +## libs/shared — upload + +**No findings.** + +`upload.adapter.ts` mixes reads (`categoriesResource`, `status`) and writes (`xhrUpload`, +`deleteDocument`), and `upload-shell.service.ts` mixes `upload`/`delete`/`cancel` (commands) +with `pollReturning` (a query). It is tempting to file, and it is deliberately not: **BL-010** +records that this whole folder sits outside the layer convention by design and is carved out +by name in the `apiclient-infrastructure-only` dependency-cruiser rule. §7 lists no adapter +or command factory here to extend — `upload.adapter.ts` is explicitly footnoted as the +adapter that is _not_ in an `infrastructure/` folder. Resolving BL-010 (agent 03's call) has +to come first; a read/write split layered on top of an already-exceptional layout would +entrench the exception. Worth noting for whoever takes BL-010: `UploadShellService` is +otherwise the FE's most complete command implementation — every method takes a `Dispatch` and +reports its outcome as a Msg, which is the CLAUDE.md §3 shape done exactly right. + +--- + +## libs/shared — domain, contracts, kernel, ui, layout, testing, environments + +**No findings.** No application services, no adapters, no writes. `kernel/fp.ts`'s `Result` +is the return type the command side is built on, not a service. + +--- + +## libs/beheer + +### CQ-005 — both stamdata reads run through `runSubmit`, in a file that documents itself as write-free + +- **Module / file:line** — `libs/beheer/src/infrastructure/stamdata.adapter.ts:27` (`list`) + and `:42` (`load`). +- **Extends** — the same `submit.ts` split proposed in CQ-003. **Fix them in one ticket**; + they are listed separately only because the module scope requires it. +- **Baseline citation** — **BL-007** (which names `stamdata.adapter.ts` explicitly among the + ~13 "mutations living inline in adapters"); §7 Frontend "Infrastructure adapters (read + side) | 20". +- **The mixing, concretely** — sharper here than anywhere else in the repo, because the + file's own docstring (`:16-20`) says: _"Both endpoints are reads … There is no write method + — the edit is downloaded and lands as a PR."_ Both nevertheless call `runSubmit`. BL-007 + counts this adapter on the write side of the inventory on the strength of that call, when + the module is in fact the repo's only genuinely CQRS-clean context: `StamdataStore` has no + write command at all (`download()` at `:137` is a local `Blob` + anchor click, zero + network), and `AuditStore` is read-only. The tooling and the baseline both mis-classify + this module purely because of the helper name. +- **Proposed change, minimal** — point both at `runResult` per CQ-003. Nothing else in this + library changes. +- **Effort** — S. Ships with CQ-003 in the same deploy. + +### Not filed — `libs/beheer` application/domain/ui/contracts otherwise + +`stamdata.store.ts` and `audit.store.ts` are query-only (see above). `stamdata-editor.machine.ts` +is a pure reducer. The `ui/` layer holds no adapter calls. + +--- + +## backend — Program.cs + +### CQ-006 — the read/write banner split is established, honoured once, then abandoned for 5 of 7 feature sections + +- **Module / file:line** — `backend/src/BigRegister.Api/Program.cs`. Banners at `:133` + ("GET: screen-shaped reads"), `:185` ("POST: submits"), `:441` ("read side only") and + `:464` ("record a behandelaar's decision"). Mixed sections: `:197` Document upload, + `:275` Applications, `:598` Brief, `:721` Organization templates, and admin-cases split + non-contiguously across `:424`, `:554`, `:566`. +- **Extends** — the banner convention _inside this file_, specifically the WP-65 pair at + `:441`/`:464`, which already splits one feature's query endpoints from its command endpoint + under two banners. This proposal applies the `:441`/`:464` treatment to the five sections + that predate it. **No handler types, no mediator, no `Features/` folders** — see the + out-of-mandate section for why that larger move is not proposed here. +- **Baseline citation** — **BL-003** (940 lines, 48 endpoints, file CC 78 vs next-highest 27, + "read/write separated only by comment banner"); §7 Backend CQRS-light row ("Read/write + split exists as _comment banners_"); §3c `Program.cs` 97.4% line / 84.8% branch. +- **The mixing, concretely** — the file opens by declaring direction as its organising + principle (`:133` reads, `:185` writes), then from `:197` switches to feature grouping + without saying so, and every subsequent section interleaves: + | Section | Line | Reads | Writes | + | -------------------------------------------------------------------------------------------- | ------------: | ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | + | Document upload | 197 | `GET /uploads/categories`, `GET /uploads/{id}/content`, `GET /uploads/status` | `POST /uploads`, `DELETE /uploads/{id}`, `DELETE /admin/uploads/{id}` | + | Applications | 275 | `GET /applications`, `GET /applications/{id}` | `POST`, `PUT /{id}`, `DELETE /{id}`, `POST /{id}/submit` | + | Admin cases | 424, 554, 566 | `GET /admin/cases`, `GET /admin/audit` | `DELETE /admin/cases/{id}` — **129 lines away** from its list, with werkvoorraad, beoordeling, besluit and the ZGW notification hook in between | + | Brief | 598 | `GET /brief`, `GET /brief/preview` | `PUT /brief`, `POST submit/approve/reject/send/reveal-bignummer/reset` | + | Org templates | 721 | `GET /admin/org-templates`, `GET /admin/org-template/{id}` | `PUT /{id}`, `POST /{id}/publish`, `POST /{id}/rollback` | + | `GET /admin/org-template/{subOrgId}/preview` (`:703`) is additionally filed under the Brief | + | banner rather than the Org-templates one. The consequence is not a bug — §3c confirms the | + | file is well tested, and this is a **structure finding, not a correctness one** — it is that | + | a reader cannot answer "what can mutate state here?" without reading all 940 lines, and that | + | the two cross-cutting write wrappers (`Submit`'s idempotency replay, `RecordZgwDivergence`) | + | have no visible scope. | +- **Proposed change, minimal** — within each existing feature section, order reads first then + writes and insert the `:441`/`:464`-style sub-banners; move `DELETE /admin/cases/{id}` + (`:554`) and `GET /admin/audit` (`:566`) up beside `GET /admin/cases` (`:424`); move the + org-template preview from the Brief section to the org-template one. **Pure reordering and + comments** — no signature, route, DTO or behaviour change, so §3c's 97.4%/84.8% coverage is + the regression net and the diff is reviewable line-for-line. +- **Effort** — S. Independently shippable in one deploy. One caveat for whoever schedules it: + it is a large-diff/zero-semantic-change commit, so land it alone, never mixed with a + behaviour change. + +### CQ-007 — `GET /brief` creates a brief, though the explicit create command already exists + +- **Module / file:line** — `backend/src/BigRegister.Api/Program.cs:603` + (`api.MapGet("/brief", …)`) → `backend/src/BigRegister.Api/Data/BriefStore.cs:50` + (`GetOrCreate` — `db.Briefs.Add(created); db.SaveChanges();`). +- **Extends** — the command/query direction split that `Contracts/Dtos.cs` encodes + (`*Request` in / `*Dto` out) and that the banners at `:133`/`:185` state as the file's + premise; and concretely, **`POST /brief/reset` (`:712` → `BriefStore.ResetAndCreate`)** — + the create-a-fresh-brief command already exists as a POST. The write half of `GetOrCreate` + has a command counterpart; the query does not need to duplicate it. +- **Baseline citation** — **BL-003**; §7 Backend CQRS-light row. +- **The mixing, concretely** — this is the only endpoint in the backend where a GET performs + a persisted write. Everything else respects the direction, and notably the read side goes + out of its way to _avoid_ writing: `ToDetailDto(DateTimeOffset.UtcNow)` / + `ToDto(now)` project Concept → InBehandeling → Goedgekeurd from stored timestamps on every + read rather than mutating a status column (`Program.cs:284`, `Data/AanvraagMapper.cs`), which + is a textbook CQRS read-model projection and the strongest evidence the convention is + intended. `GET /brief` breaks it: a plain read is non-idempotent on first call, allocates a + row, and — since the FE retries GETs automatically (`api-client.provider.ts:66`, + `retry({ count: 2, delay: 500 })`, GET-only, precisely because GETs are assumed safe) — + a transient failure can enter the create path more than once. `BriefStore.GetOrCreate` is + `lock`-guarded so no duplicate row results today; the objection is that the safety depends + on the lock rather than on the endpoint being a query. +- **Proposed change, minimal** — `GET /brief` returns 404 when no brief exists for the owner; + `BriefStore.GetOrCreate` splits into `Get` (query) and the existing `ResetAndCreate` (already + there). `BriefStore.load()` on the FE (`brief.adapter.ts:55`) treats 404 by calling the + existing `reset()` command once. **This is the least certain finding in this file** and the + only one with a behaviour change: it costs one extra round-trip on a first visit and touches + the brief tests. If the demo-seeding convenience is judged to outweigh the principle, the + acceptable alternative is to leave the code alone and add one line at `:603` saying the GET + seeds on first call — the defect is as much that it is undocumented as that it exists. +- **Effort** — M. Independently shippable, but FE and BE must land together (the 404 contract), + so it is the one finding here that is not a single-side deploy. + +--- + +## backend — Domain + +**No findings.** `Domain/` is static classes of pure functions (`SubmissionRules`, +`IntakePolicy`, `BeoordelingRules`, `HerregistratieRule`, `OrgTemplateRules`, `Authz`, +`FeatureFlags`, `LetterHtml`, `DiplomaRules`, `DocumentRules`) with no persistence and no I/O — +verified EF-free and ASP-free per §7. A pure decision function has no read/write axis to +separate. `Domain/Applications/Aanvraag.cs`'s `Concept`/`Submitted`/`Decided` tagged union is +the write model; §3c records 94.2% line coverage. + +--- + +## backend — Data + +**No findings within mandate.** The seven static stores (`ApplicationStore`, `DocumentStore`, +`BriefStore`, `OrgTemplateStore`, `FeatureFlagStore`, `AuthzAuditStore`, `IdempotencyStore`) +each expose reads and writes on one type — `ApplicationStore` alone has 6 reads +(`Get`/`List`/`GetAny`/`GetByReferentie`/`ListAll` + `ToDetailDto`) and 7 writes. That is +ordinary repository design, and §7 records these as "Not behind any port … Deliberate, +documented in `Data/Db.cs`". Splitting them into read/write repositories would be +_introducing_ the pattern into a module where §7 records it absent — out of mandate. See the +out-of-mandate section. One observation to hand on rather than file: `AanvraagMapper` / +`ToDetailDto(now)` is a real read-model projection and is cited approvingly in CQ-007; do not +let a future ticket "simplify" it into a stored status column. + +--- + +## backend — Zgw + +**No findings.** `OpenZaakZaakSource` / `OpenZaakDocumentSource` implement ports whose +interfaces (`IZaakSource`, `IDocumentSource`) already separate by operation +(`ListMyCases`/`ListCases` vs `CreateZaak`), and §7 records the ACL as "Fully built" under +ADR-0005. §3c gives it the second-highest branch coverage in the backend (85.5%). Nothing to +extend. + +--- + +## backend — Contracts + +**No findings.** `Contracts/Dtos.cs`'s 65 records split by direction is the backend's +strongest CQRS-light artifact and is cited as the pattern several findings above extend. +§3c's 65.0% branch coverage (the backend's weakest, per **BL-005**) is agent 02's axis, not +this one — a DTO record has no read/write mixing to fix. + +--- + +## backend — Stamdata + +**No findings.** Config-as-code tables (ADR-0004), validated at build by +`StamdataValidationTests`, never runtime-editable — read-only by definition. Its only +endpoints (`Program.cs:164`, `:173`) are both GETs behind the `StamdataAdmin` gate, correctly +placed under the reads banner at `:158`. Together with `libs/beheer` (CQ-005) this is the +cleanest end-to-end query slice in the repo. + +--- + +## Out of mandate (pattern absent) + +Filed here rather than as tickets, per the "name the pattern you extend or don't file it" +rule. Agent 08 or a human decides whether any of these becomes a ticket. + +**OOM-A — extracting `Program.cs` into `Features/` folders with handler types.** +This is the change BL-003 most obviously invites: 940 lines, 48 endpoints, file CC 78 against +a next-highest of 27. It is out of mandate because §7 is explicit that the backend has +**"No handler types, no mediator, no `Features/` folders"**, and the local helpers (`Submit`, +`StamdataAdmin`, `CasesAdmin`, `Beoordelen`, `OrgAdmin`, `FlagsAdmin`) are authorization and +idempotency wrappers, not handlers — verified by reading them at `Program.cs:772-940`. There +is no CQRS-light structure here to extend, only one to introduce. CQ-006 is the largest step +available _within_ the mandate, and it deliberately stops at ordering and comments. Note also +that CQ-006 is a strict prerequisite for OOM-A should it ever be taken: you cannot cut a +940-line file into vertical slices while five of its seven sections interleave directions. + +**OOM-B — read/write repository split in `backend/Data`.** +`ApplicationStore` (file CC 27, the highest in `Data`) and the six sibling stores each mix +reads and writes. Splitting them into query and command repositories would introduce the +pattern where §7 records it absent. It would also collide with the documented static/no-DI/ +`Db.Create()`-per-call design (§7: "Deliberate, documented in `Data/Db.cs` and +`Program.cs:40-45`"), which agent 06 may have views on. + +**OOM-C — no read model, no event sourcing, and none proposed.** +Stated explicitly so a later phase does not read this file as a step toward one. Baseline §7 +records no separate read model or event store, so per the role definition neither is in scope. +The `ToDetailDto(now)` status projection is a read-side _derivation_, not a materialised read +model, and CQ-007 argues it should stay that way. + +**OOM-D — BL-011 affects every acceptance criterion here.** +Not a finding, a scheduling note: the FE suite is flaky under parallel load, so "CI green" +alone does not verify CQ-001..005. Verify against §3a/§4a numbers, per **BL-009** (no +coverage threshold is enforced anywhere, so nothing ratchets). + +--- + +## Summary + +| ID | Module | Title | Extends | Baseline | Effort | One deploy? | +| ------ | ----------------------- | ---------------------------------------------------------------- | ----------------------------------------------------------- | -------------- | ------ | ----------------------- | +| CQ-001 | ssp/registratie | `createDraftSync` is a command factory owning 3 query paths | the command-factory idiom (`submit-change-request.ts`) | §7, §4a, §9 | M | yes | +| CQ-002 | ssp/registratie | 2 read stores write without the `runSubmit` fold; errors dropped | `runSubmit` + `createSubmitChangeRequest` | BL-007, §7 | S | yes | +| CQ-003 | ssp/brief | `runSubmit` (write fold + idempotency mint) used for 3 reads | `submit.ts` fold vs `withIdempotencyKey` | BL-007, §7 | S | yes | +| CQ-004 | libs/shared/application | `FeatureFlagStore.set` skips the fold, drops the error entirely | `runSubmit`/`SUBMIT_FAILED` (same folder) | BL-007, §7 | S | yes | +| CQ-005 | libs/beheer | 2 stamdata reads run through `runSubmit`; ship with CQ-003 | `submit.ts` fold vs `withIdempotencyKey` | BL-007, §7 | S | yes (with CQ-003) | +| CQ-006 | backend/Program.cs | read/write banner split abandoned in 5 of 7 feature sections | the `:441`/`:464` WP-65 banner pair, in-file | BL-003, §7,§3c | S | yes — land it alone | +| CQ-007 | backend/Program.cs | `GET /brief` creates; `POST /brief/reset` already exists | direction split in `Contracts/Dtos.cs`; `POST /brief/reset` | BL-003, §7 | M | **no** — FE+BE together | + +**Modules with no findings:** ssp/auth · ssp/herregistratie · ssp/showcase+shell+root · +bhp/auth · bhp/behandeling (the reference implementation) · bhp/shell+root · +libs/shared/{infrastructure, upload, domain, contracts, kernel, ui, layout, testing, +environments} · backend/{Domain, Data, Zgw, Contracts, Stamdata}. + +**Suggested sequencing.** CQ-003 + CQ-005 are one ticket (one shared-file split, five call +sites) and should go first — they make the direction legible at every call site, which is +what CQ-002 and CQ-004 then apply consistently. CQ-006 is independent and can run in +parallel on the backend. CQ-001 is the only FE ticket with real design content. CQ-007 is +the only one needing a coordinated deploy and the only one whose premise is arguable — +schedule it last, or take its documentation-only alternative. + +**Honest scale.** Six of seven findings are S/M and none is a correctness defect except +CQ-004's dropped error and CQ-002's silent rollback. Baseline §6 (0 dependency violations, +textbook instability gradient) and §3c (97.6% backend line coverage) are accurate: this is a +well-structured codebase, and the CQRS-light work available is consistency work, not repair. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/05-bdd.md b/docs/project/refactor-backlog-setup/refactor-backlog/05-bdd.md new file mode 100644 index 0000000..37451f0 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/05-bdd.md @@ -0,0 +1,9 @@ +## Scope: [to be filled by agent] + +## Status: not_started + +## Last updated: - + +## Depends on: [see agent prompt] + +## --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/06-adr-conformance.md b/docs/project/refactor-backlog-setup/refactor-backlog/06-adr-conformance.md new file mode 100644 index 0000000..cdf9cbd --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/06-adr-conformance.md @@ -0,0 +1,582 @@ +## Scope: ADR-0001…0006 (all six), checked against apps/ssp, apps/behandelportal, libs/shared, libs/beheer, backend/src/BigRegister.Api, backend/tests, e2e + +## Status: complete + +## Last updated: 2026-08-26 + +## Depends on: 00-baseline.md + +## --- + +# 06 — ADR conformance + +Every claim below was verified by reading the code, not by trusting the ADR text or +CLAUDE.md. Where the two disagree with each other **and** with reality, that is flagged +as an ADR-fix, per CLAUDE.md's own "the docs win — update this file" rule. + +**Headline.** The architecture is in good shape. All six ADRs are substantially +conformed to in their **decisions**; there is no case where the codebase has quietly +adopted a different architecture. What has decayed is the ADRs' **descriptive prose** — +four of the six describe a repo layout, a set of files, or an out-of-scope list that +WP-49…WP-75 has since overtaken. Five of the eleven findings are therefore ADR-fixes. + +**On BL-002 (the auth duplication), my verdict up front, because the brief asked for an +argument rather than an assertion:** ADR-0002's non-sharing rule **needs no amendment and +should not be relaxed**. The prediction has not _failed_; it is _unexecuted_. See +ADR-C-004/ADR-C-006 for the reasoning and for what should actually be filed instead. + +--- + +## ADR-0001 — BFF-lite endpoints + decision DTOs (Accepted) + +**Core decision: conformed.** The two policy shapes the ADR prescribes are both live and +correctly applied. + +- _Decision flag_ — verified server-computed and rendered, never recomputed: + `apps/ssp/src/app/brief/application/brief.store.ts:103-108` derives `canEdit`/ + `canApprove`/`canReject`/`canSend`/`canRevealBigNummer` purely from + `BriefState.loaded.decisions`, with an explicit comment at line 31 ("this store never + computes them itself"). Same shape in + `apps/behandelportal/src/app/behandeling/infrastructure/beoordeling.adapter.ts:87-97` + (`decisions.canBesluiten`, rejected at the parse boundary if absent). +- _Config value_ — `apps/ssp/src/app/herregistratie/domain/intake.machine.ts:52` takes + `scholingThreshold` as a parameter; `SCHOLING_THRESHOLD_DEFAULT` (line 43) survives only + as the offline fallback the ADR sanctions, wired through + `application/intake-policy.store.ts:18-19`. No hardcoded `1000` is used as authority. +- _Parse boundary_ — 30 `parse*` functions (baseline §7). ADR-0001's own "out of scope" + item _"Runtime DTO validation on every endpoint (only the dashboard view has it)"_ is + substantially discharged. +- `libs/shared/src/application/remote-data.ts` + ``, and `dep:check`'s + `ui-not-infrastructure` rule at 0 violations (baseline §6), keep the "infrastructure is + the only network layer" clause enforced — with one carve-out, ADR-C-002. + +### ADR-C-001 — ADR-0001's worked example describes a POC that no longer exists + +- **Type: ADR-fix** +- **ADR cited:** ADR-0001, §"Worked example in this POC" — opening sentence _"This POC has + no real backend (static mock JSON + fake submit timers), so the 'BFF output' is a static + file"_; §"Out of scope here", bullets 1 and 4. +- **Evidence (verified):** + - There is a real backend: `backend/src/BigRegister.Api/Program.cs` (940 lines, 48 + endpoint mappings — baseline §2/BL-003). + - Every artifact the worked example names by path is gone. `find` returns nothing for + `public/mock/dashboard-view.json`, `public/mock/intake-policy.json`, or + `src/app/herregistratie/contracts/intake-policy.dto.ts`. `apps/ssp/public/mock/` does + not exist. The surviving contract file moved to + `apps/ssp/src/app/registratie/contracts/dashboard-view.dto.ts` (WP-67's `src/` → + `apps/ssp/src/` move), so §A's three cited paths are all wrong. + - Out-of-scope bullet 1 ("runtime DTO validation on **every** endpoint — only the + dashboard view has it") is stale: 30 `parse*` boundaries exist. Out-of-scope bullet 4 + ("Real OpenAPI/TypeSpec codegen toolchain") is stale: `npm run gen:api` generates + `libs/shared/src/infrastructure/api-client.ts` (2372 lines, NSwag) and CI drift-checks + it. +- **Baseline citation:** baseline §2 (`Program.cs` 940 lines), §7 pattern inventory + (`parse*` trust boundaries = 30; excluded-as-generated api-client 2372 lines), BL-003. +- **Proposed resolution:** rewrite §"Worked example" against the shipped system and prune + the discharged out-of-scope bullets. No code changes. +- **What the amended ADR should say:** the worked example should read as _"implemented + against `backend/src/BigRegister.Api`"_, cite + `apps/ssp/src/app/registratie/{contracts/dashboard-view.dto.ts,infrastructure/dashboard-view.adapter.ts}` + and `GET /api/v1/dashboard-view` / `GET /api/v1/intake/policy` as the endpoints, and + reduce §"Out of scope" to the two items still genuinely open (the `BigProfileStore` + optimistic-update race, and session persistence / multi-tab sync). +- **Blocked code tickets:** none. This is purely descriptive drift; the decision is intact. +- **Effort: S** + +### ADR-C-002 — `libs/shared/src/upload/` does network outside `infrastructure/` + +- **Type: code-violates-ADR** +- **ADR cited:** ADR-0001, §Decision — the DTO/adapter seam; operationalised in CLAUDE.md + §4 as _"`infrastructure/` is the **only** layer that touches the network — the + anti-corruption boundary"_. +- **Evidence (verified):** `libs/shared/src/upload/upload.adapter.ts` injects `ApiClient` + (line 3 import, line 56 `inject(ApiClient)`) and opens a raw `XMLHttpRequest` at line + 118 — i.e. it is a genuine network adapter — yet sits in a top-level `upload/` folder, + not under `libs/shared/src/infrastructure/`. Its sibling `upload.machine.ts` is an + Elm-style reducer sitting outside any `domain/` folder (the only one of 9 machines to do + so). The exception is **hard-coded into the enforcement itself**: + `.dependency-cruiser.base.js:105` reads + `from: { pathNot: '/infrastructure/|^libs/shared/src/upload/' }` — the rule is written + around the violation rather than the violation being fixed, which is why baseline §6 + reports 0 violations despite this. +- **Baseline citation:** BL-010 (verbatim: the exception "is already encoded rather than + resolved"); baseline §7 FE pattern inventory ("Infrastructure adapters (read side): 20 + **+1 outside an `infrastructure/` folder**"; "Elm-style machines: 9 — 8 under a `domain/` + folder; outlier `libs/shared/src/upload/upload.machine.ts`"). +- **Proposed resolution:** move `upload.adapter.ts` → `libs/shared/src/infrastructure/`, + `upload.machine.ts` (+ its spec) → `libs/shared/src/domain/`, and + `upload-controller.ts`/`upload-shell.service.ts` → `libs/shared/src/application/`. Then + **delete the `^libs/shared/src/upload/` carve-out** from + `.dependency-cruiser.base.js:105` — that deletion is the acceptance criterion, since it + is what proves the exception is resolved rather than relocated. Note the side benefit: + `libs/shared/src/domain` currently has 0% spec reach across 3 files (baseline §3b), and + this moves a well-specced machine into it. +- **Effort: M** (mechanical move + import updates across 30 dependents — `libs/shared/upload` + has Ca 30, baseline §6 — plus the depcruise rule edit) + +### ADR-C-003 — `contracts/` vs the generated client: ADR-0001 and CLAUDE.md §4 no longer agree with the code + +- **Type: ADR-fix** +- **ADR cited:** ADR-0001, §"Why DTOs _decouple_ rather than couple" — _"Manage it with + **one source of truth** (OpenAPI or TypeSpec) that **generates types for both sides**"_ — + read against CLAUDE.md §4's flat rule _"DTO lives in `contracts/`"_. +- **Evidence (verified):** only 4 hand-written `contracts/` DTO files exist + (`apps/ssp/src/app/registratie/contracts/{brp-address,dashboard-view,duo-diplomas}.dto.ts`, + `libs/beheer/src/contracts/stamdata.dto.ts`), against 20 infrastructure adapters, 19 of + which import types from `@shared/infrastructure/api-client` directly. Crucially, the + surviving hand-written contract **documents its own obsolescence**: + `dashboard-view.dto.ts:10-11` says _"In production these types are GENERATED from the + OpenAPI/TypeSpec spec (one source of truth for both sides)"_. So the code has reached + ADR-0001's stated target state, and the hand-written `contracts/` files are the + pre-codegen scaffolding — but CLAUDE.md §4 still states the pre-codegen rule as the + standing law, which will mislead the next feature author. +- **Baseline citation:** baseline §7 FE pattern inventory — _"`contracts/` DTO files: 4 — + most adapters consume NSwag-generated types directly instead"_; agent-brief note "Only 4 + `contracts/` DTO files exist". +- **Proposed resolution:** amend ADR-0001, then correct CLAUDE.md §4 to match (CLAUDE.md's + own precedence rule requires this direction). +- **What the amended ADR should say:** add a short §"Where the contract lives, after + codegen": the generated client (`libs/shared/src/infrastructure/api-client.ts`, + regenerated by `npm run gen:api`, drift-checked in CI) **is** the wire contract and the + single source of truth; a hand-written `contracts/*.dto.ts` is warranted only where + codegen does not reach — a hand-rolled `fetch`/XHR endpoint, or a shape the generator + types too loosely — and in either case the hand-written file must still import nothing. + The `parse*` trust boundary in `infrastructure/` is **unchanged and still mandatory** + regardless of where the type came from: a generated type is a compile-time claim about + the wire, not a runtime guarantee. Then decide explicitly whether the 4 survivors stay + (they are more precise than the generated shapes) or are retired — and record which. +- **Blocked code tickets:** any ticket that would either (a) delete the 4 remaining + `contracts/` files or (b) add new hand-written DTOs for already-generated endpoints must + wait for this amendment; today CLAUDE.md §4 can be cited to justify both directions. +- **Effort: S** (ADR + CLAUDE.md edit; the follow-on code decision is separately sized) + +--- + +## ADR-0002 — User groups as actors, not bounded contexts (**Proposed**, amended WP-67) + +This is the ADR with the most divergence, and the one the baseline pointed me at. Three +findings. The core modelling decision — contexts drawn by capability, not by who logs in — +**is** conformed to: there is no `zorgverlener/` or `behandelaar/` folder anywhere, the +contexts are `registratie`/`herregistratie`/`brief`/`behandeling`/`beheer` (capability +names), and both apps integrate through one backend aggregate via decision DTOs. + +### ADR-C-004 — the `Principal` union never landed, although actor #2 did + +- **Type: code-violates-ADR** +- **ADR cited:** ADR-0002 §3 ("Separate identity from authorization") — _"Model the + principal as a **discriminated union** … This replaces the flat `Session` the day a second + actor arrives"_; §Consequences — _"The one concrete FE change when actor #2 lands is + `Session → Principal` in the `auth` context"_; §"Out of scope" — deferred _"until a second + actor is actually introduced"_. Actor #2 was introduced in WP-61 and consolidated in + WP-67. **The deferral condition has been met and the change was not made.** +- **Evidence (verified):** + - `grep -rn "Principal" apps libs --include=*.ts` returns exactly **one** hit, and it is + a comment: `libs/shared/src/infrastructure/role.ts:8`. The type does not exist in the + frontend. + - `apps/behandelportal/src/app/auth/domain/session.ts` is byte-identical to the SSP's and + still reads `interface Session { readonly bsn: string; readonly naam: string }` — a + Behandelaar carrying a BSN, which §3 names as the precise thing the union exists to make + unrepresentable. + - The backoffice login is literally the citizen login. `apps/behandelportal/src/app/auth/ui/login.page.ts:31` + is `async login(bsn: string)`, renders `intro="Log in op uw persoonlijke BIG-register +omgeving."`, and calls `SessionStore.login(bsn)` → + `auth/infrastructure/digid.adapter.ts:14`, which returns + `ok({ bsn: r.value, naam: 'Dr. A. (Anna) de Vries' })`. A behandelaar logs into the + backoffice as a zorgverlener, by DigiD, with a citizen's name in the header. + - The medewerker identity that _does_ exist bypasses the auth model entirely: + `apps/behandelportal/src/app/auth/infrastructure/medewerker.interceptor.ts:14-19` stamps + `X-Medewerker`/`X-Rollen` onto every `/api/v1/` request. It never touches `Session`. +- **Baseline citation:** BL-002 (`ssp/auth` 211/211 significant lines duplicated; + `session.store.ts` 39 windows, `login-form.component.ts` 35, `login.page.ts` 23) — the + 100% figure is the _measurement_ of this finding: the files are identical **because** the + modelling change that would differentiate them was skipped. +- **Proposed resolution:** land `Session → Principal` as ADR-0002 §3 specifies. In + `apps/behandelportal`: replace `Session` with the `medewerker` variant, replace + `DigidAdapter` with a `MedewerkerAdapter` that resolves `MEDEWERKER_ID` + `currentRollen()` + (the values `medewerker.ts` already holds) into a `Principal`, and make `login.page.ts` + an SSO-stand-in entry rather than a BSN form. In `apps/ssp`: the `zorgverlener` variant. + This makes the two `auth` contexts genuinely differ — which is what ADR-0002 §3 asserted + would happen and is the honest resolution of BL-002. +- **Effort: M** + +### ADR-C-005 — ADR-0002 is still `Proposed` after two apps shipped against it + +- **Type: ADR-fix** +- **ADR cited:** ADR-0002 header, `Status: Proposed · Date: 2026-07-01`. +- **Evidence (verified):** `apps/behandelportal` exists with 29 source files and 3 contexts + (baseline §2); the ADR has been amended once in-document (§"Amendment (WP-67, + 2026-08-01)"); its structural rulings are enforced in CI today — + `.dependency-cruiser..js`'s `-no-other-app` and `shared-no-beheer` rules run at + `severity: error` with 0 violations. An architectural decision that CI enforces is not + "Proposed". The other five ADRs are all `Accepted`, so this is an inconsistency in the + ADR set itself, not a deliberate signal. +- **Baseline citation:** baseline §7 "ADRs on record" — _"`0002` … (**Proposed**, amended + WP-67)"_; baseline §6 (11 `severity: error` rules, 0 violations, 223 modules cruised); + baseline §2 (apps/behandelportal: 29 src files, 1 309 lines). +- **Proposed resolution:** promote to `Accepted`, dated to WP-67. +- **What the amended ADR should say:** `Status: Accepted · Date: 2026-07-01 · Amended +2026-08-01 (WP-67)`. Also update §"Out of scope here", which still lists _"Building the + Behandeling backoffice application"_ and _"The backend aanvraag status lifecycle + + authorization endpoints/DTOs"_ as unbuilt — both shipped (WP-61…67; `AanvraagStatusTag`, + `GET /me` capabilities, `Domain/Authz.cs`). The one bullet that stays is real + authentication. Leave the `Session → Principal` bullet in scope but re-word it from + "deferred until a second actor is introduced" to a stated debt — it is ADR-C-004. +- **Blocked code tickets:** none strictly, but agent 08 should surface this **before** + ADR-C-004, because ADR-C-004's justification is "the ADR says to do this" and a `Proposed` + ADR is weak grounds for a refactor ticket. +- **Effort: S** + +### ADR-C-006 — extract the actor-agnostic route guards to `libs/shared` (the part of BL-002 that will never diverge) + +- **Type: code-violates-ADR** — but note carefully: it violates CLAUDE.md §2's + "composition over duplication" and the DRY intent, **not** ADR-0002 §3. ADR-0002 §3 is + about _identity and login flow_. A route guard is neither. +- **ADR cited:** ADR-0002 §3 — the scope of the non-sharing decision is `Principal` and the + login flow ("the two groups **authenticate differently**"); §Consequences names + `auth.guard.ts` and `session.store.ts` only as the _seams that localise_ the change, not + as things that must be duplicated. Read with CLAUDE.md §1's rule that a genuinely + cross-app concern belongs in `libs/shared`. +- **Evidence (verified):** `diff -ru apps/ssp/src/app/auth apps/behandelportal/src/app/auth` + reports **no content differences at all** — 9 of 11 files are byte-identical; the only + delta is two _additional_ files in behandelportal (`medewerker.ts`, + `medewerker.interceptor.ts`). Within those 9, `auth.guard.ts` is entirely actor-agnostic: + `authGuard` (lines 8-12) reads only `SessionStore.isAuthenticated()` and `Router`; + `capabilityGuard` (lines 25-33) adds only `AccessStore.can(capability)` and + `whenReady()` — and `AccessStore` already lives in `libs/shared/src/application`. + Both apps' routes redirect to the same `/login` and `/dashboard` paths + (`apps/behandelportal/src/app/app.routes.ts:10,17`). Both app configs already register + `{ provide: SESSION_PORT, useExisting: SessionStore }` + (`apps/behandelportal/src/app/app.config.ts:65`), so the seam for a shared guard exists + today. +- **Baseline citation:** BL-002 top clone pairs — `auth.guard.spec.ts` **36 windows** and + `auth.guard.ts` **21 windows**, i.e. 57 of the 211 duplicated lines, the single largest + block after `session.store.ts`. +- **Proposed resolution:** move `authGuard`/`capabilityGuard` + `auth.guard.spec.ts` to + `libs/shared/src/application/` (or a `libs/shared/src/routing/`), injecting `SESSION_PORT` + instead of the app-local `SessionStore`. One small widening is needed: + `libs/shared/src/application/session.port.ts:9-12` currently exposes only + `session: Signal<{naam: string} | null>` and `logout()` — add + `readonly isAuthenticated: Signal` (or have the guard derive it from + `session() !== null`, which both `SessionStore`s already do at + `session.store.ts:40`). Each app keeps a two-line re-export at `@auth/auth.guard` so its + `app.routes.ts` is untouched. +- **Explicitly NOT proposed, and why.** No ticket to merge `session.store.ts`, + `session.ts`, `digid.adapter.ts`, `login-form.component.ts` or `login.page.ts`, and **no + ADR-fix relaxing ADR-0002 §3.** The brief invited me to treat BL-002 as a prediction the + code failed to bear out. It is not. Those five files are identical because ADR-C-004 was + never executed — the divergence the ADR predicted has in fact already arrived, it just + arrived through an orthogonal side door (`medewerkerInterceptor`, a dev-only HTTP header + stamp) instead of through the `Principal` seam the ADR designated. Merging them now would + cement a citizen DigiD/BSN login as the backoffice's shared login, which is the one + outcome ADR-0002 §3 was written to prevent. The correct sequencing is ADR-C-005 (accept + the ADR) → ADR-C-004 (land `Principal`) → **re-measure**. My expectation is that + post-ADR-C-004 the residual `ssp/auth` ↔ `bhp/auth` duplication drops from 211 lines to + under 40 on its own. If ADR-C-004 is still unstarted at the **next backlog cycle**, that + is the point at which the ADR-fix conversation becomes legitimate — not now. +- **Effort: S** + +### Observation for agent 07 (BIO2), not a ticket here + +`medewerkerInterceptor` is registered **only** inside `isDevMode()` +(`apps/behandelportal/src/app/app.config.ts:57-63`). In a production build the backoffice +therefore sends no `X-Medewerker`/`X-Rollen` at all, and `StubIdentityProvider` falls +through to its zorgverlener default. ADR-0002 §3's "authorization enforced at the backend +boundary" holds structurally, but the behandelportal's _identity_ has no non-dev path. The +ADR lists real employee SSO as out of scope, so this is not an ADR conformance defect — +flagging it because it is the kind of thing a compliance pass should see stated, and +because ADR-C-004 is the natural place to close it. + +--- + +## ADR-0003 — CIBG Huisstijl (Bootstrap 5.2) as the design system (Accepted) + +**Core decision: conformed.** All five decision points hold. The package is vendored at +`public/cibg-huisstijl/` (`css`, `fonts`, `icons`, `images` present, licensed RO/Rijks text +fonts absent per point 5); the token bridge is intact at `libs/shared/styles.scss`; atoms +emit Bootstrap classes with their `input()` APIs preserved; `check:tokens` +(`scripts/check-tokens.sh`) runs in `npm run ci`; the gap register exists at +`libs/shared/docs/cibg-gaps.mdx` with 9 `// CIBG-GAP EXTENSION:` markers in code. + +### ADR-C-007 — ADR-0003's file paths and its `app-alert` example are both stale + +- **Type: ADR-fix** +- **ADR cited:** ADR-0003 §Decision point 1 (`src/index.html`), point 2 (`src/styles.scss`), + point 4 (_"CIBG omits Bootstrap's `.alert` and `.navbar`, so `app-alert` is a small + token-styled surface"_), §Consequences (`.storybook/`, `src/docs/cibg-gaps.mdx`). +- **Evidence (verified):** + - Every path moved in WP-67: `src/styles.scss` → `libs/shared/styles.scss`; + `src/index.html` → `apps/ssp/src/index.html` **and** `apps/behandelportal/src/index.html` + (two now, not one); `.storybook/` → `.storybook-ssp/` and `.storybook-behandelportal/`; + `src/docs/cibg-gaps.mdx` → `libs/shared/docs/cibg-gaps.mdx`. + - Point 4's `app-alert` claim is **factually wrong about the current code**. + `libs/shared/src/ui/alert/alert.component.ts:13-16` documents itself as a _"Thin wrapper + over the vendored `.feedback feedback-*` classes: the design system owns surface + icon"_, + and the template (lines 31-37) binds `.feedback-info`/`.feedback-success`/ + `.feedback-warning`/`.feedback-error`. Its only local CSS is a 3-line flex fix. It is not + hand-rolled and carries no `CIBG-GAP` marker — correctly, since it is not a gap. + - **CLAUDE.md §2 repeats the same stale claim** verbatim: _"(Where CIBG lacks a class — + e.g. `alert` — the atom is a small hand-rolled surface built from the token bridge; see + ADR-0003.)"_ Both documents must be corrected, ADR first. +- **Baseline citation:** baseline §2 (the monorepo layout the paths must be rewritten + against: apps/ssp, apps/behandelportal, libs/shared, libs/beheer); baseline §7 config-seam + tokens (`HEADER_NAV_ITEMS`/`HEADER_ADMIN_LINKS`/`DEBUG_PANEL`) — the WP-67 two-app split + these paths belong to. +- **Proposed resolution:** repoint all five paths; replace the `.alert` example in point 4. +- **What the amended ADR should say:** point 4 should keep the principle ("hand-roll what + CIBG's build drops, and mark it") but pick a live example — `skeleton` and `spinner` are + the cleanest (both in the register, both genuinely absent from the vendored build) — and + note that `.alert` was subsequently resolved onto the vendored `.feedback` classes, so + it is no longer a gap. CLAUDE.md §2's parenthetical must be corrected in the same diff. +- **Blocked code tickets:** none. +- **Effort: S** + +### ADR-C-008 — the CIBG gap register is one row behind the markers in code + +- **Type: code-violates-ADR** (the artifact the ADR mandates is incomplete; the fix is a + one-row doc edit, not a code change) +- **ADR cited:** ADR-0003 §Consequences, final bullet — _"Hand-rolled components (point 4) + are tracked in the **CIBG gap register** … every deviation from the design system carries + a `// CIBG-GAP EXTENSION:` marker so it's auditable rather than silently drifting."_ +- **Evidence (verified):** 9 files carry a `CIBG-GAP` marker + (`debug-state`, `language-switcher`, `wizard-shell`, `application-link`, + `placeholder-chip`, `rich-text-editor`, `skeleton`, `spinner`, `status-badge`); the + register table at `libs/shared/docs/cibg-gaps.mdx:26-33` has **8** rows. + `libs/shared/src/layout/language-switcher/language-switcher.component.ts:7-9` carries a + full, well-formed marker (`"Taal instellen" … no vendored Huisstijl class ships for it … +See cibg-gaps.mdx`) and has no corresponding row. The register's own §"Keeping this + register honest" concedes there is no automated diff and instructs _"If markers and this + table drift, trust the code and fix the table"_ — so this is exactly the drift it + predicted, caught by review as designed. +- **Baseline citation:** baseline §2 (libs/shared 86 src files / 5 194 lines — the surface + the register must cover); baseline §6 (`libs/shared/src/layout` is a cruised module with + Ca 22). +- **Proposed resolution:** add the `language-switcher` row (CIBG concept: "Taal instellen"; + reason: no vendored class in this build). Optionally add the CI script the register + declines — a ~10-line `grep -l CIBG-GAP | diff` in `scripts/` folded into `check:tokens` + would make the drift impossible to reintroduce. I would file the row as the ticket and + the script as an explicitly optional second step, matching the register's own + proportionality argument. +- **Effort: S** + +--- + +## ADR-0004 — Stamdata as code (Accepted) + +**Core decision: conformed.** `backend/src/BigRegister.Api/Stamdata/` holds 15 files — +typed C# (`Beroep.cs`, `PolicyQuestions.cs`, `Professions.cs`, `StamdataCatalog.cs`, +`StamdataTable.cs`, `StamdataFile.cs`) plus the checked-in JSON data-files +(`beroepen.json`, `documentconfidentialiteit.json`, `opleidingen.json`, +`professions.json`, `specialismen.json`) the WP-29 follow-on introduced. Coverage is 96.8% +line (baseline §3c). The `beheer/stamdata` editor is read-only-plus-download, not a write +path, exactly as the ADR's own WP-29 note states. There is **no** runtime write endpoint +for any stamdata table. + +### ADR-C-009 — feature flags are a second runtime-editable exception the ADR does not acknowledge + +- **Type: ADR-fix** +- **ADR cited:** ADR-0004 §Decision — _"Never a production database, never runtime-editable"_ + — and §"The deliberate exception: org-templates", which names **one** exception in the + singular and justifies it narrowly ("specific to one sub-organization's identity"). +- **Evidence (verified):** `backend/src/BigRegister.Api/Data/FeatureFlagStore.cs` is a + second admin-writable SQLite surface, added by WP-47 — after ADR-0004 (2026-07-20) — and + **its own doc-comment states the equivalence the ADR does not**: _"Runtime feature-flag + state (WP-47). SQLite-backed like `OrgTemplateStore`, same single-gate idiom."_ It exposes + `Set(key, enabled)` writing `db.FeatureFlags`, surfaced through the admin page + `/beheer/functies` (`apps/behandelportal/src/app/app.routes.ts:44-50`, gated by + `capabilityGuard('flags:manage')`). +- **Assessment — and why this is an ADR-fix, not a violation.** The design is genuinely + ADR-0004-shaped, not a breach of it: the _catalog_ (which flags exist, their descriptions + and defaults) is compiled-in code (`Domain/Features/FeatureFlags.Catalog`), only the + boolean override persists, and `IsEnabled` fails closed for an unknown key + (`FeatureFlagStore.cs:45-47`), so a bad DB row cannot invent a flag. That is the ADR's + actual principle — schema and values gated at compile time — applied correctly. What is + wrong is the ADR's _text_: it states the rule as a closed list of one exception, which + means the next operational-config surface has no principle to test itself against and + will either be waved through or blocked on a technicality. +- **Baseline citation:** baseline §7 backend pattern inventory — the 7 static stores listed + as "Not behind any port", which includes both `OrgTemplateStore` **and** + `FeatureFlagStore`; baseline §3c (`backend/Stamdata` 96.8% line / 71.7% branch; + `backend/Data` 99.0% / 75.5%). +- **Proposed resolution:** amend ADR-0004. No code change; the code is right. +- **What the amended ADR should say:** replace §"The deliberate exception: org-templates" + with §"The deliberate exception: operational configuration", stating the **test** rather + than a list — runtime-editable persistence is permitted only when (1) the catalog/schema + of what may be set lives in code, (2) an unknown or unlisted key is rejected/fails closed, + (3) the value is operational (per-organisation identity, an on/off rollout switch) and not + a shared business rule whose wrong value breaks the register for everyone, and (4) writes + are admin-capability-gated and audited. Then list the two surfaces that pass it today — + `OrgTemplateStore` (WP-23/26) and `FeatureFlagStore` (WP-47) — and note that both are + admin-gated. Add a matching sentence to CLAUDE.md §4, which currently repeats the + singular framing ("Org-templates are the deliberate exception"). +- **Blocked code tickets:** any ticket proposing a third runtime-editable config surface + should wait for this test to be written down, rather than arguing by analogy to + org-templates. +- **Effort: S** + +--- + +## ADR-0005 — OpenZaak (ZGW APIs) behind the BFF (Accepted) + +**Conformed, with no findings.** This is the cleanest ADR in the set and I am recording +that plainly rather than manufacturing a ticket. + +Verified point by point: + +- The anti-corruption layer is in the .NET BFF and nowhere else. Every ZGW type lives under + `backend/src/BigRegister.Api/Zgw/` (8 files: `OpenZaakZaakSource`, + `OpenZaakDocumentSource`, `ZgwHttpClient`, `ZgwTokenProvider`, `ZgwZaakMapper`, + `ZgwOptions`, `ZgwDiagnosticHandler`, `NotificatieDto`). `grep` for + `OpenZaakZaakSource|OpenZaakDocumentSource` outside `Zgw/` returns only the two DI + registrations in `Program.cs:67,69` and eight explanatory comments — no ZGW shape reaches + a consumer. +- The config-switched port pair is exactly as decided: `Program.cs:59-84` reads + `Zgw:Enabled` and binds either `OpenZaakZaakSource`/`OpenZaakDocumentSource` (via + `AddHttpClient`, with the WP-60 15s timeouts) or `LocalZaakSource`/`LocalDocumentSource`. + Default is local, so the POC still runs fully offline as the ADR's second constraint + requires. +- The FE is untouched by the switch: consumers inject the interface only + (`Program.cs:206,281,340,425,434,446,473`), and the same `ApplicationSummaryDto` is + returned either way — zero DTO/api-client drift, matching the ADR's first `+`. +- The ADR's stated **minus** is still accurate and honestly scoped: `Program.cs:465` + confirms _"The local write runs against ApplicationStore directly (not the IZaakSource + seam)"_, which is precisely the "only some endpoints have a source interface; each future + slice introduces its own seam" consequence the ADR wrote down. An ADR that predicted its + own remaining gap and the gap stayed where predicted is conformance, not drift. +- `backend/Zgw` is the best-covered backend module at 98.1% line / 85.5% branch (baseline + §3c) and the second-lowest duplication (1.8%, baseline §5) — the seam the ADR claimed + would be "unit-testable without a live server" measurably is. + +Note for agent 03: BL-006 (no NetArchTest/ArchUnitNET, one assembly) means nothing +_enforces_ the "ZGW shapes never leave `Zgw/`" property that this ADR depends on. It holds +today by convention. That is agent 03's ticket to size, not mine — I record only that +ADR-0005's conformance is currently review-maintained, not CI-maintained. + +--- + +## ADR-0006 — Test data through the production door (Accepted) + +**Core decision: partially conformed.** Three of the five mechanisms are properly in place; +the frontend replay idiom — the ADR's own flagship — is adopted in one spec out of nine. + +Conformed: + +- §1 backend type-state builder — `backend/tests/BigRegister.Tests/Builders/AanvraagBuilder.cs` + exists and is used in 5 test files (`Acceptance/BesluitLifecycleTests.cs`, + `Acceptance/IntakeSubmissionTests.cs`, `ApplicationTests.cs`, + `Domain/BeoordelingRuleTests.cs`, `OpenZaakZaakSourceTests.cs`) — the exact two files the + ADR's Context named as the problem, plus three more. +- §4 `RemoteData` named constructors — `libs/shared/src/testing/remote-data.ts` exists. +- §5 e2e actors/seed-refs — `e2e/support/actors.ts` exists; no page-object layer was added. +- No `'x' as BrandedType` cast appears in any spec (`grep` for + `as Postcode|as Uren|as BigNummer|as Bsn` in `*.spec.ts`: zero hits) — the §3 illegal + route is genuinely closed. + +### ADR-C-010 — four machine specs hand-roll the exact state literal ADR-0006 §2 forbids + +- **Type: code-violates-ADR** +- **ADR cited:** ADR-0006 §2 (_"No object is built directly. A fixture is the result of + running real `Msg`s through the real `reduce`"_) and the Decision table's "Pure reducer / + state machine (frontend)" row: **Do not** — _"A literal returning `{ tag: 'Editing', ... }` + by hand"_; plus §Consequences (_"a hardcoded `errors: {}` fixture literal can't drift from + what validation actually produces"_). +- **Evidence (verified) — each of these returns a state literal, and three of the four + hardcode the `errors: {}` the ADR calls out by name:** + - `apps/ssp/src/app/herregistratie/domain/intake.machine.spec.ts:19-25` — + `const answering = (...): IntakeState => ({ tag: 'Answering', answers, cursor, errors: {}, scholingThreshold })`. + This is the worst instance: `apps/ssp/src/app/herregistratie/domain/intake.testing.ts` + exists **in the same folder**, exports `givenIntake`, and is the ADR's own quoted example + (§2's code block is `export const givenIntake = given(reduce, initial)`) — yet + `givenIntake` is imported only by `intake.acceptance.spec.ts`, never by the machine spec + it was written for. + - `apps/ssp/src/app/registratie/domain/registratie-wizard.machine.spec.ts:25-31` — + `const invullen = (...): RegistratieState => ({ tag: 'Invullen', draft: {...}, cursor, errors: {}, upload: initialUpload })`. + - `apps/behandelportal/src/app/behandeling/domain/besluit.machine.spec.ts:5-9` — + `const editingWith = (...): BesluitState => ({ tag: 'Editing', draft: {...}, errors: {} })`. + - `apps/ssp/src/app/brief/domain/brief.machine.spec.ts:67-74` — + `const loaded = (...): BriefState => ({ tag: 'loaded', brief, availablePassages, decisions })`. + - For contrast, two specs **do** conform and should be the pattern to copy: + `libs/beheer/src/domain/stamdata-editor.machine.spec.ts:17-23` (`seedLoaded()` = + `reduce(initial, {tag:'Loaded', …})`) and + `apps/ssp/src/app/brief/domain/org-template.machine.spec.ts:29-30` + (`reduce({tag:'loading'}, {tag:'DraftLoaded', …})`). Only 1 of 9 machines has a + `*.testing.ts`; `change-request.machine.spec.ts:6` honours the idiom but declares + `given(reduce, initial)` inline in the spec rather than in a `*.testing.ts`. +- **Baseline citation:** baseline §7 FE pattern inventory (Elm-style machines: 9); + baseline §3a — `ssp/herregistratie` 70.9% line / 67.8% branch and `ssp/brief` 75.3% / + 68.8%, i.e. the two modules whose specs hand-roll states are also two of the three + weakest-covered non-auth FE modules, which is consistent with fixtures asserting shapes + the reducer may not actually produce. +- **Proposed resolution:** add `*.testing.ts` next to each of the four machines + (`registratie-wizard`, `brief`, `besluit`, plus adopt the existing `intake.testing.ts`), + each a one-liner `export const givenX = given(reduce, initial)` per §2, and rewrite the + four literal helpers as message replays. Where a state genuinely is not reachable by + replay, that is a finding in its own right and should be recorded in the ticket rather + than worked around — it means the reducer cannot produce a state the spec asserts. +- **Effort: M** (four specs; mechanical but each needs the right message sequence worked out, + and `brief.machine.spec.ts` is the largest) + +### ADR-C-011 — `unwrapOk` has zero adopters; its one call site reimplements it inline + +- **Type: code-violates-ADR** +- **ADR cited:** ADR-0006 §3 (_"Value objects → `unwrapOk`, never a cast"_) and the Decision + table's "Value object / parser" row. +- **Evidence (verified):** `unwrapOk` is defined in + `libs/shared/src/testing/value-object.ts` and referenced nowhere else in `apps/` or + `libs/` except `libs/shared/docs/testing.mdx` — zero spec consumers. The one place that + needs it hand-rolls the same three lines: + `apps/ssp/src/app/registratie/application/submit-change-request.spec.ts:8-9` reads + `const telefoon = parseTelefoonnummer('0612345678'); if (!telefoon.ok) throw new Error('fixture phone should parse');` + — semantically identical to `unwrapOk(parseTelefoonnummer('0612345678'))`, which is + exactly the duplication the ADR shipped the helper to remove. +- **Baseline citation:** BL-004 (122 of 220 FE source files never loaded by any Vitest run + — `libs/shared/src/testing` is one of the few modules at 100% reach, so a helper here that + nothing imports is measurably dead weight, not merely unexercised); baseline §3a + `libs/shared/testing` 3 files, 100% line. +- **Proposed resolution:** one-line change in `submit-change-request.spec.ts` to call + `unwrapOk`. Then judge honestly whether one call site justifies keeping the helper — if a + future ticket finds it still has one consumer, deleting it and keeping the inline guard is + the equally valid answer, and ADR-0006 §3's real requirement (never a cast) is satisfied + either way. File it as "adopt or delete", not "adopt". +- **Effort: S** + +--- + +## ADR-fix tickets (require architect approval) + +Agent 08 must surface these five for human sign-off. None of them are code changes; all +five change what the repo's architecture documents _claim_, and two of them gate code +tickets. + +| ID | ADR | What the amendment does | Gates | Effort | +| ------------- | ---- | ------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- | ------ | +| **ADR-C-001** | 0001 | Rewrite the worked example against the real backend; drop the two discharged out-of-scope bullets | nothing | S | +| **ADR-C-003** | 0001 | State that the generated client **is** the wire contract post-codegen; scope `contracts/` to codegen gaps; correct CLAUDE.md §4 | any ticket deleting the 4 surviving `contracts/*.dto.ts`, or adding a hand-written DTO for a generated endpoint | S | +| **ADR-C-005** | 0002 | `Proposed` → `Accepted`; refresh §"Out of scope" (backoffice + status lifecycle + authz DTOs all shipped) | **ADR-C-004** — a `Proposed` ADR is weak grounds for a refactor ticket, so this must land first | S | +| **ADR-C-007** | 0003 | Repoint five WP-67-stale paths; replace the false `app-alert` hand-rolled example; correct CLAUDE.md §2 | nothing | S | +| **ADR-C-009** | 0004 | Generalise "the org-templates exception" into a stated four-part test; list both passing surfaces; correct CLAUDE.md §4 | any future third runtime-editable config surface | S | + +**Ordering dependency, called out explicitly as the brief requires:** ADR-C-005 → ADR-C-004 +→ (re-measure BL-002) → possibly-nothing. ADR-C-006 is deliberately **not** in this chain: +extracting the actor-agnostic route guards is compatible with ADR-0002 §3 as written and +needs no approval. **No ADR-fix is proposed against ADR-0002 §3's non-sharing rule.** I +considered it, as instructed, and rejected it: the rule's prediction has not been falsified, +it has not been tested, because the change that would test it (ADR-C-004) was never made. +Amending an ADR to match code that never executed its decision would ratify the omission +rather than the evidence. + +--- + +## Summary + +| ADR | Verdict | Findings | +| -------- | --------------------------------------------------- | --------------------------------------- | +| **0001** | Decision conformed; prose stale | ADR-C-001 (fix), 002 (code), 003 (fix) | +| **0002** | Decision conformed; §3 unexecuted; still `Proposed` | ADR-C-004 (code), 005 (fix), 006 (code) | +| **0003** | Conformed; two documentation defects | ADR-C-007 (fix), 008 (code/doc) | +| **0004** | Conformed; exception clause under-general | ADR-C-009 (fix) | +| **0005** | **Fully conformed — no findings** | — | +| **0006** | Partially conformed; §2 adopted 1 of 9 | ADR-C-010 (code), 011 (code) | + +Six code tickets (1×M, 1×M, 1×M, 3×S) and five ADR-fixes (all S). No finding proposes +introducing a pattern that does not already exist in the repo, and every one cites a +baseline observation or metric row. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md b/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md new file mode 100644 index 0000000..37451f0 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md @@ -0,0 +1,9 @@ +## Scope: [to be filled by agent] + +## Status: not_started + +## Last updated: - + +## Depends on: [see agent prompt] + +## --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md new file mode 100644 index 0000000..e69de29 diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md new file mode 100644 index 0000000..e952668 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md @@ -0,0 +1,13 @@ +# Agent run status + +| Agent | Status | Last module processed | Last updated | Notes | +| --------------- | ----------- | -------------------------- | ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| baseline | complete | all (18 FE + 7 BE modules) | 2026-08-26 | 0 layering violations. 11 observations BL-001..BL-011. Thresholds for Phase 1 in §9 — agent 01's `[N] lines` = 40 (TS fn) / 60 (C# method) / 400 (file). Read BL-001, BL-002 and BL-004 before filing complexity, duplication or coverage findings. | +| readability | not_started | - | - | | +| testability | complete | all 24 modules | 2026-08-26 | 9 findings TE-001..TE-009; 15 modules explicit "no findings". Corrected BL-004 — `libs/shared/domain` and `libs/beheer/contracts` are pure type declarations, 0% is unimprovable (amendment note in 00-baseline.md §10). TE-009 is also a latent correctness bug (dead `ActiveOn` branches). Acquitted `createDraftSync`, `httpClientFetch`, `Contracts/Mappers.cs` in writing. | +| ddd-hexagonal | not_started | - | - | | +| cqrs-light | complete | all (16 FE + 6 BE modules) | 2026-08-26 | 7 findings CQ-001..CQ-007; 12 modules clean. Corrected BL-007 (see the amendment note in 00-baseline.md §10) and found 3 mutations the baseline missed. `Program.cs` → `Features/`+handlers filed as out-of-mandate OOM-A, not a ticket; CQ-006 is its prerequisite. `bhp/behandeling` named the reference implementation — do not "clean it up". | +| bdd | not_started | - | - | | +| adr-conformance | complete | all 6 ADRs | 2026-08-26 | 11 findings: 6 code, **5 ADR-fix (architect approval required)**. Sharpened BL-002 — `Principal` was never built, so ADR-0002 was untested not falsified (amendment note in 00-baseline.md §10). ADR-0005 fully conformed. Gates: ADR-C-005→ADR-C-004; ADR-C-003 gates contracts/ cleanup. 3 ADR-fixes need a matching CLAUDE.md correction in the same diff. | +| bio2-compliance | not_started | - | - | | +| consolidation | not_started | - | - | | diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/00-baseline.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/00-baseline.prompt.md new file mode 100644 index 0000000..f9e6bac --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/00-baseline.prompt.md @@ -0,0 +1,46 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/00-baseline.md +DEPENDS ON: none + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +ROLE: Metrics Baseline Agent + +Before any refactoring suggestions, establish a baseline for the scoped codebase: +- Test coverage (line/branch) per module, .NET and Angular separately. +- Cyclomatic complexity per method/function (flag >10). +- Duplication percentage (tool-based, e.g. jscpd/SonarQube if configured). +- Dependency graph / layering violations (existing static analysis if present). +- Count and location of existing CQRS-light and hexagonal architecture patterns + already in use (so later agents compare against actual current state, not + assumed absence). + +Output: a metrics table per module, plus a short list of modules ranked +worst-to-best on each metric. This file is fixed input to every Phase 1 agent — +no agent may propose a change without citing which baseline metric it improves. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/01-readability.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/01-readability.prompt.md new file mode 100644 index 0000000..b5e3c6b --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/01-readability.prompt.md @@ -0,0 +1,39 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/01-readability.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: Readability Agent + +Junior = fluency in language constructs, not domain knowledge. Assume familiarity +with generics, async/await, LINQ, DI, RxJS operators, TS type system — do NOT flag +idiomatic use of these as "unreadable". Flag only: unclear naming, methods/components +exceeding [N] lines, nesting >3 levels, magic values, misleading types, missing guard +clauses. Cite baseline complexity score per finding. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/02-testability.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/02-testability.prompt.md new file mode 100644 index 0000000..a304df5 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/02-testability.prompt.md @@ -0,0 +1,38 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/02-testability.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: Testability Agent + +Flag constructs that block unit testing without excessive mocking: static/singleton +dependencies, hidden I/O, constructors doing work, mixed pure/impure logic. Cite +baseline coverage gap per finding. Propose the minimal seam needed (interface +extraction, pure function split) — not a rewrite. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/03-ddd-hexagonal.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/03-ddd-hexagonal.prompt.md new file mode 100644 index 0000000..ba28367 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/03-ddd-hexagonal.prompt.md @@ -0,0 +1,40 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/03-ddd-hexagonal.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: DDD/Hexagonal Agent + +Target architecture: hexagonal (ports/adapters), already partially present in the +codebase per baseline.md — treat that as the pattern to extend, not reinvent. Do NOT +introduce hexagonal structure into modules where it is absent; only propose closing +gaps where the pattern is already started. Flag anemic domain models, domain logic +leaked into controllers/services/components, primitive obsession, missing ubiquitous +language. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/04-cqrs-light.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/04-cqrs-light.prompt.md new file mode 100644 index 0000000..51057b1 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/04-cqrs-light.prompt.md @@ -0,0 +1,39 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/04-cqrs-light.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: CQRS-light Agent + +Target: command/query separation at the application-service level (not event +sourcing or separate read models unless already present per baseline.md). Only +extend existing CQRS-light patterns — do not introduce the pattern into modules +where it's absent. Identify handlers/services mixing reads and writes within +modules that already show the pattern elsewhere. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/05-bdd.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/05-bdd.prompt.md new file mode 100644 index 0000000..d463b1f --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/05-bdd.prompt.md @@ -0,0 +1,38 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/05-bdd.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: BDD Agent + +Check whether existing tests express behavior in domain/business language mapped +to acceptance criteria, or only technical steps. Flag test names/structure gaps. +Do not propose new BDD tooling if none is present — flag as a separate structural +item instead, not a per-module ticket. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/06-adr-conformance.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/06-adr-conformance.prompt.md new file mode 100644 index 0000000..71f2765 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/06-adr-conformance.prompt.md @@ -0,0 +1,40 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/06-adr-conformance.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: ADR-Conformance Agent + +Read all ADRs/docs in the repo. Compare code against each. Two outcomes per +deviation: +(a) code violates a correct ADR → refactoring ticket, cite ADR. +(b) ADR itself appears outdated/wrong given current code or constraints → propose + an ADR amendment as a separate ticket type ("ADR-fix"), with rationale — not + a code ticket. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/07-bio2-compliance.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/07-bio2-compliance.prompt.md new file mode 100644 index 0000000..5d3f9e4 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/07-bio2-compliance.prompt.md @@ -0,0 +1,48 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/07-bio2-compliance.md +DEPENDS ON: 00-baseline.md (complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- + +AGENT: BIO2/Compliance Agent + +No explicit control list supplied — using the following BIO2/ISO 27002:2022 +controls, selected for privacy and security relevance. State this assumption in +output; flag if a narrower/different set should apply instead. + +- Access control (9.1, 9.2, 9.4): authorization checks, RBAC, least privilege. +- Logging & monitoring (8.15, 8.16): audit trails, esp. BIG-register/DUO data access. +- Data classification & handling (5.12, 5.13): BSN, health data, AVG-sensitive fields. +- Cryptography (8.24): encryption at rest/in transit. +- Secure development (8.25, 8.28, 8.29): secure coding, review, security testing gates. +- Change control (8.32): deployment register / change approval exceptions. +- Input validation (8.26): boundary validation on public-facing forms/APIs. + +Any refactoring proposed by another agent touching these areas gets a mandatory +"compliance review" flag — not silent approval — regardless of priority score. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/08-consolidation.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/08-consolidation.prompt.md new file mode 100644 index 0000000..8f382b2 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/08-consolidation.prompt.md @@ -0,0 +1,59 @@ +MODEL: Opus +OUTPUT FILE: /refactor-backlog/99-backlog.md +DEPENDS ON: 01-readability.md through 07-bio2-compliance.md (all complete) + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- +Phase 1 file changed since last run] + +ROLE: Consolidation & CD-Sequencing Agent + +Input: all Phase 1 files (01–07) + 00-baseline.md. + +1. Deduplicate overlapping findings across agents — merge into one ticket, list all + contributing reasons/agents. +2. Score priority: + P1 = violates a correct ADR, blocks testability, or is a BIO2 compliance risk. + P2 = significant maintainability cost, moderate effort. + P3 = low urgency. +3. Sequence for continuous delivery: every ticket must be independently deployable + without a big-bang release. Reject/split any ticket that can't ship alone — + decompose into a dependency chain of smaller tickets. +4. Any ticket touched by the BIO2 agent requires compliance sign-off before merge, + regardless of priority score — mark explicitly. +5. Output final table: + +| ID | Module | Category | Description | Baseline metric improved | Effort (S/M/L) | +Risk | Priority | CD batch # | Depends on | Status | + +6. Separately list "ADR-fix" tickets — require human/architect approval before any + dependent code ticket proceeds. + +HALT CONDITION: after writing 99-backlog.md, stop and report to the human for +approval before any Implementation Agent (Phase 3) starts — even if no tickets +carry a compliance or ADR-fix flag. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/09-implementation.prompt.md b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/09-implementation.prompt.md new file mode 100644 index 0000000..ca8de86 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/final-prompts/09-implementation.prompt.md @@ -0,0 +1,57 @@ +MODEL: Sonnet +OUTPUT: status update in /refactor-backlog/99-backlog.md + + /refactor-backlog/implementation/[ticket-id].md +DEPENDS ON: ticket status = not_started, no unresolved compliance/ADR-fix flag, + all tickets in "Depends on" column = implemented or needs_review + +--- + +PERSISTENCE & RESUME PROTOCOL + +Before starting work: +1. Read /refactor-backlog/_status.md. If your row says "complete", stop — do not re-run. +2. If "in_progress", read your own output file. Treat modules already listed as done. + Resume from "Last module processed" + 1. +3. If "not_started", confirm your dependencies show "complete" in _status.md. If not, + stop and report a blocking dependency instead of guessing. + +While working: +4. Append findings incrementally, one module at a time. After each module, update + _status.md: "Last module processed" and "Last updated". +5. Each finding gets a stable ID (e.g. RD-014) that never changes across runs. +6. If interrupted, the file + status row is the full recovery state. + +On completion: +7. Mark your _status.md row "complete" only once every module in scope has a + corresponding section in your output file. + +Every output file starts with: +## Scope: [modules covered] +## Status: [not_started | in_progress | complete] +## Last updated: [timestamp] +## Depends on: [file(s)] +## --- +module list] + +AGENT: Implementation Agent + +Input: one ticket from 99-backlog.md (fill in TICKET-ID below), the Phase 1 +file(s) that produced it, and 00-baseline.md. + +TICKET-ID: [fill in before dispatching this agent] + +Scope discipline: +- Implement exactly the change described in the ticket. No scope expansion, no + incidental fixes. +- If the ticket is ambiguous or underspecified for implementation, do not guess — + write a blocker note to the ticket's status and stop. +- Do not modify architecture/pattern decisions (hexagonal boundaries, CQRS-light + structure) — those are Opus-level design calls already made in the ticket. If + implementation reveals the design call was wrong, flag back to Consolidation + rather than deciding unilaterally. +- Tickets touching a BIO2-flagged item are blocked until human compliance + sign-off is recorded in the ticket status — do not implement first and flag + after. + +Update ticket status in 99-backlog.md: not_started → in_progress → implemented +→ needs_review. Append implementation notes to implementation/[ticket-id].md. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-006.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-006.md new file mode 100644 index 0000000..0b20dca --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/adr-c-006.md @@ -0,0 +1,58 @@ +# ADR-C-006 — extract the actor-agnostic route guards to `libs/shared` + +Status: **implemented** · 2026-08-26 · Source finding: `06-adr-conformance.md` §ADR-C-006 + +## What changed + +| File | Change | +| ----------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| `libs/shared/src/application/auth.guard.ts` | **new** — `authGuard` + `capabilityGuard`, injecting `SESSION_PORT` instead of an app-local `SessionStore` | +| `libs/shared/src/application/auth.guard.spec.ts` | **new** — the single spec, provides `SESSION_PORT`; one case added asserting the guard reads the port | +| `libs/shared/src/application/session.port.ts` | widened by one member: `readonly isAuthenticated: Signal` | +| `apps/ssp/src/app/auth/auth.guard.ts` | now a re-export | +| `apps/behandelportal/src/app/auth/auth.guard.ts` | now a re-export | +| `apps/{ssp,behandelportal}/src/app/auth/auth.guard.spec.ts` | **deleted** — both were byte-identical to the new shared spec | + +Neither `app.routes.ts` was touched: both still `import { authGuard, capabilityGuard } +from '@auth/auth.guard'`. Routing asks the auth context for its guards, which is the +direction the boundary should read. + +## Why the port widening was free + +Both `SessionStore`s already exposed `readonly isAuthenticated = computed(() => +this._session() !== null)` (`session.store.ts:40` in each app), and both apps already +registered `{ provide: SESSION_PORT, useExisting: SessionStore }` +(`app.config.ts:64` / `:65`). `SessionPort` is satisfied structurally, so adding the +member required no change in either app — the seam existed, it was just narrower than +what it already carried. + +## Scope discipline + +Only the guards moved. Per the finding, **no** ticket to merge `session.store.ts`, +`session.ts`, `digid.adapter.ts`, `login-form.component.ts` or `login.page.ts`, and no +relaxation of ADR-0002 §3. Those five are identical because ADR-C-004 (`Session → +Principal`) was never executed; merging them would cement a citizen DigiD/BSN login as +the backoffice's shared login, which is the outcome §3 exists to prevent. + +## Measured effect + +Re-ran `tools/baseline-scan.mjs --dup` after the change: + +| Metric | Before | After | +| --------------------------- | -----: | --------: | +| `ssp/auth` duplicated lines | 211 | **151** | +| `bhp/auth` duplicated % | 86.8% | **82.5%** | +| Repo-wide duplication | 7.1% | **6.6%** | + +The `auth.guard.spec.ts` (36 windows) and `auth.guard.ts` (21 windows) clone pairs have +dropped out of the top-clones list entirely. The remaining `ssp/auth` ↔ `bhp/auth` +duplication is `session.store.ts` (39), `login-form.component.ts` (35) and `login.page.ts` +(23) — exactly the three ADR-C-004 is expected to differentiate. Re-measure BL-002 after +that lands; the finding's expectation is a drop to under 40 lines. + +## Verification + +`npm run lint` · `npm run typecheck` · `npm run dep:check` (0 violations, 224 modules) +· `npx prettier --check apps libs` — all clean. +Tests: shared 122, ssp 235, behandelportal 27, beheer 23 — **407 passed, 0 failed**. +`ng build --localize` for both apps. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/tools/baseline-scan.mjs b/docs/project/refactor-backlog-setup/refactor-backlog/tools/baseline-scan.mjs new file mode 100644 index 0000000..d8cbed3 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/tools/baseline-scan.mjs @@ -0,0 +1,255 @@ +#!/usr/bin/env node +// Baseline scanner for 00-baseline.md: duplication % and C# cyclomatic complexity. +// Both are the same crude text scan, so they share one file. +// +// ponytail: line-window hashing, not token-based like jscpd, and regex method +// detection that does not understand C# expression-bodied members or nested +// lambdas. Deterministic and zero-install, which is what a *baseline* needs. +// Upgrade path: jscpd for duplication, a Roslyn analyzer for C# complexity — +// only if a ticket's before/after needs more precision than "did it move". +// +// Usage: node baseline-scan.mjs [--dup] [--complexity] (default: both) + +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { createHash } from 'node:crypto'; +import { join, relative } from 'node:path'; + +const ROOT = new URL('../../../../..', import.meta.url).pathname.replace(/\/$/, ''); +const SKIP_DIR = + /(^|\/)(node_modules|dist|coverage|bin|obj|\.angular|\.git|storybook-static.*|TestResults|public|openzaak)$/; +const SKIP_FILE = /(api-client\.ts|\.Designer\.cs|AppDbContextModelSnapshot\.cs)$/; +const SCAN = ['apps', 'libs', 'backend/src', 'backend/tests', 'e2e']; +const WINDOW = 6; // duplicate = >= 6 consecutive normalized lines seen elsewhere + +function walk(dir, out = []) { + for (const e of readdirSync(dir)) { + const p = join(dir, e); + if (SKIP_DIR.test(p)) continue; + if (statSync(p).isDirectory()) walk(p, out); + else if (/\.(ts|cs)$/.test(p) && !SKIP_FILE.test(p)) out.push(p); + } + return out; +} + +// Module attribution: the unit the baseline table reports on. +function moduleOf(rel) { + let m; + if ((m = rel.match(/^apps\/(ssp|behandelportal)\/src\/app\/([^/]+)\//))) + return `${m[1] === 'ssp' ? 'ssp' : 'bhp'}/${m[2]}`; + if (rel.startsWith('apps/ssp/')) return 'ssp/root'; + if (rel.startsWith('apps/behandelportal/')) return 'bhp/root'; + if ((m = rel.match(/^libs\/(shared|beheer)\/src\/([^/]+)\//))) return `libs/${m[1]}/${m[2]}`; + if (rel.startsWith('libs/')) return rel.split('/').slice(0, 2).join('/'); + if (rel === 'backend/src/BigRegister.Api/Program.cs') return 'backend/Program.cs'; + if ((m = rel.match(/^backend\/src\/BigRegister\.Api\/([^/]+)\//))) return `backend/${m[1]}`; + if (rel.startsWith('backend/tests/')) return 'backend/tests'; + if (rel.startsWith('e2e/')) return 'e2e'; + return 'other'; +} + +const files = SCAN.flatMap((d) => { + try { + return walk(join(ROOT, d)); + } catch { + return []; + } +}).map((p) => ({ abs: p, rel: relative(ROOT, p) })); + +// --- Duplication ------------------------------------------------------------ +// Normalize away formatting/comments, hash every WINDOW-line run, and mark a run +// duplicated when its hash occurs in more than one place. +function duplication() { + const norm = new Map(); // rel -> [{line, text}] + for (const f of files) { + const kept = []; + readFileSync(f.abs, 'utf8') + .split('\n') + .forEach((raw, i) => { + const t = raw + .replace(/\/\/.*$/, '') + .replace(/\s+/g, ' ') + .trim(); + if (t.length > 3) kept.push({ line: i + 1, text: t }); + }); + norm.set(f.rel, kept); + } + + const seen = new Map(); // hash -> [{rel, idx}] + for (const [rel, lines] of norm) + for (let i = 0; i + WINDOW <= lines.length; i++) { + const h = createHash('sha1') + .update( + lines + .slice(i, i + WINDOW) + .map((l) => l.text) + .join('\n'), + ) + .digest('hex'); + (seen.get(h) ?? seen.set(h, []).get(h)).push({ rel, idx: i }); + } + + const dupLines = new Map(); // rel -> Set(line) + const clones = new Map(); // "relA|relB" -> count + for (const occ of seen.values()) { + if (occ.length < 2) continue; + for (const { rel, idx } of occ) { + const set = dupLines.get(rel) ?? dupLines.set(rel, new Set()).get(rel); + for (let k = 0; k < WINDOW; k++) set.add(norm.get(rel)[idx + k].line); + } + const pair = [...new Set(occ.map((o) => o.rel))].sort(); + if (pair.length > 1) { + const key = pair.slice(0, 2).join(' | '); + clones.set(key, (clones.get(key) ?? 0) + 1); + } + } + + const per = new Map(); // module -> {total, dup} + for (const [rel, lines] of norm) { + const mod = moduleOf(rel); + const e = per.get(mod) ?? per.set(mod, { total: 0, dup: 0 }).get(mod); + e.total += lines.length; + e.dup += dupLines.get(rel)?.size ?? 0; + } + + console.log('## Duplication (normalized lines, window=' + WINDOW + ')\n'); + console.log('| Module | Sig. lines | Duplicated | % |'); + console.log('|---|---:|---:|---:|'); + let T = 0, + D = 0; + for (const [mod, e] of [...per].sort((a, b) => b[1].dup / b[1].total - a[1].dup / a[1].total)) { + T += e.total; + D += e.dup; + console.log(`| ${mod} | ${e.total} | ${e.dup} | ${((100 * e.dup) / e.total).toFixed(1)}% |`); + } + console.log(`| **TOTAL** | **${T}** | **${D}** | **${((100 * D) / T).toFixed(1)}%** |`); + + console.log('\n### Top clone pairs (distinct duplicated windows)\n'); + for (const [pair, n] of [...clones].sort((a, b) => b[1] - a[1]).slice(0, 15)) + console.log(`- ${n} × — ${pair}`); +} + +// --- C# cyclomatic complexity (approximate) --------------------------------- +// Two numbers per file. FILE CC (sum of decision points) is exact enough to trust. +// Per-METHOD CC uses depth-aware regex detection and is the approximate one. +const BRANCH = + /\bif\s*\(|\bwhile\s*\(|\bfor\s*\(|\bforeach\s*\(|\bcase\s+|\bcatch\s*[({]|\?\?|&&|\|\||\?\.|\bwhen\s+/g; +const TYPE_DECL = + /^\s*(?:\[[^\]]*\]\s*)*(?:public|private|internal|protected|static|sealed|abstract|partial|file|\s)*\b(?:class|record|struct|interface|enum|namespace)\b/; +const NOT_A_CALL = + /^(if|for|foreach|while|switch|catch|using|lock|return|throw|new|await|yield|else|do|fixed|checked)$/; +// A member: optional attrs/modifiers, a return type, a name, then `(`. +const SIG = + /^\s*(?:\[[^\]]*\]\s*)*(?:(?:public|private|internal|protected|static|async|override|virtual|sealed|partial|extern|new|unsafe)\s+)*[\w<>,\[\]?.]+\s+(\w+)\s*(?:<[^>()]*>)?\s*\(/; + +function branchesIn(line) { + return (line.replace(/\/\/.*$/, '').match(BRANCH) ?? []).length; +} + +function csComplexity() { + const rows = []; + const fileCc = []; + for (const f of files.filter((f) => f.rel.endsWith('.cs'))) { + const lines = readFileSync(f.abs, 'utf8').split('\n'); + let depth = 0, + cur = null, + total = 1, + code = 0; + for (let i = 0; i < lines.length; i++) { + const l = lines[i]; + const bare = l.replace(/\/\/.*$/, ''); + const b = branchesIn(l); + total += b; + if (bare.trim().length > 1 && !/^\s*(\/\/|\/\*|\*)/.test(l)) code++; + + const opens = (bare.match(/{/g) ?? []).length; + const closes = (bare.match(/}/g) ?? []).length; + + if (cur) { + cur.lines++; + cur.cc += b; + // Expression-bodied member: `=> expr;` with no block of its own. + if (cur.depth === null && /=>/.test(bare)) { + if (/;\s*$/.test(bare) && opens === closes) { + rows.push(cur); + cur = null; + } else if (opens > closes) cur.depth = depth; + } else if (cur.depth === null && opens > closes) cur.depth = depth; + else if (cur.depth !== null && depth + opens - closes <= cur.depth) { + rows.push(cur); + cur = null; + } + } else if (!TYPE_DECL.test(l)) { + const m = SIG.exec(bare); + // `Name(` must not be a call/keyword, and the line must not be a statement. + if ( + m && + !NOT_A_CALL.test(m[1]) && + !/^\s*(var|return|await)\b/.test(bare) && + !/;\s*$/.test(bare.replace(/=>.*/, '')) + ) + cur = { + file: f.rel, + name: m[1], + line: i + 1, + cc: 1 + b, + lines: 1, + depth: opens > closes ? depth : null, + }; + } + depth += opens - closes; + } + if (cur) rows.push(cur); + fileCc.push({ file: f.rel, cc: total, code }); + } + + const p = (arr, q) => + arr.slice().sort((a, b) => a - b)[Math.min(arr.length - 1, Math.floor(q * arr.length))] ?? 0; + + const perFile = new Map(); + for (const r of fileCc) { + const mod = moduleOf(r.file); + (perFile.get(mod) ?? perFile.set(mod, []).get(mod)).push(r); + } + console.log('\n\n## C# complexity — per module\n'); + console.log( + '| Module | Files | Σ file CC | max file CC | Methods | max method CC | p90 method CC | CC>10 |', + ); + console.log('|---|---:|---:|---:|---:|---:|---:|---:|'); + const perMethod = new Map(); + for (const r of rows) { + const mod = moduleOf(r.file); + (perMethod.get(mod) ?? perMethod.set(mod, []).get(mod)).push(r); + } + for (const [mod, fs] of [...perFile].sort( + (a, b) => Math.max(...b[1].map((r) => r.cc)) - Math.max(...a[1].map((r) => r.cc)), + )) { + const ms = perMethod.get(mod) ?? []; + const ccs = ms.map((r) => r.cc); + console.log( + `| ${mod} | ${fs.length} | ${fs.reduce((a, r) => a + r.cc, 0)} | ${Math.max(...fs.map((r) => r.cc))} | ` + + `${ms.length} | ${ccs.length ? Math.max(...ccs) : 0} | ${p(ccs, 0.9)} | ${ms.filter((r) => r.cc > 10).length} |`, + ); + } + + console.log('\n### C# files by CC (top 12)\n'); + console.log('| File CC | Code lines | File |'); + console.log('|---:|---:|---|'); + for (const r of fileCc.sort((a, b) => b.cc - a.cc).slice(0, 12)) + console.log(`| ${r.cc} | ${r.code} | ${r.file} |`); + + console.log('\n### C# methods over CC 10 (approximate detection)\n'); + console.log('| CC | Lines | Method | Location |'); + console.log('|---:|---:|---|---|'); + for (const r of rows.filter((r) => r.cc > 10).sort((a, b) => b.cc - a.cc)) + console.log(`| ${r.cc} | ${r.lines} | \`${r.name}\` | ${r.file}:${r.line} |`); + + const all = rows.map((r) => r.lines); + console.log( + `\nMethod-length distribution (n=${rows.length}): p50 ${p(all, 0.5)}, p90 ${p(all, 0.9)}, p99 ${p(all, 0.99)}, max ${Math.max(...all)}`, + ); +} + +const args = process.argv.slice(2); +const all = args.length === 0; +if (all || args.includes('--dup')) duplication(); +if (all || args.includes('--complexity')) csComplexity(); diff --git a/docs/project/refactor-backlog-setup/setup.sh b/docs/project/refactor-backlog-setup/setup.sh new file mode 100755 index 0000000..5e8c2b1 --- /dev/null +++ b/docs/project/refactor-backlog-setup/setup.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# Sets up the multi-agent refactoring backlog workspace. +# Run from the root of the target repo: bash setup.sh +set -euo pipefail + +WORK_DIR="./refactor-backlog" +AGENTS_SRC="$(dirname "$0")/agents" +PROTOCOL="$AGENTS_SRC/_persistence-protocol.md" + +echo "Setting up $WORK_DIR ..." +mkdir -p "$WORK_DIR/implementation" +mkdir -p "$WORK_DIR/final-prompts" + +# 1. Initialize _status.md +cat > "$WORK_DIR/_status.md" << 'EOF' +# Agent run status + +| Agent | Status | Last module processed | Last updated | Notes | +|---|---|---|---|---| +| baseline | not_started | - | - | | +| readability | not_started | - | - | | +| testability | not_started | - | - | | +| ddd-hexagonal | not_started | - | - | | +| cqrs-light | not_started | - | - | | +| bdd | not_started | - | - | | +| adr-conformance | not_started | - | - | | +| bio2-compliance | not_started | - | - | | +| consolidation | not_started | - | - | | +EOF +echo " created _status.md" + +# 2. Initialize empty output files with headers +for f in 00-baseline 01-readability 02-testability 03-ddd-hexagonal \ + 04-cqrs-light 05-bdd 06-adr-conformance 07-bio2-compliance; do + cat > "$WORK_DIR/${f}.md" << EOF +## Scope: [to be filled by agent] +## Status: not_started +## Last updated: - +## Depends on: [see agent prompt] +## --- +EOF +done +touch "$WORK_DIR/99-backlog.md" +echo " initialized 8 phase-1 output files + 99-backlog.md" + +# 3. Assemble final prompts: inject persistence protocol into each agent prompt +# at the "[Insert contents of _persistence-protocol.md here...]" marker. +for src in "$AGENTS_SRC"/*.prompt.md; do + name="$(basename "$src")" + out="$WORK_DIR/final-prompts/$name" + awk -v protofile="$PROTOCOL" ' + /\[Insert contents of _persistence-protocol\.md here/ { + while ((getline line < protofile) > 0) print line + close(protofile) + next + } + { print } + ' "$src" > "$out" + echo " assembled final-prompts/$name" +done + +echo "" +echo "Done. Structure:" +echo " $WORK_DIR/_status.md (run tracker — all agents not_started)" +echo " $WORK_DIR/00-baseline.md ... 07-bio2-compliance.md (empty, agent-writable)" +echo " $WORK_DIR/99-backlog.md (empty, Consolidation writes here)" +echo " $WORK_DIR/implementation/ (Phase 3 notes land here)" +echo " $WORK_DIR/final-prompts/ (ready-to-dispatch prompts, protocol" +echo " already merged in — no manual copy-paste)" +echo "" +echo "Next: dispatch final-prompts/00-baseline.prompt.md first (blocks everything)," +echo "then the 7 Phase 1 prompts in parallel, then 08-consolidation.prompt.md," +echo "then per-ticket 09-implementation.prompt.md (fill in TICKET-ID each time)."