Skip to content

Drop the prerender tab's module graph only when a pass changed a module - #6203

Open
habdelra wants to merge 4 commits into
mainfrom
cs-13040-an-isolated-incremental-index-re-derives-the-cards-type-and
Open

habdelra wants to merge 4 commits into
mainfrom
cs-13040-an-isolated-incremental-index-re-derives-the-cards-type-and

Conversation

@habdelra

@habdelra habdelra commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Background: what an index pass asks a browser tab to do

When a card is written, the realm enqueues an index job. That job does not
parse the card itself — it renders it. A pool of headless Chrome tabs is kept
warm on the prerender server, each one tagged with the realm it has been
serving ("affinity"), and the job asks a tab to visit the card's /render
route. The tab loads the card's document, resolves the class it adopts from,
instantiates the card, renders it, and hands back a search doc plus the
prerendered HTML.

Resolving the class is the expensive part, because it means loading the card's
module and everything that module transitively imports — the base card API,
field components, icons, and every schema module in the realm the card's own
schema reaches. That is several hundred modules for a realm of any size. The
tab's Loader caches every module it has evaluated, so this is paid once per
tab and then never again: the second card of the same type resolves its class
out of the cache in about a millisecond.

An index pass sent clearCache on its first render, and that flag did two
quite different jobs at once:

  • It dropped the tab's module graph (and its HTTP fetch cache). This is the
    expensive half, and it is only ever needed when a module's bytes changed.
  • It dropped the tab's store — the instances it holds resident, the
    documents it has cached, and the local-id pairings it has resolved. This is
    the cheap half, and it is needed on every pass, because a pass must never be
    handed an instance another pass left behind.

The flag was armed for every pass, so every pass paid the expensive half to get
the cheap one.

The problem

For a bulk pass that is nearly free: one cold render, then thousands of warm
ones behind it. For an incremental pass it is the entire cost, because a pass
over a single card save has exactly one render to amortize it against — the
cold one.

Measured on a deployed environment, saving one card repeatedly to a quiet
realm: the PATCH took about 2.8 s server-side, of which 2.7 s (97%) was the
handler waiting for the index visit. Inside that visit, 1.9 s of a 2.9 s total
was class resolution, which re-fetched and re-evaluated 370 modules. The tab
had been reused, and it had rendered the same card type dozens of times in the
preceding minutes. Across the environment's index rows, class resolution
averages about 1 ms when nothing is evaluated and hundreds to thousands of
milliseconds whenever anything is, so the split is entirely "did this visit
still have a module graph".

What changed

The two halves are now separate signals. clearCache keeps its meaning —
drop everything, module graph included — for the callers that need the module
bytes re-read, which is the prerenderer's own retry after a render error. An
index pass sends the new resetStore on its first render instead, and only
adds clearCache when its invalidation set contains an executable.

That executable test is the same condition that mints a fresh loader epoch, and
both now read it through one predicate (passInvalidatesExecutables, beside
hasExecutableExtension), so IndexWriter and IndexRunner cannot disagree
about whether a pass changed a module.

Why dropping the store still has to happen every pass. The store can notice
a job boundary by itself — GcCardStore.observeIndexingJob drops residency
when the visit's render scope moves. But only the out-of-process prerender
server tags visits with a render scope; the in-browser driver
(CardPrerender, which is what host-test indexing runs through) tags nothing,
so there the store cannot see the boundary and resetStore is the only thing
that moves it. Sending it per pass keeps both drivers behaving the same way.

Affinity ownership no longer depends on clearing. The prerender server
tracks which indexing batch owns a tab's warm loader and uses it to strip
clearCache off a render that does not belong to a batch. Ownership used to be
established by a batch's first clearing visit, which no longer happens for
most passes — so any batch visit now claims the affinity. It also takes over
from another batch rather than only claiming an unowned one: since is never
read as a deadline and nothing but a matching release or affinity disposal
clears the entry, so a batch that died mid-pass would otherwise own the
affinity indefinitely.

