Re-render a card in place when its adoptsFrom (type) changes after indexing#5567
Conversation
The host store's reload path applied incoming JSON to the existing card instance in place via updateFromSerialized. A card instance's JavaScript class is fixed at construction, so a change to meta.adoptsFrom (the card's type) — e.g. a realm index card re-pointed from CardsGrid to a custom index card — left the old class in place and kept rendering the old type until a full browser reload. reloadInstance now resolves the incoming adoptsFrom and, when the instance is no longer an instance of that type, re-instantiates via createFromSerialized (carrying over the existing local id so the identity map's id-resolver accepts the swap) and returns the new-typed instance for reloadTask to swap into the tracked identity map. Same-type edits keep the in-place update. A failure to resolve the incoming type surfaces as a card error rather than being mistaken for a deletion. Adds a host integration test and a full-path code-submode acceptance test (edit adoptsFrom in the editor -> reindex -> index event -> re-render). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d-json-with-an-adoptsfrom-change-should-cause
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6905abc3f3
ℹ️ 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 fixes a host refresh correctness bug where a card instance whose meta.adoptsFrom (type) changes after indexing would keep rendering as its previous type because the existing in-memory instance was only updated in place. The store now detects a type mismatch on reload and re-instantiates the card so the identity map points at the new-typed instance and renders update without a full page reload.
Changes:
- Update
StoreService.reloadInstance()to resolve the incomingadoptsFromand, when the existing instance is not an instance of that type, re-instantiate viacreateFromSerialized()(carrying over the existing local id). - Adjust reload autosave subscription teardown logic so the old instance’s subscription is detached when it’s been superseded by an error state or a new-typed instance.
- Add integration and acceptance tests that change
meta.adoptsFromand assert the store + rendered preview update to the new type.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/host/app/services/store.ts | Rebuilds card instances on reload when the incoming adoptsFrom resolves to a different type; updates autosave subscription handling accordingly. |
| packages/host/tests/integration/store-test.gts | Adds an integration test that rewrites a card’s meta.adoptsFrom and asserts the stored instance and isolated render become the new type. |
| packages/host/tests/acceptance/code-submode/editor-test.ts | Adds an acceptance test that edits meta.adoptsFrom in the code editor and asserts the preview re-renders as the new type after indexing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Compare the resolved adoptsFrom against the instance's exact class instead of a subtype check, so re-pointing from a subclass to one of its ancestors rebuilds instead of keeping the subclass instance. This also removes the instanceof narrowing that tripped the type-check. - Build the new-typed instance directly (rather than via createFromSerialized, whose cached-instance reuse would keep the old object for an ancestor type) and let updateFromSerialized swap it into the identity map. - Preserve the CardError detail from a failed loadCardDef, only dropping the 404 status so it surfaces as an error state rather than a phantom delete. - Add an integration test for the subclass-to-ancestor case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When you change a card's type by editing its JSON
meta.adoptsFrom— for example re-pointing a realm's index card from the defaultCardsGridto a custom index card — the realm re-indexes correctly, but the host kept rendering the card as its old type until a full browser reload.How host refresh works
The host doesn't poll. When indexing completes, the realm broadcasts an
index/incrementalevent listing the invalidated cards; the store (StoreService.handleInvalidations) re-fetches each loaded card's JSON and applies it to the instance it's holding. Instances live in a tracked identity map, so anything rendering a card by id re-renders when the map entry changes.The bug
The reload applied the new JSON in place via
card-api.updateFromSerialized. A card instance's JavaScript class is fixed at construction —updateFromSerializedreads the class off the existing object and never reconsultsmeta.adoptsFrom. So field edits worked, but a type change copied the new fields onto an instance that was still the old class, which kept rendering with the old template.The fix
reloadInstancenow resolves the incomingadoptsFrom; when the current instance is no longer an instance of that type, it re-instantiates viacreateFromSerialized(which builds the new-typed instance and swaps it into the tracked identity map) instead of updating in place. Same-type field edits are unchanged — still updated in place, preserving object identity.Two details worth calling out:
Tests
Integration | Store): loads a card as one type, rewrites its JSON to an unrelated type, and asserts both the stored instance and the rendered isolated template become the new type.code submode | editor tests): exercises the whole path — editadoptsFromin the code editor → realm reindex → index event → store reload → the preview re-renders as the new type.Both tests were confirmed to fail without the fix and pass with it. The full
Integration | Store(82) and editor acceptance (17) suites stay green.🤖 Generated with Claude Code