Repository navigation
fix: Drop held changes to models the client has not been sent - #8906
Open
philippjfr wants to merge 1 commit into
Open
philippjfr wants to merge 1 commit into
philippjfr wants to merge 1 commit into
Conversation
Under the combine hold policy a later change is merged into the slot of an earlier one, which can discard the only definition that preceded a change to a new model, e.g. a range shared between two plots that re-render twice in one hold. The client then rejects the whole patch with "reference isn't known". Assisted-by: Claude Code:claude-opus-5-5
Open
2 tasks
This branch has not been deployed
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.
Description
Changes made inside a
hold()with the default'combine'policy can produce a patch the client cannot apply. The browser then rejects the whole patch withDeserializationError: reference pXXXX isn't known, and none of the held changes appear.The combine policy merges a later change to a property into the slot of the earlier event for the same property. If the earlier event was the only one that defined a new model, and a change to that model was recorded in between, the change ends up ahead of any definition. A concrete case is two HoloViews plots with a shared range that re-render twice in one hold:
holder.childrento a figure that defines a newRange1d.startchanges.other.x_range = range, which only references it).holder.childrenagain. This is combined into slot 1, so slot 1 no longer defines the range, and slot 2 now references a model the client does not have.This change drops held changes to models the client has not been sent yet (Bokeh's
doc.models._new_models), both in_flush_eventsand in the threaded unhold. These changes are redundant, because the message that first defines a model serializes its current state.This was found while debugging Lumen's
ComponentController(holoviz/lumen#2056), which writes several widgets inside onehold(). On the penguin copilot example, the agent's tool call succeeded on the server, but nothing changed in the browser.Minimal reproducer (click
go, then check the browser console):Before this change, the browser logs
reference … isn't knownand keeps the old widget values. After it, all three changes apply.Tests:
test_combined_patch_references_unsent_modelreproduces the undecodable patch with plain Bokeh models and Bokeh's ownDeserializer.test_drop_unsent_model_changes_keeps_combined_patch_decodablechecks that the filtered patch decodes. All ofpanel/tests/io/test_document.pypasses locally.AI Disclosure
Tool & Model: Claude Code + Claude Opus 5.5
Usage: Debugged the issue from the Lumen copilot down to Bokeh's event combining, wrote the fix and tests, and drafted this description.
Checklist