Skip to content

Render each card type's icon once per indexing job#5481

Merged
habdelra merged 3 commits into
mainfrom
cs-12060-indexing-hot-path-render-the-icon-once-per-card-type-per-job
Jul 13, 2026
Merged

Render each card type's icon once per indexing job#5481
habdelra merged 3 commits into
mainfrom
cs-12060-indexing-hot-path-render-the-icon-once-per-card-type-per-job

Conversation

@habdelra

@habdelra habdelra commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

During indexing, the prerenderer's index visit renders a card's icon alongside its search-doc data. Icon markup is a pure function of the card's type — it comes from the type's static icon component, never from instance data — yet the visit rendered it once per file. A from-scratch index of an 885-card realm with 11 distinct card types performs 885 icon renders where 11 would do; each one is a route transition plus render-settle inside the hot path's per-visit floor.

The prerender server now keeps a job-scoped memo of icon markup keyed on the card's type. The first index visit for a type renders the icon; every later visit of that type in the same job reuses the captured markup and skips the icon route entirely. File icons get the same treatment keyed on the file's type — a file visit whose icon is memoized performs no page work at all in its file-render pass (the file-data stash is skipped too).

Rows are unchanged: icon_html on an index row is byte-identical to what a fresh render produces.

How it works

  • The index visit's card pass enters the render app via render.meta and renders the icon afterward as an in-page child transition. Meta goes first because the memo key is types[0], which the meta pass resolves. The fused visit is unchanged: it still runs meta last, after the format renders mark the linked fields they read as used.
  • A meta error short-circuits the pass: the page is showing the error route at that point, so a subsequent icon capture would wait on fresh render output the page can no longer produce and ride out the full render timeout, evicting the tab. The row lands as an error row with no icon — the same skip the html-route entry applies when its isolated render errors. A card whose meta errors deterministically (e.g. a computed that reads a broken link) takes this path on every visit.
  • The memo lives on the RenderRunner, one slot per affinity, keyed by the visit's jobId plus the loader epoch. Indexing serializes per realm, so at most one job is active per slot; a visit carrying a different job key replaces the slot outright. A module edit arrives as a new job, so stale icon markup cannot survive a module change.
  • Visits without a jobId (on-demand renders) never read or write the memo.
  • clearCache visits bypass the memo read — they are asked for a pristine render — and overwrite the entry with their fresh capture.
  • releaseBatch also drops the affinity's memo. Correctness doesn't depend on this (the job-key check already isolates jobs); it just frees the memory promptly.

Testing

New tests in prerendering-test.ts cover same-job reuse (byte-identical markup, hit/miss counters), per-type keying (a second card type renders its own distinct icon), job-change reset, jobless visits leaving the memo untouched, clearCache bypass, releaseBatch and affinity-disposal cleanup, and the meta-error short-circuit (an instance whose computed reads a broken link produces an error row promptly, with no icon render, no render timeout, and no tab eviction). The existing lossless-partition test continues to pin the index visit's output against the fused visit's, byte for byte.

🤖 Generated with Claude Code

Icon HTML is a per-type constant (the type's static icon component), yet
the index visit rendered it once per file. The index visit's card pass now
enters the render app via render.meta and renders the icon afterward as an
in-page child transition, so a job-scoped memo keyed on the type the meta
pass resolves (types[0]) can skip the icon route for every subsequent
visit of the same type. The file pass memoizes the same way, keyed on the
extract pass's types[0]; on a memo hit it performs no page work at all.

The memo lives on the RenderRunner, one slot per affinity, keyed by
jobId + loader epoch: indexing serializes per realm so at most one job is
active per slot, a new job replaces the slot outright, and visits without
a jobId never touch it. clearCache visits bypass the memo read and
overwrite the entry with their fresh capture. releaseBatch drops the
owning affinity's memo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfeb5a24ef

ℹ️ 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".

Comment thread packages/realm-server/prerender/prerenderer.ts
Affinity disposal clears the batch-ownership entry, which makes the
owner-matched memo clear in releaseBatch a no-op for a job whose affinity
is disposed mid-run (cancel, idle eviction, capacity pressure). The
onAffinityDisposed hook drops the icon memo along with the ownership
entry so every disposal path releases it; a still-running job re-warms
the memo with at most one icon render per type.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes realm indexing prerenders by memoizing rendered icon HTML per card/file type within an indexing job, so repeated visits of the same type can reuse captured markup and skip the icon route work.

Changes:

  • Add an affinity-scoped, job-keyed icon HTML memo to RenderRunner and use it during visitType: 'index' card + file passes to avoid redundant icon rendering.
  • Clear the icon memo when an indexing batch releases an affinity, and expose a test-only accessor for memo stats/keys.
  • Add coverage in prerendering-test.ts for memo hits/misses, per-type behavior, job boundaries, jobless visits, clearCache bypass, and releaseBatch cleanup.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/realm-server/prerender/render-runner.ts Introduces job-scoped icon memoization and integrates it into index-visit card/file render flows.
packages/realm-server/prerender/prerenderer.ts Clears the icon memo on releaseBatch and exposes a test-only accessor for memo observability.
packages/realm-server/tests/prerendering-test.ts Adds new tests validating icon memo correctness and lifecycle behaviors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/realm-server/prerender/render-runner.ts
@habdelra habdelra requested a review from a team July 13, 2026 17:57
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files  ±0      1 suites  ±0   2h 33m 23s ⏱️ +35s
3 470 tests ±0  3 455 ✅ ±0  15 💤 ±0  0 ❌ ±0 
3 489 runs  ±0  3 474 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 3335610. ± Comparison against earlier commit 1edeb49.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   11m 27s ⏱️ - 1m 53s
1 822 tests +1  1 822 ✅ +2  0 💤 ±0  0 ❌  - 1 
1 901 runs  +1  1 901 ✅ +2  0 💤 ±0  0 ❌  - 1 

Results for commit 3335610. ± Comparison against earlier commit 1edeb49.

A meta error leaves the page on the error route, so the icon render
that follows would wait on fresh output the page can no longer produce,
ride out the full render timeout, and evict the tab — a 60s+ stall for
every card whose meta errors deterministically, such as a computed that
reads a broken link. Mirror the entry-error semantics of the html-route
visits: stop the pass at the failed entry and land the row as an error
row with no icon.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@lukemelia lukemelia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels funny to me that you define the icon on card def the same way you define fitted, embedded, etc, but it has different semantics. No need to solve this in this PR though, lol!

@habdelra habdelra merged commit 028c37d into main Jul 13, 2026
65 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.

3 participants