Skip to content

Conversation

@dlqqq
Copy link
Collaborator

@dlqqq dlqqq commented Sep 10, 2025

Description

Adds a new option on_reset callback argument to the YRoom public get methods. The new signatures of these methods are:

  • get_awareness(on_reset: Callable[[pycrdt.Awareness], Any] | None = None)
  • async get_jupyter_ydoc(on_reset: Callable[[YBaseDoc], Any] | None = None)
  • async get_ydoc(on_reset: Callable[[pycrdt.Doc], Any] | None = None)

These on_reset callbacks, if passed, will be called with the new object of that type whenever the YDoc is reset.

  • Passing on_reset to get_awareness() will result in on_reset being called with the new pycrdt.Awareness object after the YDoc is reset.
  • This works similarly for get_jupyter_ydoc() and get_ydoc(), which will be called with the new YBaseDoc and pycrdt.Doc objects after the YDoc is reset.

Currently, a YDoc is reset only when an out-of-band change occurs on the source document. However, we may implement auto-resets in the future to save memory (see #114).

Technical details

This PR adds a mock_server_docs_app: MockServerDocsApp fixture that simulates the root ServerDocsApp defined in app.py. This can be passed as the parent argument to any singleton "manager" class initialized by the root ExtensionApp, making it much easier to write integration tests in the future.

This PR uses this to define new fixtures for YRoom and YRoomManager, and adds a unit test verifying the behavior of the on_reset callback arguments.

@dlqqq dlqqq added the enhancement New feature or request label Sep 10, 2025
@dlqqq
Copy link
Collaborator Author

dlqqq commented Sep 10, 2025

The check_release and enforce-label jobs are failing, but these failures don't seem related to this PR. It seems that the GitHub token used in these jobs needs to be updated:

# from check_release

{'type': 'FORBIDDEN', 'path': ['search', 'nodes', 0], 'extensions': {'saml_failure': False}, 'locations': [{'line': 8, 'column': 5}], 'message': 'Resource not accessible by integration'}
...

# from enforce-label

RequestError [HttpError]: Resource not accessible by integration
Error: Unhandled error: HttpError: Resource not accessible by integration
    at /home/runner/work/_actions/actions/github-script/v7/dist/index.js:9537:21
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async eval (eval at callAsyncFunction (/home/runner/work/_actions/actions/github-script/v7/dist/index.js:36187:16), <anonymous>:6:18)
    at async main (/home/runner/work/_actions/actions/github-script/v7/dist/index.js:36285:20) {
  status: 403,
  response: {
    url: 'https://api.github.com/repos/jupyter-ai-contrib/jupyter-server-documents/issues/152',
    status: 403,
...

Copy link
Collaborator

@ellisonbg ellisonbg left a comment

Choose a reason for hiding this comment

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

This looks good. For JSD, this API changes are backwards compatible. For extensions that need to work with both this and jupyter collaboration, we will need to add try except handling until jupyter collaboration also supports this API. The only thing that I am wondering (as a follow up) is if we can mark the old ydocs as beign stale in a way such that if someone tries to use them, they will get an exception. Basically, how can we ensure that extensions that have handles on ydocs are properly registering an on_reset handler. We can figure this out as a follow up.

@ellisonbg ellisonbg merged commit ec93b8b into jupyter-ai-contrib:main Oct 13, 2025
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants