Fix Layout hooks crash and stuck placeholder#810
Merged
SimonHeybrock merged 5 commits intomainfrom Mar 19, 2026
Merged
Conversation
Tests for both sub-issues:
(a) has_layout guard misses Layout when plotter's cached state is
transiently None, causing ValueError when hooks are applied to
an already-evaluated Layout-type DynamicMap.
(b) last_seen_version is bumped before the rebuild, so a failed
rebuild is never retried — the cell stays as a placeholder.
(a) Check `dmap.type is hv.Layout` in addition to
`plotter.get_cached_state()`. Once Bokeh has evaluated a
DynamicMap, its `.type` is set; `.opts(hooks=...)` rejects
Layout-typed DynamicMaps. The plotter's cached state can be
transiently unavailable during a background plotter replacement,
making the old guard miss the Layout.
(b) Defer `last_seen_version` bump until after the cell rebuild
succeeds. Previously the version was marked as "seen" before
the rebuild, so a failed rebuild was never retried and the cell
stayed as a placeholder permanently.
Fixes #805
Replace isolated mechanism-proof tests with three integration tests that exercise the actual poll+rebuild code path: - Layout plotter creates session layer with components - Rebuild succeeds after Bokeh evaluates DynamicMap as Layout - Failed rebuild leaves version stale for retry
The (a) race requires thread interleaving between two property reads in the same synchronous function — not reproducible deterministically. Keep only the happy-path Layout poll test and the (b) version-bump test.
SimonHeybrock
changed the title
nvaytet
approved these changes
Mar 18, 2026
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
2 participants
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.
Summary
Two fixes for cells getting permanently stuck as placeholders when a workflow produces
hv.Layoutplots:_get_session_composed_plot): Additionally checkdmap.type is hv.Layout— the DynamicMap's resolved type after Bokeh renders it. The existingplotter.get_cached_state()check can miss a Layout during a background plotter replacement (thread race between two reads ofstate.plotterwithin the same function). Thedmap.typecheck is race-free because it inspects the object that hooks will be applied to._poll_for_plot_updates): Deferlast_seen_versionbump until after the cell rebuild succeeds. Previously the version was marked "seen" before the rebuild, so any failure (not just the Layout error) left the cell stuck permanently.Testability note
The (a) race requires thread interleaving between two property reads in the same synchronous function — not reproducible deterministically in a single-threaded test. The fix is a code-review-level improvement (strictly better guard). Only (b) has a deterministic regression test.
Test plan
test_poll_creates_session_layer_for_layout_plotter— Layout plotter poll happy pathtest_failed_rebuild_does_not_bump_version— confirms (b) regression caught by reverting the fixFixes #805
🤖 Generated with Claude Code