Skip to content

Skip redundant aset on out-of-band changes#556

Merged
krassowski merged 4 commits into
jupyterlab:mainfrom
jordanhboxer:jupyter-server-ydoc-2.2.1+hrt0
Mar 27, 2026
Merged

Skip redundant aset on out-of-band changes#556
krassowski merged 4 commits into
jupyterlab:mainfrom
jordanhboxer:jupyter-server-ydoc-2.2.1+hrt0

Conversation

@jordanhboxer
Copy link
Copy Markdown
Contributor

@jordanhboxer jordanhboxer commented Mar 24, 2026

Summary

  • Only call aset when the document content actually differs from the incoming model
  • Avoids unnecessary Y document mutations that can trigger duplicate cells during concurrent syncs

Test plan

  • Existing tests pass
  • Manual verification with concurrent notebook editing

Related

#545
jupyter-server/jupyter_ydoc#376
jupyter-server/jupyter_ydoc#385

@github-actions
Copy link
Copy Markdown
Contributor

Binder 👈 Launch a Binder on branch jordanhboxer/jupyter-collaboration/jupyter-server-ydoc-2.2.1%2Bhrt0

Comment thread projects/jupyter-server-ydoc/jupyter_server_ydoc/_version.py Outdated
@krassowski krassowski changed the title Skip redundant aset on out-of-band changes Skip redundant aset on out-of-band changes Mar 25, 2026
@krassowski krassowski added the bug Something isn't working label Mar 25, 2026
@krassowski
Copy link
Copy Markdown
Member

Assuming that we want it in this repo then a pytest test would be helpful.

Jordan Boxer and others added 2 commits March 25, 2026 08:57
Only call aset when the document content actually differs from the
incoming model, avoiding unnecessary Y document mutations that can
trigger duplicate cells during concurrent syncs.

PY-1189

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test that aset is skipped when document content matches the incoming
model in both the _on_outofband_change and _maybe_save_document
(OutOfBandChanges) code paths.

PY-1189

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jordanhboxer jordanhboxer force-pushed the jupyter-server-ydoc-2.2.1+hrt0 branch from e49a199 to eb83c39 Compare March 25, 2026 12:59
@jordanhboxer
Copy link
Copy Markdown
Contributor Author

done!

Copy link
Copy Markdown
Collaborator

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we could use the file hash from the server?

@jordanhboxer
Copy link
Copy Markdown
Contributor Author

correct me if wrong but the way i see it is the hash is only set once the document is saved so i think there is the potential for the ydoc to have unsaved edits which would make comparing the hashes not possible

Comment thread tests/test_rooms.py Outdated
Copy link
Copy Markdown
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should not hurt beside some performance overhead from iterating the cell list as by default we would attempt to deudplicate)

I am ok with merging this if @davidbrochart gives thumbs up too.

@krassowski krassowski merged commit 9293fa9 into jupyterlab:main Mar 27, 2026
55 of 56 checks passed
@krassowski
Copy link
Copy Markdown
Member

I will start prep for release of 4.3.0 now, unless we want to wait for #558?

@davidbrochart
Copy link
Copy Markdown
Collaborator

#558 is far from being ready, so please go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants