diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 796f718a73..13e4b1469f 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -252,4 +252,5 @@ 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 | 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 | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 1d89933df3..23b0ea40fa 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -140,7 +140,6 @@ removed after current-main verification; it is not missing recommended work. | #103 | P2 | issue | Wide table-facts trigram index missing from `schema.sql` | **Outcome:** the migration chain and `schema.sql` agree on `document_table_facts` trigram indexes. `supabase/migrations/20260714190000_document_table_facts_trgm_idx.sql` creates a wide 5-column trigram index that is **absent from `supabase/schema.sql`**, so local replay and the live database can diverge. Distinct from #102: different owner and verification path. **Next:** confirm whether the wide index exists live, then take one of exactly two routes — **retained:** mirror `document_table_facts_text_trgm_idx` into `supabase/schema.sql` beside the narrow one and regenerate `drift-manifest.json`; **redundant:** drop it through a new forward migration, never by deleting `20260714190000`. **`drift-allowlist.json` is NOT a third option** (PR #1377 review): its own header scopes it to _"Known live-vs-`schema.sql` divergence"_, so it can silence a live drift finding but cannot reconcile the migration chain with the mirror — a fresh `supabase db reset` still runs `20260714190000` and creates the index while `schema.sql` still omits it, leaving this row's stated outcome unmet. **No offline gate catches this today:** the migration↔`schema.sql` parity test (`tests/drift-detection.test.ts:59-68`) only asserts one migration's `schema_drift_snapshot` function definition, not an index inventory — which is why this sits open rather than red in CI, and why a replay-to-schema inventory comparison is the check that would have caught it. Note the narrow `document_table_facts_title_row_param_trgm_idx` (`schema.sql:6425`) is the one the effective RPC expression (`:6726`) actually matches, so the wide index may be genuinely redundant — do not drop it without live scan evidence, per the monitored-not-auto-fixed index policy. | `docs/audit/latency-audit-2026-07-28.md` limitations; `npm run check:drift` | 2026-07-29 | | #105 | P3 | task | Verify the `#017`-exempt client latency wins in a browser | **Outcome:** the two zero-payload client fixes are confirmed in a real browser. `#017` gates _payload_ decisions (#012/#013/#016 are all byte-count items); a `loading` fallback ships zero bytes and a resource hint ships ~60, so neither can be justified or refuted by a Lighthouse number — that is why neither was held behind #017. **Preconnect half DONE 2026-07-30.** Verified against a dev server started with a placeholder `NEXT_PUBLIC_SUPABASE_URL` — locally the var is unset, `supabaseOrigin()` returns null and nothing renders, so the check fails silently without it. React 19 hoists both tags out of ``: SSR HTML and the live DOM both show `preconnect` and `dns-prefetch` in ``, zero stray `preconnect` in ``, and `crossOrigin="anonymous"` preserved — load-bearing, since without it supabase-js's CORS fetches open a second connection. **`LoadingPanel` half STILL PENDING, and a claimed verification of it was WITHDRAWN 2026-07-30 — read this before re-verifying.** It was briefly recorded as done on the strength of `role="status" aria-label="Loading"` appearing in the SSR HTML of `/`, `/dsm` and `/forms`. Those hits are real but they are **not `LoadingPanel`**: that exact string has exactly one renderer in `src/`, `ModeHomePageSkeleton` (`src/components/mode-home-page-skeleton.tsx:8`), and `LoadingPanel` never emits a bare `"Loading"` label — it renders `aria-label={label}`, always a specific string such as `"Loading differentials"`. Re-checked per route against a running server: `/`, `/dsm`, `/forms`, `/differentials` and `/favourites` return **zero** `aria-label="Loading "` matches. **Why, and what it means for the technique:** Codex's mechanism finding on PR #1459 is correct — the installed Next 16 loader sets `hasSuspenseBoundary` when a `loading` element is supplied, and `BailoutToCSR` throws on the server, so a mounted `ssr:false` surface DOES emit its fallback into the response HTML. But that only fires for a surface that mounts during the initial render, and none of the eleven in `clinical-dashboard-lazy.tsx` do: every one sits behind interaction state (`answer` present, a drawer open, results present — see the mount conditions in `ClinicalDashboard.tsx:3666-4070`). So neither a cold-load grep nor a throttled cold load can observe them. **Next:** verify with a driven browser session — navigate, perform the interaction that mounts the surface, and assert its specific `LoadingPanel` label — or accept that these fallbacks are unobservable on cold load and re-scope the row to say so. **Stop:** the two sidebar dialogs are intentionally excluded — they mount on open, so a fallback would render into a closed dialog. Do not re-close this row on a cold-load grep; that is the exact mistake being withdrawn here. | `docs/audit/latency-audit-2026-07-28.md` L3-4/L3-5; `src/components/clinical-dashboard/clinical-dashboard-lazy.tsx` | 2026-07-29 | | #106 | P2 | rec | Ingestion worker and indexing agent are verified by grepping their own source | **Outcome:** the ingestion worker and indexing agent are verified by executing code, not by asserting on their own source text. **Detail:** measured 2026-07-29 via `npm run test:coverage` — `worker/main.ts` (2,015 lines) and `supabase/functions/indexing-v3-agent/index.ts` (1,966 lines) each report **0% executed lines**; no test imports either module. Both are covered only by `readFileSync` + `toContain` assertions in `worker-safe-logging.test.ts`, `worker-visual-capture.test.ts` and `document-metadata-merge.test.ts`, which pass whenever a string is present and break on harmless refactors; `document-metadata-merge.test.ts` additionally reimplements the SQL deep-merge in TypeScript and tests the reimplementation rather than the worker. Area totals: `worker/` 18.6% lines, `supabase/functions/` 4.5%. **Next:** continue the extraction pattern that already works here — `indexing-v3-agent/behavior.ts` (167 lines, 96%) and `ingestion-worker/auth.ts` (30 lines, 90%) — pulling the highest-risk decision points out of `worker/main.ts` (job claim/retry, generation commit, failure classification) into importable modules with executing tests, retiring the matching source-text assertion as each lands. Roughly cost-neutral: each extracted test replaces a grep assertion. **Stop:** do not try to make the 2,000-line entrypoint importable in one pass; extract incrementally and keep each step green. | session 2026-07-29 test-coverage analysis | 2026-07-29 | -| #107 | P2 | rec | Component state matrices are the largest untested surface | **Outcome:** loading / empty / error / disabled states on interactive components are covered by executing tests, not only by E2E happy paths. **Detail:** measured 2026-07-29 — production components (excluding mockups) sit at **38.2% lines / 22.8% branch** across 12,602 lines, with **83 of 208 files at zero executed lines**; there are 51 `.dom.test.tsx` files against 195 components. Playwright does visit these routes, so they are smoke-covered, but branch coverage is where the state matrix lives and smoke journeys rarely reach it. Worst by uncovered lines: `global-search-shell.tsx` (7%), `mode-action-popup.tsx` (21%), `answer-content.tsx` (27%), `document-search-results.tsx` (32%), `universal-search-command-surface.tsx` (39%), `master-search-header.tsx` (43%). A concrete first target with clinical meaning: `calculator-ui.tsx` now covers all exported scoring logic, but `seedCheckboxDefaults`, `toggleCheckboxAnswer` and `selectOptionAnswer` stay uncovered because they are module-private and only reachable through React event handlers — `seedCheckboxDefaults` is what makes an all-negative CAGE / SAD PERSONS screen read as a valid 0 rather than incomplete, so a regression there is a false-negative risk. **Next:** treat as a per-PR convention rather than a backfill push — `docs/testing.md` already prescribes the state matrix, so the gap is enforcement. Start with `global-search-shell.tsx`, which `docs/search-chrome-behaviour.md` treats as a contract surface. Keep additions in the jsdom tier (measured ~0.54s per file) instead of new Playwright journeys (~231 production journeys already run serially at `workers: 1` against a 45-minute CI budget). **Stop:** do not chase the coverage percentage by backfilling low-risk components; the re-ratcheted broad floor in `vitest.config.mts` holds the line. | session 2026-07-29 test-coverage analysis | 2026-07-29 | | #110 | P3 | task | Design-system project token manifest lags its stylesheet | **Outcome:** the claude.ai/design token panel matches the shipped stylesheet. **Detail:** PR #1375 pushed a recompiled `_ds_bundle.css` (Clinical Sky, `--e0`–`--e4`, 4px radius grid, `--tracking-eyebrow`/`--leading-display`/`--leading-prose`) plus the four changed guideline docs to project `08d6f126`, but `_ds_manifest.json` is converter-generated and still advertises `--text-4xs: 0.5rem`, the old `--radius-lg/xl/2xl` values, and `--tw-leading`/`--tw-tracking` entries scoped to the retired `.leading-[…]` / `.tracking-[0.08em]` utilities. Rendering is correct; only the token inventory lags. Hand-editing was rejected — `kind`/`scope`/`annotation` are converter heuristics and a wrong panel is worse than a stale one. **Next:** in a session with the `/design-sync` skill, `npm ci`, then `npm install --prefix .ds-sync --no-save --package-lock=false esbuild ts-morph @types/react @tailwindcss/cli geist`, read `.design-sync/NOTES.md`, and run `resync.mjs --remote` so bundle and manifest regenerate together. **Stop:** do not hand-author `_ds_manifest.json`; the converter is not a published npm package and ships with the skill. | PR #1375; `.design-sync/NOTES.md`; project `08d6f126` (`_ds_needs_recompile` marker present) | 2026-07-29 | | #121 | P3 | issue | Container Playwright browser build lags the pinned client | **Outcome:** browser gates run in remote sessions without hand-patching. **Evidence 2026-07-30:** the repo's Playwright client resolves headless-shell build `1234`; the container image provides `1194` at `/opt/pw-browsers`, so every browser test fails at launch. Worked around in-session by symlinking `chromium_headless_shell-1234/chrome-headless-shell-linux64/chrome-headless-shell` to the `1194` `headless_shell` binary plus its sibling resources — container-local, nothing committed, and it disappears with the session. `PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1` means the mismatch cannot self-heal. **Next:** decide whether the image pins the browser build or the repo pins a client matching the image; until then any remote session claiming browser proof must state which it used. **Added 2026-07-30 (session closing `#120`):** the mismatch reproduced unchanged on `main` at `c5c1a86` — `npx playwright install --dry-run chromium` reports `chromium v1234` while `/opt/pw-browsers` holds only `chromium-1194` and `chromium_headless_shell-1194`, and one `verify:phone-chrome` run lost all 13 browser tests at launch. It has now been misread twice: the 2026-07-30 handoff records 13 launch failures taken as "my change is wrong", and `#120` was filed as a gate defect from a reading taken under this condition (closed as not reproducible; the gate exits 1 correctly). **Detection, before trusting or filing anything from a browser gate:** compare `npx playwright install --dry-run chromium` against `ls /opt/pw-browsers`. **Stop:** do not file a gate defect from a run whose tests never launched — zero assertions executed, so the output describes the environment, not the diff. | `docs/testing.md`; container `/opt/pw-browsers` | 2026-07-30 | | #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 | @@ -176,6 +175,7 @@ Move resolved rows here with the resolution date and a one-line outcome. Keep th | ID | Type | Summary | Outcome | Resolved | | ---- | ----- | ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| #107 | rec | Component state matrices are the largest untested surface | RESOLVED 2026-07-30. The state-matrix convention is enforced through executing jsdom coverage on two previously weak, high-branch interactive surfaces rather than a low-value percentage backfill. `DocumentSearchResultsPanel` now proves loading → settled-empty transition, empty-query home actions, unavailable/error reporting, registry loading/error/unauthorized states, and successful governed results. `ModeActionPopup` now proves closed → open → action-close, keyboard entry/navigation/Escape focus restoration, and disabled mode selection. Together with the existing `docs/testing.md` per-PR state-matrix requirement, the re-ratcheted broad component coverage floor, and the source-preview lifecycle suite, this closes the recommendation without adding serial Playwright journeys or chasing low-risk components. | 2026-07-30 | | #131 | issue | `@codex fix` produced commits that never reached the repository | RESOLVED 2026-07-30. Automatic repair requests now bind Codex to the exact repository, pull-request head branch, and starting SHA; explicitly forbid detached/synthetic `work` branches and stacked pull requests; and require the authenticated GitHub connector to publish and verify the fix before success is reported. Thread closure is fail-closed: a fixed reply must name its 40-character pushed commit SHA, and the workflow resolves the thread only when that SHA is the pull-request head. No-code dispositions carry a distinct marker, while missing, conflicting, or unlanded result claims leave the thread open with a visible workflow failure. The guard and executable workflow tests pin the destination prompt, both valid outcomes, malformed results, and mismatched-head refusal. Source: issue #131; session 2026-07-30 | 2026-07-30 | | #136 | rec | Reuse Next build cache across isolated Playwright production builds | CLOSED 2026-07-30 after an end-to-end implementation benchmark rejected the proposed cache as a net CI loss. Keeping the reusable cache outside each disposable run root preserved the runner cleanup contract, but a warm build saved only 34 seconds (97s cold to 63s warm) while producing an 804 MB cache. Persisting that entry per commit would consume the repository cache budget and evict the substantially more valuable Playwright browser cache. No cache wiring ships; reconsider only if Next materially reduces the cache size or a later measurement changes the storage/time trade-off. Source: `scripts/run-playwright.mjs`; `docs/testing.md`; session 2026-07-30 | 2026-07-30 | | #085 | rec | Upload-limit client/server sync is unguarded | Resolved 2026-07-30. A provider-free parity checker now validates both configured limits, their shared 150 MB default, invalid and over-ceiling values, and mismatches. It runs in `verify:cheap`, the static PR job, and before every production build so a client-side value cannot silently diverge from the server runtime limit. | 2026-07-30 | diff --git a/tests/document-search-record-fault.dom.test.tsx b/tests/document-search-record-fault.dom.test.tsx index 8b22e78783..a8f70c1250 100644 --- a/tests/document-search-record-fault.dom.test.tsx +++ b/tests/document-search-record-fault.dom.test.tsx @@ -1,4 +1,5 @@ import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { DocumentSearchResultsPanel } from "@/components/clinical-dashboard/document-search-results"; @@ -123,3 +124,54 @@ describe("document search record path fault reporting", () => { expect(screen.getByRole("button", { name: `Answer from ${lithiumMatch.title}` })).toBeInTheDocument(); }); }); + +describe("document search state matrix", () => { + const documentProps = { + ...baseProps, + showRecordMatches: false, + recordMatches: [], + }; + + it("announces loading and replaces it with the empty result after the request settles", () => { + const { rerender } = render(); + + const loadingLabel = screen.getByText("Finding matching documents"); + expect(loadingLabel.closest('[role="status"]')).toBeInTheDocument(); + expect(screen.queryByText("No matching documents")).not.toBeInTheDocument(); + + rerender(); + expect(screen.queryByText("Finding matching documents")).not.toBeInTheDocument(); + expect(screen.getByText("No matching documents")).toBeInTheDocument(); + }); + + it("renders the document home for an empty query and wires every escape action", async () => { + const user = userEvent.setup(); + const onOpenRecentDocuments = vi.fn(); + const onOpenLibrary = vi.fn(); + const onOpenSourcePdf = vi.fn(); + render( + , + ); + + expect(screen.getByTestId("document-search-empty-state")).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: /Recent documents/i })); + await user.click(screen.getByRole("button", { name: /Browse sources/i })); + await user.click(screen.getByRole("button", { name: /Open a source PDF/i })); + expect(onOpenRecentDocuments).toHaveBeenCalledTimes(1); + expect(onOpenLibrary).toHaveBeenCalledTimes(1); + expect(onOpenSourcePdf).toHaveBeenCalledTimes(1); + }); + + it("reports an unavailable document search alongside the empty-result guidance", () => { + render(); + + expect(screen.getByRole("alert")).toBeInTheDocument(); + expect(screen.getByText(/No matching documents/)).toBeInTheDocument(); + }); +}); diff --git a/tests/mode-action-popup.dom.test.tsx b/tests/mode-action-popup.dom.test.tsx new file mode 100644 index 0000000000..d9dcad1193 --- /dev/null +++ b/tests/mode-action-popup.dom.test.tsx @@ -0,0 +1,97 @@ +import { useState } from "react"; + +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { Search, UploadCloud } from "lucide-react"; +import { describe, expect, it, vi } from "vitest"; + +import { + ModeActionPopup, + modeActionItemsFor, + type ModeActionModeOption, +} from "@/components/clinical-dashboard/mode-action-popup"; + +const modeOptions: ModeActionModeOption[] = [ + { id: "documents", label: "Documents", icon: Search }, + { id: "answer", label: "Answer", icon: Search }, + { id: "upload", label: "Upload", icon: UploadCloud, disabled: true }, +]; + +function Harness({ onAction = vi.fn(), onModeSelect = vi.fn() }) { + const [open, setOpen] = useState(false); + + return ( + + ); +} + +describe("ModeActionPopup state transitions", () => { + it("starts closed, opens its action menu, and closes after an action", async () => { + const user = userEvent.setup(); + const onAction = vi.fn(); + render(); + + const trigger = screen.getByRole("button", { name: "Open document actions" }); + expect(trigger).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByRole("menu", { name: "Documents" })).not.toBeInTheDocument(); + + await user.click(trigger); + expect(trigger).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByRole("menu", { name: "Documents" })).toBeInTheDocument(); + + await user.click(screen.getByRole("menuitem", { name: "Upload PDF" })); + expect(onAction).toHaveBeenCalledWith("documents-upload"); + expect(screen.queryByRole("menu", { name: "Documents" })).not.toBeInTheDocument(); + expect(trigger).toHaveAttribute("aria-expanded", "false"); + }); + + it("opens from the keyboard and moves focus through the action list", async () => { + const user = userEvent.setup(); + render(); + + const trigger = screen.getByRole("button", { name: "Open document actions" }); + trigger.focus(); + await user.keyboard("{ArrowDown}"); + + const first = screen.getByRole("menuitem", { name: "Upload PDF" }); + await waitFor(() => expect(first).toHaveFocus()); + await user.keyboard("{End}"); + expect(screen.getByRole("menuitem", { name: "Open source PDF" })).toHaveFocus(); + await user.keyboard("{Escape}"); + expect(screen.queryByRole("menu", { name: "Documents" })).not.toBeInTheDocument(); + await waitFor(() => expect(trigger).toHaveFocus()); + }); + + it("selects enabled modes and exposes disabled modes without selecting them", async () => { + const user = userEvent.setup(); + const onModeSelect = vi.fn(); + render(); + + await user.click(screen.getByRole("button", { name: "Open document actions" })); + const modeTrigger = screen.getByRole("button", { name: "Documents" }); + await user.click(modeTrigger); + + expect(screen.getByRole("menu", { name: "Choose search mode" })).toBeInTheDocument(); + expect(screen.getByRole("menuitemradio", { name: "Documents" })).toHaveAttribute("aria-checked", "true"); + const disabledMode = screen.getByRole("menuitemradio", { name: "Upload" }); + expect(disabledMode).toBeDisabled(); + await user.click(disabledMode); + expect(onModeSelect).not.toHaveBeenCalled(); + + await user.click(screen.getByRole("menuitemradio", { name: "Answer" })); + expect(onModeSelect).toHaveBeenCalledWith("answer"); + expect(screen.queryByRole("menu", { name: "Choose search mode" })).not.toBeInTheDocument(); + await waitFor(() => expect(modeTrigger).toHaveFocus()); + }); +});