perf: batch image signed URLs + cache seeded catalog fetches (round 2, phase 2)#479
Conversation
Round-2 phase 2 (client network & caching). - New POST /api/images/signed-urls resolves up to 50 images with one rate-limit consume, one document_images read, one owner-scoped documents read, and one batch storage signing call — replacing N API calls of 3 DB round trips each on image-heavy document views. Authorization is fail-closed per item (unknown, unauthorized, and uncommitted-generation ids all resolve null), mirroring the single route's checks; covered by mixed-ownership, uncommitted-generation, and anonymous-scope tests. - signed-url-cache.ts gains a 25 ms batching window keyed by authorization header (sessions never share a batch); DocumentImage (viewer) and SourceImage (answer surface) fetch through it. Headless Playwright probe: the document page renders its image via exactly one batch POST. - The single-image GET now sends Cache-Control: private, max-age=540 + Vary: Authorization so reloads reuse the 10-minute signed URL. - client-fetch-cache.ts adds short-TTL + in-flight dedup for the registry and medication catalog hooks, which several surfaces mount concurrently; 401s and errors are never cached so auth loading re-resolves. - Seven seeded content routes (registry records, medications, differentials, presentations — lists and slugs) move from no-store to private, max-age=300, stale-while-revalidate=600 + Vary: Authorization, 200 responses only (404s/errors stay no-store). Verified live via curl. - docs/site-map.md regenerated for the new route. Verified: tsc, eslint, prettier, full vitest 1429 passed (site-map test caught the missing route doc — fixed), targeted suites green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
💡 Codex Review
Database/src/lib/client-fetch-cache.ts
Lines 17 to 18 in 7cea285
fetchJsonCached keys entries only by the explicit headers object, but these same-origin fetches still carry Supabase cookies while useAuthSession() is loading or after auth state changes, and the catalog routes resolve those cookies via publicAccessContext. A 200 owner-scoped response fetched with {} headers is cached under the same key as any later cookie/no-header request, so signing out or switching users in the same tab can show the previous user's registry/medication data for up to 60s. Skip caching when no bearer header is present or include a stable session key in the cache key.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 200 responses only — errors and 404s must never be cached. | ||
| export const seededContentCacheHeaders = { | ||
| "Cache-Control": "private, max-age=300, stale-while-revalidate=600", | ||
| Vary: "Authorization", |
There was a problem hiding this comment.
Vary private cached API responses on Cookie
The new cache header varies only on Authorization, but these routes also treat Supabase session cookies as an auth signal (shouldResolvePublicCatalogAccess/publicAccessContext). When an owner-scoped 200 is produced from a cookie-only request, for example before the client has a bearer token, the browser's private HTTP cache may reuse it for a later cookie/no-header request by a signed-out or different user because the Cookie value is not part of the cache key. Include Cookie in Vary or keep cookie-authenticated responses no-store; the same issue applies to the new image signed-url GET cache header.
Useful? React with 👍 / 👎.
Summary
Phase 2 of the round-2 performance initiative (stacked on #475). Collapses per-image and per-component fetch storms and adds HTTP caching to rarely-changing owner-scoped catalog content.
Image signed URLs (high impact on image-heavy documents)
POST /api/images/signed-urls(≤50 ids): one rate-limit consume + onedocument_imagesread + one owner-scopeddocumentsread + onecreateSignedUrlsbatch call — previously each image cost its own API call with 3 DB round trips.withOwnerReadScopeand whose generation metadata matches the committed index generation resolve to URLs. Unknown / unauthorized / uncommitted ids are indistinguishable (all null). Tests: mixed ownership, uncommitted replacement generation, anonymous public-only scope.signed-url-cache.ts: 25 ms batching window, queues keyed by authorization header so sessions never mix in one batch.DocumentImage(viewer) +SourceImage(answer surface) both fetch through it.POST /api/images/signed-urls, zero per-image GETs.Cache-Control: private, max-age=540+Vary: Authorization(URLs live 600 s), so reloads reuse it.Catalog fetch dedup + HTTP caching
client-fetch-cache.ts: short-TTL + in-flight dedup foruseRegistryRecords/useMedicationCatalog(several surfaces mount them concurrently and previously issued identical GETs). Non-OK responses are never cached, so the auth-loading 401 race still re-resolves correctly.no-store→private, max-age=300, stale-while-revalidate=600+Vary: Authorization, 200 responses only (the shared slug-route helpers are status-aware; 404s/errors stay no-store). Verified live via curl.Tenancy review note
The batch endpoint is a new owner-scoped read surface. It reuses
enforceDocumentReadRateLimit,withOwnerReadScope, andisCommittedGenerationMetadataunchanged from the single route and fails closed per item; please review with the tenancy defense-in-depth lens.Verification
tsc,eslint,prettiercleandocs/site-map.mdregenerated)🤖 Generated with Claude Code