Two legitimate batches do overlap on one realm — an index pass and the
prerender_html job it spawns are in different concurrency groups and run
concurrently by design — so they will trade the entry between them. That costs
nothing: no reader asks which batch owns an affinity, only whether one does.

A dropped loader is now recorded on the row. The visit's diagnostics gain
loaderResetReason ('clearCache' or 'loaderEpoch'), present only when the
model build actually replaced the tab's loader, and naming the epoch when both
fire. moduleEvaluationCount has been on the row for a while, but a large
value was ambiguous between "this visit threw its graph away" and "this tab had
never evaluated the graph at all" — which are different problems with different
fixes.

What covers a module the pass's own realm does not own

The loader epoch is per realm, and a tab's evaluated graph is not: a tab affine
to one realm also holds the base realm's modules, and a base-realm write moves
only the base realm's epoch. What covers that is the deploy rather than
indexing. A deployed realm's base modules only change by releasing; the
prerender fleet recycles its browsers whenever the host-shell token changes;
and that token is the digest of the host's index.html, which carries the
build's own version. So no release leaves a tab holding modules from the one
before it.

Scope

This does not change how much work an index visit does when a module has
changed, and it does not take awaitIndex off the write's request path; a save
still waits for its index visit.

Where the changes live

  • packages/runtime-common/index.tspassInvalidatesExecutables, beside the
    extension test it is built on.
  • packages/runtime-common/index-runner.ts, index-writer.ts — the two
    one-shots (store every pass, loader only for an executable pass) and the
    shared predicate.
  • packages/runtime-common/render-route-options.ts,
    index-runner/visit-file.ts, packages/realm-server/prerender/render-runner.ts
    resetStore on the wire, consumed by the first pass of a visit.
  • packages/host/app/routes/render.ts,
    packages/host/app/components/card-prerender.gts — the two consumers.
  • packages/realm-server/prerender/batch-ownership-gate.ts — ownership claimed
    by any batch visit.

Tests

  • packages/realm-server/tests/index-loader-reset-test.ts (new) — the arming
    predicate: instances alone do not arm it, a .gts or .ts anywhere in the
    set does, a .d.ts does not, and the extension is read at the end of the
    path rather than anywhere in it.
  • packages/realm-server/tests/prerender-batch-ownership-test.ts — a batch
    visit claims an unowned affinity without clearing, and reclaims one whose
    owner never released.
  • packages/realm-server/tests/indexing-test.ts — two consecutive saves of one
    card record no loader reset and evaluate no modules; a pass that rewrites the
    module they adopt from records a reset and re-evaluates the graph. The second
    case is what keeps the first from holding equally well for a version that
    never arms the reset at all.

🤖 Generated with Claude Code

habdelra and others added 2 commits September 18, 2026 11:29
…odule

An index pass sends `clearCache` on its first visit, which resets the
prerender tab's loader and its fetch cache. The tab's evaluated module
graph is what makes a card render cheap, so the first card after the drop
re-fetches and re-evaluates every module it reaches. A pass over thousands
of rows amortizes that across all of them; a pass over one row — which is
what one card save produces — pays it whole.

The drop was armed for every pass rather than for the passes that need it.
Nothing but a change to an executable can make an evaluated graph describe
something other than what is on disk, and that condition already mints a
fresh loader epoch, which resets every tab holding a superseded graph
rather than only the one this pass's first visit reaches. Arm the pass-local
drop from the same condition, decided from the same URL set, so the two can
never disagree about whether the pass changed a module.

Ownership of an affinity was established by that same first clearing visit,
and it is what keeps a non-batch render from dropping a batch's warm loader
mid-pass. A pass that no longer clears would never claim it, so a batch
visit now claims an unowned affinity whether or not it clears. The takeover
rule is unchanged: an owner is replaced by a clearing successor, never by a
visit that merely arrived.

A row whose model build dropped the loader now records which
synchronization did it, so a large `moduleEvaluationCount` can be read as
the price of a drop rather than as a property of the card.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-18T15:48:16.445434Z 3d7bf08 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@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: 3d7bf08d80

