Keep keyed children that move out of a replaced container - #65
Merged
Merged
Conversation
maartenbreddels
force-pushed
the
fix/fast-keyed-move
branch
from
September 29, 2026 09:38
def267e to
76cb953
Compare
Review of the previous fuzz test found a case it cannot reach: a child with an explicit key that moves to a sibling container while the container it leaves is replaced by one of another type. The random trees now have a Mover component that does this with an element made once (use_memo), so it is the same object in every render. On master, 2 of the 12 seeds fail: the fast renderer has a closed widget in the tree. Over 300 seeds, about 1 in 5 fail, some with a KeyError in the fast renderer. Fixed in the next commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
When reconciliation replaces a widget by one of another type (a VBox becomes an HBox), it first removes the old widget with all its children, by key. An explicit key is the same in any container of a component, so a child with an explicit key that moved to a sibling container in the same render was removed too: - if the sibling was reconciled first, the moved child's widget was closed while it was in the tree (both renderers); - if it came later, the fast renderer raised a KeyError, since it had kept the child's subtree as it was. While a replaced element is removed, its children with an explicit key that the new tree still uses are now left alone, and reconciliation updates them where the new tree has them. A keyed child that stays under the new container keeps its widget instead of getting a new one (its state was kept already). The fuzz test: on master 2 of the 12 seeds fail, and about 1 in 5 of 300; with this fix 1000 of 1000 pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review found that keeping a keyed child of a replaced container also kept shared elements (.shared()), whose bookkeeping needs the removal: the element stayed in _shared_elements, so reconciliation raised "Element not reconsolidated", and old _shared_widgets entries piled up. Shared elements now keep the old behavior. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maartenbreddels
force-pushed
the
fix/fast-keyed-move
branch
from
September 29, 2026 09:48
76cb953 to
821f7a3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
When reconciliation replaces a widget by one of another type (a VBox becomes an HBox), it first removes the old widget and all its children, by key. An explicit
.key()is the same in any container of a component. So a keyed child that moved to a sibling container in the same render was removed too:KeyError. The default renderer survives.This is the follow-up that the crossreview of #63 found.
What
Three commits:
Movercomponent keeps one child with an explicit key (made once withuse_memo, so it is the same object in every render). On its own state, it moves the child between two sibling containers and flips their types. On master, 2 of the 12 seeds fail, and about 1 in 5 of 300 seeds..shared()) keep the old behavior, because their bookkeeping needs the removal.Tests
test_keyed_child_moves_out_of_replaced_container(both move orders): fails on master in both renderers.test_shared_keyed_child_in_replaced_container: passes on master, and guards the review fix.pytest reacton/withREACTON_FAST=0and=1: all pass.Review
Crossreview by three reviewers (astra, opus, glm).
close()or the stale-key sweepsMoveris deterministicVBox(children=[VBox(children=[child]) if s else child]). That removal runs through the stale-key sweep, which this fix does not change. It fails the same way on master, in both renderers. This is the next follow-up.🤖 Generated with Claude Code