doc/REVIEW_02.md: simplification + maintainability audit

Sister document to REVIEW_01 (security). Catalogs over-complicated
patterns that can be rewritten more simply with no functional change:
HIGH severity items are the capacity-formula PHP/JS duplication, the
buildTaskRow → Twig partial drift (3 historical hotfixes), and the
form-endpoint admin+CSRF guard inlined ~7× despite gateJsonAdmin
already existing. Plus 7 MEDIUM and 3 LOW findings, and a "things
that look complex but are NOT findings" section pre-empting re-flags
of intentional scar tissue (cell popover, cascade audit verbosity).
Same R02-Nxx / Severity / Status format as REVIEW_01 so the /loop
workflow can plug in directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doc/REVIEW_02.md

@@ -0,0 +1,488 @@
+# REVIEW_02 — Simplification & maintainability audit
+
+Date: 2026-05-08
+Scope: full repo at `main` (commit `1dba877`), against SPEC.md as of the
+same SHA. Sister document to `doc/REVIEW_01.md` — that one captures
+security and "fishy patterns"; this one captures **overcomplicated
+solutions that could be rewritten more simply with no functional change.**
+
+This document is meant to be referenced on each refactor loop alongside
+SPEC.md and REVIEW_01.md. Findings are ordered by severity (highest
+first) and each carries:
+
+- **ID**: stable handle (`R02-Nxx`) — quote when committing a fix.
+- **Severity**: HIGH / MEDIUM / LOW. Severity here measures
+  _maintenance pain_, not security. HIGH = a real bug surface (already
+  caused regressions, or two-place edits routinely diverge); MEDIUM =
+  meaningful churn / readability / tested-once-then-forgotten; LOW =
+  cosmetic / churn for churn's sake unless the area is being touched
+  anyway.
+- **Status**: `open` / `accepted-by-design` / `fixed-in-<sha>`.
+- **Where**: file paths + line ranges (current; will shift after edits).
+- **What**: the pattern that's overcomplicated.
+- **Why it's worth fixing**: what breaks today, or what the next
+  contributor will trip over.
+- **Suggested rewrite**: shape of the change, not a full diff. Keep the
+  spec invariants intact: vanilla JS, strict CSP, Twig auto-escape,
+  audit-row-per-mutation, capacity math under §5.
+
+When a finding is closed, set Status to `fixed-in-<sha>` and append a
+short note. Do not delete entries — they're history, same contract as
+REVIEW_01.
+
+---
+
+## How to use this with /loop
+
+1. Pick the lowest-numbered `open` finding whose severity ≥ MEDIUM.
+2. Re-read SPEC.md §5 (capacity math), §7 (audit), and §9 (phase log)
+   to confirm the invariant the simplification must preserve.
+3. Apply the rewrite, add/adjust tests, update SPEC.md if a public
+   contract changes. Most findings here are pure internals — no spec
+   touch required.
+4. Commit; flip Status to `fixed-in-<sha>` and add a one-line note.
+5. Commit the doc update separately.
+
+---
+
+## HIGH
+
+### R02-N01 — Capacity formula duplicated in PHP and JS
+- **Severity**: HIGH (already a documented two-place-edit hazard).
+- **Status**: open.
+- **Where**:
+  - `src/Services/CapacityCalculator.php` — authoritative implementation.
+  - `public/assets/js/sprint-planner.js` lines 172–222 — `recomputeRow()`
+    + `applyServerCapacity()` reproduce the same `round_half`,
+    `after_reserves`, `committed_p1`, `available` chain on the client.
+  - SPEC.md §5 calls this out explicitly: _"Runs identically in
+    `App\Services\CapacityCalculator` (PHP) and in `sprint-planner.js`
+    (JS). Any edit must touch both."_
+- **What**: The capacity rule (the core domain invariant of the app)
+  lives in two places. The JS copy exists so the on-screen totals
+  update during typing without waiting for the server round-trip. Every
+  PATCH response already returns the freshly computed `per_worker`
+  values though, and `applyServerCapacity()` is called immediately
+  after each save (lines 249, 559, 612).
+- **Why it's worth fixing**: any future change to capacity (a new
+  reserve type, a different rounding rule, priority-3 tasks, anything)
+  must be made twice. The invariant is the kind of thing that gets
+  changed during product evolution; missing one side has produced subtle
+  display bugs in similar projects historically.
+- **Suggested rewrite**: drop `recomputeRow()`'s arithmetic. Two paths:
+  - **Path A (preferred):** keep typing-responsive feel by relying on
+    the existing 400 ms debounce — the user usually pauses before
+    expecting Available to update anyway. Render `Available` from
+    server values only; on edit, optionally show a "saving…" indicator.
+  - **Path B:** keep an immediate visual update but make it a *plain
+    sum* (no reserve arithmetic) so the formula stays single-source.
+    Server PATCH response is authoritative for Reserves/Available.
+  Either way the JS file shrinks by ~30 lines and the spec's "any edit
+  must touch both" line in §5 goes away.
+
+### R02-N02 — `buildTaskRow()` mirrors the Twig partial by hand
+- **Severity**: HIGH (already the source of two phase-bound hotfixes).
+- **Status**: open.
+- **Where**: `public/assets/js/sprint-planner.js` ~lines 361–510 —
+  `buildTaskRow(task, ...)`.
+  - Twig source of truth: `views/sprints/_task_list.twig` lines 200–283.
+  - Historical regressions documented in SPEC.md §9 Phase 10:
+    - hotfix `7c298d3` — owner dropdown empty in JS-built rows because
+      `ownerChoices()` scraped the pre-Phase-10 `[data-owner-filter]
+      option` selector that no longer existed.
+    - hotfix `23ab365` — JS-built cells missed `data-col`, breaking the
+      Columns dropdown and Phase 13 focus auto-hide.
+  - Plus the `whitespace-nowrap` re-mirror in the "Task table polish"
+    commit `f204611`.
+- **What**: when an admin clicks "+ Add task", the new row is built in
+  JS to avoid a page reload. The function manually emits ~15 data
+  attributes + ~10 CSS classes that the Twig partial already emits on
+  server-rendered rows. Every time the partial changes, this function
+  silently drifts.
+- **Why it's worth fixing**: it's a recurring foot-gun. The partial is
+  the spec for what a task row looks like; `buildTaskRow` is a
+  forgetful copy that has bitten the project at least three times.
+- **Suggested rewrite**: server-rendered hidden `<template
+  data-task-row-template>` block at the bottom of `_task_list.twig`
+  containing one rendered row with placeholder values (or just the
+  blank shell — the JS already fills the data). On task creation:
+  ```js
+  const tr = template.content.firstElementChild.cloneNode(true);
+  tr.dataset.taskId = data.task.id;
+  // …apply server-returned task fields
+  ```
+  Eliminates the manual mirror. The template renders inside the
+  existing autoescape, so no XSS surface change. ~80 LoC removed from
+  `sprint-planner.js`. New test: assert the template element exists in
+  `_task_list.twig` (drift guard, same shape as the autoescape pin
+  R01-N21).
+
+### R02-N03 — Form-endpoint admin+CSRF guard inlined ~7×
+- **Severity**: HIGH (the JSON path already has the helper; form path
+  doesn't, and it's the same boilerplate). Real fix is small but stops
+  ongoing copy-paste.
+- **Status**: open.
+- **Where**:
+  - `src/Controllers/SprintController.php::create` lines 91–99
+  - `…::delete` lines ~1020–1026
+  - `src/Controllers/WorkerController.php::create`/`::update`
+  - `src/Controllers/UserController.php::update`/`::tombstone`
+  - `src/Controllers/SettingsController.php::update`
+  - `src/Controllers/ImportController.php::upload`/`::commit`
+  Existing parallel: `SprintController::gateJsonAdmin()` at line
+  1169 — used 8× across the JSON endpoints (lines 349, 510, 589, 649,
+  705, 762, 806, 928). The pattern works. The form path just lacks the
+  twin.
+- **What**: every form-handling action begins with the same three
+  lines:
+  ```php
+  $actor = SessionGuard::requireAdmin($this->users);
+  if ($actor instanceof Response) { return $actor; }
+  if (!SessionGuard::verifyCsrf($req)) { return Response::text('CSRF token invalid', 403); }
+  ```
+- **Why it's worth fixing**: ~7 sites, three lines each, all identical.
+  More importantly: a developer adding a new form endpoint can forget
+  the CSRF check or the admin gate and the omission won't be obvious in
+  review (the surrounding code looks fine). Both the JSON
+  (`gateJsonAdmin` returning a JSON envelope) and the form (a
+  `Response::text` 403) flavours are reasonable; missing either is a
+  vulnerability.
+- **Suggested rewrite**: add `SessionGuard::requireAdminForm(Request,
+  UserRepository): User|Response` mirroring `requireAdminJson` (already
+  at SessionGuard:256). It runs both the admin check and CSRF, returns
+  the user on success or a 403 text response on either failure. Each
+  controller method drops 4 lines to one. `gateJsonAdmin` in
+  `SprintController` can also move into `SessionGuard` for symmetry —
+  it's not sprint-specific.
+
+---
+
+## MEDIUM
+
+### R02-N04 — Three near-identical debounced-batch-save pipelines
+- **Severity**: MEDIUM.
+- **Status**: open.
+- **Where**: `public/assets/js/sprint-planner.js`
+  - Arbeitstage cells: lines 228–258 (`pendingCells`/`cellDebounce`/
+    `queueCell`/`flushCells`)
+  - Task assignment days: ~lines 590–615 (`pendingAssign`/`assignTimers`)
+  - Task assignment status: ~lines 644–668 (`pendingStatus`/
+    `statusTimers`)
+  Plus a fourth, simpler one in `sprint-settings.js` lines 94–141 for
+  meta + week masks.
+- **What**: each pipeline is the same shape — accumulator Map +
+  per-key debounce timer + flush function that PATCHes a JSON envelope
+  and applies the server response. The only differences are URL,
+  payload shape, and post-save UI hooks (which row total to refresh).
+- **Why it's worth fixing**: every behaviour change ("show
+  saving-spinner"/"retry on network error"/"coalesce two pending writes
+  to the same cell") has to be applied 3×. Already happened during
+  Phase 18 — the status pipeline is structurally identical to the days
+  pipeline but written in parallel.
+- **Suggested rewrite**: a small factory in `sprint-planner.js`'s
+  helper section:
+  ```js
+  function makeBatchSaver(url, debounceMs, onSuccess) {
+      const pending = new Map();
+      let timer = null;
+      function flush() {
+          if (pending.size === 0) return;
+          const items = Array.from(pending.values());
+          pending.clear();
+          request('PATCH', url, items)
+              .then(onSuccess)
+              .catch((e) => flash(e.message, true));
+      }
+      return {
+          queue(key, item) {
+              pending.set(key, item);
+              clearTimeout(timer);
+              timer = setTimeout(flush, debounceMs);
+          },
+      };
+  }
+  ```
+  Three callers, each ~5 lines. ~80 LoC saved across both JS files. The
+  cell-popover-status pipeline keeps its own onSuccess closure; the
+  rest is shared.
+
+### R02-N05 — `SprintController::updateMeta()` mixes 4 concerns in 144 lines
+- **Severity**: MEDIUM.
+- **Status**: open.
+- **Where**: `src/Controllers/SprintController.php::updateMeta` lines
+  346–489.
+- **What**: a single method does (a) JSON body parsing, (b) date
+  validation, (c) reserve fraction validation, (d) detection of which
+  fields actually changed, (e) the auto-resync of `sprint_weeks` when
+  start_date/end_date moves (Phase 21), (f) the audit emission, (g) the
+  envelope construction. Reading top-to-bottom is hard.
+- **Why it's worth fixing**: the date-resync flow is the most
+  consequential change in Phase 21, and it's tucked inside this
+  method. The next time someone touches it (e.g. to add a "shrink also
+  trims tasks" rule) they'll need to mentally re-trace the whole
+  surrounding flow.
+- **Suggested rewrite**: extract three private methods, each pure
+  (returns a value or throws):
+  - `parseAndValidateMetaPatch(Request): array` — returns the
+    field-set or a `Response` on failure, like `gateJsonAdmin`.
+  - `applyDateRangeChange(Sprint, string $start, string $end): array`
+    — does the realign + audit, returns the diff descriptor for the
+    response envelope.
+  - `applyReserveChange(Sprint, float $reserve): bool` — same shape.
+  The method body becomes ~30 lines of orchestration. Tests can call
+  the helpers directly without spinning up a full transaction.
+
+### R02-N06 — localStorage key registry duplicated between init/setItem/Reset/`:beamer`
+- **Severity**: MEDIUM.
+- **Status**: open.
+- **Where**: `public/assets/js/sprint-planner.js`
+  - Five keys: `ownerFilterKey` (~line 1515), `focusKey` (~1565),
+    `statusFilterKey` (~1596), `columnsKey` (~1740), `tabKey` (~1989).
+  - Each is built inline as `'sp:' + sprintId + ':<name>' +
+    keySuffix` (the `:beamer` namespacing for the present view).
+  - Reset button handler clears them by hand (~lines 1851–1864).
+- **What**: adding a new persisted filter is a five-step ritual: pick a
+  name, build the key, init from localStorage, persist on change, add
+  to Reset. Step 5 is the easy one to forget — the symptom is "Reset
+  doesn't clear my new filter." Two-place edit (Reset + the persistence
+  site) per state slot.
+- **Why it's worth fixing**: a small registry removes the second
+  edit. Pure cleanup; no behaviour change.
+- **Suggested rewrite**: at the top of the IIFE,
+  ```js
+  const stateKeys = ['ownerFilter', 'focusWorker', 'statusFilter',
+                     'hiddenCols', 'tab'];
+  const key = (n) => `sp:${sprintId}:${n}${keySuffix}`;
+  function resetState() { stateKeys.forEach(n => localStorage.removeItem(key(n))); }
+  ```
+  Reset becomes one line. New filters add one entry to `stateKeys`.
+
+### R02-N07 — Form-field markup repeated ~20× across views
+- **Severity**: MEDIUM (medium because every view-touch nibbles at the
+  problem; small individually, large in aggregate).
+- **Status**: open.
+- **Where**: across `views/sprints/new.twig`, `views/sprints/settings.twig`,
+  `views/workers/index.twig`, `views/users/index.twig`,
+  `views/settings/index.twig`, `views/auth/local.twig`, the Phase 20
+  `views/sprints/import_*.twig`. Roughly 20 instances of the
+  `<label>…<input class="mt-1 block w-full rounded-md border-slate-300
+  border shadow-sm px-3 py-2 …">` pattern + optional hint span.
+- **What**: every form field hand-rolls the same 4-line label/input/
+  hint block. Tailwind class strings are duplicated verbatim (or — worse
+  — _almost_ verbatim, with a stray class missing in 1–2 spots).
+- **Why it's worth fixing**: any visual tweak (focus ring colour, dark
+  mode text, error state styling) requires touching 20+ sites. Already
+  happens for dark-mode adjustments — the Phase 16 commit notes
+  "every view file gets a systematic `dark:` sweep."
+- **Suggested rewrite**: a `views/_macros/form.twig` (or inline at top
+  of `layout.twig`) with:
+  ```twig
+  {% macro field(name, label, type='text', value='', hint=null, attrs={}) %}
+    <label class="block">
+      <span class="text-sm font-medium ...">{{ label }}</span>
+      <input type="{{ type }}" name="{{ name }}" value="{{ value }}"
+             class="mt-1 block w-full rounded-md border-slate-300 ..."
+             {% for k, v in attrs %} {{ k }}="{{ v }}"{% endfor %}>
+      {% if hint %}<span class="text-xs ...">{{ hint }}</span>{% endif %}
+    </label>
+  {% endmacro %}
+  ```
+  Saves ~80 lines. Same auto-escape applies. Audit each replacement
+  with the existing TwigViewTest smoke renders.
+
+### R02-N08 — Three filter dropdowns hand-coded with identical scaffolding
+- **Severity**: MEDIUM.
+- **Status**: open.
+- **Where**: `views/sprints/_task_list.twig`
+  - Owners dropdown: lines 31–58
+  - Status dropdown: lines 61–90
+  - Columns dropdown: lines 103–132
+  Each is `<div data-X-root>` + button + panel + clear button + option
+  list, with identical Tailwind class strings.
+- **What**: the structural shell (positioning, classes, open/close
+  trigger pair) is the same in all three; only the option list differs.
+  A new dropdown filter (e.g. "Status of last update") adds ~30 lines
+  of mechanical scaffolding before any real content.
+- **Why it's worth fixing**: a `{% macro filter_dropdown(id, label,
+  triggerText) %}` that yields a slot for the option list would cut
+  ~60 lines. The dropdown-close-on-outside-click logic in
+  `sprint-planner.js` (lines that listen for `data-*-root`) already
+  treats them uniformly — the markup should follow.
+- **Suggested rewrite**: macro accepting `id` (used as `data-{id}-root`,
+  `data-{id}-trigger`, `data-{id}-dropdown`), `triggerLabel`,
+  `headerLabel`, and renders the three rendered to `{% block options %}`.
+
+### R02-N09 — Tailwind class soup repeated for buttons/inputs/alerts
+- **Severity**: MEDIUM.
+- **Status**: open.
+- **Where**: scattered across `views/**/*.twig`. Notable cases:
+  - Secondary button class string (~35 chars + dark variants) appears
+    ≥4× across `show.twig`, `present.twig`, settings, audit.
+  - Number-input class (`w-14 rounded border …`) appears 3× across
+    `show.twig` and `_task_list.twig`.
+  - Error/warning alert classes appear ≥4× across `home.twig`,
+    `show.twig`, `settings.twig`, `users/index.twig`.
+- **What**: the same ~10 inline class strings repeat verbatim. The
+  `.cell-popover` / `.task-menu` / `.task-modal` blocks already exist
+  in `assets/css/input.css` under `@layer components` (per SPEC.md
+  Phase 17/18/22 notes), proving the team is comfortable with `@apply`-
+  based component classes when the markup gets too thick — these other
+  recurrences just haven't been promoted yet.
+- **Why it's worth fixing**: visual drift between "supposedly the same"
+  buttons. Easier theming. Less to read.
+- **Suggested rewrite**: add `@layer components` blocks for
+  `.btn-secondary`, `.input-number`, `.alert-error`, `.alert-warning`
+  in `assets/css/input.css`. Replace inline duplicates. ~30 individual
+  copies eliminated; central styling for review.
+
+### R02-N10 — `public/index.php` does ~60 lines of inline service wiring
+- **Severity**: MEDIUM.
+- **Status**: open.
+- **Where**: `public/index.php` lines 158–196.
+- **What**: 11 repository constructors + 9 controllers, each with 4–12
+  args, instantiated by hand in a flat block. Reading the file
+  top-to-bottom you have to step over the wiring to reach the dispatch
+  logic.
+- **Why it's worth fixing**: every new repository or controller touches
+  this block. SPEC.md `§3` already lists every dependency, so a small
+  factory class is a one-time cost. Today the bootstrap mixes safety-
+  critical work (early `FatalErrorHandler::register`, schema-pending
+  503, HTTPS redirect) with mechanical wiring. Separating them makes
+  the safety-critical sequence easier to audit (which is itself a
+  REVIEW_01 concern: R01-N13 had to register the handler twice to cover
+  the wiring window).
+- **Suggested rewrite**: `App\Container::build(PDO $pdo, View $view):
+  array` returning `['controllers' => [...], 'services' => [...]]`.
+  `index.php` becomes ~25 lines: env + autoload + early handler +
+  schema check + `$container = Container::build(...)` + router setup.
+  No DI library required — same plain `new` calls, just relocated.
+  Tests don't change (they construct individual controllers directly).
+
+---
+
+## LOW
+
+### R02-N11 — `recordForRequest()` always passes `$req, $actor` as the trailing pair
+- **Severity**: LOW (named args make it readable; bulk reduction is the
+  win).
+- **Status**: open.
+- **Where**: 50+ call sites across `SprintController.php`,
+  `TaskController.php`, `WorkerController.php`, `UserController.php`,
+  `ImportController.php`, `SettingsController.php`.
+- **What**: every audit emission inside a controller method is 8 lines
+  with two positional invariants (`req: $req, actor: $actor`).
+- **Why it's worth fixing**: the controller already has both — they're
+  the per-request constants of the call. A `$this->record('CREATE',
+  'sprint', $id, $before, $after, $req, $actor)` style helper, or
+  better, a per-request audit closure built once at the top of each
+  method (`$audit = $this->auditFor($req, $actor); $audit('CREATE',
+  'sprint', $id, null, $snap);`) — would shrink the bulk by ~30%.
+- **Suggested rewrite**: thin protected method on a base controller, or
+  a request-scoped closure. Spec §7's audit contract is unaffected.
+
+### R02-N12 — Field whitelists declared inline in `TaskController::update`
+- **Severity**: LOW.
+- **Status**: open.
+- **Where**: `src/Controllers/TaskController.php` lines 133–181.
+- **What**: an if/elseif cascade walks `title`, `priority`,
+  `owner_worker_id`, `description`, `url` with bespoke validation per
+  field. URL validation (regex + length) inline at lines 170–181.
+- **Why it's worth fixing**: each field's rule is buried in the cascade.
+  Adding a sixth field (already happened twice in Phase 22) keeps
+  growing this method.
+- **Suggested rewrite**: a static array `private const UPDATABLE = ['title' =>
+  ['string', 1, 200], 'priority' => ['int_in', [1,2]],
+  'owner_worker_id' => ['nullable_int'], 'description' => ['string',
+  0, 8000], 'url' => ['url_or_empty', 2048]];` with a small
+  `validateField($name, $value)` switch on the type tag. Same per-
+  field semantics, table-driven, easier to extend.
+
+### R02-N13 — Date validation duplicated in `create()` and `updateMeta()`
+- **Severity**: LOW.
+- **Status**: open.
+- **Where**: `src/Controllers/SprintController.php` lines 110–124
+  (`create()`) and ~lines 373–401 (`updateMeta()`).
+- **What**: both methods strict-parse `Y-m-d` with
+  `DateTimeImmutable::createFromFormat`, check `$endD < $startD`, and
+  call `weeksBetween(...)` to enforce the 26-week ceiling.
+  R01-N12's `validateDateFilters()` in `AuditController` is yet another
+  variant of the same idea (round-trip strict parse).
+- **Why it's worth fixing**: 3× similar parse-and-validate. A shared
+  `Domain\DateRange::parse(string, string): self|array` would let the
+  controllers do `$range = DateRange::parse($start, $end); if
+  (is_array($range)) return $errorRedirect($range['error']);`.
+- **Suggested rewrite**: small value object in `src/Domain/`. Static
+  helpers `weeksBetween`, `isIsoDate`, `validateDateFilters` collapse
+  into one type. Tests for it are already half-written across the
+  three call sites.
+
+---
+
+## Things that look complex but are NOT findings
+
+Listed so a future reviewer doesn't re-flag them.
+
+- **The cell popover (`sprint-planner.js` ~lines 787–885).** SPEC.md
+  Phase 17 documents seven iterations of a number-stepper popover that
+  was eventually deleted entirely. The _current_ popover (commit
+  `10ea4b8`) is a different one — slider + status pills, ~100 lines,
+  uses element-local pointer events with a 250 ms grace timer. That
+  scar tissue is worth preserving: the previous attempts at
+  document-level pointer tracking failed in production. Don't refactor
+  this without reading the Phase 17 saga first.
+- **`buildTaskRow` itself was scaffolded once in vanilla JS** when the
+  team migrated off jQuery in Phase 19. The right fix is the
+  template-clone approach in R02-N02; rewriting the inner helpers is
+  not worth it.
+- **Twig macro `capacity_table` is called twice on the same page**
+  (Phase "tabs"). That is _already_ the simplification — a macro for
+  what would otherwise be 50 lines of duplicated table.
+- **The audit-row-per-cascade rule** (SPEC.md §7) — every cascade
+  controller (`TaskController::delete`, `SprintController::removeWorker`,
+  `SprintController::delete`, etc.) snapshots descendants before the
+  FK fires. This _is_ verbose, but the verbosity is the audit
+  contract. Don't try to "abstract" it; the explicit shape is the
+  point.
+- **The R01-N04 `SESSION_SECRET` removal** demonstrates the team's
+  preference for "delete the dead config" over "wire it up." Apply the
+  same posture to any future "the var is documented but unused" find:
+  delete first.
+
+---
+
+## Suggested order to attack
+
+If a single refactor sprint is allotted, the highest leverage-per-hour
+sequence:
+
+1. **R02-N02** (template-clone for `buildTaskRow`) — kills a class of
+   recurring bugs and is mechanically simple. Worth doing first.
+2. **R02-N03** (form-endpoint guard helper) — five lines added to
+   `SessionGuard`, ~25 lines removed across controllers, security-
+   relevant.
+3. **R02-N04** (debounce factory) — pure JS internal cleanup, lets
+   future "saving…" indicators land in one place.
+4. **R02-N01** (capacity formula) — biggest semantic win, but worth
+   landing _after_ the JS-side debounce factory because Path A's "let
+   the server be authoritative" works cleanly with the factory.
+5. **R02-N07 / R02-N08 / R02-N09** (Twig macros + component classes) —
+   pure markup cleanup; can be done piecemeal as views are touched for
+   other reasons. Don't churn 10 view files in one PR for purely
+   cosmetic reasons.
+
+R02-N05/N06/N10 and the LOW findings are good "while I'm in here"
+opportunistic cleanups, not standalone work items.
+
+---
+
+## Maintenance contract
+
+Same posture as REVIEW_01:
+
+1. Pick a finding with severity ≥ MEDIUM that has Status `open`.
+2. Apply the rewrite. Adjust tests. Make sure SPEC.md still describes
+   reality (most findings here are pure internals — no spec touch
+   needed).
+3. Commit code first; commit the doc update separately, marking Status
+   `fixed-in-<sha>` with a one-line note.
+4. Don't delete entries. The history is part of the document's value.