refactor: split the prerender visit into index and prerender-html passes#5399
Open
habdelra wants to merge 6 commits into
Open
refactor: split the prerender visit into index and prerender-html passes#5399habdelra wants to merge 6 commits into
habdelra wants to merge 6 commits into
Conversation
Bifurcate the consolidated prerender visit into two visit types selected
by a new `visitType`:
- index visit: file extract + card meta (search doc / serialized / types /
display names / deps) + card & file icon; never runs the html route.
- prerender-html visit: the html route per format (fitted, embedded, atom,
head, isolated) + markdown.
The indexing job runs both visits per file and merges the halves into the
same updateEntry writes as the fused visit; RenderRunner and the in-browser
card-prerender implement the same seam. A parity test asserts the two halves
losslessly partition the fused output.
Splitting the visit removed the html render the search doc implicitly relied
on for link loading, so searchable independence is completed here:
- /render/meta drives the instance's searchable-path and computed link loads
and awaits store.loaded() until the load generation is stable, reproducing
via searchable the settle a template render provides.
- "used" for usedLinksToFieldsOnly serialization is searchable-driven: a link
is kept iff it is in a searchable path or actually set, independent of what
a template rendered. Rename includeUnrenderedFields to
includeNotSearchableFields.
- Broken searchable links plant the terminal sentinel on the owner field so
getBrokenLinks records them.
- Record the searchable module as an explicit meta dependency; its
per-loader-cached load hook is otherwise nondeterministic across tabs.
Contract: card+json carries a non-searchable link only when it is set, so
unset or explicitly-null non-searchable links are omitted rather than emitted
as { self: null }. Written card sources are unchanged (they persist
relationships as authored).
Co-Authored-By: Claude Opus 4.8 (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: 8d46d3c9fd
ℹ️ 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
Preview deploymentsHost Test Results 1 files 1 suites 2h 39m 31s ⏱️ Results for commit 95ad4ab. For more details on these errors, see this check. Realm Server Test Results 1 files ±0 1 suites ±0 8m 54s ⏱️ +31s Results for commit 95ad4ab. ± Comparison against earlier commit d0fe5f9. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR splits the consolidated prerender “visit” into two selectable halves (visitType: 'index' | 'prerender-html') so indexing work (extract + meta/search-doc + icons) can run independently from HTML/markdown prerendering, while the indexing job still merges both halves back into the same persisted row shape as the prior fused visit. It also makes search-doc generation independent of template rendering by explicitly settling searchable-driven link loads and revising “used link” serialization to be searchable/set driven (not render-driven).
Changes:
- Add
visitType+cardTypesplumbing across runtime-common types, prerender server, remote prerenderer, index runner, and browser-sidecard-prerenderimplementation. - Implement two-visit orchestration in the index runner, including merged diagnostics (timing-summed) and merged card/file render payloads.
- Update searchable/link-settling + serialization behavior and adjust/expand tests to match new output shapes and contract.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/index.ts | Adds PrerenderVisitType, visitType/cardTypes args, and prerenderHtmlRequestId diagnostics field. |
| packages/runtime-common/index-runner/visit-file.ts | Replaces fused visit with two-visit orchestration + merge helpers (card/file results + diagnostics). |
| packages/runtime-common/index-runner/file-indexer.ts | Updates docs/comments to reflect merged visit inputs. |
| packages/runtime-common/index-runner/card-indexer.ts | Updates docs/comments to reflect merged visit inputs. |
| packages/runtime-common/index-runner.ts | Switches index runner to the new two-visit visitFileForIndexing. |
| packages/realm-server/prerender/render-runner.ts | Implements visitType bifurcation inside server-side composite visit; adds captured-deps path for prerender-html visits. |
| packages/realm-server/prerender/remote-prerenderer.ts | Sends visitType and cardTypes through the remote visit request. |
| packages/realm-server/prerender/prerenderer.ts | Threads visitType + cardTypes into attempt execution. |
| packages/realm-server/prerender/prerender-app.ts | Validates/parses visitType + cardTypes; tightens fileData requirements for prerender-html visits; logs include visitType. |
| packages/host/app/routes/render/meta.ts | Adds searchable-driven settle loop and deps union to make search-doc independent of HTML render order. |
| packages/host/app/routes/render.ts | Publishes settle-time deps snapshot on a global for prerender-html visits; resets global between cards. |
| packages/host/app/components/card-prerender.gts | Mirrors visitType bifurcation in the browser composite visit; captures deps for prerender-html visits. |
| packages/base/searchable.ts | Makes broken searchable links plant terminal sentinels so brokenLinks diagnostics work without template render. |
| packages/base/field-support.ts | Changes “used link” logic to searchable/set driven; adds routesForField usage and supporting helpers. |
| packages/base/card-serialization.ts | Renames includeUnrenderedFields → includeNotSearchableFields and updates used-links gating. |
| packages/realm-server/tests/prerendering-test.ts | Adds parity test asserting index + prerender-html partition the fused output losslessly. |
| packages/realm-server/tests/indexing-test.ts | Updates expectations for dropped unset non-searchable relationships; adds clarifying comments. |
| packages/realm-server/tests/realm-endpoints-test.ts | Updates expectations for omitted unset base-card relationships. |
| packages/realm-server/tests/card-source-endpoints-test.ts | Updates expectations for omitted unset base-card relationships. |
| packages/realm-server/tests/card-endpoints-test.ts | Updates expectations broadly and adds explicit null-vs-absent relationship contract test. |
| packages/host/tests/integration/realm-test.gts | Updates relationship expectations to omit unset non-searchable links. |
| packages/host/tests/integration/realm-indexing-test.gts | Updates relationship expectations to omit unset non-searchable links. |
| packages/host/tests/integration/components/serialization-test.gts | Renames option usage and updates expectations accordingly. |
| packages/host/tests/integration/components/serialization-rri-form-audit-test.gts | Renames option usage. |
| packages/host/tests/integration/components/operator-mode-links-test.gts | Updates expectations to omit unset base-card relationships. |
| packages/host/tests/integration/components/linksto-sentinel-roundtrip-test.gts | Renames option usage. |
| packages/host/tests/integration/components/card-copy-test.gts | Updates expectations to omit unset base-card relationships. |
| packages/host/tests/integration/commands/patch-instance-test.gts | Updates expectations for empty relationship maps when links are unset/non-searchable. |
| packages/host/tests/helpers/index.gts | Updates system card fixtures to omit unset base-card relationships. |
| packages/host/tests/acceptance/file-def-test.gts | Updates relationship expectations to omit unset base-card relationships. |
| packages/host/tests/acceptance/code-submode/editor-test.ts | Updates relationship expectations to omit unset base-card relationships. |
| packages/host/tests/acceptance/code-submode/create-file-test.gts | Updates relationship expectations (now {}) for omitted unset relationships. |
| packages/boxel-cli/tests/commands/ingest-card.test.ts | Updates fixture expectations to omit unset base-card relationship. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The search doc and the file/card serialization are separate concerns, but
`usedLinksToFieldsOnly` had come to conflate them: a link was serialized into
the card+json / written source based on whether it was in a `searchable` path.
That's wrong — whether a relationship is empty vs. absent is a property of the
card's data, independent of searchability.
Decouple them:
- Serialization keys on the data bucket, not searchable annotations: a link is
kept iff the card actually has it (an authored `{ self: null }`, stored as a
`null` bucket entry, or a set target) and omitted when never authored. An
empty `linksToMany` backing array is treated as "not had". `searchable.ts`
drives only the search doc.
- The field getter no longer persists a nullish empty value (an unset
`linksTo`): a mere read used to fabricate a `null` bucket entry, which made
"in the bucket" unreliable and is why an unset link couldn't be told from an
authored empty. Now `undefined` in the bucket = never set, `null` = authored
empty. (`searchable.ts` already worked around this pollution; the fix is at
the source.)
- Rename `includeNotSearchableFields` back to `includeUnrenderedFields`.
- `copy-and-edit` resolves a relationship's dotted path by walking the declared
field structure (getFields), not by serializing — it previously relied on the
getter pollution to surface an unset target.
Contract: an authored empty `linksTo` (`{ self: null }`) is now preserved
distinctly from a never-set one in both the written source and the served
card+json — the empty-vs-absent distinction is a data property, no longer lost.
Never-set and empty-linksToMany links are omitted (as before, but now for the
right reason).
An errored card records its full render closure as deps rather than only its
own module, so any change to a transitive dependency reindexes it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors the /render template settle loop, which warns when it hits its max-passes bound; makes a pathological link graph diagnosable in production. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rchable The serialization filter is data-driven (kept iff the card has the link, authored or set), not searchable-driven; update the SerializeOpts doc, the render-fields-cache token comment, a copy-and-edit comment, and two test comments to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `[CS-11221 DIAG] getBrokenLinks finding` console.error was a leftover diagnostic. This branch's broken-searchable-link sentinels make getBrokenLinks report more findings, so the stray log now fires far more often — remove it. The pure-read contract it documented is already covered by the function-level block comment. Also reword the block comment: the "used" signal is now the data-bucket-driven `isFieldUsed`, not the removed `getUsedFields`. Co-Authored-By: Claude Opus 4.8 (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.
Prerender visit split
The consolidated prerender visit splits along the search-doc / HTML seam into two visit types, selected by a new
visitTypeonprerenderVisit:htmlroute; its entry into the render app is the icon route.htmlroute per format (isolated,head,atom,fitted,embedded) plusmarkdown, for both the card and the FileDef rendering. Produces no search-doc data.No
visitTyperuns the fused union of both halves (the user-initiated prerender proxy and direct callers keep that shape).The indexing job runs both visits per file and merges the halves into the same
updateEntry()writes as before. The prerender-html visit chains off the index visit: the extract's resource becomes the FileDef rendering'sfileData, the extract's types drive the FileDef fitted/embedded renders, and the card meta's types ride a newcardTypesarg. Merge rules: HTML + markdown from the prerender-html visit; meta + icons from the index visit;depsthe union of both;errorpreferring the index visit's;diagnosticstiming-summed with the prerender-html visit's request id underprerenderHtmlRequestId.RenderRunnerand the in-browsercard-prerendercomponent implement the same bifurcation, so host-test indexing follows the identical seam. A parity test (prerenderVisit - composite pass orchestration) asserts the two halves losslessly partition the fused output.Searchable-driven search doc
The index visit builds the search doc from the
searchableannotations rather than from what a template render pulled into the store, so it no longer needs the HTML render to have run first:/render/metasettle loop. The meta route drives the instance's searchable-path link loads (and the loads its computeds/contained fields trigger through the field getter) and awaitsstore.loaded(), repeating until the store's load generation is stable. This gives the search doc, viasearchable, the same settle a template render provides — and composes with the/renderroute's template settle (still needed for HTML prerendering, where the store starts empty). A computed reading a not-yet-loaded link throws on the first pass; the loop swallows that (the load it fired is what the nextstore.loaded()waits on) and a later pass re-evaluates against the loaded target.brokenLinksdiagnostics. A failed searchable expansion plants the terminallink-error/link-not-foundsentinel on the owner's field, sogetBrokenLinksrecords it, and records the broken target as a dependency so the card reindexes if the target becomes reachable.Decoupling file/card serialization from searchability
The search doc and the file/card serialization are separate concerns. The
usedLinksToFieldsOnlyfilter (which decides what a card+json / written source emits) keys off the card's data, notsearchablemembership:{ links: { self: null } }) — and omitted when never authored. Searchability plays no part; it drives only the search doc.linksTo). A mere render used to fabricate anullbucket entry, so an unset link was indistinguishable from an authored empty. Now the bucket state is the signal: key absent = never set,null= authored empty, a value/sentinel = set. (An emptylinksToManybacking array is treated as "not had".)copy-and-editresolves a relationship's dotted path by walking the declared field structure, not by serializing — the target it links into is typically unset, which the data-driven serialization (correctly) omits.Contract
An authored empty link (
{ links: { self: null } }) is preserved distinctly from a never-set one in both the written source and the served card+json GET/POST/PATCH: the empty stays{ self: null }, the never-set is omitted. This is data fidelity, independent of searchability — so base-card links likecardInfo.themeno longer appear on the wire unless a card actually authored them, while a deliberately-emptied link is kept. This resolves the empty-vs-absent distinction tracked in CS-11779 forlinksTo.An errored card records its full render closure as
depsrather than only its own module, so any change to a transitive dependency — potentially the one that fixes the render — reindexes it.Tests
Relationship expectations across the realm-server and host suites are reconciled to the data-driven shape: never-set (and empty-
linksToMany) links are omitted; authored{ self: null }links are preserved; set links and searchable-driven search-doc expansions are unchanged. Acard-endpointstest locks the empty-vs-absent distinction end to end (source and served card+json both keep the authored empty and omit the never-set). New parity coverage inprerendering-testasserts the index and prerender-html visits partition the fused output losslessly.