Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/process-hardening.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,11 @@ For each: trace which module-scope helpers/icons/types it uses; move solely-cons

- Making the dashboard and document viewer client-only via `dynamic(..., { ssr: false })` (PRs #144/#147) meant "page loaded" no longer implies "app mounted". Firefox/WebKit paint the client chunk later than Chromium, so three release-browser-matrix specs raced and failed on `main` while Chromium stayed green. All three were test-timing gaps, not product regressions — the app renders correctly in every browser.
- `tests/ui-overlap.spec.ts`: `gotoHome` waited on `networkidle` (Playwright discourages it) and then measured the header, which had not mounted yet — every failure was `header#search not found` (count 0), never a real overlap. Now waits for `header#search` to be visible before measuring.
- `tests/ui-tools.spec.ts` (forms detail → shared search): the shell re-syncs its query from the URL on mount via `requestAnimationFrame`, which on Firefox/WebKit can land just after a programmatic `fill` and wipe the value (button stays disabled) or drop the submit before the router navigates. The fill-and-submit now runs as one `toPass` unit that retries until the search routes. Root-cause fix: the `requestAnimationFrame` effect in `GlobalMockupSearchShellClient` now skips the `setQuery("")` reset on detail pages (where the URL carries no `q`/`query` param), so the programmatic fill is never wiped.
- `tests/ui-tools.spec.ts` (forms detail → shared search): the shell re-synced its query from the URL on mount via `requestAnimationFrame`, which on Firefox/WebKit could land just after a programmatic `fill` and wipe the value (button stays disabled) or drop the submit before the router navigates. The fill-and-submit runs as one `toPass` unit that retries until the search routes. **Root-cause fix (see the 2026-07-03 follow-up below):** `GlobalMockupSearchShellClient` no longer uses a `requestAnimationFrame` sync at all — it seeds the composer mode/query from the URL and re-syncs only when the search string actually changes, so a mount-time frame can never wipe an in-progress fill.
- `tests/ui-stress.spec.ts` (desktop evidence panel): the evidence `<details>` is opened by focusing its `<summary>` and pressing Enter; in CI WebKit the key event could fire before focus landed, so it never toggled. Now asserts `toBeFocused()` before pressing Enter.
- Rule of thumb for these client-only surfaces: never gate an interaction on `networkidle` or a bare `goto`. Wait for the specific mounted element, and wrap fill→submit→navigate races in `toPass` (the same idiom `openAppModeMenu`/`openDailyActions` already use). The `verify` + `ui-smoke` PR gates run Chromium only, so Firefox/WebKit-specific races surface solely in the gated `release-browser-matrix` (main/release/dispatch/schedule) — keep that job green rather than letting these re-accumulate.
- **Post-merge outcome (PR #178):** `ui-overlap` and `ui-stress` fixes verified green in CI WebKit. `ui-tools.spec.ts:264` (forms-detail search) still fails on **CI WebKit only** — the composer input stays focused-but-empty and the submit disabled across the full retry, and it does **not** reproduce on local WebKit, so it can't be iterated locally. Ruled out: the inline `availableModeIds={["forms"]}` arrays in the `forms`/`services`/`favourites` layouts churning the effect (those layouts are Server Components, so the ref is stable). On WebKit the test now runs its **structural half** (the detail page renders inside the shell with the Forms composer present) and returns before the known-broken **submit-and-route half** (`if (browserName === "webkit") return;`); Chromium + Firefox still verify the full wiring. The root-cause fix (shell mount `requestAnimationFrame` query-sync) is deferred and needs CI-based iteration — removing that WebKit early-return is its exit criterion.
- **Post-merge outcome (PR #178):** `ui-overlap` and `ui-stress` fixes verified green in CI WebKit. `ui-tools.spec.ts:264` (forms-detail search) still failed on **CI WebKit only** — the composer input stayed focused-but-empty and the submit disabled across the full retry, and it did **not** reproduce on local WebKit, so it couldn't be iterated locally. PR #186 added an interim `isDetailPage` guard to the mount `requestAnimationFrame` but stayed `[WIP]`, keeping a `if (browserName === "webkit") return;` early-return that ran only the **structural half** on WebKit.
- **Follow-up resolution (2026-07-03):** removed the `requestAnimationFrame` mount sync in `GlobalMockupSearchShellClient` entirely. The composer mode/query are now seeded from the URL at initial state, and a `lastSyncedSearchParamsRef` gates the re-sync effect so it fires **only when the search string actually changes** (a real navigation). Because typing never changes the URL and the initial mount is a no-op, no deferred frame can wipe an in-progress `fill` on a no-query detail route — the CI-WebKit-only race. This also retired the `isDetailPage`/`previousUrlHadQueryRef` special-casing from PR #186. With the race fixed at the source, the WebKit early-return and its comment were deleted from `ui-tools.spec.ts:264`, so all three browsers now verify the full submit-and-route wiring. Ruled out earlier (and still true): the inline `availableModeIds={["forms"]}` arrays are stable because those layouts are Server Components.

## Suspense fallback must not re-render page children (2026-07-02)

Expand Down
71 changes: 30 additions & 41 deletions src/components/clinical-dashboard/global-mockup-search-shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,21 @@ function GlobalMockupSearchShellClient({
const requestedQuery = (searchParams.get("q") ?? searchParams.get("query") ?? "").trim();
const requestedMode = searchParams.get("mode");
const searchParamString = searchParams.toString();
// Mode resolved from the URL (?mode=), falling back to this shell's default when
// the param is missing, unknown, or not offered here. Seeds the initial mode and
// re-syncs it after a navigation.
const resolvedSearchMode =
isAppModeId(requestedMode) &&
isAppModeVisible(requestedMode) &&
(!availableModeIds?.length || availableModeIds.includes(requestedMode))
? requestedMode
: initialSearchMode;
const [query, setQuery] = useState(requestedQuery);
const previousUrlHadQueryRef = useRef(currentUrlHasQuery);
const [searchMode, setSearchMode] = useState<AppModeId>(initialSearchMode);
// The search string we last synced into local state, so the effect below only
// reacts to genuine navigations. Seeded with the current string so the initial
// mount is a no-op — the state above is already derived from the URL.
const lastSyncedSearchParamsRef = useRef(searchParamString);
const [searchMode, setSearchMode] = useState<AppModeId>(resolvedSearchMode);
const [queryMode, setQueryMode] = useState<ClinicalQueryMode>("auto");
const [scopeFilters, setScopeFilters] = useState<SearchScopeFilters>({});
const [mobileMenuOpen, setMobileMenuOpen] = useState(false);
Expand All @@ -104,12 +116,6 @@ function GlobalMockupSearchShellClient({
const { theme, toggleTheme } = useTheme();
const auth = useAuthSession();
const sidebarIdentity = useMemo(() => deriveSidebarIdentity(auth.session?.user.email), [auth.session?.user.email]);
const dashboardSearchMode =
isAppModeId(requestedMode) &&
isAppModeVisible(requestedMode) &&
(!availableModeIds?.length || availableModeIds.includes(requestedMode))
? requestedMode
: initialSearchMode;
const shouldRenderDashboardSearch = requestedRun && requestedQuery.length > 0;
const isFormsOnlyShell = availableModeIds?.length === 1 && availableModeIds[0] === "forms";
const isStandaloneModeHome =
Expand All @@ -119,41 +125,24 @@ function GlobalMockupSearchShellClient({
(searchMode === "favourites" && pathname === "/favourites") ||
(searchMode === "differentials" && pathname === "/differentials"));
const isDifferentialPresentationWorkflow = pathname.startsWith("/differentials/presentations");
// True when on a sub-route of a mode home (e.g. /forms/transport-crisis-form,
// /services/13yarn) rather than the mode home itself (/forms, /services).
const isDetailPage =
/^\/(forms|services|favourites)\/.+/.test(pathname) || /^\/differentials\/diagnoses\/.+/.test(pathname);

useEffect(() => {
const frame = window.requestAnimationFrame(() => {
const params = new URLSearchParams(window.location.search);
const requestedMode = params.get("mode");
const nextMode =
isAppModeId(requestedMode) &&
isAppModeVisible(requestedMode) &&
(!availableModeIds?.length || availableModeIds.includes(requestedMode))
? requestedMode
: initialSearchMode;
setSearchMode(nextMode);
// Re-derive the mode and query from the URL, but only when the search string
// actually changes (a real navigation). Reacting on every render — as the old
// requestAnimationFrame sync effectively did — let a deferred frame land after
// a programmatic/user fill and wipe the controlled input; on slow CI WebKit
// that raced the forms-detail composer to empty (input focused-but-empty,
// submit stuck disabled). Typing never changes the URL, so a URL-gated sync
// cannot clobber in-progress input, and the initial mount is skipped entirely
// because the state above is already seeded from the URL.
if (lastSyncedSearchParamsRef.current === searchParamString) return;
Comment thread
BigSimmo marked this conversation as resolved.
lastSyncedSearchParamsRef.current = searchParamString;

const urlHasQuery = params.has("q") || params.has("query");
const hadQueryBeforeThisSync = previousUrlHadQueryRef.current;
previousUrlHadQueryRef.current = urlHasQuery;
if (urlHasQuery) {
// Sync the controlled query state from the URL query param.
const requestedQuery = (params.get("q") ?? params.get("query"))?.trim();
setQuery(requestedQuery ?? "");
} else if (!isDetailPage || hadQueryBeforeThisSync) {
// On no-query routes, clear any stale URL-derived query. Initial detail
// page mounts still skip the deferred clear so programmatic fills are
// not wiped by the WebKit requestAnimationFrame race.
setQuery("");
}
setSearchMode(resolvedSearchMode);
setQuery(currentUrlHasQuery ? requestedQuery : "");

if (params.get("focus") === "1") inputRef.current?.focus({ preventScroll: true });
});
return () => window.cancelAnimationFrame(frame);
}, [availableModeIds, initialSearchMode, isDetailPage, pathname, searchParamString]);
if (searchParams.get("focus") === "1") inputRef.current?.focus({ preventScroll: true });
}, [currentUrlHasQuery, requestedQuery, resolvedSearchMode, searchParamString, searchParams]);

useEffect(() => {
let cancelled = false;
Expand Down Expand Up @@ -231,14 +220,14 @@ function GlobalMockupSearchShellClient({
navigateToMode("answer", { query: recentQuery, focus: true });
}

if (shouldRenderDashboardSearch && dashboardSearchMode === "forms" && isFormsOnlyShell) {
if (shouldRenderDashboardSearch && resolvedSearchMode === "forms" && isFormsOnlyShell) {
return <FormsSearchResultsPage query={requestedQuery} focusSearch={searchParams.get("focus") === "1"} />;
}

if (shouldRenderDashboardSearch) {
return (
<ClinicalDashboard
initialSearchMode={dashboardSearchMode}
initialSearchMode={resolvedSearchMode}
initialQuery={requestedQuery}
focusSearch={searchParams.get("focus") === "1"}
autoRunSearch
Expand Down
24 changes: 7 additions & 17 deletions tests/ui-tools.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ test.describe("Clinical KB applications launcher", () => {
await expectNoPageHorizontalOverflow(page);
});

test("form detail pages keep the shared forms search wired to form results", async ({ page, browserName }) => {
test("form detail pages keep the shared forms search wired to form results", async ({ page }) => {
await page.setViewportSize({ width: 1280, height: 900 });
await gotoLauncher(page, "/forms/transport-crisis-form");

Expand All @@ -274,22 +274,12 @@ test.describe("Clinical KB applications launcher", () => {
const formsSearchInput = page.locator('input[placeholder="Search forms..."]:visible').first();
await expect(formsSearchInput).toBeVisible();

// Submit-and-route half is known-broken on CI Linux WebKit only: the shell's
// mount requestAnimationFrame query-sync wipes the composer value there (the
// input stays focused-but-empty and the submit disabled), so the search never
// routes. It does not reproduce on local WebKit and needs CI-based iteration
// on the shell to fix. Skip ONLY this half on WebKit (tracked as follow-up);
// Chromium and Firefox still verify the full wiring, and WebKit keeps the
// structural checks above. See docs/process-hardening.md "Cross-browser test
// robustness".
if (browserName === "webkit") return;

// Under client-only (ssr:false) rendering the shell re-syncs its query from
// the URL on mount via requestAnimationFrame. On Firefox that frame can land
// right after a programmatic fill — wiping the value, disabling the submit, or
// dropping the submit before the router navigates. Drive the fill-and-submit
// as one retried unit until the search actually routes to the forms results
// URL; the assertions below still verify the result.
// The shell now seeds its composer state from the URL and only re-syncs on a
// real navigation, so a programmatic fill on this no-query detail route is no
// longer wiped by a mount-time frame — the race that used to break CI WebKit
// (and could flake Firefox). Drive the fill-and-submit as one retried unit
// regardless, so any residual cross-browser navigation-timing jitter cannot
// flake the route assertion; the assertions below still verify the result.
const formsSearchButton = page.getByRole("button", { name: "Search forms" });
await expect(async () => {
// A previous attempt's click may have navigated late — after the inner URL
Expand Down
Loading