From a8c7a573fca0b8297be2208263ac8430e24e1eaa Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Fri, 4 Sep 2026 18:45:04 +0200 Subject: [PATCH] refactor: move SaveState into debounced-save.ts, delete action-state.ts (RD-14) RD-12 and RD-13 moved both ActionState consumers into their machines. This left ActionState with zero users, so this ticket deletes it outright. SaveState is different. It has two four-way consumers (brief.page.ts and org-template.page.ts) that still switch on all four cases, so it survives. It moves into debounced-save.ts, next to createDebouncedSave, the only function that sets it. Both store imports merge into the existing debounced-save import line. The doc comments that named ActionState are reworded, since the type no longer exists. No store's public saveState signature changes, so brief/ui/ needs no edit. This completes the phase. Two encodings survive: RemoteData for fetched data, and each machine's own state union. SaveState stays as an explicitly justified third encoding, for a separate concern (debounced autosave status) that the other two do not cover. Co-Authored-By: Claude Sonnet 5 --- .../src/app/brief/application/brief.store.ts | 3 +- .../brief/application/org-template.store.ts | 3 +- ...D-14-move-savestate-delete-action-state.md | 126 ++++++++++++++++++ docs/project/readable-codebase/README.md | 2 +- libs/shared/src/application/action-state.ts | 9 -- libs/shared/src/application/debounced-save.ts | 7 +- 6 files changed, 135 insertions(+), 15 deletions(-) create mode 100644 docs/project/readable-codebase/RD-14-move-savestate-delete-action-state.md delete mode 100644 libs/shared/src/application/action-state.ts diff --git a/apps/ssp/src/app/brief/application/brief.store.ts b/apps/ssp/src/app/brief/application/brief.store.ts index 83c04bf..a556947 100644 --- a/apps/ssp/src/app/brief/application/brief.store.ts +++ b/apps/ssp/src/app/brief/application/brief.store.ts @@ -1,9 +1,8 @@ import { Injectable, computed, inject, signal } from '@angular/core'; import { Result } from '@shared/kernel/fp'; import { createStore } from '@shared/application/store'; -import { SaveState } from '@shared/application/action-state'; import { createHistory } from '@shared/application/history'; -import { createDebouncedSave } from '@shared/application/debounced-save'; +import { SaveState, createDebouncedSave } from '@shared/application/debounced-save'; import { fromLoadLifecycle } from '@shared/application/remote-data'; import { Brief, diff --git a/apps/ssp/src/app/brief/application/org-template.store.ts b/apps/ssp/src/app/brief/application/org-template.store.ts index 2834d5d..87ce84a 100644 --- a/apps/ssp/src/app/brief/application/org-template.store.ts +++ b/apps/ssp/src/app/brief/application/org-template.store.ts @@ -1,7 +1,6 @@ import { Injectable, computed, effect, inject, signal } from '@angular/core'; import { createStore } from '@shared/application/store'; -import { SaveState } from '@shared/application/action-state'; -import { createDebouncedSave } from '@shared/application/debounced-save'; +import { SaveState, createDebouncedSave } from '@shared/application/debounced-save'; import { fromLoadLifecycle } from '@shared/application/remote-data'; import { UploadAdapter, uploadContentUrl } from '@shared/infrastructure/upload.adapter'; import { UploadShellService } from '@shared/application/upload-shell.service'; diff --git a/docs/project/readable-codebase/RD-14-move-savestate-delete-action-state.md b/docs/project/readable-codebase/RD-14-move-savestate-delete-action-state.md new file mode 100644 index 0000000..cd5cf17 --- /dev/null +++ b/docs/project/readable-codebase/RD-14-move-savestate-delete-action-state.md @@ -0,0 +1,126 @@ +# RD-14 — Move `SaveState` beside its producer, delete `action-state.ts` + +Status: done +Source: PLAN.md 1b#2b + +## Why + +RD-12 and RD-13 moved both `ActionState` consumers into their machines, so **`ActionState` +now has zero real users.** Word-anchored, it survives only in its own definition and in one +doc-comment mention. + +`SaveState` is different and must survive: it has two genuine consumers that keep all four +cases (`brief.page.ts:150` and `org-template.page.ts:102` both `switch` on it, and +`brief.page.ts:77` reads `=== 'Error'`). The original plan called for deleting both types; +that was corrected once the consumers were read. + +So the file's remaining job is to hold one type whose only producer lives elsewhere. Move +`SaveState` next to `createDebouncedSave`, which is what sets it, and the file has no reason +to exist. + +## Read first + +- `libs/shared/src/application/action-state.ts` — 9 lines, both types +- `libs/shared/src/application/debounced-save.ts` — `SaveState`'s new home; note the comment + at line 16, which names `ActionState` +- `apps/ssp/src/app/brief/application/brief.store.ts:4,57` and + `org-template.store.ts:3,66` — the two importers + +## Decisions (pre-made, don't relitigate) + +1. **Delete `ActionState` outright.** Zero users after RD-12 and RD-13. Do not deprecate it, + do not keep a re-export. + +2. **Move `SaveState` verbatim into `debounced-save.ts`**, keeping its doc comment. That file + already owns the debounced-autosave concern and `createDebouncedSave` is the only thing + that drives the state, so the type belongs beside it. Keep the four cases exactly as they + are — `Idle | Saving | Saved | Error`. + +3. **Delete `libs/shared/src/application/action-state.ts`.** Nothing else lives in it. + +4. **Update the two store imports** to `@shared/application/debounced-save`. Both stores + already import from that module for `createDebouncedSave`, so this should merge into an + existing import line rather than adding one. + +5. **Reword `debounced-save.ts:16`**, which currently reads "it touches that store's + `SaveState`/`ActionState` + adapter". Drop the `ActionState` half — the type will not + exist. + +6. **Change no UI file and no page.** `saveState`'s public signature on both stores stays + identical, so the three consumer sites need no edit. + +## Files + +- `libs/shared/src/application/debounced-save.ts` — gains `SaveState`, comment reworded +- `libs/shared/src/application/action-state.ts` — **deleted** +- `apps/ssp/src/app/brief/application/brief.store.ts` — import only +- `apps/ssp/src/app/brief/application/org-template.store.ts` — import only + +No spec files. No UI files. No machine files. + +## Steps + +1. Move the `SaveState` declaration and its doc comment into `debounced-save.ts`. +2. Reword the `ActionState` mention at line 16 (decision 5). +3. Re-point both store imports (decision 4). +4. `git rm libs/shared/src/application/action-state.ts`. +5. Update this ticket's `Status:` to `done` and the README's RD-14 row to `done`. +6. Commit all of it together. + +## Acceptance criteria + +Measured baselines, dry-run before handover. + +```bash +# The file is gone, and nothing imports it. +ls libs/shared/src/application/action-state.ts # MUST be "No such file" +git grep -l "application/action-state" -- apps libs # was 2 files -> MUST return nothing + +# ActionState is gone entirely, word-anchored (a name containing it would defeat a bare grep). +git grep -nw "ActionState" -- apps libs # MUST return nothing + +# SaveState survives, in its new home, with all four cases. Anchor on the DECLARATION: +# a bare `-w SaveState` grep already returns 1 today, from the line-16 comment. +D=libs/shared/src/application/debounced-save.ts +git grep -c "export type SaveState" -- $D # was 0 -> MUST be 1 +git grep -c "'Idle'\|'Saving'\|'Saved'\|'Error'" -- $D # MUST be >= 4 + +# The render seam did not move: the three consumer sites are untouched. +git diff --name-only c599fee | grep -c "brief/ui/" || true # MUST be 0 +git grep -c "readonly saveState" -- \ + apps/ssp/src/app/brief/application/brief.store.ts \ + apps/ssp/src/app/brief/application/org-template.store.ts # still 1 each +``` + +```bash +npm run ci # exits 0 +``` + +## Verification + +`npm run ci`. No story, no `.mdx`, no `libs/shared/src/ui/**`, so `--full` is not required. + +`dep:check` matters here: `debounced-save.ts` is in `libs/shared/src/application`, the same +layer `action-state.ts` was in, so no boundary changes. If `dep:check` fails, the type landed +in the wrong layer. + +If `dotnet test` fails with `SQLite Error 1: 'no such table: …'`, that is the stale +`bigregister.db` artifact in this README's Troubleshooting section, unrelated to your change. + +## Out of scope + +- Anything under `brief/ui/` (decision 6). +- The machines. RD-12 and RD-13 already moved the action lifecycles. +- `NO_SUBORGS` becoming `Empty` — optional RD-34. +- `UploadStatus`'s `type:` discriminant — optional RD-35. + +## Risks + +- **Do not delete `SaveState` along with the file.** It has two four-way consumers. The + original plan said to delete both types; reading the consumers corrected that, and this + ticket is the corrected version. +- **Merge into the existing `debounced-save` import** in both stores rather than adding a + second import line from the same module — lint will not complain, but it reads badly. +- **This is the last ticket that touches `action-state.ts`.** After it, the phase's claim + holds: two encodings survive, `RemoteData` for fetched data and each machine's own state + union, plus `SaveState` as an explicitly-justified third for a different concern. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index 80560a4..c0edf56 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -108,7 +108,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | done | | RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | done | | RD-13 | Same for org-template, folding `pendingPublish` in | 12 | | done | -| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | todo | +| RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | done | | RD-15 | Remove 22 abandoned agent worktrees (4.7 GB) | 01 | | todo | | RD-16 | `parseDashboardView` returns `BigProfile`; delete `DashboardView` | 01 | | todo | | RD-17 | `successOf`/`successOr` sweep — 10 sites, 8 files | 01 | | todo | diff --git a/libs/shared/src/application/action-state.ts b/libs/shared/src/application/action-state.ts deleted file mode 100644 index ca63529..0000000 --- a/libs/shared/src/application/action-state.ts +++ /dev/null @@ -1,9 +0,0 @@ -/** Transient state of a one-shot action (submit/approve/publish/reset/…): one tagged - union instead of a busy boolean + a nullable error sitting side by side. Shared by the - editor stores (WP-31). */ -export type ActionState = { tag: 'Idle' } | { tag: 'Busy' } | { tag: 'Failed'; error: string }; - -/** Debounced-autosave indicator, shown in a small status line near a toolbar — a separate - concern from ActionState (a stale autosave error doesn't block submit/approve), but - tag-aligned with it for one consistent idiom. */ -export type SaveState = { tag: 'Idle' } | { tag: 'Saving' } | { tag: 'Saved' } | { tag: 'Error' }; diff --git a/libs/shared/src/application/debounced-save.ts b/libs/shared/src/application/debounced-save.ts index 7b48206..823f87e 100644 --- a/libs/shared/src/application/debounced-save.ts +++ b/libs/shared/src/application/debounced-save.ts @@ -1,3 +1,8 @@ +/** Debounced-autosave indicator, shown in a small status line near a toolbar — a separate + concern from a store's one-shot action lifecycle (a stale autosave error doesn't block + submit/approve), but tag-aligned with it for one consistent idiom. */ +export type SaveState = { tag: 'Idle' } | { tag: 'Saving' } | { tag: 'Saved' } | { tag: 'Error' }; + export interface DebouncedSave { /** (Re)arm the debounce timer; no-op when `canSave()` is false. */ schedule(): void; @@ -13,7 +18,7 @@ export interface DebouncedSave { /** * The debounced-autosave timer shared by the editor stores (WP-31). It owns ONLY the timer * bookkeeping; the actual write + save-state transitions live in the caller's `flush` - * (store-specific — it touches that store's SaveState/ActionState + adapter). The handle is + * (store-specific — it touches that store's SaveState + adapter). The handle is * nulled the moment it fires, so `hasPendingSave()` means "a write is still owed". Integrates * with the `PendingSave` seam (pending-saves.ts): a store delegates hasPendingSave/flushPending * here so the CanDeactivate guard / beforeunload handler can flush a pending edit.