Add widget comm state sync for multi-window support#293
Merged
Conversation
Track active comm channels in daemon and send CommSync broadcast to newly connected clients, enabling widget reconstruction in secondary windows. - Add CommState module for tracking comm_open/msg/close - Wire CommState into NotebookRoom and kernel manager - Send CommSync after initial Automerge sync on client connect - Handle comm_sync in frontend to synthesize comm_open messages - Clear comm state on kernel shutdown Known limitations: - Third window opened after execution may not receive comm_sync (needs investigation) - Widget state not synced in real-time (by design - avoids feedback loops) - Buffer paths not fully reconstructed (Codex review finding #1) - Replay order nondeterministic (Codex review finding #2) Closes #276 Co-Authored-By: QuillAid <261289082+quillaid@users.noreply.github.com>
- Use sequence numbers for deterministic comm replay order (finding #2) Widget models can reference other widgets via IPY_MODEL_. Returning comms in insertion order ensures dependencies are created before dependents. - Clear comm state when launching new kernel (finding #3) If a kernel crashes without proper shutdown, stale comm snapshots could persist. Now cleared on new kernel launch, not just explicit shutdown. Co-Authored-By: QuillAid <261289082+quillaid@users.noreply.github.com>
Previously, failed deserializations during init were silently ignored. Now logs a warning with the error and payload size to help diagnose issues like comm_sync not being received. Co-Authored-By: QuillAid <261289082+quillaid@users.noreply.github.com>
Helps diagnose third window widget sync issue by logging: - When comm_sync event is received - Whether onCommMessage handler is available - Unknown broadcast event types Co-Authored-By: QuillAid <261289082+quillaid@users.noreply.github.com>
rgbkrk
force-pushed
the
widget-comm-sync
branch
from
563090f to
6ac03e7
Compare
rgbkrk
added
daemon
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
1 participant
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
Track active comm channels in daemon and send
CommSyncbroadcast to newly connected clients, enabling widget reconstruction in secondary windows.Fixes #276
Changes
CommStatemodule for trackingcomm_open/msg/closemessagesCommStateintoNotebookRoomand kernel manager's iopub handlerCommSyncbroadcast after initial Automerge sync on client connectcomm_syncin frontend to synthesizecomm_openmessages for widget storeKnown Limitations (WIP)
comm_sync(needs investigation - may be timing issue with broadcast delivery)HashMapiteration) - model dependencies may not resolve correctlyTest plan
Co-authored-by: QuillAid 261289082+quillaid@users.noreply.github.com