# RB-33 — `unwrapOk`: adopt or delete Status: **implemented** · 2026-08-28 · Source finding: `06-adr-conformance.md` ADR-C-011 · `99-backlog.md` RB-33 ## Decision: delete The ticket names this "adopt or delete", not "adopt", and asks for the judgment call, not the default. I deleted `unwrapOk`. ## Why delete, not adopt `unwrapOk` (`libs/shared/src/testing/value-object.ts`) has had zero consumers across the whole codebase since ADR-0006 shipped it, except its own definition and one sentence in `libs/shared/docs/testing.mdx`. I verified this before changing anything: ``` grep -rn "unwrapOk" apps libs --include=*.ts --include=*.mdx libs/shared/docs/testing.mdx:92: ...unwrapOk(parseX(raw))... libs/shared/src/testing/value-object.ts:9:export function unwrapOk(...) libs/shared/src/testing/value-object.ts:11: throw new Error(`unwrapOk: ...`); ``` The one call site the finding names, `apps/ssp/src/app/registratie/application/submit-change-request.spec.ts`, still has the exact hand-rolled guard the finding quotes: ```ts const telefoon = parseTelefoonnummer('0612345678'); if (!telefoon.ok) throw new Error('fixture phone should parse'); ``` I also checked whether any other spec has the same shape, in case the finding's "one call site" undercounted the real duplication: ``` grep -rln "if (!.*\.ok)\s*throw" apps libs --include=*.spec.ts apps/ssp/src/app/registratie/application/submit-change-request.spec.ts ``` Only this one file, anywhere. There is no cast (`'x' as Telefoonnummer`) to close off either — the spec already calls the real `parseTelefoonnummer` and checks `.ok` before touching `.value`. ADR-0006 §3's actual requirement ("never a cast") is already met by the inline code, with or without the helper. Weighing it honestly: - **For adopt:** it is a one-line change, and the ADR's own worked example literally shows this exact call. Doing it would make the finding's "zero adopters" claim technically false. - **For delete:** a helper that gains its _only_ real-codebase consumer by an agent adding that one call site as an act of ticket compliance is not organic adoption — it is manufacturing a usage to justify keeping the file. `unwrapOk` has sat available, exported, and documented since ADR-0006 (well before this session) without a single spec reaching for it on its own. One caller, forever, is not "removing duplication" (the stated point of a shared test helper) — there is no duplication with only one occurrence. The inline guard is also arguably clearer here: its error message (`'fixture phone should parse'`) names the actual fixture, where `unwrapOk`'s generic message (`unwrapOk: expected ok, got error: ...`) does not. Delete wins: it removes dead, unadopted code and its stale doc reference, changes no runtime behaviour anywhere, and costs nothing to reverse if a second real need for this idiom shows up later (three lines, trivial to re-add against actual duplication instead of a single hypothetical site). ## What changed | File | Change | | ------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `libs/shared/src/testing/value-object.ts` | Deleted. Its only export, `unwrapOk`, is what this ticket removes; the file had nothing else in it. | | `libs/shared/docs/testing.mdx` | Rewrote the sentence that named `unwrapOk` and the deleted file's path. It now states the same rule in plain terms — call the real `parse*` and check `.ok`, never a cast — and keeps the `RemoteData` half of the sentence pointing at `remote-data.ts` (unchanged, still in use). | | `apps/ssp/src/app/registratie/application/submit-change-request.spec.ts` | **Not touched.** Its inline guard already satisfies ADR-0006 §3; this is the "delete" branch, so the fixture-construction behaviour stays exactly as it was. | | `99-backlog.md` | RB-33's status cell: `open` → `implemented`. | ## What this ticket did not touch `docs/reference/architecture/0006-test-data-builders.md` (the ADR itself) still shows `unwrapOk` in its worked example and decision table. That is deliberate: RB-33 is a code ticket, not one of the five ADR-fix tickets that need architect sign-off (`06-adr-conformance.md`'s "ADR-fix tickets" section). The ADR's illustrated pattern ("call the real parser, unwrap through a checked path, never a cast") is still the correct principle — this ticket only removes one now-unused concrete implementation of it, which the inline guard in `submit-change-request.spec.ts` already satisfies without the named helper. Amending the ADR's own text is out of this ticket's scope and is left for a future ADR-fix ticket if one is ever raised. The finding document (`06-adr-conformance.md`) and the historical WP-70/WP-71 backlog notes that mention `unwrapOk` are left as-is — they are records of what was true when written, not living code. No other file in `libs/shared/src/testing/` was touched (`expect-tag.ts`, `machine.ts`, `remote-data.ts` are all unrelated and still have real consumers). ## Verification - `grep -rn "unwrapOk" apps libs --include=*.ts --include=*.mdx` — zero occurrences. - `apps/ssp/src/app/registratie/application/submit-change-request.spec.ts` — unchanged file, still passes (see `npm run ci` result below). - No new test added. The ticket is a deletion of unused code plus a doc-sentence rewrite; the surviving inline guard in the spec is exercised the same way it always was, by the spec's three existing `it` blocks. - `npm run ci` (foreground): see the session report for the exit code and step count.