Skip module pre-warming on incremental indexing#5121
Conversation
Incremental indexing no longer pre-warms the definition cache for the modules in the change set. The module cache is already warm from the prior from-scratch, and any module a render needs that isn't cached resolves through the on-demand `lookupDefinition` read-through during the visit (PagePool-safe: the sub-prerender materializes its own tab). From-scratch indexing is unchanged — it still runs the realm-wide `.gts`/`.gjs` sweep plus the per-invalidation deps warm, where the module cache is cold by definition. 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: fb47438b25
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR removes module pre-warming from the incremental indexing path in IndexRunner, relying instead on the existing on-demand lookupDefinition read-through during the visit phase. From-scratch indexing continues to do the realm-wide .gts/.gjs sweep plus per-invalidation warming, preserving cold-cache behavior.
Changes:
- Removed
preWarmModulesTable()invocation during incremental indexing and adjustedtotalFilesaccounting accordingly. - Updated incremental indexing comments to describe the new “no pre-warm” behavior.
- Clarified (in comments) that the realm-wide sweep is from-scratch-only.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Describe how query-backed field expansion stays correct without pre-warming: a prerender `_search` reads the `queryFieldDefs` pre-extracted onto each result instance's stored meta (`populateQueryFieldsFromMeta`), so it needs no `modules`-table row for the queried type, and the prerender-search definition path is cache-only by design. Drops the imprecise "cache is already warm from the prior from-scratch" claim — the realm-wide sweep is from-scratch-only because that is when the cache is cold, not because incremental assumes a warm one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Incremental indexing no longer pre-warms the definition cache for the modules involved in a change set (the invalidated module files, their per-row
boxel_indexdeps, and theadoptsFrommodule of a novel.json). It now does no module pre-warming at all.The module cache is already warm from the prior from-scratch index, and any module a render needs that isn't cached resolves through the on-demand
lookupDefinitionread-through during the visit phase. That read-through is PagePool-safe — the sub-prerender materializes its own tab rather than queueing behind the render that triggered the lookup, so there's no affinity deadlock to pre-empt. Leaning on it keeps the incremental path's cost bounded by the cards it actually visits.From-scratch indexing is unchanged: it still runs the realm-wide
.gts/.gjssweep plus the per-invalidation deps warm, because there the module cache is cold by definition.The existing
indexing-test.tscase "the full-realm module pre-warm sweep runs on from-scratch indexing but not on incrementals" continues to encode the contract — it asserts an orphan module (never visited, never a dependency) is not cached after an incremental, which holds with zero pre-warming.🤖 Generated with Claude Code