diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 99024ecb45..2bd799438b 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -166,7 +166,7 @@ removed after current-main verification; it is not missing recommended work. | 113 | `#276` | Optional | High — design-system links | Next LinkAction tone follow-up | 30–60 min | LinkAction accepts `tone` via `BaseProps` but never reads it, so `tone="inherit"` is silently ignored while sibling links honour tone. Decide honour vs refuse on `LinkActionProps`; refusing is probably right for a forward-action link. **Gate:** `npm run check:design-system-contract` plus the Links block in `tests/ui-v2-components.dom.test.tsx`. **Stop:** do not bundle with unrelated link policy work (#263 follow-on rows). | - + ## Open items > **Merged-main canary update (2026-07-23, run `30018289898`):** the new structured report correctly recorded evaluated tree `c24f2e8f2d30d0c59fc1eba025d3dcd63478137e`, run/attempt identity and `cross-region-runner` latency context. Golden retrieval remained 36/36 with document/content recall 1.0 and no failed cases. The 44-case answer gate had grounded-supported and unsupported-correct rates of 1.0, but failed because `neuroleptic-side-effect-escalation` again returned one citation where two are required (citation-failure rate 0.0227). `admission-discharge-comparison` again omitted the specific AKG admission document after `comparison_source_extractive_fallback`; `admission-discharge-coverage-paraphrase` was advisory-only at 24,870 ms. Answer cost was reported as `$0.234736`. Do not retry immediately: retain this as the first structured datapoint, compare it with the scheduled 2026-07-26 report, and keep retrieval/ranking unchanged. @@ -215,7 +215,7 @@ removed after current-main verification; it is not missing recommended work. | #118 | P2 | task | Adopt the visual and Lighthouse baselines so the two new gates actually gate | **Outcome:** `visual-baseline` and `lighthouse-budget` stop reporting and start blocking. **Detail:** PR #1404 added both as `continue-on-error` jobs outside `pr-required`, deliberately. `tests/ui-visual-baseline.spec.ts` has no committed baselines, so all six targets (dashboard-shell, dashboard-shell-phone, search-results-band, search-results-band-phone, document-viewer, therapy-compass-home) fail with a missing-snapshot error by design; the job uploads them on every run (run 30513537912, artifact 8748062487, 31 files). `tests/__screenshots__/` still holds only README.md, re-checked 2026-08-08. `lighthouse-budget.json` ships `enforce: false` with `baseline: null`, so the grader warns rather than grades. **Owner decision 2026-08-08: adopt the baselines, advisory-only first, and promote to required only after they have held across a few runs** — the workflow comment asks for that soak explicitly. **Sequencing constraint recorded the same day:** the owner had design changes merging at the time, and pixel baselines frozen while the look is still moving go red on the next merge, which is the churn the hold exists to avoid. Trigger the ubuntu job once that merge wave has landed, not before. **Next:** (1) trigger `visual-baseline` on a UI PR, download the artifact, review the six PNGs and commit them under the platform-scoped screenshots directory that `playwright.visual.config.ts` names in its `snapshotPathTemplate` — from CI, never a developer machine, because font hinting differs between them; (2) run `npm run check:lighthouse-budget -- --update` against a known-good CI build and flip `enforce`, but not before #117 or the baseline pins a known-slow route; (3) then add each job to `pr-required` and drop `continue-on-error` in the same edit. **Also:** PR #1404 added the first rendered-effect contract for #094, but 37 of the 38 unlayered visual classes still carry exemptions in `tests/helpers/style-contracts.ts` rather than contracts; and `scripts/run-lighthouse-budget.mjs` duplicates about 50 lines of the isolated-server boot in `scripts/run-playwright.mjs`, deferred to avoid destabilising the required UI gate in the same change. **Stop:** do not make a missing baseline skip instead of fail — that is the soft-skip-green pattern `AGENTS.md` forbids. | session 2026-07-30; PR #1404 | 2026-07-30 | | #142 | P3 | task | Four loose dated docs need source and migration edits before they can be filed | **Outcome:** every dated point-in-time doc lives in `docs/audit/` or `docs/archive/` as `docs/README.md` requires, not loose at the `docs/` top level. **Detail:** PR #1436 filed the five that were docs-only moves. These four are referenced from outside `docs/`, so relocating them means editing source, tests and migration SQL comments — a different risk class than a docs tidy, and not worth bundling into one: `capacity-review.md` (`scripts/soak-test.ts`), `tenancy-defense-in-depth-review.md` (`src/lib/owner-scope.ts`, `tests/owner-scope-guard.test.ts`, two migrations, `SECURITY.md`, `.claude/agents/clinical-governance-reviewer.md`), `operator-apply-july8-batch.md` (three migrations plus `supabase/schema.sql`), `scale-readiness-review.md` (one migration). Also note `forward-codify-retrieval-rpcs-workorder.md` is indexed as a completed workorder but live is still ahead of the repo on those RPCs, so archiving it would misrepresent open operator work. **Next:** treat as low priority — the docs are correctly indexed and reachable where they are; only file them if a pass is already editing those migrations. Editing applied migration SQL is subject to `npm run check:migration-role`'s immutability pin. | PR #1436; session 2026-07-30 | 2026-07-30 | | #147 | P2 | rec | Mobile CLS breaches four routes, and it reproduces exactly offline | **Outcome:** mobile CLS is under 0.1 on every measured route, or each remaining breach has a recorded reason. **Measured 2026-07-30 with the LOCAL OFFLINE production harness** (`CHROME_PATH=/opt/pw-browsers/chromium npm run verify:lighthouse -- --keep`; Lighthouse 12.8.2, Chromium 141.0.7390.37, demo corpus, inert loopback Supabase). Mobile CLS: `/dsm` 0.363, `/documents/search` 0.220, `/therapy-compass` 0.142, `/` 0.023 — `/forms` did not measure locally (Lighthouse `NO_NAVSTART`, its own "run again" transient), but the live dispatch put it at 0.212. Desktop passes everywhere: 0.016–0.097. **The finding that matters is that these reproduce the live production dispatch EXACTLY.** Against run `30548662649` on `psychiatry.tools`, all four locally measurable mobile routes match to three decimals — 0.023 / 0.142 / 0.220 / 0.363 on both — and the desktop range matches at both endpoints, 0.016–0.097. So CLS on these routes is deterministic layout structure, not network, latency, or production data. Three consequences: (a) the CLS half of `#017` is debuggable and gate-able locally at zero provider cost; (b) `#017`'s recorded blocker "prod server hard-requires Supabase secrets" is stale — `scripts/run-lighthouse-budget.mjs` builds and serves an offline production app precisely to sidestep that, and its earlier note that dev-mode CLS looked "excellent at 0.00–0.04" was measuring DEV, which does not reproduce production; (c) re-dispatching the live workflow to re-measure CLS buys nothing. **LCP does NOT reproduce and must not be read from local runs:** local mobile LCP is 2014–2051 ms against 3639–4716 ms live, because the loopback server has no network latency. The LCP _ranking_ does survive — `/therapy-compass` is the outlier both locally (4978 ms, ~2.4x every other route) and live — which corroborates `#117` rather than replacing it. Mobile TBT, local: `/documents/search` 581 ms, `/dsm` 448, `/therapy-compass` 379, `/` 338. **Ranking by measured contribution, which is what `#017` asked for:** 1. `/dsm` CLS 0.363 (3.6x the threshold, the worst single number anywhere); 2. `/documents/search` 0.220; 3. `/forms` 0.212 (live only); 4. `/therapy-compass` 0.142, and separately the sole LCP outlier via `#117`; 5. `/` 0.023, which passes and needs nothing. **ATTRIBUTED 2026-07-30 — one cause dominates, and it is a self-inflicted round trip.** Driving Chromium directly against the same offline production build (Lighthouse mobile emulation: 412x823, DPR 1.75, 4x CPU throttle) with a `PerformanceObserver` on `layout-shift` reading `entry.sources[].node`: on `/dsm`, `/documents/search`, `/forms` and `/therapy-compass` the entire main content region moves **down 128px and straight back up 128px** within 15-60ms. Both moves score, so the round trip is pure cost with zero net movement. It is **100% of `/documents/search`** (0.110 down + 0.110 up = 0.220) and about 75% of `/dsm`. The shifting element is the `max-sm:pt-[var(--phone-overlay-chrome-h)]` wrapper around `
`, i.e. the phone overlay chrome reserve. **Mechanism, pinned by a MutationObserver timeline on the root style attribute rather than inferred:** the property goes CSS seed -> `200px` -> `72px`, and the decisive line is that the 200px is written when the header stack **already measures 72px** — `t=1552ms reserve=200px stack=72`, corrected to `72px` at `t=1612ms`. So `usePhoneOverlayChromeReserve` (`src/components/clinical-dashboard/use-phone-overlay-chrome-reserve.ts`) publishes a **stale** measurement: its `useLayoutEffect` reads `stack.offsetHeight` while the stack is still transiently 200px, and the write lands after the stack has already collapsed to 72px; its `ResizeObserver` then corrects it. The CSS seed at `globals.css:375` (`calc(max(0.5rem, var(--safe-area-top)) + var(--shell-header-h))` = 72px on a zero-inset profile) is **correct for the settled stack** — the seed is not the problem. **This corrects the mechanism recorded on `#130`** (now archived), which framed the defect as the seed under-reserving by `max(0, 0.5rem - inset)`, i.e. 0px on a notched iPhone and 8px elsewhere. Measured, the CLS driver is not an 8px seed shortfall but a 128px transient over-reserve written by the hook. Do not reopen `#130` on this; it is recorded here. **Control that makes the attribution trustworthy:** `/` never writes the property at all (no overlay stack) and is the one clean route at 0.023. **Variance, stated rather than smoothed over:** `/dsm` measured 0.363 (3 shift entries) and 0.219 (2 entries) on two runs — the round trip is the stable core and the third entry is intermittent. This harness has no network throttling, so `/forms` (0.310 local vs 0.212 live) and `/therapy-compass` (0.240 vs 0.142) run high locally; only `/dsm` 0.363, `/documents/search` 0.220 and `/` 0.023 reproduced the live dispatch exactly, and only those three should be treated as production-equivalent. **Next:** stop the hook publishing a measurement it is about to revise — defer the first publish until the stack has settled, or let the `ResizeObserver` be the only writer and trust the seed until it fires. That is a phone-chrome change, so it is governed by `docs/search-chrome-behaviour.md` and needs `npm run verify:phone-chrome` plus a before/after CLS pair from this harness. **Harness trap worth keeping:** attaching a `MutationObserver` to `document.documentElement` inside a Playwright `addInitScript` throws when the document element does not exist yet, which silently takes the CLS observer down with it and reports a uniform `CLS=0.000` across every route — a false clean bill, not a pass. Guard the attach. **Original next step, now done:** attribute the shifts to elements. Lighthouse's `layout-shift-elements` audit returned **zero items on every route** — the `cumulative-layout-shift` audit carries only `debugdata` — so attribution needs a driven Chromium session with a `PerformanceObserver` on `layout-shift`, reading `entry.sources[].node` and `entry.value`, against the same offline production build. Start with `/dsm`. **Gate — `#118` owns it, and this row supplies a constraint it was missing.** `#118` already tracks flipping `lighthouse-budget.json` to `enforce: true` and says not to do it "before `#117` or the baseline pins a known-slow route". These numbers make that concrete: pinning a baseline today would bake CLS 0.363 on `/dsm` in as the accepted value, so the CLS fixes must land first or the gate ratifies the breach. Separately, no baseline could have been committed from this run anyway — the grader correctly refused it as incomplete evidence because `/forms` produced no report, and that refusal was not overridden. **Reproduction trap:** without `CHROME_PATH`, `chrome-launcher` cannot find a browser in this container and every route fails; the harness reads `CHROME_PATH`/`PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH` (`run-lighthouse-budget.mjs:291`) but nothing sets either, even though `PLAYWRIGHT_BROWSERS_PATH` is set. Related to `#121`. Also seen: a truncated `progress` package in the npx cache broke Lighthouse startup entirely; clearing the npx cache entry fixed it. **Stop:** do not re-dispatch the live Web-Vitals workflow to re-measure CLS — it is reproducible offline for free. Do not commit a budget baseline from a run with a failed route. Do not treat local LCP numbers as production LCP. | `#017` live run `30548662649`; local `verify:lighthouse` 2026-07-30; `lighthouse-budget.json` | 2026-07-30 | -| #149 | P2 | issue | `check:installed-lock-parity` only compares seven top-level packages, so transitive drift passes | **Outcome:** a session cannot run local gates against a `node_modules` that silently disagrees with the lockfile. **Evidence 2026-07-30:** the `SessionStart` hook reported "node_modules matches the lockfile, skipping install" while `brace-expansion` was installed at **1.1.16** and the lockfile pinned **1.1.18** — the CVE-2026-14257 patch. Every local gate run in that window was weaker than it appeared, including several `verify:cheap` runs reported as green during PR #1430/#1444 work. It surfaced only because `tests/installed-lock-parity.test.ts` asserts the patched behaviour directly (`maxLength` honoured); its failure was initially mistaken for a pre-existing repo defect, since it also failed on clean `main` **in the same stale container**. `npm ci --include=dev` fixed it. **CORRECTION 2026-07-30 (PR #1496 review, Codex):** an earlier version of this row said the parity check lives only in `verify:ui` and proposed adding it to `verify:cheap`. **Both were wrong** — `verify:cheap:internal` already runs `check:installed-lock-parity`, so that follow-up was a no-op, and it ran green throughout the stale window. The real gap is its **scope**: `scripts/check-installed-lock-parity.mjs` compares only seven top-level packages (`criticalInstalledPackages` = next, react, react-dom, eslint, playwright, typescript, vitest), so a stale **transitive** dependency is invisible to it. Proved by faking `node_modules/brace-expansion` back to `1.1.16` against the `1.1.18` lockfile pin: `npm run check:installed-lock-parity` **exits 0**. That also explains why the hook and the gate both reported parity while the tree was stale — neither was looking at the package that drifted. **Next:** broaden the check beyond the seven names — validate the full installed tree against the lockfile, or stamp the install with the lockfile hash and compare that — so transitive drift is caught rather than assumed absent. **Stop:** do not treat a green `check:installed-lock-parity` as proof the install is lockfile-current; today it means seven packages match. **Do not archive on the audit-worktree disposition (checked 2026-07-31):** several `comprehensive-audit-*` worktrees carry an uncommitted row archiving this as “Resolved 2026-07-31 — `check:installed-lock-parity` now validates every concrete package location in `package-lock.json`, including nested transitives”. That fix exists nowhere: not on `main`, not in any of the 42 worktrees, and not in any open PR. `main` still reads `criticalInstalledPackages = ["next", "react", "react-dom", "eslint", "playwright", "typescript", "vitest"]`. The sibling `#103` disposition from the same worktrees WAS verified true and landed in PR #1528; this one was not. | `scripts/check-installed-lock-parity.mjs`; `tests/installed-lock-parity.test.ts`; `package.json` | 2026-07-30 | +| #149 | P2 | issue | `check:installed-lock-parity` only compares seven top-level packages, so transitive drift passes | **Outcome:** a session cannot run local gates against a `node_modules` that silently disagrees with the lockfile. **Evidence 2026-07-30:** the `SessionStart` hook reported "node_modules matches the lockfile, skipping install" while `brace-expansion` was installed at **1.1.16** and the lockfile pinned **1.1.18** — the CVE-2026-14257 patch. Every local gate run in that window was weaker than it appeared, including several `verify:cheap` runs reported as green during PR #1430/#1444 work. It surfaced only because `tests/installed-lock-parity.test.ts` asserts the patched behaviour directly (`maxLength` honoured); its failure was initially mistaken for a pre-existing repo defect, since it also failed on clean `main` **in the same stale container**. `npm ci --include=dev` fixed it. **CORRECTION 2026-07-30 (PR #1496 review, Codex):** an earlier version of this row said the parity check lives only in `verify:ui` and proposed adding it to `verify:cheap`. **Both were wrong** — `verify:cheap:internal` already runs `check:installed-lock-parity`, so that follow-up was a no-op, and it ran green throughout the stale window. The real gap is its **scope**: `scripts/check-installed-lock-parity.mjs` compares only seven top-level packages (`criticalInstalledPackages` = next, react, react-dom, eslint, playwright, typescript, vitest), so a stale **transitive** dependency is invisible to it. Proved by faking `node_modules/brace-expansion` back to `1.1.16` against the `1.1.18` lockfile pin: `npm run check:installed-lock-parity` **exits 0**. That also explains why the hook and the gate both reported parity while the tree was stale — neither was looking at the package that drifted. **Next:** broaden the check beyond the seven names — validate the full installed tree against the lockfile, or stamp the install with the lockfile hash and compare that — so transitive drift is caught rather than assumed absent. **Stop:** do not treat a green `check:installed-lock-parity` as proof the install is lockfile-current; today it means seven packages match. **Do not archive on the audit-worktree disposition (checked 2026-07-31):** several `comprehensive-audit-*` worktrees carry an uncommitted row archiving this as “Resolved 2026-07-31 — `check:installed-lock-parity` now validates every concrete package location in `package-lock.json`, including nested transitives”. That fix exists nowhere: not on `main`, not in any of the 42 worktrees, and not in any open PR. `main` still reads `criticalInstalledPackages = ["next", "react", "react-dom", "eslint", "playwright", "typescript", "vitest"]`. The sibling `#103` disposition from the same worktrees WAS verified true and landed in PR #1528; this one was not. **Second, worse instance measured 2026-08-08 — the gap is not only transitive version drift, it is structural completeness.** Two `npm install` runs were killed mid-extraction (a concurrent session ran `npm cache clean`; both exited **127 with an empty log**, no npm error). What survived looked complete: every package directory present at its locked version, and `check:installed-lock-parity` **passed**. But `@testing-library/dom/types/index.d.ts` was **0 bytes** and `@types/node` was **six files short**, missing `web-globals/importmeta.d.ts`. The result was **163 phantom `tsc` errors** — 135 x TS2305 'no exported member screen', plus `ImportMeta.dirname`, `Timeout` and `unref` failures — that read exactly like real code breakage, and `@rolldown/binding-win32-x64-msvc` was absent so Vitest died with `ERR_DLOPEN_FAILED`. A version comparison of any width cannot catch this; the installed files were the right version and simply incomplete. Cheap detection that worked: run the gate against pristine `origin/main` content as well — an identical error count proves the tree innocent and the install guilty. Repair that worked: `rm -rf node_modules` then ONE `npm install `, which reifies the whole tree from the lockfile without touching `package.json` or `package-lock.json`. Worth folding into this row's fix: a completeness check alongside the version comparison — but zero-byte tracked `.d.ts` plus each package's `main`/`types` entry point is NOT enough on its own. The measured miss was `@types/node/web-globals/importmeta.d.ts`, a non-entry file; `@types/node/index.d.ts` stayed present and non-empty, so that shallow check would still pass. Require a completeness method that rejects structurally incomplete installs when arbitrary package files are missing (for example a lockfile/package file inventory, an extract-integrity stamp, or an equivalent that fails closed on a deleted non-entry path), and prove it with a fixture that deletes a non-entry file while leaving entry points intact. | `scripts/check-installed-lock-parity.mjs`; `tests/installed-lock-parity.test.ts`; `package.json` | 2026-07-30 | | #150 | P2 | issue | CodeRabbit reviewed none of a full day's PRs; spending cap reached | **Outcome:** the repo's second automated reviewer is either funded or acknowledged as absent, rather than appearing to review while skipping. **Evidence 2026-07-30:** CodeRabbit posted "Review limit reached … Your organization has reached its usage spending cap" on **every** PR opened that day — #1404, #1430, #1444, #1445, #1479 — reviewing none of them. Each notice renders as an ordinary bot comment, so a skimming reader sees reviewer activity where there was no review. The Codex connector was the sole substantive reviewer across those PRs and found three real defects that had survived local gates and self-review: a proxy-variable inference in #1430, an `unset` vs `unspecified` git-attribute conflation in #1444, and an earlier P1 recursive-delete on an unvalidated `--dir`. **Next:** decide whether to raise the cap, switch to label-based opt-in so the budget lands on PRs that need it, or accept single-reviewer coverage explicitly. **ESCALATED 2026-07-30 — both reviewers are now capped, so this row's premise no longer holds.** The analysis above rests on the Codex connector being the surviving reviewer. On PR #1505 the Codex connector posted "You have reached your Codex usage limits for code reviews" while CodeRabbit posted its own spending-cap notice on the same PR. **That PR therefore received zero automated review**, and so will anything opened while both caps hold. This is not a second issue — it is the same one, with the fallback removed. **Why it is worth more than a status note:** on 2026-07-30 the single Codex finding on PR #1459 was correct and changed the outcome — it showed that a claimed `LoadingPanel` verification had matched `ModeHomePageSkeleton` instead, which caused `#105` to be closed on wrong evidence. Local gates did not catch it and neither did self-review; the review did. A window in which neither reviewer runs is a window in which that class of error lands. **Next:** the three options above now need deciding rather than deferring, because "accept single-reviewer coverage" is no longer one of them. Until then, treat any PR merged during a cap window as locally-gated only. **Stop:** do not read a CodeRabbit *or* Codex comment as a completed review without checking it is not a usage-limit notice — during this window both bots posted comments on every PR while reviewing none of them. | PRs #1404/#1430/#1444/#1445/#1479; `.coderabbit.yaml` | 2026-07-30 | | #151 | P3 | issue | `gh pr checks` cannot read CI, but the Actions API can | **Outcome:** nobody concludes CI is unverifiable when it is merely reached through a different endpoint. **Detail:** the session `gh` credential is a fine-grained PAT with **Actions: read** but **not Checks: read**. `gh pr checks` fails per context with `Resource not accessible by personal access token` and `GET /repos/:o/:r/commits/:sha/check-runs` returns 403, so the obvious routes look like a hard wall. `GET .../commits/:sha/status` succeeds but returns `total: 0`, because this repo reports through Actions rather than legacy commit statuses — a silent empty result that reads like an absence of checks instead of an absence of permission. **The working route** is `gh api "repos/BigSimmo/Database/actions/runs?head_sha="` with a jq filter over `.workflow_runs[]`, which returned CI, PR Policy, PR mergeability, SAST and Secret Scan all `completed/success` for PR #1490. **Cost of not knowing this:** two PRs were handed off as green-locally-but-unverifiable and armed for auto-merge instead of merged; #1490 was then closed unmerged and its unique content lost (re-landed as #152). Separately, `scripts/guard-push.mjs` logs `auto-merge: gh not available` only when the CLI cannot execute; that is a distinct #132 prerequisite, not a Checks-permission symptom. **Next:** either add Checks: read to the PAT so `gh pr checks` works, or document the Actions query as the supported way to read CI here. **Stop:** do not report CI as unreadable without trying the Actions endpoint. | session 2026-07-30; PRs #1490, #1494 | 2026-07-30 | | #152 | P2 | issue | Uncommitted work sits in worktrees whose branches are already merged | **Outcome:** work that exists in no branch and no PR is either committed or knowingly discarded, not lost to a disk reclaim. **Inventory 2026-07-30**, all from worktrees whose pre-snapshot branch tips were fully merged into `origin/main`, so the uncommitted changes existed nowhere else: `codex/reconcile-immediate-20260730` — 21 files, +395/-200 across 19 tracked, including `.github/workflows/ci.yml`, `package.json` and `docs/scripts-index.md`; `codex/document-results-mockup-20260730` — 8 files (+13/-3 tracked) plus an untracked `document-search-results/page.tsx` under `src/app/mockups/` (named without a full path here because it does not resolve in this repo); `codex/chat-ledger-triage-d344` — `docs/outstanding-issues.md` +59/-61; `claude/section-spy-browser-coverage` — `tests/ui-smoke.spec.ts` +51. A fifth (`claude/frosty-mayer-2c6167`) self-resolved to clean during the session. **Preserved 2026-07-30, not reviewed:** each was committed on its own branch as an unpushed `wip: preserve uncommitted work before worktree cleanup` snapshot, so the work now survives a worktree reclaim — `codex/reconcile-immediate-20260730` `748ef018f`, `codex/document-results-mockup-20260730` `5dbd9f965`, `codex/chat-ledger-triage-d344` `b7eae51a4`, `claude/section-spy-browser-coverage` `d949859c3`. All four worktrees are now clean. None is pushed and none is reviewed or verified; the snapshots exist to stop silent loss, not to endorse the content. First captured in PR #1490, which was closed unmerged; re-landed on `main` by PR #1508. **Next:** per snapshot, review and either promote it to a real branch/PR or `git reset --hard HEAD~1` to drop it. Do not bulk-delete worktrees without this check — it is why only one of the two "safe" candidates was removed in the 2026-07-30 cleanup. | session 2026-07-30 worktree cleanup; PR #1490 (closed); PR #1508 | 2026-07-30 | @@ -315,6 +315,7 @@ removed after current-main verification; it is not missing recommended work. | #274 | P2 | task | Services' Clear quick filters leaves the q param in the URL, so a reload restores the filter | **Outcome:** clearing a services quick filter survives a reload. **Detail:** `services-navigator-page.tsx`'s Clear runs `setLocalQuery({ urlQuery, value: "" })`, which clears local state only; the route still carries `?q=crisis`, so a remount or reload re-derives `activeQuickFilter` and the filter comes back. Raised by CodeRabbit against the phone sheet's `onClearAll` in PR #1706, but the phone path was deliberately wired to the *same* handler the desktop Clear button already used — the behaviour is pre-existing on desktop and was mirrored, not introduced. Fixing it in that PR would have changed a shipped desktop control's semantics under a phone-scoped change, so it was deferred rather than bundled. **Next:** add one clear path that both resets local state and pushes a route without `q` (or with the mode's default query), and point both the desktop Clear and the sheet's `onClearAll` at it; assert the URL after Clear in `ui-tools`. **Stop:** do not fix only the phone path — two Clear buttons that disagree about whether the URL changes is worse than one that is consistently wrong. | CodeRabbit review on PR #1706; services-navigator-page.tsx Clear handler | 2026-08-07 | | #275 | P2 | task | The shared filter trigger carries arbitrary spacing values inherited from DocumentFilterTrigger | **Outcome:** the phone filter trigger expresses its measurements as named tokens rather than bracketed values. **Detail:** `result-filter-control.tsx`'s `ResultFilterTrigger` uses `pr-[0.6875rem]`, `h-[1.0625rem]`/`min-w-[1.0625rem]` for the badge, and the raw breakpoint window `min-[414px]:max-[429px]` for the label. CodeRabbit flagged these against the design-token rule in PR #1706. Every one of them is copied *verbatim* from `DocumentFilterTrigger`, which shipped on main earlier and is the component this one was deliberately lifted from so the two cannot drift — so the finding is real but its scope is both call sites, not the new one. Changing only the copy would reintroduce exactly the drift the extraction removed, and each value carries a measured justification in its own comment (the asymmetric padding answers a stroked glyph against a filled pill; the breakpoint window is the one band that is single-line and short of width). **Next:** tokenise in `@theme` once, then update the trigger — there is now only one implementation, so it is a single edit. Confirm the badge and padding render identically at 393/402/414/430px before and after. **Stop:** do not tokenise the trigger without also retiring the values from the documents original, and do not treat this as licence to change the measurements themselves. | CodeRabbit review on PR #1706; DocumentFilterTrigger on main | 2026-08-07 | | #276 | P3 | issue | LinkAction accepts a tone prop it never reads | src/components/ui/link.tsx: LinkActionProps = BaseProps & { href: string }, and BaseProps declares tone?: 'accent' \| 'inherit'. LinkAction destructures only { href, children, className }, so a call site can pass tone='inherit' and get the accent styling anyway, with no type error and no DOM leak to notice it by. The three sibling links (TextLink, ExternalTextLink, DownloadLink) all honour tone, so this is an inconsistency inside one module rather than a deliberate omission. Found while closing #263 (Track A4) and left alone because it is a behaviour change to a link's colour on adopted surfaces, not a defect on the #263 list. Next: decide whether LinkAction should honour tone (apply toneClass[tone] like its siblings) or refuse it (drop BaseProps' tone from LinkActionProps so the compiler rejects the call). Refusing is probably right — a forward-action link is always the accent by design — and is the smaller diff. Gate: npm run check:design-system-contract plus the Links block in tests/ui-v2-components.dom.test.tsx. | session 2026-08-07 closing #263 / COMPONENTS section 0.4; commit e674f6e20 | 2026-08-07 | +| #277 | P2 | issue | docs/design-system/HANDOVER-2026-08-07.md is cited as provenance by nine ledger rows but is measurably wrong | Outcome: no session scopes design-system work from a document whose figures have already been disproved. Evidence: rows #261, #262, #264, #265, #266, #267, #268, #269 and #270 all carry 'session 2026-08-07 — design-system HANDOVER-2026-08-07 Track A1 handoff (PR #1678)' as their Source, and docs/design-system/README.md links it as 'measured state, the ordered plan'. Measured wrong so far, all corrected into the rows themselves rather than the document: its '229 --shadow-tight aliases' is the seven-token legacyShadowAliases total mislabelled as one token (real figure 100 sites across 55 files, total 228); its adoption figure was 24 unadopted against a measured 23; its claim that visual baselines cannot be generated on Windows is half true and led to the wrong conclusion, since the ubuntu CI job already produces the ones that count; and #270's '22 call sites pair a tap token with a dead numeric height' does not survive re-measurement at all (zero same-variant pairs, 84 cross-variant responsive step-downs that are not dead). The document itself still asserts the originals. Next: cheapest fix is a superseded banner at the top naming the ledger rows as the current source of truth, plus the same in docs/design-system/README.md's link text — not a rewrite, because the corrections already live in the rows and duplicating them re-creates the drift. If the live path should leave docs/design-system/, move the file to docs/archive/ (or the design-system archive) and update inbound links per docs/README.md; do not delete it, because the nine Source citations, the PR/commit record, and the handover's verification/gotcha sections are provenance the ledger is meant to preserve. Stop: do not re-copy its figures into any new plan or handover, do not delete the evidence, and do not silently correct it in place, which would leave the nine Source citations pointing at a document that no longer says what those rows were derived from. | session 2026-08-08 — measured while closing #263 follow-ups across PRs #1719 and #1720 | 2026-08-08 | ## Resolved / archive