ℹ️ 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/batch-ownership-gate.ts Outdated
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files  ±0      1 suites  ±0   2h 37m 52s ⏱️ + 4m 50s
4 914 tests ±0  4 900 ✅ +3  14 💤 ±0  0 ❌ ±0 
4 929 runs  ±0  4 915 ✅ +6  14 💤 ±0  0 ❌  - 3 

Results for commit d00056b. ± Comparison against earlier commit 54f8706.

Realm Server Test Results

    1 files  ±0    245 suites  ±0   1h 27m 49s ⏱️ + 6m 9s
3 625 tests ±0  3 625 ✅ +2  0 💤 ±0  0 ❌  - 2 
3 676 runs  ±0  3 676 ✅ +2  0 💤 ±0  0 ❌  - 2 

Results for commit d00056b. ± Comparison against earlier commit 54f8706.

@habdelra habdelra left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] This review went after one thing: whether narrowing the reset can leave a tab serving a module graph that no longer matches disk. I traced the epoch's scope against the graph a tab actually holds, both producers of a batchId and their queue groups, every hop loaderResetReason takes to the row, and what the new tests can and cannot fail on. I did not run the realm-server or host suites, and I did not review the render/settle paths the diff does not touch.

No blocking issues — the narrowing is right for the realm the pass is over, and the ownership change does not mis-sequence two batches. The one thing worth an answer before merge is (1): the loader epoch is scoped per realm, the module graph a tab caches is not, and clearCache was the only thing bridging that.

Recommendations, in the order I'd act on them:

  1. The epoch does not reach out-of-realm modules. A tab affine to realm A holds the base realm's modules, and nothing moves realm A's epoch when those change — see the thread on passInvalidatesExecutables in index-runner.ts. Needs an answer (probably "the host-shell recycle covers it"), and that answer belongs in the comment.
  2. Nothing tests that a module pass still arms the reset. Both call sites could be deleted with the suite green; see the thread on the new case in indexing-test.ts for the one-case fix.
  3. The gate's rationale paragraph says same-realm batches never run concurrently. They do, and this change is what makes prerender-html batches claim ownership for the first time — thread on batch-ownership-gate.ts.
  4. loaderResetReason is overwritten when both synchronizations fire, which is every executable pass's first visit — one-character suggestion in the render.ts thread.
  5. The timeout path cannot report the reason even though its type says it can — follow-up, thread on index.ts.
  6. The executable scan has two implementations over one list — follow-up, thread on the from-scratch arming site.

Two things I checked that you asked about in the description and that came back clean, so they are not threads: tasks/indexer.ts is the only construction site of IndexRunner, and the from-scratch arming reads exactly the list noteInvalidatedURLs feeds the epoch scan — including deletions, which discoverInvalidations folds into urls rather than leaving in deletedUrls alone.

Adjacent, out of scope: on an executable pass's first visit the route now builds a Loader and immediately replaces it, since both branches call resetLoader back to back. Nothing is evaluated in between so it costs nothing measurable, but it does mean two store.resetCache() calls for one visit, and it is the reason the diagnostics field has to pick a winner at all.

Comment thread packages/runtime-common/index-runner.ts Outdated
Comment on lines +1370 to +1375
// The tab-local drop is not the whole of the mechanism, and is not what makes
// a module change safe: the realm's loader epoch is re-minted by the same
// condition and threaded on every render, which resets every tab holding a
// superseded graph rather than only the one this pass's first visit reaches.
// This stays as the reset for that one tab, decided from the same set so the
// two can never disagree about whether the pass changed a module.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] This says the epoch "resets every tab holding a superseded graph". That holds inside the pass's realm; the graph the tab holds is not confined to one realm, so it does not hold outside it — and the reset this change removes is what used to cover the difference.

