diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 6b569caaf..d7e99d935 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -672,10 +672,12 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-06 | a24f74fdf0134487a03dce37dd9f1e9bd18502f5 | a24f74fdf0134487a03dce37dd9f1e9bd18502f5 | PR #1614 post-merge RAG index restoration audit | Pass - guard-only migration, no DDL, no ranking/RPC change; pr-policy ragRanking=false so no eval-canary required; 1 P3 doc nit (#248 renumber note says 237->246, row is #248) | check:migration-role; npx vitest run tests/supabase-schema.test.ts (74 passed); check:outstanding-issues | | 2026-08-06 | PR #1614 / codex/restore-rag-indexes-20260804 | a24f74fdf0134487a03dce37dd9f1e9bd18502f5 | PR #1614 post-merge RAG index restoration audit | Pass - guard-only migration, no DDL, no ranking/RPC change; pr-policy ragRanking=false so no eval-canary required; 1 P3 doc nit (#248 renumber note says 237->246, row is #248); supersedes 2026-08-06 row (ref column mistakenly held commit SHA instead of PR ref, breaking ledger:lookup per Devin/Sentry review on PR #1636) | check:migration-role; npx vitest run tests/supabase-schema.test.ts (74 passed); check:outstanding-issues | | 2026-08-06 | claude/implement-97vpz7 | 00ab7bfd34684bc854d15a3f28987674098a7130 | PR #1646 soft-tail answer-cache skip + soft-tail test hardening | fixed — answer-path soft-tail skip via rag-query-guard helpers; soft-tail fixture pins; duplicate memo test removed; in-corpus assert narrowed; budget 4362 | test:rag-query-guard+unsupported-cache+classifier-memo 22/22,check:maintainability-budgets 4362/4362 | +| 2026-08-06 | claude/settings-nav-freeze-desktop-tdzh7z (PR #1641) | d4ecd2b5fc48a8d8f37f081cac2d80beb749df01 | Run PR sweep: CI fix + threads + drift | before: mergeable/BLOCKED, 0 unresolved threads, 0 behind main; prior CI reds were infra (npm ECONNRESET on Unit coverage; e7f380f3 cancelled at Set up job). after: pushed d4ecd2b5 ResizeObserver pin-clamp distance reset; all review threads remain resolved; CI re-triggered but GitHub Actions major_outage — runs pending/queued, no product failure to fix; merge tree clean | local typecheck PASS; local lint (settings-dialog) PASS; format unchanged; no provider-backed checks run; hosted CI awaiting Actions recovery (run 31120931239) | +| 2026-08-06 | claude/settings-nav-freeze-desktop-tdzh7z (PR #1641) | c31827e63c20788c5dabb22b96ddbf239c65f893 | Run PR sweep: CI fix + threads + drift | before: mergeable/BLOCKED, 0 unresolved threads, 0 behind main; prior CI reds were infra (npm ECONNRESET; cancelled Set up job). after: product fix d4ecd2b5 (ResizeObserver pin-clamp distance reset) + ledger row; all review threads resolved; merge tree clean; hosted CI pending on GitHub Actions major_outage (no product failure) | local typecheck PASS; local lint (settings-dialog) PASS; no provider-backed checks run; hosted CI awaiting Actions recovery | +| 2026-08-06 | temp-rebase | 868a8a2800351ce85a2ad13e14d80550cbc3e668 | Merge conflict resolution and CI fixes | Verified and ready for PR | verify:pr-local | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | fff524d73ebfafeef7bdc50752a6d14f253ebc2e | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in fff524d73ebfafeef7bdc50752a6d14f253ebc2e, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | f67e5c9103c3e1483ad8e034fe7a6757c2362d74 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in f67e5c9103c3e1483ad8e034fe7a6757c2362d74, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 36c1bccd89f976bcae3dabf8f5787198db5df078 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in 36c1bccd89f976bcae3dabf8f5787198db5df078, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 3403126bc6cd86145d6921a3fe4d081181e8a113 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in 3403126bc6cd86145d6921a3fe4d081181e8a113, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 057a5579e538fcacc15feb8a0ac7fa580b3decb4 | PR #1649 review-and-fix | fixed: Bugbot cross-session prune disarm + marker-write fail-open context + deny gh pr comment/review; superseded dead Run PR ledger HEADs; merge-tree clean vs main; threads cleared; CI re-trigger on push | vitest:pr-handoff-stop 11/11; verify:cheap 516 files/5457 tests; verify:pr-local pass (format+lint+typecheck+test+rag-fixtures); bash -n hook OK; security-review: no P0/P1; no provider gates | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 057a5579e538fcacc15feb8a0ac7fa580b3decb4 | Run PR sweep: CI fix + threads + drift | supersede: prior stacked Run PR rows used unresolvable HEADs (fff524d7/f67e5c91/3403126b); reviewed product tip is this SHA (ledger bookkeeping may sit one commit above); Bugbot prune+context+comment/review fixes landed | vitest:pr-handoff-stop 11/11; verify:cheap pass; verify:pr-local pass | -| 2026-08-06 | temp-rebase | 868a8a2800351ce85a2ad13e14d80550cbc3e668 | Merge conflict resolution and CI fixes | Verified and ready for PR | verify:pr-local | diff --git a/src/components/clinical-dashboard/settings-dialog.tsx b/src/components/clinical-dashboard/settings-dialog.tsx index 0a04dd2c9..b469684d2 100644 --- a/src/components/clinical-dashboard/settings-dialog.tsx +++ b/src/components/clinical-dashboard/settings-dialog.tsx @@ -81,6 +81,28 @@ const APPEARANCE_OPTIONS: ReadonlyArray<{ value: ThemePreference; label: string; { value: "system", label: "System", icon: Monitor }, ]; +// The section rail is `hidden lg:flex`. Match the repo's desktop seam literally +// (`1024px` in globals.css / JS media queries), not Tailwind's `64rem` token — +// rem-based seams drift from that CSS when the root font size is not 16px. +const settingsRailMediaQuery = "(min-width: 1024px)"; + +// Scroll offsets are fractional on hidpi displays, so "the same position" needs +// a pixel or two of slack. The spy marker uses the same tolerance so a settle +// that lands within it still answers as the clicked section. +const scrollSettleTolerance = 2; + +// Sticky title bar clearance for focus-scroll into section content. Matches the +// desktop header's pt-6 + title leading-8 + pb-3 budget with a little slack so +// keyboard focus does not land under the opaque bar. +const settingsSectionScrollMarginClass = + "scroll-mt-[max(4.5rem,calc(env(safe-area-inset-top)+3.5rem))] lg:scroll-mt-24"; + +/** + * A section chosen from the rail, held while the scroll travels to it and until + * the reader scrolls somewhere else themselves. + */ +type PinnedSection = { id: SettingsSectionId; offset: number; distance: number; settled: boolean }; + function sectionDomId(id: SettingsSectionId) { return `settings-section-${id}`; } @@ -116,13 +138,23 @@ export function SettingsDialog({ const closeButtonRef = useRef(null); const guideButtonRef = useRef(null); const scrollRef = useRef(null); + // The title bar is sticky inside the scroll region on every breakpoint, so its + // height is the amount of the scroll port a section would otherwise land + // underneath — both the scroll-spy root and the click-to-scroll offset have to + // subtract it. + const headerRef = useRef(null); + // Section chosen from the rail, held until the reader scrolls for themselves. + const pinnedSectionRef = useRef(null); + // Memoised so phone scroll does not reconstruct a MediaQueryList on every event. + const settingsRailMqlRef = useRef(null); + const scrollSpyFrameRef = useRef(null); const settingsEmailInputRef = useRef(null); const { theme, preference: themePreference, setPreference: setThemePreference } = useTheme(); const { preferences, setPreference, resetPreferences } = useAppPreferences(); // Hide-on-scroll for the mobile glass header (phone-gated inside the hook), so // the top goes fully edge-to-edge while scrolling — the same behaviour as the - // app's search bar. Desktop keeps a static in-panel header. + // app's search bar. Desktop keeps its title bar pinned and never hides it. const { hidden: headerHidden, reportScroll } = useScrollHideReporter(); const auth = useAuthSession(); @@ -169,51 +201,245 @@ export function SettingsDialog({ setDataCounts(readDataCounts()); }, []); - // Desktop scroll-spy: highlight the section nearest the top of the scroll - // region so the rail mirrors what the reader is looking at. + /** + * Desktop scroll-spy: the rail highlights the last section whose heading has + * passed under the sticky title bar. + * + * Read from geometry on scroll rather than from an IntersectionObserver. An + * observer callback receives only the entries whose intersection *changed* in + * that batch, so picking "the topmost visible entry" from it picks the topmost + * of a partial set — which is how selecting the last rail item could leave the + * rail highlighting the one above it. + */ + const syncActiveSection = useCallback((container: HTMLDivElement) => { + const railMql = settingsRailMqlRef.current; + if ( + railMql ? !railMql.matches : typeof window !== "undefined" && !window.matchMedia(settingsRailMediaQuery).matches + ) { + return; + } + const sectionEls = container.querySelectorAll("[data-settings-section]"); + if (sectionEls.length === 0) return; + const maxOffset = container.scrollHeight - container.clientHeight; + // The final rendered section is shorter than the scroll port, so its heading + // can never reach the marker line. The end of the runway belongs to it — + // take the id from the DOM, not from SETTINGS_SECTIONS, so a conditional + // section cannot desync the two sources of truth. + if (maxOffset > 0 && maxOffset - container.scrollTop <= scrollSettleTolerance) { + const lastId = sectionEls[sectionEls.length - 1]?.getAttribute("data-settings-section"); + if (lastId) setActiveSection(lastId as SettingsSectionId); + return; + } + const marker = + container.getBoundingClientRect().top + (headerRef.current?.offsetHeight ?? 0) + scrollSettleTolerance; + let next = + (sectionEls[0]?.getAttribute("data-settings-section") as SettingsSectionId | null) ?? SETTINGS_SECTIONS[0].id; + for (const element of sectionEls) { + if (element.getBoundingClientRect().top > marker) break; + next = element.getAttribute("data-settings-section") as SettingsSectionId; + } + setActiveSection(next); + }, []); + useEffect(() => { - if (!open || typeof IntersectionObserver === "undefined") return; + if (typeof window === "undefined") return; + settingsRailMqlRef.current = window.matchMedia(settingsRailMediaQuery); + }, []); + + useEffect(() => { + if (!open) return; + // The Sheet unmounts its children while closed, so the scroll port comes + // back at offset 0 — but this component stays mounted, so a pin left over + // from the last visit would survive and hold the spy inert on the fresh + // surface. Cleared here rather than in the open-reset block above, which + // runs during render where a ref must not be written. + pinnedSectionRef.current = null; const container = scrollRef.current; - if (!container) return; - const sectionEls = Array.from(container.querySelectorAll("[data-settings-section]")); - if (sectionEls.length === 0) return; + if (container) syncActiveSection(container); + }, [open, syncActiveSection]); - const observer = new IntersectionObserver( - (entries) => { - const visible = entries - .filter((entry) => entry.isIntersecting) - .sort((a, b) => a.boundingClientRect.top - b.boundingClientRect.top); - const next = visible[0]?.target.getAttribute("data-settings-section"); - if (next) setActiveSection(next as SettingsSectionId); - }, - { root: container, rootMargin: "0px 0px -62% 0px", threshold: [0, 0.35] }, - ); - sectionEls.forEach((element) => observer.observe(element)); - return () => observer.disconnect(); - }, [open]); + // Geometry changes (viewport resize, panel height tracking 88dvh, email form + // expanding a section) used to re-fire via IntersectionObserver. Re-run the + // spy on resize / content size without releasing an in-flight rail pin. + useEffect(() => { + if (!open || typeof ResizeObserver === "undefined") return; + let cancelled = false; + let observer: ResizeObserver | null = null; + let frame = 0; + const reevaluate = () => { + const container = scrollRef.current; + if (!container) return; + const pin = pinnedSectionRef.current; + if (pin) { + // Content/viewport growth can make the click target unreachable; keep + // the pin, but clamp its offset so arrival remains possible. Reset + // distance from the live scrollTop too: leaving the pre-clamp gap + // would make the next scroll frame look like a takeover (distance + // suddenly larger than pin.distance) and drop the hold mid-animation. + const maxOffset = Math.max(0, container.scrollHeight - container.clientHeight); + if (pin.offset > maxOffset) { + pin.offset = maxOffset; + pin.distance = Math.abs(container.scrollTop - pin.offset); + pin.settled = pin.distance <= scrollSettleTolerance; + } + return; + } + syncActiveSection(container); + }; + + const detachWindow = () => { + window.removeEventListener("resize", reevaluate); + settingsRailMqlRef.current?.removeEventListener("change", reevaluate); + }; + + const attach = () => { + const container = scrollRef.current; + if (!container || cancelled) return false; + observer = new ResizeObserver(reevaluate); + observer.observe(container); + for (const child of container.children) { + if (child instanceof Element) observer.observe(child); + } + window.addEventListener("resize", reevaluate); + settingsRailMqlRef.current?.addEventListener("change", reevaluate); + return true; + }; + + if (!attach()) { + // Sheet children mount with `open`; one frame is enough for the ref. + frame = window.requestAnimationFrame(() => { + frame = 0; + attach(); + }); + } + + return () => { + cancelled = true; + if (frame) window.cancelAnimationFrame(frame); + observer?.disconnect(); + detachWindow(); + }; + }, [open, syncActiveSection]); + + // Scroll the settings scroll port itself rather than calling + // `target.scrollIntoView()`. `scrollIntoView` walks every scrollable ancestor, + // and an ancestor with `overflow: hidden` is still programmatically + // scrollable — so it used to drag the whole two-column panel up, taking the + // section rail and the close control out of the dialog with it. const scrollToSection = useCallback( (id: SettingsSectionId) => { setActiveSection(id); + pinnedSectionRef.current = null; const container = scrollRef.current; const target = container?.querySelector(`[data-settings-section="${id}"]`); - if (!target) return; + if (!container || !target) return; const prefersReducedMotion = preferences.motion === "reduced" || (typeof window !== "undefined" && window.matchMedia("(prefers-reduced-motion: reduce)").matches); - target.scrollIntoView({ behavior: prefersReducedMotion ? "auto" : "smooth", block: "start" }); + const headerOffset = headerRef.current?.offsetHeight ?? 0; + const top = + container.scrollTop + target.getBoundingClientRect().top - container.getBoundingClientRect().top - headerOffset; + // Clamp to what the port can actually reach. The last sections are shorter + // than the port, so their ideal offset lies past the end of the runway — + // an unclamped target is one the scroll can never arrive at, and the pin + // below would wait for that arrival forever. + const maxOffset = Math.max(0, container.scrollHeight - container.clientHeight); + const offset = Math.min(Math.max(0, top), maxOffset); + const distance = Math.abs(container.scrollTop - offset); + pinnedSectionRef.current = { id, offset, distance, settled: distance <= scrollSettleTolerance }; + // Avoid `behavior: "instant"` / `"auto"`: `"auto"` defers to the container's + // `scroll-smooth`, and engines that do not recognise `"instant"` can fall + // back to that CSS smooth path — the exact reduced-motion failure. Write + // `scrollTop` under an inline `scroll-behavior: auto` instead. + if (prefersReducedMotion) { + const previousBehavior = container.style.scrollBehavior; + container.style.scrollBehavior = "auto"; + container.scrollTop = offset; + container.style.scrollBehavior = previousBehavior; + } else { + container.scrollTo({ top: offset, behavior: "smooth" }); + } }, [preferences.motion], ); + /** + * Decide whether a scroll belongs to the reader or to a rail click still in + * flight, and return whether the spy may act on it. + * + * Driven by scroll position, never by which input event arrived. Releasing on + * `wheel`/`touchmove`/`keydown` alone missed the one interaction this dialog + * just regained — dragging the native scrollbar emits `scroll` and nothing + * else — which left the rail highlighting the clicked section while the reader + * was somewhere else entirely. + */ + const admitScrollToSpy = useCallback((container: HTMLDivElement) => { + const pin = pinnedSectionRef.current; + if (!pin) return true; + const distance = Math.abs(container.scrollTop - pin.offset); + if (distance <= scrollSettleTolerance) { + // Arrived. Hold the highlight here: a section shorter than the port cannot + // reach the marker line, so geometry would answer a click on "Shortcuts" + // with whichever neighbour sits at the top of the runway's end. + pin.settled = true; + pin.distance = distance; + return false; + } + // Use `<=`, not `<`: a coalesced rAF can run after the pin is armed but + // before smooth-scroll has moved `scrollTop`. Strict `<` treats that + // no-progress frame as a reader takeover and drops the pin immediately, + // so a rapid second rail click (or any scroll listener still in flight) + // loses the hold before the animation starts. + if (!pin.settled && distance <= pin.distance) { + // Still closing the gap (or not yet moved), so this is the click's own + // scroll animation. + pin.distance = distance; + return false; + } + // Either the reader scrolled after it settled, or they took the scroll over + // mid-flight and the gap stopped shrinking. The choice is theirs now. + pinnedSectionRef.current = null; + return true; + }, []); + const handleScroll = useCallback( (event: UIEvent) => { const el = event.currentTarget; reportScroll({ offset: el.scrollTop, maxOffset: el.scrollHeight - el.clientHeight, source: el }); + // Coalesce spy geometry reads to one frame, matching useHideOnScroll. + if (scrollSpyFrameRef.current != null) return; + scrollSpyFrameRef.current = window.requestAnimationFrame(() => { + scrollSpyFrameRef.current = null; + // Sheet returns null while closed and unmounts this port, but the dialog + // component stays mounted. A frame armed before close must not sync from + // the detached node and overwrite the reopen reset (`activeSection` → + // account) after the fresh port is already at offset 0. + if (scrollRef.current !== el || !el.isConnected) return; + if (admitScrollToSpy(el)) syncActiveSection(el); + }); }, - [reportScroll], + [admitScrollToSpy, reportScroll, syncActiveSection], ); + // Cancel a coalesced spy frame on close (and on unmount). The empty-deps + // cleanup alone left the frame live across `open` flips. + useEffect(() => { + if (!open) { + if (scrollSpyFrameRef.current != null) { + window.cancelAnimationFrame(scrollSpyFrameRef.current); + scrollSpyFrameRef.current = null; + } + return; + } + return () => { + if (scrollSpyFrameRef.current != null) { + window.cancelAnimationFrame(scrollSpyFrameRef.current); + scrollSpyFrameRef.current = null; + } + }; + }, [open]); + async function submitSettingsEmail(event: FormEvent) { event.preventDefault(); if (!settingsEmail.trim()) return; @@ -283,7 +509,14 @@ export function SettingsDialog({ contentClassName="w-full max-w-none border-[color:var(--border-lux)] bg-[color:var(--background)] font-sans shadow-none max-lg:!pb-0 lg:max-w-[940px] lg:bg-[color:var(--surface-lux)] lg:shadow-[var(--shadow-lux)]" bodyClassName="p-0" > -
+ {/* The desktop height must be definite, not `h-auto` + `max-h-`. With an + auto height the single grid row sizes to max-content (the full ~2800px + of settings), overflows the capped container, and is clipped by + `overflow-hidden` — so the scroll column below never overflows its own + box and `overflow-y-auto` never engages. That left desktop settings + unscrollable, with the section rail stretched off the bottom of the + panel. A definite height bounds the row, which bounds the column. */} +