onDidChangeNotebookAssociation not fired on initial notebook open

[colombod]

Version: 1.56.0-insider (user setup)
Commit: 7ce1499
Date: 2021-04-22T05:13:14.711Z
Electron: 12.0.4
Chrome: 89.0.4389.114
Node.js: 14.16.0
V8: 8.9.255.24-electron.0
OS: Windows_NT x64 10.0.19042

Steps to Reproduce:

Clone pr dotnet/interactive#1286
Go to folder src/dotnet-interactive-vscode/nsiders adn open with VSCode insiders
Launch debugger
set breakpoint in src/dotnet-interactive-vscode/insiders/src/notebookControllers.ts at line 61
Open .ipynb notebook

this not firing will not let us fix languages and metadata if missing. At this point users cannot work until they actually trigger a ReopenWith

Does this issue occur when all extensions are disabled?: Yes/No

[vscodebot]

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

• Webview post messages are not posted to the notebook controller (#121895)

[colombod]

cc @brettfo , @rebornix , @jrieken

[jrieken]

Yes there the following cases where a control is selected and the event should be send

1. use picks controller from the picker
2. controller is auto-picked, user executes cell/notebook
3. controller has been picked in a prior session and is restored as the active one

I believe cases 1 and 2 work but that case 3 isn't covered.

[colombod]

I can't get 2 and 3 to work, the events for me is triggered only with scenario 1

[jrieken]

I have pushed a fix for 3 and a test that asserts that 2 is working. I guess you need to specify the "can't work further"

[colombod]

in the second scenario I right click on notebook and select openwith

I select the jupyter kernel preview

The event is not triggered and I am stuck here

not that language is C#, trying to change it is not easy with mouse (see #121927) and until is set properly run cell won't work

Using the command palette I do perform the change

When I run now the event is fired but I don't feel this sequence is something users should face

[jrieken]

In you steps you are not picking a controller, you are just opening a notebook editor. The controller UI is in the status bar

[colombod]

[jrieken]

Yes, that. So, if you now run a cell the event should be fired (2) unless it has already been fired (1) or unless you are running into (3)

[brettfo]

The issue is that we want to be notified when a notebook is first opened. Our scenario is that .NET Interactive supports .ipynb files and if the user does Open With... on a .NET Interactive .ipynb and the Python controller is selected, we want that notification so we can peek at the metadata and suggest that this is a .NET Interactive notebook and prompt them to change. If they manually switch from .NET Interactive to Python then we get that notification and we can peek/prompt, but we'd like to know the first time it's opened. We've had users get really confused when they open a notebook, hit run, and they get a bunch of errors from IPython saying Console.WriteLine couldn't be found.

[brettfo]

Also, even if we're selected as the default controller, we still need to know immediately on open so we can update the cell languages. We do some metadata probing to properly set the language and if we're not called, call cells think they're C# and it breaks our intellisense.

[jrieken]

The issue is that we want to be notified when a notebook is first opened.

You will have to use the onDidOpenNotebook event for that. Tho, you still have the challenge of not knowing when (iff ever) you are picked as the selected provider. There is a fix that makes sure you are receiving the event immediately after a notebook is opened (assuming have been selected before) but it isn't send in the same "ipc message" as the open event.

It also remains unclear how selection should work in the presence of a single controller. Today, this happens when the user presses run (because you don't get the picker UI for a single controller). We cannot auto-select a single controller because we don't know how many other controllers come after that. So, it's either when run is pressed, or by clicking the status bar item, or by showing a single item picker. Whatever we do, you need to be prepared for a large gap between opening a notebook and becoming the selected controller.

hit run, and they get a bunch of errors from IPython saying Console.WriteLine couldn't be found.

I don't wanna get involved here but it sounds like an IPython bug. Do they not know upfront that they cannot run C# code?

[brettfo]

Each cell in one of our notebooks is potentially a different language, and that language we have set in the cell's metadata, so on initial open, what VS Code thinks the cell language is, is incorrect. We tie into the onDidChangeNotebookAssociation event to "fix" the cell languages to what they should be. If the user manually selects our controller, then that event fires and all is good. If, however, our controller was already selected, either because (1) a session was restored where we were previously selected, or (2) we're the only controller available, that event doesn't fire and we never get an opportunity to update the languages, and because we didn't update the languages, the cell languages are all wrong so the user can't ever execute a cell, so the path where on first execute we get notified of a controller change, that won't happen.

That's why even if our controller was already determined to be correct, we still need that event to fire, because we won't work otherwise.

[brettfo]

If always firing the onDidChangeNotebookAssociation event on open isn't a possibility, we could accomplish the same thing with either (1) a way to figure out what the current controller was for a given NotebookDocument, or (2) figure out what NotebookDocuments are associated with a given controller. In either of those cases we could re-use the onDidOpenNotebookDocument event and then correct the cell languages.

[jrieken]

If, however, our controller was already selected, either because (1) a session was restored where we were previously selected,

The restore should should be fixed but I see how there is a bug when having multiple view types for a notebook and how a wrong controller is then restore. I will push a fix today.

or (2) we're the only controller available, that event doesn't fire

That we can not do because we don't know that it is the only controller. The very first controller would always be the "only controller" and automatically associated. That is unwanted.

[jrieken]

So, the rules of notebook association are still

1. use picks controller from the picker
2. controller is auto-picked and user executes cell/notebook
3. controller has been picked in a prior session and is restored as the active one

I fixed a bunch of issues for (3) that would only occur when having multiple providers/serialiser for a notebook file. That case went under the radar and was basically not working at all.

Again, "auto-select controller because it's the only one" unfortunately doesn't work because we never know if a controller is the only one. Tho, that should only be a corner case and "the only one" should always be the suggested controller which, upon execution, is being selected.

[colombod]

The scenario 2

controller is auto-picked and user executes cell/notebook

Will be quite common and we won't be able to run until a user changes language. Right now having Jupyter extension to handle the file operations and us being the only controller installed will always land on that scenario. Language list and all won't be addressed until the user realises "oh I need to do something about this".

[jrieken]

Language list and all won't be addressed until the user realises "oh I need to do something about this".

You still receive the selection event, just directly before receiving the execute request. It should give you the same "chances" as if a user manually selects the controller and then very quickly selects run cell

[brettfo]

You still receive the selection event, just directly before receiving the execute request.

This doesn't work for us for the following reason: to maintain compatibility with Jupyter Lab, a .ipynb file that uses the .NET Interactive kernel states that its language is C#, which means the Jupyter extension simply sets that as each cell language, but from a VS Code standpoint, our language isn't C# (or csharp), it's dotnet-interactive.csharp because we have our own special variant of the language, and when a user first opens an .ipynb, the user can't execute the cell because csharp isn't in our controller's list of supported languages (but dotnet-interactive.csharp is). Because of this, they can't click execute which means the initial selection event never fires which puts the user into the situation of needing to manually set each cell's language on open. This is made even more complicated by the fact that each cell in a .NET Interactive notebook can have a different language, but the .ipynb format assumes a document-level language. We get around this limitation by setting metadata on each cell, but if we never know that our controller is associated with a document, we can never read this metadata and set the cell languages appropriately.

This partly goes back to one of my earlier requests that would help us; some way to associate a NotebookDocument with a NotebookController, then we could do our own check on onDidOpenNotebookDocument. E.g., any one of the following would work:

• isControllerAssociatedWithNotebookDocument(controller: NotebookController, document: NotebookDocument): boolean
• getAssociatedNotebookDocuments(controller: NotebookController): NotebookDocument[]
• getAssignedController(document: NotebookDocument): NotebookController)

[jrieken]

E.g., any one of the following would work:

If we would know the answer to any of those suggested functions (to be able to implement them) then we could also send the onDidChangeNotebookAssociation-event. The problem is that we don't know what controller is associated to a notebook, for us that's a user-action and only after that user-action has happened we know.

[jrieken]

The only "workaround" to the problem I can think of is auto-associate a notebook to a kernel as long as it is the only applicable one. E.g the following flow

• create first controller A for ipynb
• controller A receives select event for notebook N
• another extension creates controller B also for ipynb
• controller A receives un-select event for notebook N
• (maybe) controller B receives select event for notebook N because it was selected in the past

What worries me is that controller A has now changed the notebook, as a side-effect of having been selected for a short time. I don't know if controller B likes that and how users like that.

[jrieken]

for us that's a user-action and only after that user-action has happened we know.

A more aggressive way to interpret user-actions would be to change the meaning for the "select as default" gesture (see capture below). Today, being the default for a view type is used for sorting but we could also auto-associate notebooks that have a default controller. Tho, again that is a user-action and only happens when the user has performed that.