visit-file.ts threads loaderEpoch: batch.loaderEpoch, which is realm_generations.loader_epoch for the pass's own realm, and the tab keeps one unkeyed __boxelLoaderEpoch. But that tab's Loader also holds the base realm's modules — every card adopts from card-api, and #canaryRenderFailure in this file renders new URL('card-api', baseRealm.url) on the pass's realm affinity carrying the pass's realm epoch. Nothing moves realm A's epoch when a base-realm module's bytes change: mintRealmLoaderEpoch is only ever called with the writing realm's own url, and IndexWriter.loaderEpoch scans only the pass's own invalidation set. A base-realm definition lookup does mint and reset, but it passes the base realm as both affinity and epoch source, so it reaches a different tab.

So for a tab affine to realm A, a base-realm module change moves neither synchronization, and after this change nothing drops that graph.

Two things would settle it, and I could not settle either from the tree:

  • Does a release that changes only packages/base move the host-shell token? If it always does, the fleet recycle covers this and the answer is one sentence.
  • If the answer is instead the post-deployment full reindex, note that handlePostDeployment enqueues it only when the boxel-ui checksum moved, and that it arms per realm from that realm's own discovered URLs — so a realm holding no executable of its own would not arm even when it runs.

Whichever it is, please put it in this comment. It is the load-bearing half of the safety argument for narrowing the reset, and as written the paragraph reads as though the epoch covers everything clearCache did.

Class: regression — this change removes the only reset that reached the cross-realm case. Non-blocking on code if the recycle answer holds; the comment should still say so.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] Settled, and the answer is the first of the two you offered: the host-shell token always moves. It is the md5 of the host's index.html, and index.html embeds the build's own version — a dist built from this branch carries "version":"0.0.0+3d7bf08d", the commit it was built from. So every release produces a different digest, reportHostShellToManager reports it, the manager echoes it on heartbeat, and the fleet recycles its browsers. A deployed realm's base modules cannot change except by releasing, so no release leaves a tab holding the previous build's base-realm graph.

The paragraph in 54f8706 now says that rather than implying the epoch covers everything, and it says plainly that the epoch is per realm while a tab's graph is not.

The full-reindex path you flagged is indeed not the answer — it is gated on the boxel-ui checksum, and a base-only change would not move it. It just is not load-bearing here.

Comment on lines +1979 to +2003
assert.strictEqual(
first?.loaderResetReason,
undefined,
`a pass whose invalidation set holds no executable records no loader reset, got: ${JSON.stringify(first?.loaderResetReason)}`,
);
assert.strictEqual(
first?.moduleEvaluationCount,
0,
`and evaluates no module, got: ${JSON.stringify(first?.moduleEvaluationCount)} (a count above zero with no reason above means the tab arrived cold)`,
);

// A second isolated pass over the same card: each write is its own
// pass, so a per-pass drop would be paid again here rather than once.
await write('Richard Starkey');
let second = await diagnosticsFor('ringo.json');
assert.strictEqual(
second?.loaderResetReason,
undefined,
`the next pass over the same card records no reset either, got: ${JSON.stringify(second?.loaderResetReason)}`,
);
assert.strictEqual(
second?.moduleEvaluationCount,
0,
`and still evaluates no module, got: ${JSON.stringify(second?.moduleEvaluationCount)}`,
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] All four assertions hold a fortiori if the flag is never armed at all, so they cannot tell this fix from a version that deleted both #scheduleClearCacheForNextRender() call sites. Nothing else covers that direction either: across the whole tree, no test under packages/realm-server/tests or packages/host/tests references consumeClearCacheForRender or asserts clearCache on a render an IndexRunner issued, and index-loader-reset-test.ts imports only the pure predicate, so it cannot see the call sites.

The loaderResetReason === undefined pair has a second blind spot: it cannot distinguish "no reset happened" from "the key never reaches the row". The plumbing does carry it — every hop from render/meta.ts through flattenPrerenderMeta to IndexWriter spreads the object wholesale — but nothing in the suite demonstrates that, so the field's persistence is unpinned.

Ask: add the mirror case — write pet.gts, then assert the row records a loaderResetReason and a nonzero moduleEvaluationCount. That one case guards both arming call sites and turns these four assertions into discriminating ones.

Class: regression (new behavior, no coverage of its positive direction). Non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] Right, and fixed in 54f8706 — the module-write mirror is in the same test.

It reads the row differently than you proposed, for a reason worth recording: modules are visited before the instances that adopt from them, so pet.gts consumes the pass's one-shot and ringo.json never sees it. What re-synchronizes the tab for the instance row is the fresh epoch the module write minted, so the row records loaderResetReason from the epoch branch and a nonzero moduleEvaluationCount. That happens to pin the mechanism the narrowing actually leans on, which is the better thing to guard.

It also closes the second blind spot you named: the field is now asserted present on one row and absent on another in the same test, so a key that never reached the row would fail the mirror.

// │ no batchId + clearCache:off │ any │ run; owner unchanged │
// └─────────────────────────────┴─────────────┴──────────────────────┘
//
// Rationale: indexing jobs are serialized per-realm through the queue, so

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] This rationale is not true, and the row added above is what makes it matter.

indexingConcurrencyGroup is indexing:${realmURL} and prerenderHtmlConcurrencyGroup is prerender-html:${realmURL} — different groups, so the queue does not serialize them against each other. The index pass spawns its prerender-html job from onInvalidationsReady, whose own comment reads "Fires as soon as the invalidation set is known, so HTML rendering can start concurrently with the pass." Both mint a batchId and share the realm:<realmURL> affinity, so two legitimate batches over one realm overlap by design.

That was inert for this policy until now, because a prerender-html visit never sets clearCache and so could never claim. With the new row it claims any unowned affinity, so a concurrent peer routinely holds the entry an index pass would otherwise take. The takeover rule survives that, but for a reason this paragraph does not give: a concurrent peer never asks for a clear, so it can never present as a "successor".

Ask: say that instead. As written, the next reader has license to conclude that a different batchId always means a successor and to simplify the takeover rule the new prose above is careful to preserve.

Class: pre-existing, now load-bearing. Non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] Correct on all counts, and the paragraph is rewritten in 54f8706.

It now states that a different batchId is either a successor or a legitimate concurrent peer, names the two concurrency groups, and gives the reason the takeover rule is safe — that ownership answers 'is a batch rendering here', which every batch visit answers the same way, so no reader depends on which one holds it.

The row you were commenting on has since changed shape: ownership is no longer claimed only on an unowned affinity. Codex's P2 on this file pointed out that a batch which dies without releasing would then hold the entry indefinitely, since nothing expires it. Any batch visit now claims it, so the two peers will trade the entry — which the rewritten rationale says explicitly rather than leaving for a reader to discover.

Comment thread packages/host/app/routes/render.ts Outdated
}
}
if (parsedOptions.clearCache) {
loaderResetReason = 'clearCache';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] When both synchronizations fire, the row records clearCache and the epoch reset disappears — and they fire together on exactly the case this field exists to explain. A pass that invalidated an executable mints a fresh epoch and arms the flag from the same condition, so its first visit takes both branches: the block above sets 'loaderEpoch' and this line overwrites it. The reset that reached every other tab in that pass is the epoch, so a query counting epoch-driven resets undercounts by one row per executable pass, and the row names the second reset rather than the one that dropped the graph.

Suggested change
loaderResetReason = 'clearCache';
loaderResetReason ??= 'clearCache';

Class: regression. Non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] Taken as suggested in 54f8706. The line is loaderResetReason ??= 'clearCache', with a comment saying why the epoch wins: the two fire together on the first visit of an executable pass because one condition arms both, and the epoch is the reset that reached every other tab serving that pass.

// re-fetch and re-evaluation it pays for is the drop's price and not a
// property of the card. Its absence alongside a large count is the
// reading that says the tab had never evaluated the graph at all.
loaderResetReason?: 'clearCache' | 'loaderEpoch';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] RenderTimeoutDiagnostics extends this interface, so a timed-out row's type says it can carry loaderResetReason, but nothing populates it: the __boxelRenderDiagnostics hook in render.ts returns a fixed key literal and the prerender-side picker in prerender/utils.ts selects a fixed key list out of that, and the reason is in neither.

Worth a follow-up rather than this PR, but it is the one member of this interface a stalled page could answer. moduleEvaluationCount and the module list can only be totalled once the build finishes, which is why the preamble above excludes them; the reset reason is decided before the build starts. A render that times out cold-loading the graph is precisely where knowing the loader was dropped is worth the most.

Class: follow-up. Non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] Agreed on both the diagnosis and the disposition — left for a follow-up rather than done here.

Your point that it is the one member of the interface a stalled page could answer is right, and worth keeping: the count and the module list can only be totalled once the build finishes, but the reset is decided before it starts, so a render that times out cold-loading the graph is exactly where it would pay. It needs the key added to the __boxelRenderDiagnostics literal and to the picker in prerender/utils.ts, neither of which this change touches.

Comment on lines 366 to +367
current.batch.noteInvalidatedURLs(discoverResult.urls);
if (passInvalidatesExecutables(discoverResult.urls)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] "decided from the same set so the two can never disagree" (the helper's comment at the bottom of this file) is convention here, not construction: noteInvalidatedURLs runs its own inline urls.some(hasExecutableExtension) and this line runs a second implementation over the same list. Moving passInvalidatesExecutables next to hasExecutableExtension in runtime-common/index.ts would let IndexWriter call it too, putting one implementation behind both readers and making the promise structural.

Class: follow-up. Non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[Claude Code 🤖] Done in 54f8706 rather than deferred — the promise is cheap to make structural and was not worth leaving as a convention.

passInvalidatesExecutables now lives in runtime-common/index.ts beside hasExecutableExtension, and both readers call it: IndexWriter.loaderEpoch and noteInvalidatedURLs for the mint, IndexRunner for the arming. Their two inline .some() implementations are gone, so a change to what counts as an executable moves both at once. The comment says that instead of asserting the agreement.

An index pass sent `clearCache` on its first render, which dropped two
different things at once: the prerender tab's evaluated module graph, and the
store holding the instances, cached documents and local-id pairings the tab
had resolved. Only the first is expensive — the card after it re-fetches and
re-evaluates every module it reaches — and only the second is needed on every
pass. Arming one flag for both meant every pass paid the expensive drop to get
the cheap one, which a pass over thousands of rows amortizes and a pass over
one row, as a single card save produces, pays whole.

An index pass now sends `resetStore` on its first render, and adds
`clearCache` only when its invalidation set contains an executable — the same
condition that mints a fresh loader epoch, and now read through the same
predicate so the two cannot disagree about whether a pass changed a module.
`clearCache` keeps its meaning for the caller that needs the module bytes
re-read: the prerenderer's retry after a render error.

Dropping the store stays per pass because the boundary it enforces is not
otherwise visible from inside the tab. A store can observe the job boundary
itself when a visit carries a render scope, but only the out-of-process
prerender server tags visits with one; the in-browser driver tags nothing, so
there this flag is the only thing that moves it.

Ownership of an affinity was established by a batch's first clearing visit,
which most passes no longer make, and it is what keeps a non-batch render from
dropping a batch's warm loader. Any batch visit now claims the affinity,
including from another batch: nothing expires the entry, so a batch that died
without releasing would otherwise hold it until the affinity was disposed. Two
batches legitimately overlap on one realm and will trade the entry, which
costs nothing — no reader asks which batch owns an affinity, only whether one
does.

A row whose model build dropped the loader records which synchronization did
it, naming the epoch when both fire, so a large `moduleEvaluationCount` can be
read as the price of a drop rather than as a property of the card.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@habdelra

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] Pushed 54f8706, which reshapes the fix after the red shards on the previous commit.

clearCache was doing two jobs: dropping the tab's evaluated module graph, and dropping its store. Narrowing the flag as a whole took the second away with the first. The store drop is what keeps one index pass from being handed the instances, cached documents and local-id pairings of the last one — and the render scope that would otherwise move that boundary is tagged onto visits only by the out-of-process prerender server, so the in-browser driver has nothing else that moves it. They are separate signals now: resetStore every pass, clearCache only for a pass carrying an executable.

Verified by toggling only the store half on one build:

Integration | bxl cyclic card graphs
store reset enabled 3 pass
store reset disabled 3 fail — the same three that failed here

Also run: the two standalone suites (17 pass), and typecheck plus eslint across runtime-common, realm-server and host.

Not yet re-run locally: the indexing (mutating) > batch and incremental operations module, including the new module-write mirror case. It needs the realm-server test lane, which is occupied on this machine. CI covers it here, and I will say so plainly rather than imply otherwise.

…ader whole

Narrowing the per-pass loader drop exposed two places that had been relying on
it.

The file watcher drops the byte caches and the cached definition when a module
changes on disk under the realm, but never minted a loader epoch. The write
path mints one for exactly this reason: deleting a cached definition decides
that the next lookup re-derives one, not what it derives it from, and a
prerender tab still holding the old module answers with the old schema. Nothing
wrote an externally-edited module through the realm, so the index pass covering
that module was the only thing that minted — and a pass carrying only the
instances that adopt from it does not. Minting where the edit is observed makes
the order between those passes stop mattering, and reaches every tab rather
than the one a pass's first visit lands on.

The in-browser prerender driver shares one loader service with the application
it renders, and the application drops its own references in response to that
loader being replaced. Separating the store drop from the loader drop there
would leave the app holding instances resolved against a graph the pass has
moved on from. It now drops both. The out-of-process driver keeps the
narrowing: an index pass owns its tab, so that loader holds nothing but what
indexing put there.

The module-write half of the loader-reset test reads the re-evaluation rather
than a recorded reason. Modules are visited before the instances that adopt
from them, so the module's own visit consumes the pass's one-shot and no card
row is left to name it; what the row can still show is that the graph it
rendered against had to be rebuilt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@habdelra

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] Pushed d00056b, closing the remaining red on 54f8706. Three causes, each isolated by disabling only the suspected half and re-running:

Failure Cause Control
card playground error-handling ×2 The in-browser prerender driver shares one loader-service with the application it renders, and the app drops its own references when that loader is replaced. Separating the store drop from the loader drop left the app resolving against a graph the pass had moved on from. It now drops both; the out-of-process driver keeps the narrowing, because an index pass owns its tab. 3 fail with the old unconditional arming restored → pass with it; both target tests pass on the fix
file watcher invalidates caches after external edits The file watcher drops the byte caches and the cached definition on an external module edit but never minted a loader epoch, unlike the write path. The index pass covering the module mints; a pass carrying only the instances that adopt from it does not, and on an external edit those can be separate passes. mint disabled → exactly that test fails (3 pass / 1 fail); mint enabled → 4/4
the module-write half of my own loader-reset test It asserted loaderResetReason on a row that cannot carry it: modules are visited before the instances that adopt from them, so the module's visit consumes the pass's one-shot and no card row is left to name it. It now asserts the re-evaluation, which is what the row can show.

Run locally on this commit: file-watcher-events-test (4/4), the card playground suite (the two target tests pass; two others failed on Failed to fetch https://localhost:4201/base/cards-grid while the realm server on that port was being cycled, and pass on re-run), the two standalone suites (17), and typecheck + eslint across runtime-common, realm-server and host.

Not run locally: indexing (mutating) > batch and incremental operations. It needs :4200 serving a host built from this branch — the resetStore option is consumed by the host route, so against a host built from main the store is never reset between passes and the module fails for reasons unrelated to the change. That port is held by other work on this machine, and running it against the wrong build would produce a result I could not stand behind. CI is the first execution of that module on this commit.

@habdelra
habdelra requested a review from a team September 18, 2026 19:17
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.

2 participants