Skip query-backed linksToMany expansion in prerender requests#4934
Merged
Conversation
Inside a prerender request, the realm-server's loadLinks walker now
populates relationships.{field}.data for query-backed linksTo /
linksToMany fields but stops short of pushing the linked resources
into included[]. Static linksTo / linksToMany still expand
transitively. The signal is the umbrella relationship's links.search,
written exclusively by applyQueryResults, so per-field detection at
the loadLinks follow point is unambiguous.
On the host, the SearchResource consumes the parent doc's
relationships.{field}.data IDs as the seed's cardURLs. In prerender
context the resource short-circuits the live re-query and applySeed
loads each ID via runtimeStore.get(url). Per-URL GETs are stable
(deterministic by URL) and the realm-server's instance-GET runs the
same query-field skip in prerender mode, so each GET stays cheap.
The two prerender signals on globalThis are merged into one
(__boxelRenderContext), set by both the prerender server's
evaluateOnNewDocument and the host's prerender-shaped routes
(render.ts / module.ts / file-extract.ts / command-runner.ts). The
host's fetch wrapper and job-priority resolver now read this single
flag.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc7212ed6e
ℹ️ 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".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes prerender-time relationship resolution by preventing the realm-server from eagerly expanding query-backed linksTo / linksToMany into included[], and instead having the host prerender path resolve those relationships via stable per-URL GETs based on the parent document’s relationships.{field}.data IDs. It also consolidates prerender detection onto a single globalThis.__boxelRenderContext flag.
Changes:
- Add
skipQueryBackedExpansionplumbing from request detection → realm search / cardDocument →loadLinkswalker to suppress query-backed transitive expansion during prerender. - Extend query-field seed capture and host
SearchResource.applySeedto use captured relationship IDs (cardURLs) and fetch per-URL rather than re-querying_federated-search. - Rename prerender global from
__boxelDuringPrerenderto__boxelRenderContextand update routes/tests/headers to match.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/search-utils.ts | Extends search options to carry skipQueryBackedExpansion through searchRealms. |
| packages/runtime-common/realm.ts | Threads prerender-only skipQueryBackedExpansion into search and instance document generation. |
| packages/runtime-common/realm-index-query-engine.ts | Adds skipQueryBackedExpansion option and gates query-backed link traversal in loadLinks. |
| packages/realm-server/handlers/handle-search.ts | Enables skipQueryBackedExpansion for prerender-marked _federated-search requests. |
| packages/realm-server/prerender/prerender-constants.ts | Updates prerender global documentation to __boxelRenderContext. |
| packages/realm-server/prerender/page-pool.ts | Injects globalThis.__boxelRenderContext = true into pages. |
| packages/realm-server/tests/skip-query-backed-expansion-test.ts | Adds coverage for default vs. skip behavior for cardDocument and searchCards. |
| packages/base/card-api.gts | Extends search seed type to include cardURLs. |
| packages/base/query-field-support.ts | Captures relationship IDs into seedCardURLs for prerender seed resolution. |
| packages/host/app/resources/search.ts | Uses cardURLs to fetch per-URL instances in prerender seed mode; expands “seed is authoritative” predicate. |
| packages/host/app/lib/prerender-fetch-headers.ts | Switches prerender header gating to read __boxelRenderContext. |
| packages/host/app/services/store.ts | Renames prerender global usage and extends seed shape with cardURLs. |
| packages/host/app/routes/command-runner.ts | Aligns __boxelRenderContext teardown with other prerender routes (testing-only clear). |
| packages/host/tests/unit/job-priority-header-test.ts | Updates test descriptions/comments to the new global name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Preview deploymentsHost Test Results 1 files 1 suites 3h 38m 28s ⏱️ Results for commit f9a4060. For more details on these errors, see this check. Realm Server Test Results 1 files ±0 1 suites ±0 7m 48s ⏱️ - 1m 12s Results for commit f9a4060. ± Comparison against earlier commit 7393d33. |
…op early returns Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nder-flag check; log seed-URL load failures - query-field-support: when shouldTreatEmptySeedAsUnresolved is true (seed comes from a search result with unhydrated nested query fields, or relationship.data is absent), leave seedCardURLs undefined so SearchResource falls back to a live query instead of treating an empty seed as authoritative. - prerender-fetch-headers: require __boxelRenderContext === true (not just truthy) before stamping the prerender header, matching the surrounding prerender-gated logic. - search.ts applySeed: console.warn on runtimeStore.get failures during the per-URL hydration path so missing relationship items are diagnosable in prerender logs instead of silently dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… search result Previous fix used the broader shouldTreatEmptySeedAsUnresolved gate which also triggers on relationshipHasUnhydratedTargets — but that case (relationship.data names IDs, no records hydrated) is the expected prerender-server-skip shape, not an unresolved seed. Clearing seedCardURLs there gutted the per-URL GET path the whole contract relies on. Tighten the gate to seedComesFromSearch && seedRecords.length === 0 — that catches the only truly-untrustworthy case (nested query fields on a search-result document) while leaving the prerender's indexer-populated relationship IDs authoritative. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
With the prerender server now skipping query-backed expansion in `included[]` and the host's SearchResource resolving the listed IDs via per-URL GETs, the live fallback `_federated-search` no longer fires for query-backed `linksToMany` fields during a prerender. Flip the first assertion in the directory-ops prerender test to expect `delayedSearchPatch.getRequestCount() === 0`, matching the new contract. The HTML and searchDoc assertions stay — those verify that per-URL GETs still hydrate every level of the relationship graph correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n fires
The previous skip prevented the per-item resources from being added
to `included[]`, but left the `fieldName.N` entries on
`resource.relationships`. The host deserializer treats every
per-item entry as a follow-able relationship and expects its target
in `included[]`, so the orphan-link mismatch produced silent
"error getting instance" failures (Error.toJSON drops fields so the
log just printed `{}`).
Before traversal, when the response is in skip mode, walk each
query-backed umbrella (relationship with `links.search`) and remove
its `fieldName.N` sub-keys. The umbrella entry itself stays — it
carries `links.search` plus the `data: [array of IDs]` the host's
per-URL hydration path consumes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`captureQueryFieldSeedData` now treats `relationship.links.search` as the unambiguous signal that the indexer (not the user's raw source) populated the query-backed `linksToMany` umbrella. A raw source's `data: []` no longer arrives as an authoritative empty seed, so the SearchResource falls through to a live `_federated-search` instead of silently rendering an empty field — restoring the host-side behavior the prerender contract assumed. Also flip the `card prerender resolves query fallback via per-URL GETs` test back to `delayedSearchPatch.getRequestCount() > 0`: source-mode instance loads keep each query-backed field firing one fallback search, and the PR's win lands on the *response* side (per-search payload drops ~10-60× via `skipQueryBackedExpansion`), not on the number of searches. The test still verifies the HTML / searchDoc reach Bob / Alice / Eve, which guards both the search firing and the per-URL hydration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous gate cleared `seedCardURLs` whenever the seed came from a `_federated-search` response with empty `seedRecords` — the seed-from- search marker stayed but the resolved IDs were thrown away. That fired for every query-backed `linksToMany` hydration off a search-result resource (the IDs are in `relationship.data`, the resources are NOT in `included[]` under prerender skip), so each parent's query field re-fanned out into its own live `_federated-search` instead of using the per-URL GET path. Measured on the bxl-dependency-order-test dashboard render: a Customer search returning 120 customers caused 120 follow-up Customer.policies searches → ~296 total `_federated-search` requests, ~600s wall-clock. `relationship.links.search` is the unambiguous "indexer wrote this" signal (`applyQueryResults` is the only writer), and the IDs in `relationship.data` next to it are the indexer's pre-resolved pointers — authoritative regardless of whether `included[]` also carries the resources. The prior secondary clause was a stand-in from before `links.search` was the gate; with the gate in place it's redundant and actively wrong. Same dashboard render after this change: ~10 top-level queries + ~200 per-URL Policy GETs (the cardURLs cascade collapse) + ~230 nested Policy.claims searches = ~117s wall-clock, matching the PR's claimed target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
The file-extract route's runtime-dep tracker was missing
`${baseRealm.url}file-api` whenever matrix-service's
`importResource(() => '…/file-api')` did not happen to fire inside the
route's active tracker context. On this branch the timing shift makes
the miss deterministic, so `prerendering-test.ts > file prerender
returns extracted metadata` fails reliably.
Explicitly importing the URL inside the file-extract route's
`withRuntimeDependencyTrackingContext` makes the dep deterministic
without touching the card prerender path. Adds a console.warn that
fires if the URL still slips out of the snapshot.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ace)
The previous fix pinned `${baseRealm.url}file-api` as a runtime dep by
calling `loader.import(fileApiURL)` inside the file-extract route's
`withRuntimeDependencyTrackingContext` window. That call's synchronous
`trackRuntimeModuleDependency` sits behind three layers of loader
indirection — the moduleShims set, the `resolveImport` URL rewrite,
and the `advanceToState` state machine — and a fourth indirection if
the loader instance was replaced between page boot and this call (e.g.
after `BrowserManager.restartBrowser()` from the preceding test).
Empirically the test still flaked with that fix in place.
Stamp the tracker directly with `trackRuntimeModuleDependency(...)`.
That's one synchronous call with no moving parts and no dependency on
the current loader instance's bookkeeping. Also include `fileApiURL`
in the merged deps unconditionally as a belt-and-suspenders: even if
the tracker session is ever closed prematurely between this call and
the snapshot, the indexer's invalidation contract still gets the URL
it needs to invalidate file extracts on `file-api` changes.
The extractor doesn't need `file-api` to be physically loaded — it
imports `card-api` for the FileDef class — so dropping the
`loader.import(...)` call is safe.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Inside a prerender request, the realm-server's
loadLinkswalker now stops short of expanding query-backedlinksTo/linksToManyfields intoincluded[], and the host's SearchResource resolves those relationships through stable per-URL GETs against the parent doc's already-serializedrelationships.{field}.dataIDs instead of firing a live_federated-searchre-query.Net effect on a card render whose template fans out across query-backed
linksToManyfields:_federated-searchQUERY requests during the renderlinksToManygetter fires a_federated-searchand recursively expandsdata-prerender-status=ready(local stack, ~900-instance realm with 5 query-backedlinksToManyfields per top-level card)_federated-searchwith the prerender header)The live-SPA path is unchanged.
Why is N+1 small GETs faster than one big
_federated-search?It's counter-intuitive that replacing one server response that side-loads the linked resources with many per-URL GETs is faster. Two reasons:
1. Today's "one big response" isn't one request — it's a cascade
The current host behavior is: every query-backed
linksToManygetter fires its own live_federated-searchbecause the resource isisLive: true. So a Dashboard with M query-backed fields per card and N matched parents fans out into M×N×… cascaded searches, each of which itself returns a doc whose query-backed fields the next render layer also re-queries.Concrete count during the local ~900-instance Dashboard render:
_federated-searchrequests during renderreadyThe 200 requests we eliminate aren't "the parent's expansion" — they're cascaded re-queries from every child layer the template touches.
2. Each
_federated-searchwas paying the full graph walk; per-URL GETs aren'tA
_federated-searchfor a top-level type doesn't just read N rows — it executes the filter, then walks every query-backedlinksToManyon every matched card, then walks their query-backed fields, serializing every reachable resource intoincluded[]. Realm-server CPU and the SQL graph walk dominate; the actual filter is single-digit ms.Measured directly with
curl -X QUERY /_federated-search(TTFB, prerender header on, ~900-instance realm):datarowsincludedrows (before)The per-URL GETs that replace the included-expansion hit
boxel_indexby primary key, return the doc already-serialized at index time, and (post-CS-11176) reuse a job-scoped search cache when one of those linked instances is itself looked up by URL more than once during the render. Each GET is bounded by its own static-linksTo closure under the same prerender skip — no recursive query-field expansion through the cascade.So the trade is: 1 expensive
_federated-search(full-graph serialization) → 1 cheap_federated-search(just the top-level rows) + N cheap GETs (PK lookups, deduped by URL, multiplexed over HTTP/2). The wall-clock number is the integral of both, and the cascade-collapse is what makes it net-faster.The contract change
Server side:
loadLinksskips query-backed expansion in prerenderpackages/runtime-common/realm-index-query-engine.tsgains a newskipQueryBackedExpansionoption on theloadLinkswalker. When set, the walker still populatesrelationships.{field}.datafor query-backedlinksTo/linksToManyfields but does not push the linked resources onto the next layer of the BFS. StaticlinksTo/linksToManycontinue to expand transitively.The per-field follow point reads the umbrella relationship's
links.search.applyQueryResultsis the only writer of that key, so its presence on a relationship is the unambiguous "this field is query-backed" signal at follow time.packages/realm-server/handlers/handle-search.tssets the opt for/_federated-searchwhenx-boxel-during-prerenderis on the request, mirroring the same gating used today forcacheOnlyDefinitions. The fourcardDocument(..., { loadLinks: true })call sites inpackages/runtime-common/realm.ts(writeMany / patch / patch-noop / GET) thread the same opt viaisDuringPrerenderRequest(request).The walker is gated, not the indexer.
boxel_indexrows are written exactly as before. Only response shape changes — and only for requests inside a prerender.Host side: SearchResource consumes the relationship IDs via per-URL GETs
packages/base/query-field-support.ts::captureQueryFieldSeedDatacaptures the parent'srelationships.{field}.dataIDs into a newseedCardURLsarray on the field's state, alongside the existingseedRecords/seedSearchURL/seedRealms.packages/host/app/resources/search.ts::SearchResource.applySeedconsumes those IDs: when the seed has emptycardsbut non-emptycardURLs, it loads each by URL through the runtime store (store.get(url)). The realm-server's instance-GET runs the same prerender skip, so each GET returns the bare resource plus its static-linksTo closure — no transitive query-backed walk.The "seed is authoritative" predicate now reads three signals:
seed.cards.length > 0(parent serialized resolved instances inincluded)seed.cardURLs !== undefined(parent's relationship.data array was captured, including the empty-no-items case)seed.searchURLset (legacy signal — only present when the relationship is fully resolved)In prerender, any of these short-circuits the live re-query. Outside prerender (
isLive: true) this branch is bypassed entirely and the resource subscribes / re-validates as before.Globals: one prerender flag, not two
Two
globalThisflags signaled "this code is running in a prerender" at different layers and they didn't agree:__boxelDuringPrerenderwas set by the prerender server'sevaluateOnNewDocumentand read by the host's fetch wrapper.__boxelRenderContextwas set by the host's route entry points and read by the seed-resolution logic.The split caused the prerender header to silently miss in any test or driven render where the route flag was set but the server flag wasn't. Converged on
__boxelRenderContext. The prerender server'sevaluateOnNewDocumentsets it; host routes (render / module / file-extract / command-runner) set it; the fetch wrapper, job-priority resolver, and seed-resolution path read it. The route teardown that previously cleared the flag now matches the other routes'isTesting()guard so the prerender server's persistent value survives between consecutive renders in a pooled tab.How the new prerender request flow looks
_federated-searchfor the card it's rendering.data+includedcontaining only the static-linksTo closure (no recursion through query-backed fields).relationships.{field}.datastill names the matched IDs for every query-backed field.applySeedreadscardURLsfrom the captured seed and issues per-URL GETs to materialize the listed cards.The cascade depth is bounded by how far the template walks the relationship graph, not by what the server eagerly includes.
Files changed
packages/runtime-common/realm-index-query-engine.ts—skipQueryBackedExpansionopt + per-field follow gate inloadLinks.packages/runtime-common/realm.ts— thread the opt throughRealm.searchand all fourcardDocument(..., { loadLinks: true })call sites.packages/runtime-common/search-utils.ts— extendSearchableRealm/searchRealmsopts.packages/realm-server/handlers/handle-search.ts— set the opt for/_federated-searchbased on the prerender header.packages/realm-server/prerender/page-pool.ts/prerender-constants.ts— set__boxelRenderContext(was__boxelDuringPrerender).packages/base/card-api.gts— extend the seed type withcardURLs.packages/base/query-field-support.ts— capturerelationships.{field}.dataIDs intoseedCardURLs.packages/host/app/resources/search.ts— new per-URL load path inapplySeed; widen the seed-authoritative predicate.packages/host/app/lib/prerender-fetch-headers.ts— read__boxelRenderContext(was__boxelDuringPrerender).packages/host/app/services/store.ts— extend theseedshape withcardURLs; same global rename.packages/host/app/routes/command-runner.ts—isTesting()guard on the route's__boxelRenderContextteardown so the persistent value survives between renders in a prerender tab (matches existing pattern inrender.ts/module.ts/file-extract.ts).packages/host/tests/unit/job-priority-header-test.ts— global rename in test descriptions / comments.packages/realm-server/tests/skip-query-backed-expansion-test.ts— NEW unit test exercising bothcardDocumentandsearchCardspaths with and without the skip opt.Test plan
pnpm lintclean inpackages/runtime-commonandpackages/base.skip-query-backed-expansion-test.tscovers default-expand vs. skip on both code paths.linksToManyfields on a realm-server build of this branch.Out of scope
store.get(url)instance-GET. No new endpoint is introduced.Risks
links.search. If a future code path writes that key from somewhere other thanapplyQueryResults, the gate would mis-classify; the test in this PR pins the current writer.isDuringPrerenderRequest; non-prerender GETs are unchanged.🤖 Generated with Claude Code