Skip to content

feat: migrate from unstable_cache to use cache, cleanup SSR prefetch stub#69

Merged
suguanYang merged 10 commits into
stagingfrom
fix/wangbinqi/ssr-demo-chunks-cleanup
May 13, 2026
Merged

feat: migrate from unstable_cache to use cache, cleanup SSR prefetch stub#69
suguanYang merged 10 commits into
stagingfrom
fix/wangbinqi/ssr-demo-chunks-cleanup

Conversation

@suguanYang
Copy link
Copy Markdown
Contributor

@suguanYang suguanYang commented May 13, 2026

Clean up issues from #67 and migrate from unstable_cache to the use cache directive.

  1. Minimal SourceView stub — only title and documentId are consumed by toParsedChunkView, so building a full fake DemoSource was misleading.
  2. Named page size constant — DEMO_CHUNK_PREFETCH_PAGE_SIZE = 50, aligned with workspaceClientConfig.sourceChunkPageSize.
  3. Migrate to use cache directive — enables cacheComponents in next.config, replaces unstable_cache with use cache + cacheLife(max) + cacheTag(demo-chunks). The home page uses Suspense + connection() for partial prerendering compatibility.

Replace over-specified fake DemoSource with a minimal SourceView since
toParsedChunkView only uses title and documentId. Extract magic page size
100 to a named constant aligned with the client-side page size (50). Add
cache tags for future on-demand invalidation and a migration note for
"use cache" when cacheComponents is enabled.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
knowhere-notebook-staging Ready Ready Preview, Comment May 13, 2026 3:52pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
knowhere-notebook Ignored Ignored Preview May 13, 2026 3:52pm

Request Review

Enable cacheComponents in next.config.ts and replace the unstable_cache
wrapper with a "use cache" async function using cacheLife("max") and
cacheTag("demo-chunks", demoSourceId). The cache key is now automatically
derived from the function's arguments rather than manually specified.

The home page is restructured with a Suspense boundary so the static
shell can be prerendered while dynamic content (auth, guest context)
streams in at request time via connection().
@suguanYang suguanYang changed the title fix: clean up SSR demo chunk prefetch stub and hardcoded values feat: migrate from unstable_cache to use cache, cleanup SSR prefetch stub May 13, 2026
connection() can't be called in test environment (no request scope).
Extract the render logic into an exported renderWorkspaceShell() so
the test can exercise the data-fetching path without triggering the
connection() boundary.
Replace the fetch-level cache:force-cache with "use cache" directives
at the async wrapper layer in knowhere-demo.ts. This catches all callers
(SSR page, API routes) and integrates with Next.js 16's cache tagging
for on-demand invalidation.

- fetchCatalog: cacheLife("hours") + cacheTag("demo-catalog")
- fetchChunkPage: cacheLife("hours") + cacheTag("demo-chunks", id)

The outer "use cache" in initial-state.ts is removed since the
integration layer now handles caching. Effect functions are exported
so tests can call them without triggering cacheLife().
Both functions call Dashboard oRPC endpoints on every SSR request.
Caching for "minutes" (1-minute revalidate) reduces Dashboard load
for rapid repeat requests while keeping session data reasonably fresh.

- getCurrentUser: cookie read extracted to outer function, Effect
  execution moved to cached inner function getCurrentUserCached
- fetchKnowhereJwt: Effect execution moved to cached inner function
  fetchKnowhereJwtCached (cookieHeader already an explicit argument)
- Tests mock next/cache as no-ops since cacheLife/cacheTag require
  the Next.js runtime
The cached function was calling getCurrentUserEffect which reads
headers() internally, ignoring the cookieHeader parameter. This meant
the cache key was always empty — the first user's session would be
returned for all subsequent users.

Extract callGetCurrentUser(cookieHeader) as a shared Effect so the
cached path uses the explicit cookie header (different cache key per
session) while getCurrentUserEffect continues to read headers() for
the Auth service layer.
The login page reads headers() to resolve the Notebook public URL for
the Dashboard callback. With cacheComponents enabled, request-time APIs
must be wrapped in a Suspense boundary with connection().

Extracts renderLoginPage() for testability — tests call it directly to
avoid the connection() call which requires Next.js runtime.
- Export HomeContent/LoginContent directly instead of renderWorkspaceShell/
  renderLoginPage wrappers. Tests mock next/server (connection) so the
  components work without Next.js runtime.
- Un-export fetchCatalogEffect/fetchChunkPageEffect. Tests mock next/cache
  (cacheLife/cacheTag) so knowhereDemoApi.fetchChunkPage works directly.
- Add comment documenting the fixed 1-min JWT cache vs. expiresInSeconds
  tradeoff.
findCitationSource matches source.documentId === citation.source.documentId,
but these were populated from different API fields:
- source.documentId <- canonical_document_id
- citation.source.documentId <- source.document_id (nested)

When the demo API returns different values for these fields, the lookup
returns null and citation clicks silently do nothing. Use the citation's
canonicalDocumentId instead, which maps to the same canonical_document_id
field as the source.
SSR-prefetched chunks bypassed resolveChunkConnectionTargets, leaving
targetChunkId null on all chunk-to-chunk references. With isResolved
=== false, the reference buttons were disabled in the chunks panel.

Wrap prefetched chunks through resolveChunkConnectionTargets so cross-
chunk references are clickable in guest mode.
@suguanYang suguanYang merged commit f2f2752 into staging May 13, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant