Repository navigation
Render only changed cards when republishing a realm - #6624
Conversation
The publish copy now keeps file mtimes, so a republish indexes only the files that changed and their dependents. Every card still renders again on a first publish, when the published realm.json changed, and when the realm was last published before this server process started (a deploy). Files written in the ambiguous second of the previous publish copy get a fresh mtime, and a publish's index pass no longer joins an in-flight pass that may have read mtimes before the swap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jurgenwerk
left a comment
jurgenwerk
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review checked which inputs a skipped row still renders from, and whether the tests pin each guard that forces a full render. It did not look at the cost of the file walk or at the readiness gating.
One blocking decision: content from other realms is no longer refreshed by a republish. The rest holds.
- Decide how a republish handles rows that depend on other realms, and whether prerendered query results matter (see the comment on
renderEveryCard). - Add tests that fail without
touchFilesModifiedSinceand without the process-start guard (see the comment ontouchFilesModifiedSince). - The PR description narrates history ("The publish copy gave…", "Before,
clearLastModifiedblocked this join; nowawaitedByPublishdoes"). Restate it as the current contract.
| let renderEveryCard = | ||
| isNewRealm || | ||
| previousPublishedAt < PROCESS_STARTED_AT || | ||
| !sameRealmConfig( | ||
| previousRealmConfig, | ||
| await readRealmConfig(publishedRealmPath), | ||
| ); |
There was a problem hiding this comment.
[Claude Code 🤖] Two inputs that a skipped row renders from are missing from this list:
- Other realms.
discoverInvalidationscompares only this realm's own file mtimes, and nothing invalidates a published row when a module or card it depends on in another realm changes (for example a catalog module, or a linked card in a different workspace). A full render on every republish refreshed these rows. Now only the first republish after a server restart does, and a catalog deploy does not restart the realm server. - Query-backed fields.
index-writer.ts(above#findPeerConflicts) says that a query's matches are not recorded as dependencies. Thus a card that lists other cards through a query keeps its old results when a republish adds, removes or changes a matching card. I could not show this with the default index card: its prerendered count reads "0 cards" with and without this change. So this part is a question: do prerendered query results matter on a published page?
Options for the first point: (a) accept it, and say so in this comment and the PR description; (b) also set renderEveryCard when any published row has a dependency outside the realm and outside the base realm, which is one query over boxel_index.deps. I recommend (b), because a republish is how a user refreshes a published page.
Regression, blocking: it needs a decision before merge.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 04d61137e5. dependsOnOtherRealms sets renderEveryCard when any published row has a dependency in a realm from realm_registry other than this realm and the base realm. A prefix-form dependency is placed through virtualNetwork.realmForReference. Base realm code and code outside any realm (host packages, icons, CDN modules) do not count, because a deploy ships them and the process-start guard covers that. The new test it renders every card again when a card depends on another realm fails without the check.
On query-backed fields: the comment above renderEveryCard now states it as a known limit. A full render does not make those results current either, because its queries run against the committed index, which does not yet hold the rows the pass writes.
| // with the current time, so the index renders it again. Such a file renders | ||
| // once more at the next republish, since its index row then holds the stamp | ||
| // and not the source mtime. | ||
| async function touchFilesModifiedSince( |
There was a problem hiding this comment.
[Claude Code 🤖] No test pins this function or the PROCESS_STARTED_AT guard. I disabled the touchFilesModifiedSince call, and all 24 publish tests still passed. The existing republish tests hit the same-second case only when a publish and its index finish in less than one second.
Suggested tests in the republishing a realm module:
- Set both versions of a card file to an mtime inside the second of the previous
last_published_at(withutimesSync), republish, and assert that the card renders again. - Set
realm_registry.last_published_atto0before a republish, and assert that an unchanged card renders again.
Test coverage, non-blocking.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 04d61137e5. Two new tests: it renders a card again that changed in the second the previous publish copied it (plants the same-second mtime in boxel_index, then writes the second version in that second) and it renders every card again when the last publish was before the server process started. With touchFilesModifiedSince disabled, only the first test fails. With the process-start guard disabled, only the second test fails.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[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: cbd45c9cba
ℹ️ 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".
| // differently. | ||
| let renderEveryCard = | ||
| isNewRealm || | ||
| previousPublishedAt < PROCESS_STARTED_AT || |
There was a problem hiding this comment.
When an unchanged card imports or links to content in another public realm and that dependency changes without restarting this process, the copied files and realm.json retain the same mtimes and bytes, so this predicate leaves renderEveryCard false. Cross-realm changes explicitly do not propagate to the consuming realm's index (packages/runtime-common/realm.ts:13033-13043), meaning the from-scratch pass skips the card and preserves stale prerendered HTML until a local edit or restart. Track upstream dependency generations or force affected cards to render on republish.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 04d61137e5: dependsOnOtherRealms makes a republish render every card when a published row depends on another realm (see the reply on the renderEveryCard thread above).
| let renderEveryCard = | ||
| isNewRealm || | ||
| previousPublishedAt < PROCESS_STARTED_AT || |
There was a problem hiding this comment.
Every republish updates realm_registry.last_published_at before indexing, and that value is part of the RealmInfo embedded in card responses (packages/runtime-common/realm.ts:16478-16482). A card component that renders realmInfo.lastPublishedAt therefore has new output even when its files and realm.json are unchanged, but renderEveryCard remains false and its existing prerendered HTML is retained; cards otherwise invalidated in the same publish can consequently show a different publication time from skipped cards. This runtime metadata change needs to invalidate consumers or be included in the full-render decision.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Not changed in code; documented in 6ed4c7cdaf. To keep realmInfo.lastPublishedAt current, every card would have to render on every publish, and that is the cost this change removes. No base template shows the value for a published realm: workspace.gts reads only the source-realm map shape. The comment above renderEveryCard now states that a skipped card keeps the time of the publish that last rendered it.
| // more) into cards that do not depend on it in the index; and code | ||
| // deployed since the last publish can render the same files | ||
| // differently. | ||
| let renderEveryCard = |
There was a problem hiding this comment.
[Claude Code 🤖] Blocking: a card whose published HTML is an error keeps that error across republishes. With clearLastModified false, discoverInvalidations re-renders a row only when indexEntry.hasError is set or its mtime moved, and getModifiedTimes reads boxel_index.has_error alone. An error recorded on the HTML channel lives in prerendered_html.error_doc (a render timeout, a deterministic render error, a visit failure past the reconcile sweep's retry cap), while that card's boxel_index row stays clean. The reconcile sweep treats such rows as terminal until the URL's next invalidation, and a full republish was that invalidation. With this change the card stays broken on the published site until its file changes or the server restarts.
Could the republish also render rows whose HTML channel holds an error? For example, invalidate the URLs with prerendered_html.error_doc IS NOT NULL at the realm's current generation (the effectiveHasError condition) alongside the mtime diff, or set renderEveryCard when any exist. A test that plants an HTML error row for an unchanged card and asserts that a republish renders it again would pin it.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 04d61137e5. When a republish does not render every card, invalidateErroredRows clears last_modified before the job is enqueued, for each row of the published realm where effectiveHasError() holds. It reuses the same condition the query engine reads, so the two cannot drift. The pass then renders those rows again. The new test it renders an unchanged card again when its HTML holds a render error plants a prerendered_html.error_doc on an unchanged card. It asserts that a republish replaces the error, and that a card without an error is not rendered again. It fails without the invalidation.
…nd render errored rows again Address review: a published row that depends on a card or module in another realm is not invalidated when that realm changes, so such a realm renders every card on republish. Rows whose HTML holds a render error get their mtime cleared so the pass renders them again. Document that query results are not dependencies, and add tests that pin the same-second touch, the process-start guard, the other-realm check and the render-error invalidation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 participants
Republishing a realm is slow, because it renders every card again, even when only one card changed. The publish copy gave every file a new mtime, and
clearLastModifiedcleared the remaining index mtimes, so the indexer saw every file as changed. Now the copy keeps file mtimes, and a republish renders only the changed files and the files that depend on them. URLs stay correct: the published URL is the same as last time, so the rows of unchanged cards already hold it.A republish still renders every card when skipping is not safe:
realm.jsonchanged (byte compare of the published copy before and after): realm info feedsog:titleand other values into cards that have no index dependency onrealm.json. The publish's own edits to the copiedrealm.json(policy pointer, default-index opt-in) would give it a new mtime each time, so its mtime is set back to the source's.Two edge cases:
lastPublishedAtis now taken before the copy, so it marks that second. The cost is that these files render once more at the next republish.clearLastModifiedblocked this join; nowawaitedByPublishdoes.🤖 Generated with Claude Code