diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 045467909f..03efc6d73c 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -252,6 +252,8 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | PR #1477 | 26d713922006c1af8187994edfa76669dc14cd46 | PR #1477 fork-safe Codex autofix routing | Fixed fork routing to the PR head repository, added fail-closed metadata handling, reconciled current main, and found no remaining actionable defects. | check:codex-autofix-workflow; check:github-actions; check:pr-policy; check:outstanding-issues; check:branch-review-ledger; docs:check-inventory; docs:check-links; docs:check-scripts; typecheck; focused Vitest 53 passed; Prettier | | 2026-07-30 | PR #1477 | 20f795da2d9d0adafa6cb3117429ab3665129c0d | PR #1477 fork-safe Codex autofix routing | Refreshed onto current main after #1465; issue and ledger reconciliation remained clean and no new actionable defects were introduced. | check:outstanding-issues; check:branch-review-ledger; check:codex-autofix-workflow; focused Vitest 53 passed | | 2026-07-30 | PR #1480 | 6c1e76f53aee87be8408cebc295744fbdce05367 | PR #1480 bounded outstanding reliability fixes | Fixed both review findings: documented the dark accent role and added partial favourites retry without hiding valid counts; no other actionable defects found. | focused Vitest 119 passed; docs index; issue and ledger guards; Actions and Codex workflow guards; Prettier; diff check; typecheck coordinator-blocked | -| 2026-07-30 | claude/global-search-mockups-mrgmzl | e79e499839e56bff66aecc37d1c915ac3127e995 | prlanded | merged and verified by content | verify:cheap exit 0 (437 files, 4576 passed); verify:pr-local exit 0; CI pr-required green on a6f2281; squash e79e499 content diff vs branch tip empty; late aria-live/role=alert commit confirmed present on main | -| 2026-07-30 | PR-1469 | 02108d5424f8a3ab50f45808a6cc3cbd872e7555 | PR #1469 component state matrix coverage | PASS after current-main sync; tests execute enabled and disabled popup transitions plus document search loading, empty, and fault states | focused Vitest 2 files, 10 tests passed; outstanding-issues and branch-review-ledger guards passed; no unresolved review threads | | 2026-07-30 | codex/close-pr1480-issues | bf8ac88b024642eb45d1fead86f4ee30fce3f98d | archive PR 1480 issue resolutions | approved: five resolved rows moved intact to archive | check:outstanding-issues; prettier check; diff check | +| 2026-07-30 | codex/close-issue-127 | 9bbb8486d399ed31b9bf43364579f466a4e66c67 | archive issue 127 after post-fix runs | approved: close condition satisfied with no post-fix recurrence | check:outstanding-issues; prettier check; diff check | +| 2026-07-30 | codex/close-issue-127 | d5fd28f75b404046f13507aa3705d55898d7669c | review finding wording correction | approved: causal wording now matches PR 1427 implementation | check:outstanding-issues; prettier check; diff check | +| 2026-07-30 | PR-1469 | 02108d5424f8a3ab50f45808a6cc3cbd872e7555 | PR #1469 component state matrix coverage | PASS after current-main sync; tests execute enabled and disabled popup transitions plus document search loading, empty, and fault states | focused Vitest 2 files, 10 tests passed; outstanding-issues and branch-review-ledger guards passed; no unresolved review threads | +| 2026-07-30 | claude/global-search-mockups-mrgmzl | e79e499839e56bff66aecc37d1c915ac3127e995 | prlanded | merged and verified by content | verify:cheap exit 0 (437 files, 4576 passed); verify:pr-local exit 0; CI pr-required green on a6f2281; squash e79e499 content diff vs branch tip empty; late aria-live/role=alert commit confirmed present on main | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index c1cef43d3f..a92db24eb0 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -145,7 +145,6 @@ removed after current-main verification; it is not missing recommended work. | #126 | P3 | task | Quarterly branch-review ledger rotation reminder | **Outcome:** live ledger stays navigable after #1418 L4 bootstrap. **Next:** each UTC calendar-quarter start (or when the live table feels unwieldy), run `npm run ledger:rotate -- --dry-run`, then `npm run ledger:rotate` and commit live+archive. Lookup/sweep/check already read archives. **Stop:** do not hand-move rows; do not delete unique review content. | session 2026-07-30; follow-up to #1418 / L4 | 2026-07-30 | | #117 | P2 | rec | Therapy Compass catalogue payload is the mobile LCP outlier | **Outcome:** `/therapy-compass` mobile LCP lands near the other mobile routes instead of double them. **Measured 2026-07-30** by the new pre-merge Lighthouse budget: mobile LCP 5229 ms, TBT 612 ms, CLS 0.142, against 2123-2460 ms on every other mobile route and 826 ms on desktop — so it is client-side work under mobile CPU/network throttling, not server latency. **Cause:** `useTherapyData` fetches `public/therapy-compass-data/therapies-index.json` (690 KB raw, 139 KB gzipped, 205 records x 16 fields) for the home/search/pathways screens, so the download plus JSON parse sits on the critical path before content paints. 90% of that weight is long-form clinical prose — indications 159 KB (26%), contraindicationsOrCautions 139 KB (23%), bestUsedFor 73 KB (12%), clinicalSummary 67 KB (11%), patientPopulation 59 KB (10%), targetSymptoms 48 KB (8%) — while name, slug, category, tags and setting together are 54 KB (7%). **Blocked on one decision per field group: rendered on the card, matched by search, or neither.** `therapy-card.tsx` references five of those prose fields and the same index feeds the search screen, so stripping fields could silently change clinical display or search recall. **Next:** settle that per-field question, then either pre-truncate prose that only feeds card display, or move search matching server-side / load prose on first keystroke. **Gate:** `check:therapy-data-index` plus the therapy Playwright journeys; re-measure with `npm run verify:lighthouse`. **Stop:** do not drop a field from the catalogue payload without confirming no card renders it and no search path matches on it. Same class as #013 (route-chunk / catalogue JSON weight), different route and now measured. | session 2026-07-30 Lighthouse budget first run; PR #1404 | 2026-07-30 | | #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 fail with a missing-snapshot error by design; the job uploads them on every run (run 30513537912, artifact 8748062487, 31 files). `lighthouse-budget.json` ships `enforce: false` with `baseline: null`, so the grader warns rather than grades. **Next:** (1) download that 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 | -| #127 | P2 | issue | Document-route phone header did not hide; likely a short drag, fixed by #1427 | **Outcome:** the document-route phone header hides when asked, and a failure names its own cause. **Observed:** `Production UI` failed on `30521269873` (1 variant) and `30524711173` (both variants, `2 failed, 339 passed`), always at the reduced-motion hide after the section-sheet round-trip, then **passed** on `30530393684` — intermittent, 2 of 3 completed runs. **Cause most likely already fixed on main by PR #1427 (2026-07-30), not by anything in this row's earlier diagnoses.** #1427 found that `addPhoneScrollRunway` slept 50 ms and merely hoped the appended 1600 px runway had reached layout, and that `dragScrollBy` clamped silently at the end of the range while reporting nothing — so under CI load the drag delivered less than it asked for and the chrome was right to stay visible. It now polls for the runway, returns the distance actually travelled, and `dragScrollUntilHidden` refuses to expect a hide until both the remaining runway and the delivered travel clear 160 px. **Two earlier diagnoses in this row were wrong and are withdrawn:** (1) `sharedChromePinned` is stuck — rested on reading DocumentViewer's composer as a proxy for the header's `scrollHidden`, which it is not, since the header runs the shell's `chromeScrollHide` and DocumentViewer runs its own reporters; (2) a short/clamped drag was ruled out early using a `maxOffset` of 2753 measured at a different moment in the trace than the failing drag — the pre-runway reading in the same trace was 1153, and a runway that had not fully landed puts the offset inside the near-bottom band where `computeScrollHideUpdate` legitimately refuses. That is the mechanism #1427 fixes. **Still open here:** #1427's own comment notes the remaining gap — `data-scroll-hidden` is the composite `scrollHidden && !sharedChromePinned`, so a reporter that never fired and a latched pin are indistinguishable. `data-scroll-signal` now publishes the raw signal and `expectChromeHidden` reports it, so if this recurs after #1427 the failure says which. **Next:** watch for a recurrence on a post-#1427 head; if none appears, close this as fixed by #1427. **Stop:** do not infer the header's scroll state from any page-owned composer, and do not read a `maxOffset` from one trace moment as the value at another. **No recurrence across four post-#1427 runs** (`30530618838`, `30532582652`, `30534158395`, `30535651470`): this test passed in every one, including the run whose shard 1 failed on a different test (#141). Per this row's own next step, that is the evidence for closing it as fixed by #1427 — held open only for a wider sample on main. | `tests/ui-phone-scroll.spec.ts`; PR #1427; `master-search-header.tsx`; `global-search-shell.tsx:332,345,876` | 2026-07-30 | | #129 | P2 | issue | GitHub's `update-branch` API doesn't honor this repo's `merge=ledger` driver | **Outcome:** `update-branch` can report a 422 "merge conflict between base and head" on a branch that a local `git merge origin/main` resolves cleanly. **Detail:** on 2026-07-30 PR #1406's branch was several commits behind `main` and touched `docs/branch-review-ledger.md`, which carries `merge=ledger` in `.gitattributes` specifically so parallel ledger appends resolve without conflict (see #088/#112). GitHub's own server-side merge/update-branch implementation does not read `.gitattributes` custom merge drivers, so it computed a real textual conflict at the same hunk the local `ledger` driver resolves. `git merge-tree --write-tree origin/main ` confirmed clean; the API call still 422'd. **Next:** when `update-branch` fails on a branch touching `docs/branch-review-ledger.md` (or any other `merge=ledger` path) and `git merge-tree` shows no real conflict, treat it as staleness rather than a genuine conflict needing manual resolution and fall back to a local `git merge origin/main` + push (per the existing "Open PR branch sync" guidance) — same as any other push, this still needs the explicit user confirmation AGENTS.md's "API and provider confirmation boundary" requires outside an authorized sweep (`Run PR`/`upload`), not a standing exemption for `merge=ledger` paths. **Stop:** do not conclude a real content conflict from `update-branch`'s response alone on a custom-merge-driven file; verify with `git merge-tree` first — same discipline as the existing GitHub `dirty`/`CONFLICTING` staleness guidance. | PR #1406; session 2026-07-30 PR babysit | 2026-07-30 | | #130 | P2 | issue | PR #1396 merged shared phone-chrome behaviour without its own declared physical-device gate | **Outcome:** a shared-chrome PR does not merge with a self-declared merge prerequisite left undone, or the ledger records that it did. **Detail:** PR #1396 ("overlay the phone header so hiding it never moves content") repeatedly stated in its own body and PR comments that `docs/phone-chrome-physical-acceptance.md` "genuinely applies before merge" because local Chromium cannot certify Safari chrome-minimisation or cold-launch PWA paint (invariant 23) — restated at least three times across the review thread, including after the final `a638b66e`/`f7347144` fix. It merged at 06:49:55 anyway. Checked 2026-07-30: `docs/phone-chrome-physical-acceptance.md` on `main` is still the blank checklist template — every "Result / evidence" cell is empty, no PR comment attaches a filled-in copy or device evidence, and no existing ledger row (`#120`, `#122`) covers this gap. Related but distinct: one Codex thread on this PR also names a still-missing guard — a pre-paint/cold-load hydration test comparing content position before and after hydration, which the author explicitly said they would "rather file it than ship a test that looks like it covers the window and does not" — and that filing never happened either. **Next:** run the physical-device matrix in `docs/phone-chrome-physical-acceptance.md` against `main`'s current tip on a real iPhone (Safari tab + cold-launch PWA, light/dark, portrait/landscape) and commit the filled-in evidence; separately, add the pre-paint/cold-load Playwright pattern this PR's own review identified as missing. **Stop:** do not treat this PR's extensive Codex/CI remediation (13 findings fixed, 9 threads resolved) as a substitute for the physical-device proof — headless Chromium was explicitly stated as unable to certify the two things this checklist exists for. **Design constraints recorded 2026-07-30, so the guard is not re-derived from scratch:** the value under test is the pre-paint reserve seed `calc(max(0.5rem, var(--safe-area-top)) + var(--shell-header-h))` in `globals.css`, refined by `useLayoutEffect` in `use-phone-overlay-chrome-reserve.ts`. The window that needs covering is _before_ hydration, so the test must sample content top on the cold load and again after hydration and compare them; a single post-hydration read passes on the broken shape and is the "looks like coverage" outcome this item exists to avoid. The `max()` is the part that actually breaks: seeding the bare inset under-reserves by `max(0, 0.5rem − inset)`, which is **zero on a notched iPhone and 8px on any phone reporting no top inset** — Android, and Playwright's default emulation — so the assertion must run on a zero-inset profile or it cannot fail. Prove it against the broken shape before trusting it (re-seed with the bare inset and confirm the test goes red), per the lesson recorded on `#120`. **Environment blocker:** this cannot be verified in a remote container. The Chromium build mismatch in `#121` means browser tests never launch, and the documented symlink bridge writes under `/opt/pw-browsers`, which the session sandbox refuses — so this needs a local session, or an operator-granted exception, before any claim that the guard works. | PR #1396 (merged 2026-07-30); session 2026-07-30 PR babysit | 2026-07-30 | | #132 | P3 | issue | Both client-side push guards are inert for agent pushes | **Outcome:** the format and auto-merge guards protect every push, or their blind spot is explicit. **Detail:** `scripts/guard-push.mjs` printed `auto-merge: gh not available — auto-merge check skipped (fail-open)` for pushes from a remote agent environment, so the auto-merge race sentinel never evaluated; and `core.hooksPath` is set only by a local `npm install`, so an agent pushing from its own checkout bypasses `.githooks/pre-push` entirely. Both guards therefore protect exactly the environment least likely to break the rule, which is why the AGENTS.md format-before-push instruction is still load-bearing even though the tooling now exists. Observed directly on PR #1400: a push landed while auto-merge was armed with nothing to stop it. **Next:** provide `gh` (or a token-based equivalent) in agent environments so the sentinel can evaluate. **Do not move the format check into `pull_request_target`** — that context carries secrets and a write token, and a format check must execute PR-head code including this repo's now-loadable dynamic `prettier.config.*`, which is the classic privileged-context vector; `.github/workflows/pr-policy.yml` deliberately checks out only `github.workflow_sha` for exactly this reason. Formatting is already enforced server-side by `Static PR checks` running `format:check` on ordinary `pull_request` CI, so the guard's only unique value is failing fast before the push — nothing to duplicate. The auto-merge sentinel reads PR metadata only and could safely live in `pull_request_target` if it is ever worth moving. | PR #1400; session 2026-07-30 | 2026-07-30 | @@ -165,6 +164,10 @@ removed after current-main verification; it is not missing recommended work. Move resolved rows here with the resolution date and a one-line outcome. Keep them — do not delete. +| ID | Type | Summary | Outcome | Resolved | +| ---- | ----- | --------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| #127 | issue | Document-route phone header intermittently failed to hide | Resolved 2026-07-30 by PR #1427, which fixed the short/clamped drag mechanism. Its runway polling and delivered-travel assertions then passed across the four recorded post-fix runs and the later PR #1480 exact-head browser/PWA document-header journeys, with no recurrence. | 2026-07-30 | + | ID | Type | Summary | Outcome | Resolved | | ---- | ----- | ------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | | #091 | issue | Results band could not express a partial-source failure | Resolved by PR #1480. Favourites and shared result bands now retain only the honest loaded count, visibly report unavailable sources, and expose a tested Retry recovery path without masking partial failures as ready. | 2026-07-30 |