diff --git a/bundle-budget.json b/bundle-budget.json index 9ebae9854d..034faf5186 100644 --- a/bundle-budget.json +++ b/bundle-budget.json @@ -2,6 +2,6 @@ "$comment": "Client JS bundle-size budget captured from a known-good production build. CI fails when total gzip size grows beyond tolerancePct; refresh intentionally with `npm run check:bundle-budget -- --update`.", "enforce": true, "tolerancePct": 10, - "totalGzipBytes": 1363382, - "updatedAt": "2026-07-17T12:11:55.267Z" + "totalGzipBytes": 1309274, + "updatedAt": "2026-07-18T15:49:00.100Z" } diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index e488e8d704..b1d8b0d1e2 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -610,3 +610,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-18 | codex/rag-merge-final-20260718-655a | 20e5964bbfdcac0311b89c96931a43544be545a6 | RAG recovery, grounding, and latency merge closure | Reviewed the recovered historical RAG fix series against fresh `origin/main`. No additional source change was needed: current main already contains the stricter source-scoped numeric grounding, blocked-retrieval-only extractive recovery, generic LAI deterministic recovery, per-case answer latency diagnostics, and agitation lexical-path protections. The only residual branch diff was proven unreachable because every medication-chart trigger yields explicit search terms; it was intentionally not carried forward. | Full local Vitest: 301 files / 2,779 tests passed. Offline RAG fixture and production-contract validation: 36 golden cases, 21 suites, 294 tests passed. Prettier and `git diff --check` passed. `verify:cheap` reached its lint stage after all preceding static guards passed, but was blocked twice by active heavyweight jobs in other registered worktrees; it was not retried. No Supabase, OpenAI, live retrieval, production-readiness, deployment, or hosted CI command was run. | | 2026-07-18 | codex/main-merge-51278-final-20260718-late | 45fa3c6c7, 14a0a898c | merge integration of 51278a70d onto fresh origin/main | Replayed the requested historical design-audit commit onto fresh `origin/main`. Kept current-main versions for five conflicts, including the regenerated drift manifest and current schema assertions. Fixed the duplicate sitemap-generator declaration, corrected redirect-section coverage, and restored the current `Clinical Guide` UI contract. | `git diff --check` and `npm run sitemap:check` passed. `npm run test` was attempted twice but blocked by the repository-wide heavy-command lock held by a separate Playwright worktree; no provider-backed checks ran. | | 2026-07-18 | origin/main framework and dependency modernization snapshot | 4057677c8b92a5e1d997ec44958764fa91f5d424 | parallel build/infra, backend, and frontend modernization audit | Changes requested. No P0. Confirmed two P1 defects: Supabase SSR 0.12 auth-cookie responses discard mandatory anti-cache headers, and reindex bypasses the server-only-aware TSX runner after mutation-capable setup. Five P2 blockers cover the Webpack-to-Turbopack production cutover, incomplete Railway image-build watch ownership, missing clean `next typegen`, Node 26 types over a Node 24 runtime, and App Router retry actions that reset without re-fetching. P3 removal-readiness debt remains in Zod and Next Image APIs. Manual rewrite zones are auth response ownership, bundler/CSP/artifact consumers, JSZip resource limits, OpenAI request typing, and React Compiler adoption in the stateful dashboard/viewer roots. | Parallel read-only source/config/test audit against the exact snapshot; Node 24.18.0/npm 11.17.0; exact-version Next 16.2.10 bundled upgrade, Turbopack, error-boundary, and codemod guidance; TypeScript 6 backend no-emit analysis and Node import probes via a separately installed exact-version local dependency tree; `git diff --check`. No install, registry outdated/audit, full repo lint/typecheck/test/build/browser/Docker, Supabase/OpenAI, deployment, or hosted CI run; this worktree had no `node_modules`. | +| 2026-07-18 | claude/clinical-kb-pwa-review-asi3wb (PR #890, plan Phase 4; single-batch implementation+ledger commit — Phase 3 landed as squash 44a4c511bc1381168474794d6f89273564659ead) | ceabc04d75fd0cea2a120d504418d44edd704bfb | App-wide performance pass: audit-then-fix + budget ratchet (plan Phase 4) | Audit on post-#872 main: enforced gzip budget +3.13% over the 1,363,382-byte baseline; analyzer treemap showed pdfjs-dist (123 KB) and cross-mode-differentials data (121 KB) already correctly lazy, and disproved the suspected forms-catalog dashboard leak (type-only import). Confirmed one real defect: the 143-line `/services` client home page value-imported `defaultServiceSlug` from `@/lib/services`, compiling the ~915 KB services snapshot (~100 KB gzip) into its route chunk. Fixed by computing the slug in the server page (`src/app/services/page.tsx`) and passing it as a prop; the client component builds its task cards from the prop. Measured result: budget swung from +3.13% to −3.97%; baseline ratcheted down 1,363,382 → 1,309,286 bytes gzip so CI locks the win. pdfjs/differentials/#718 paths deliberately untouched per audit rules. Two transient `.next/dev` generated-type corruptions from the long-lived dev server were resolved by stopping the server and setting the generated dir aside (reversible); nothing hand-deleted. | Build + `check:bundle-budget --json` before/after (artifacts in session scratchpad); `check:bundle-budget -- --update` for the ratchet; `npm run test` 2789 passed/1 failed (known container-only `pdf-extraction-budget` artifact, hosted-CI-green through #826/#835/#872); `verify:cheap` green to the same artifact; `verify:ui` 219 passed/2 failed (the two long-established container artifacts; ui-tools spec covering the services surface passed); `verify:pr-local` unit stage identical; typecheck/lint/format clean. Lighthouse not run (dev-server churn; bundle evidence sufficed for this fix set). No provider-backed checks run. | diff --git a/src/app/services/page.tsx b/src/app/services/page.tsx index 9a2148397f..ad6cf732ea 100644 --- a/src/app/services/page.tsx +++ b/src/app/services/page.tsx @@ -2,6 +2,7 @@ import { Suspense } from "react"; import { ServicesHomePage } from "@/components/services/services-home-page"; import { ServicesNavigatorPage } from "@/components/services/services-navigator-page"; +import { defaultServiceSlug } from "@/lib/services"; type ServicesSearchParams = Promise<{ [key: string]: string | string[] | undefined }>; @@ -19,7 +20,9 @@ export default async function ServicesIndexRoute({ searchParams }: { searchParam const hasSubmittedSearch = readFirstSearchParam(resolvedSearchParams.run) === "1" && query.length > 0; if (!hasSubmittedSearch) { - return ; + // Computed server-side so the client route chunk never bundles the + // services snapshot behind @/lib/services. + return ; } return ( diff --git a/src/components/services/services-home-page.tsx b/src/components/services/services-home-page.tsx index 4fd679faff..9c4bb24ceb 100644 --- a/src/components/services/services-home-page.tsx +++ b/src/components/services/services-home-page.tsx @@ -12,33 +12,37 @@ import { } from "@/components/mode-home-template"; import { appModeHomeHref } from "@/lib/app-modes"; import { modeHomeDesktopComposerSlotId } from "@/lib/mode-home-composer"; -import { defaultServiceSlug } from "@/lib/services"; import { countVerifiedRegistryRecords, useRegistryRecords } from "@/lib/use-registry-records"; -const taskCards: ModeHomeAction[] = [ - { - title: "Search services", - description: "Need, catchment, provider, keyword.", - icon: FileSearch, - href: appModeHomeHref("services", { focus: true }), - }, - { - title: "Check catchment", - description: "Region, public/private, eligibility.", - icon: MapPinned, - href: `/services/${defaultServiceSlug() ?? ""}`, - }, - { - title: "Browse referral pathways", - description: "Crisis, youth, Aboriginal health, telehealth.", - icon: Route, - href: appModeHomeHref("services", { - query: "crisis youth Aboriginal health telehealth referral pathway", - focus: true, - run: true, - }), - }, -]; +// The default service slug is computed server-side (app/services/page.tsx) and +// passed as a prop: a direct `@/lib/services` value import here would compile +// the full services snapshot (~100 KB gzip) into this client route chunk. +function buildTaskCards(defaultServiceSlug: string | null): ModeHomeAction[] { + return [ + { + title: "Search services", + description: "Need, catchment, provider, keyword.", + icon: FileSearch, + href: appModeHomeHref("services", { focus: true }), + }, + { + title: "Check catchment", + description: "Region, public/private, eligibility.", + icon: MapPinned, + href: `/services/${defaultServiceSlug ?? ""}`, + }, + { + title: "Browse referral pathways", + description: "Crisis, youth, Aboriginal health, telehealth.", + icon: Route, + href: appModeHomeHref("services", { + query: "crisis youth Aboriginal health telehealth referral pathway", + focus: true, + run: true, + }), + }, + ]; +} const commonPathways: ModeHomePill[] = [ { @@ -78,7 +82,8 @@ const commonPathways: ModeHomePill[] = [ }, ]; -export function ServicesHomePage() { +export function ServicesHomePage({ defaultServiceSlug = null }: { defaultServiceSlug?: string | null }) { + const taskCards = buildTaskCards(defaultServiceSlug); const registry = useRegistryRecords("service"); const verifiedCount = countVerifiedRegistryRecords(registry); const registryReady = registry.status === "ready"; diff --git a/tests/services-client-boundary.test.ts b/tests/services-client-boundary.test.ts new file mode 100644 index 0000000000..9ffb476459 --- /dev/null +++ b/tests/services-client-boundary.test.ts @@ -0,0 +1,160 @@ +import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; +import { dirname, join, relative, resolve } from "node:path"; +import { describe, expect, it } from "vitest"; + +// Regression guard for the PR #890 fix: the ~915 KB services snapshot reaches +// the client the moment "@/lib/services" is value-imported anywhere in a +// client module graph (~100 KB gzip in that chunk). The enforced bundle +// budget alone cannot catch a re-introduction — its 10% tolerance sits above +// the pre-fix total — so the boundary is asserted at the source level, and it +// must be TRANSITIVE: a helper without its own "use client" directive still +// lands in the client bundle when a client component imports it. Type-only +// imports are erased at compile time and stay allowed. +const SRC_ROOT = join(process.cwd(), "src"); +const TARGET_SPECIFIER = "@/lib/services"; +// Side-effect imports have no `from` clause but still execute the module. +const SIDE_EFFECT_IMPORT_PATTERN = /^import\s+["']([^"']+)["']/gm; +// Dynamic import() expressions defer loading but still emit client JavaScript +// for the target, so they count as runtime edges too — including with +// webpack/Next magic comments before the specifier. +const DYNAMIC_IMPORT_PATTERN = /\bimport\s*\(\s*(?:\/\*[\s\S]*?\*\/\s*)*["']([^"']+)["'][\s\S]*?\)/g; +// import/export ... from "..." — clause analysed by hasRuntimeBindings below so +// `import type`, `export type`, and named clauses whose specifiers are all +// `type X` (including multiline) stay allowed while default, namespace, mixed, +// star re-export, and value re-export forms are treated as runtime. +const FROM_STATEMENT_PATTERN = /^(import|export)\s+([\s\S]+?)\s+from\s+["']([^"']+)["']/gm; + +function hasRuntimeBindings(kind: string, clause: string): boolean { + const trimmed = clause.trim(); + if (/^type\b/.test(trimmed)) return false; + const named = trimmed.match(/^\{([\s\S]*)\}$/); + if (named) { + return named[1] + .split(",") + .map((specifier) => specifier.trim()) + .filter(Boolean) + .some((specifier) => !/^type\b/.test(specifier)); + } + return kind === "import" || kind === "export"; +} + +function collectSourceFiles(dir: string): string[] { + return readdirSync(dir).flatMap((entry) => { + const fullPath = join(dir, entry); + if (statSync(fullPath).isDirectory()) return collectSourceFiles(fullPath); + return /\.(?:ts|tsx)$/.test(entry) ? [fullPath] : []; + }); +} + +function resolveImport(specifier: string, fromFile: string): string | null { + let base: string; + if (specifier.startsWith("@/")) base = join(SRC_ROOT, specifier.slice(2)); + else if (specifier.startsWith(".")) base = resolve(dirname(fromFile), specifier); + else return null; + + for (const candidate of [base, `${base}.ts`, `${base}.tsx`, join(base, "index.ts"), join(base, "index.tsx")]) { + if (existsSync(candidate) && statSync(candidate).isFile()) return candidate; + } + return null; +} + +// Every on-disk shape the "@/lib/services" module could resolve to, so +// relative-path imports are treated identically to the alias. +const SERVICES_MODULE_PATHS = new Set( + ["lib/services.ts", "lib/services.tsx", "lib/services/index.ts", "lib/services/index.tsx"].map((candidate) => + join(SRC_ROOT, candidate), + ), +); + +// Strip the full directive prologue (comments and whitespace of any length) +// so a "use client" directive after a long header comment is still detected. +function isClientEntry(source: string): boolean { + const prologue = source.replace(/^(?:\s+|\/\/[^\n]*(?:\n|$)|\/\*[\s\S]*?\*\/)*/, ""); + return /^["']use client["']/.test(prologue); +} + +interface ModuleInfo { + importsServices: boolean; + isClientEntry: boolean; + valueImports: string[]; +} + +function buildModuleGraph(): Map { + const graph = new Map(); + + for (const filePath of collectSourceFiles(SRC_ROOT)) { + const source = readFileSync(filePath, "utf8"); + const valueImports: string[] = []; + let importsServices = false; + + const recordRuntimeSpecifier = (specifier: string) => { + const resolved = resolveImport(specifier, filePath); + // Match the alias text AND the resolved file, so relative specifiers + // like ../lib/services are caught identically. + if ( + specifier === TARGET_SPECIFIER || + specifier.startsWith(`${TARGET_SPECIFIER}/`) || + (resolved !== null && SERVICES_MODULE_PATHS.has(resolved)) + ) { + importsServices = true; + } + if (resolved) valueImports.push(resolved); + }; + + for (const match of source.matchAll(SIDE_EFFECT_IMPORT_PATTERN)) recordRuntimeSpecifier(match[1]); + for (const match of source.matchAll(DYNAMIC_IMPORT_PATTERN)) recordRuntimeSpecifier(match[1]); + for (const match of source.matchAll(FROM_STATEMENT_PATTERN)) { + if (hasRuntimeBindings(match[1], match[2])) recordRuntimeSpecifier(match[3]); + } + + graph.set(filePath, { + importsServices, + isClientEntry: isClientEntry(source), + valueImports, + }); + } + + return graph; +} + +describe("services snapshot client boundary", () => { + it("keeps @/lib/services value-imports out of every client module graph", () => { + const graph = buildModuleGraph(); + const offenders: string[] = []; + + for (const [entryPath, entry] of graph) { + if (!entry.isClientEntry) continue; + + const cameFrom = new Map([[entryPath, ""]]); + const queue = [entryPath]; + while (queue.length > 0) { + const currentPath = queue.shift() as string; + const current = graph.get(currentPath); + if (!current) continue; + + if (current.importsServices) { + const chain: string[] = []; + for (let step: string | undefined = currentPath; step; step = cameFrom.get(step) || undefined) { + chain.unshift(relative(process.cwd(), step)); + } + offenders.push(chain.join(" -> ")); + break; + } + for (const next of current.valueImports) { + if (!cameFrom.has(next)) { + cameFrom.set(next, currentPath); + queue.push(next); + } + } + } + } + + expect( + offenders, + "Client module graphs must not value-import @/lib/services: it compiles the full services " + + "snapshot into their chunk. Compute what you need server-side and pass it as a prop " + + "(see src/app/services/page.tsx), or use `import type` for types only. Chains shown as " + + "client entry -> ... -> importing module.", + ).toEqual([]); + }); +});