diff --git a/docs/branch-review-records/78ad337f2fcd7b8a7fe6997dde55a64c29e794a34c2cb241ddbd3fa512a3b604.record.md b/docs/branch-review-records/78ad337f2fcd7b8a7fe6997dde55a64c29e794a34c2cb241ddbd3fa512a3b604.record.md new file mode 100644 index 0000000000..bd4a9e9be2 --- /dev/null +++ b/docs/branch-review-records/78ad337f2fcd7b8a7fe6997dde55a64c29e794a34c2cb241ddbd3fa512a3b604.record.md @@ -0,0 +1 @@ +| 2026-08-13 | codex/sitemap-dom-fixes (PR #1919) | 4076d8034b61fbb3420efa398fab11d9e243b193 | PR #1919 heavy review and fix | Fixed crawler policy and added regression coverage; no other blocking finding | Focused contract and source checks passed | diff --git a/scripts/lighthouse-measurement-outcome.mjs b/scripts/lighthouse-measurement-outcome.mjs index 8c4b46ca96..3c0bee432a 100644 --- a/scripts/lighthouse-measurement-outcome.mjs +++ b/scripts/lighthouse-measurement-outcome.mjs @@ -36,11 +36,10 @@ * measured something real (including something real and slow). */ export function measurementFailureReason(exitCode, reportText) { - if (typeof exitCode !== "number" || exitCode !== 0) { - return `lighthouse exited ${exitCode ?? "without a status"}`; - } if (reportText === null || reportText === undefined || reportText === "") { - return "no report file was written"; + return typeof exitCode !== "number" || exitCode !== 0 + ? `lighthouse exited ${exitCode ?? "without a status"}` + : "no report file was written"; } let parsed; @@ -56,5 +55,10 @@ export function measurementFailureReason(exitCode, reportText) { // cell that produced no comparable numbers. if (typeof code === "string" && code.length > 0) return `lighthouse runtimeError ${code}`; + // Chrome can finish the audit, write a complete report, then fail to remove its + // temporary profile on Windows with EPERM. The report is still the evidence: the + // grader independently rejects missing metrics, runtime errors, wrong pages and + // unverified browser identities. Do not discard that evidence based only on the + // wrapper's cleanup exit status. return null; } diff --git a/scripts/run-lighthouse-budget.mjs b/scripts/run-lighthouse-budget.mjs index 4c21b77823..08fa3bca2c 100644 --- a/scripts/run-lighthouse-budget.mjs +++ b/scripts/run-lighthouse-budget.mjs @@ -21,7 +21,7 @@ * live workflow, which is grading a flaky public network): a route that produced no * report is incomplete evidence, and the grader fails closed on it. * - * A cell that produced no measurement AT ALL — a non-zero exit, no report file, or a + * A cell that produced no measurement AT ALL — no report file, invalid JSON, or a * report carrying only a `runtimeError` such as Lighthouse's own `NO_NAVSTART` — gets * exactly ONE announced retry first (`lighthouse-measurement-outcome.mjs` draws that * line). That is not a softening: a run that never started measured nothing about this @@ -426,6 +426,12 @@ try { let result = await measure(strategy, route, output, firstAttemptTimeout); let reason = measurementFailureReason(childProcessExitCode(result), readIfPresent(output)); + if (!reason && childProcessExitCode(result) !== 0) { + console.log( + `::warning::lighthouse ${cell} wrote a parseable report but ${childProcessFailureSummary(result)} after measurement; grading the report`, + ); + } + if (reason) { // ONE retry, and it is announced. A cell that never started measured nothing // about this diff, so treating it as a regression is wrong — but so is @@ -449,6 +455,11 @@ try { removePathSync(output); result = await measure(strategy, route, output, retryTimeout); const after = measurementFailureReason(childProcessExitCode(result), readIfPresent(output)); + if (!after && childProcessExitCode(result) !== 0) { + console.log( + `::warning::lighthouse ${cell} wrote a parseable report on retry but ${childProcessFailureSummary(result)} after measurement; grading the report`, + ); + } retried.push(`${cell} (${reason}${after ? ` -> still ${after}` : " -> recovered"})`); reason = after; } diff --git a/src/app/layout.tsx b/src/app/layout.tsx index 0c8308d27b..fb476a3cba 100644 --- a/src/app/layout.tsx +++ b/src/app/layout.tsx @@ -10,6 +10,7 @@ import { APP_THEME_COLORS, THEME_BOOTSTRAP_SCRIPT, THEME_COOKIE_NAME } from "@/l import { MobileKeyboardProvider } from "@/components/use-mobile-keyboard"; import { AppAnnouncements } from "@/components/app-announcements"; import { OverlayRoot } from "@/components/ui/overlay-root"; +import { PRIVATE_APP_ROBOTS_METADATA } from "@/lib/crawler-policy"; import "./globals.css"; /** @@ -55,6 +56,7 @@ const baseMetadata: Metadata = { applicationName: "Clinical KB", title: "Clinical KB", description: "Private medical guideline RAG knowledge base", + robots: PRIVATE_APP_ROBOTS_METADATA, appleWebApp: { capable: true, title: "Clinical KB", diff --git a/src/app/robots.ts b/src/app/robots.ts index eab62fe240..99a2b06d18 100644 --- a/src/app/robots.ts +++ b/src/app/robots.ts @@ -1,10 +1,6 @@ import type { MetadataRoute } from "next"; +import { PRIVATE_APP_ROBOTS_TXT } from "@/lib/crawler-policy"; export default function robots(): MetadataRoute.Robots { - return { - rules: { - userAgent: "*", - disallow: ["/mockups/"], - }, - }; + return PRIVATE_APP_ROBOTS_TXT; } diff --git a/src/components/clinical-dashboard/document-search-results.tsx b/src/components/clinical-dashboard/document-search-results.tsx index bd07efd780..0977e9e97b 100644 --- a/src/components/clinical-dashboard/document-search-results.tsx +++ b/src/components/clinical-dashboard/document-search-results.tsx @@ -312,6 +312,16 @@ function DocumentResultMoreMenu({ useEffect(() => { if (!open) return; + let positionFrame: number | null = null; + const scheduleMenuPositionUpdate = () => { + if (positionFrame !== null) return; + positionFrame = window.requestAnimationFrame(() => { + positionFrame = null; + updateMenuPosition(); + }); + }; + const scrollOptions: AddEventListenerOptions = { capture: true, passive: true }; + function closeOutside(event: PointerEvent) { if (menuRef.current?.contains(event.target as Node) || buttonRef.current?.contains(event.target as Node)) return; setOpen(false); @@ -325,14 +335,15 @@ function DocumentResultMoreMenu({ window.document.addEventListener("pointerdown", closeOutside); window.addEventListener("keydown", closeOnEscape); - window.addEventListener("resize", updateMenuPosition); - window.addEventListener("scroll", updateMenuPosition, true); + window.addEventListener("resize", scheduleMenuPositionUpdate); + window.addEventListener("scroll", scheduleMenuPositionUpdate, scrollOptions); updateMenuPosition(); return () => { + if (positionFrame !== null) window.cancelAnimationFrame(positionFrame); window.document.removeEventListener("pointerdown", closeOutside); window.removeEventListener("keydown", closeOnEscape); - window.removeEventListener("resize", updateMenuPosition); - window.removeEventListener("scroll", updateMenuPosition, true); + window.removeEventListener("resize", scheduleMenuPositionUpdate); + window.removeEventListener("scroll", scheduleMenuPositionUpdate, scrollOptions); }; }, [open, updateMenuPosition]); diff --git a/src/components/ui/tooltip.tsx b/src/components/ui/tooltip.tsx index a219f5fc80..b67233b110 100644 --- a/src/components/ui/tooltip.tsx +++ b/src/components/ui/tooltip.tsx @@ -80,13 +80,24 @@ export function Tooltip({ children, content, placement = "top", className }: Too useEffect(() => { if (!open) return; - const frame = window.requestAnimationFrame(updatePosition); - window.addEventListener("resize", updatePosition); - window.addEventListener("scroll", updatePosition, true); + + let frame: number | null = null; + const schedulePositionUpdate = () => { + if (frame !== null) return; + frame = window.requestAnimationFrame(() => { + frame = null; + updatePosition(); + }); + }; + const scrollOptions: AddEventListenerOptions = { capture: true, passive: true }; + + schedulePositionUpdate(); + window.addEventListener("resize", schedulePositionUpdate); + window.addEventListener("scroll", schedulePositionUpdate, scrollOptions); return () => { - window.cancelAnimationFrame(frame); - window.removeEventListener("resize", updatePosition); - window.removeEventListener("scroll", updatePosition, true); + if (frame !== null) window.cancelAnimationFrame(frame); + window.removeEventListener("resize", schedulePositionUpdate); + window.removeEventListener("scroll", schedulePositionUpdate, scrollOptions); }; }, [open, updatePosition]); diff --git a/src/lib/crawler-policy.ts b/src/lib/crawler-policy.ts new file mode 100644 index 0000000000..522b450c72 --- /dev/null +++ b/src/lib/crawler-policy.ts @@ -0,0 +1,30 @@ +import type { Metadata, MetadataRoute } from "next"; + +/** + * Clinical KB is a private application, not a public content catalogue. Keep its + * routes out of search results even when a crawler reaches a URL without first + * consulting robots.txt. + */ +export const PRIVATE_APP_ROBOTS_METADATA = { + index: false, + follow: false, + nocache: true, + googleBot: { + index: false, + follow: false, + noimageindex: true, + nosnippet: true, + }, +} satisfies Metadata["robots"]; + +/** + * Let compliant crawlers fetch application routes so they can observe the global + * `noindex` metadata. Search exclusion is not access control; private content must + * remain protected by authentication. Do not advertise an XML sitemap. + */ +export const PRIVATE_APP_ROBOTS_TXT = { + rules: { + userAgent: "*", + allow: "/", + }, +} satisfies MetadataRoute.Robots; diff --git a/tests/check-lighthouse-budget.test.ts b/tests/check-lighthouse-budget.test.ts index 66a9d6545a..9074337424 100644 --- a/tests/check-lighthouse-budget.test.ts +++ b/tests/check-lighthouse-budget.test.ts @@ -469,10 +469,14 @@ describe("measurementFailureReason", () => { expect(measurementFailureReason(0, report())).toBeNull(); }); - it("flags a non-zero exit", () => { + it("flags a non-zero exit that wrote no report", () => { expect(measurementFailureReason(1, null)).toContain("exited 1"); }); + it("grades a parseable report even when post-measurement cleanup exits non-zero", () => { + expect(measurementFailureReason(1, report())).toBeNull(); + }); + it("flags a run that was killed without a status", () => { expect(measurementFailureReason(null, null)).toContain("without a status"); }); diff --git a/tests/crawler-policy.test.ts b/tests/crawler-policy.test.ts new file mode 100644 index 0000000000..ab28a4fb59 --- /dev/null +++ b/tests/crawler-policy.test.ts @@ -0,0 +1,20 @@ +import { expect, it } from "vitest"; + +import robots from "../src/app/robots"; +import { PRIVATE_APP_ROBOTS_METADATA } from "../src/lib/crawler-policy"; + +it("serves restrictive search metadata through crawlable routes", () => { + expect(robots()).toEqual({ rules: { userAgent: "*", allow: "/" } }); + expect(robots()).not.toHaveProperty("sitemap"); + expect(PRIVATE_APP_ROBOTS_METADATA).toMatchObject({ + index: false, + follow: false, + nocache: true, + googleBot: { + index: false, + follow: false, + noimageindex: true, + nosnippet: true, + }, + }); +}); diff --git a/tests/document-search-record-fault.dom.test.tsx b/tests/document-search-record-fault.dom.test.tsx index f566546edc..6688a1bed8 100644 --- a/tests/document-search-record-fault.dom.test.tsx +++ b/tests/document-search-record-fault.dom.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, within } from "@testing-library/react"; +import { act, fireEvent, render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -134,6 +134,58 @@ describe("document search record path fault reporting", () => { fireEvent.click(within(menu).getByRole("menuitem", { name: "Search only this source" })); expect(baseProps.onScopeDocument).toHaveBeenCalledWith(lithiumMatch.document_id); }); + + it("coalesces more-menu viewport events into one animation-frame measurement", () => { + const pendingFrames: Array<(time: number) => void> = []; + const raf = vi.spyOn(window, "requestAnimationFrame").mockImplementation((callback) => { + pendingFrames.push(callback as (time: number) => void); + return pendingFrames.length; + }); + const rect = vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue({ + x: 20, + y: 20, + left: 20, + top: 20, + right: 120, + bottom: 60, + width: 100, + height: 40, + toJSON: () => ({}), + } as DOMRect); + + try { + render( + , + ); + + const resultCard = screen.getByTestId("document-result-card"); + fireEvent.click(within(resultCard).getByRole("button", { name: `More actions for ${lithiumMatch.title}` })); + expect(screen.getByTestId("document-result-more-menu")).toBeInTheDocument(); + expect(pendingFrames).toHaveLength(1); + act(() => pendingFrames.shift()?.(0)); + const measuredCalls = rect.mock.calls.length; + + act(() => { + window.dispatchEvent(new Event("scroll")); + window.dispatchEvent(new Event("scroll")); + window.dispatchEvent(new Event("resize")); + }); + + expect(pendingFrames).toHaveLength(1); + expect(rect).toHaveBeenCalledTimes(measuredCalls); + act(() => pendingFrames.shift()?.(16)); + expect(rect).toHaveBeenCalledTimes(measuredCalls + 2); + } finally { + raf.mockRestore(); + rect.mockRestore(); + } + }); }); describe("forms record subtitle compaction", () => { diff --git a/tests/ui-v2-components.dom.test.tsx b/tests/ui-v2-components.dom.test.tsx index c5e356b005..701189f36b 100644 --- a/tests/ui-v2-components.dom.test.tsx +++ b/tests/ui-v2-components.dom.test.tsx @@ -792,6 +792,44 @@ describe("Tooltip", () => { } }); + it("coalesces capture scroll events into one passive animation-frame measurement", async () => { + const pendingFrames: Array<(time: number) => void> = []; + const raf = vi.spyOn(window, "requestAnimationFrame").mockImplementation((callback) => { + pendingFrames.push(callback as (time: number) => void); + return pendingFrames.length; + }); + const rect = vi.spyOn(HTMLElement.prototype, "getBoundingClientRect"); + + try { + render( + + + , + ); + + screen.getByRole("button", { name: "Batched trigger" }).focus(); + await screen.findByTestId("tooltip"); + expect(pendingFrames).toHaveLength(1); + act(() => pendingFrames.shift()?.(0)); + await waitFor(() => expect(screen.getByRole("tooltip")).toBeVisible()); + const measuredCalls = rect.mock.calls.length; + + act(() => { + window.dispatchEvent(new Event("scroll")); + window.dispatchEvent(new Event("scroll")); + window.dispatchEvent(new Event("resize")); + }); + + expect(pendingFrames).toHaveLength(1); + expect(rect).toHaveBeenCalledTimes(measuredCalls); + act(() => pendingFrames.shift()?.(16)); + await waitFor(() => expect(rect.mock.calls.length).toBe(measuredCalls + 2)); + } finally { + raf.mockRestore(); + rect.mockRestore(); + } + }); + it("composes over existing child event handlers instead of replacing them", async () => { const onFocus = vi.fn(); const onKeyDown = vi.fn();