docs: Add a design doc for Language Support for notebook cells

[tanishiking]

It would be easier to read with rendered one :) https://github.com/scalameta/metals/blob/ccd1bed3a6eabe0d67d401be18ba348d1c494f3a/docs/design/language-support-notebook.md

Added a design document for scalameta/metals-feature-requests#236
This document is for

• design review the architecture before going further
• make sure we are on the same page about the goal, design, and requirements for this feature

---

This PR is just for the design review. I opened a PR because it's easier to review (maybe we could write a design doc on Google Docs or something).
Therefore, I don't need to merge this PR after people approved the architecture design, but this document could be useful as a log in the future.

[tanishiking]

Can you take a look, and give me some thoughts? @dos65 @tgodzik @alexarchambault (requesting review to someone who commented on the original feature requests).

Maybe you can skip until Problem section because I guess you guys already know about the context.

[dos65]

@tanishiking Thx for the investigations! The plan looks reasonable

I'm only not sure if there is need in adding it to metals docs.
Maybe it might be left in a issue comment to this feature request?

[dos65]

I would also include a request to execute/interpret a cell.
I don't know if we would be able to reuse existing method or it requires introduce a new one for BSP protocol.

[tanishiking]

@alexarchambault Hi, could you skim through this doc when you have time before I'm going further (to make sure we're on the same page about the implementation plan)
I'm not in a rush (as I might not have time to work on this for several weeks), take you time :)

[tgodzik]

Looks like a good plan, though might be a long term solution and will require a lot of work if I am not mistaken 🤔

[alexarchambault]

This looks like a very good expanded explanation and wrap up of earlier discussions about this.

I added a comment about a point that worries me a bit (hoping others could have better ideas about it…)

[alexarchambault]

Thinking a bit further about this (that I proposed in #4253 (comment)), I'm very slightly worried about how Metals will be able to get the PID of the Almond process corresponding to a given notebook, or more generally how Metals can find a way to connect to the right Almond kernel.

I skimmed through the notebook section of LSP 3.17 (that you linked above), and couldn't find any way of letting servers know about the kernel processes that the LSP client starts.

To work around that, maybe we could have hashed the notebook path, and used that hash instead of the kernel PID. But the kernel doesn't know about the path of the notebook it is asked to run, it only knows about individual code cells passed to it via Jupyter message protocol messages. So hashing the notebook path isn't an option either.

Another workaround could consist in having Almond leave its PID in the metadata of some display data or execute_result (AFAIK, these are persisted in the notebook). That might be a bit flaky though, as for the PID to be written in the notebook file, this would require at least a cell to have run. Not to add too much overhead, we might want to return those metadata only for the first cell say, but then the metadata would go away if users delete that cell… (and the kernel doesn't know about cell deletion…) So it might need to be put in all execute_result metadata.

Also, those metadata, once written on disk, might be stale (they may correspond to the kernel that previously ran the notebook). (Maybe writing a date alongside it could help - LSP servers would just pick the PID with the latest date.)

Don't know if asking people involved in LSP to update the protocol, so that LSP servers could know about Jupyter kernels started by LSP clients, could be an option here…

[tanishiking]

I'm very slightly worried about how Metals will be able to get the PID of the Almond process corresponding to a given notebook,

Yeah, I think it's difficult (or even impossible) to find a Kernel PID from Metals. That's why I'm thinking maybe we can use kernel name or something instead of PID (more details later)

---

To work around that, maybe we could have hashed the notebook path, and used that hash instead of the kernel PID.
Another workaround could consist in having Almond leave its PID in the metadata of some display data or execute_result (AFAIK, these are persisted in the notebook).

I think it works 👍

this would require at least a cell to have run.

Not really, we should be able to get kernel's metadata like on notebookDocument/didOpen, and we'll have access to that information without running cells

 "metadata": {
  "kernelspec": {
   "display_name": "Scala",
   "language": "scala",
   "name": "scala3"
  },
  "language_info": {
   "codemirror_mode": "text/x-scala",
   "file_extension": ".sc",
   "mimetype": "text/x-scala",
   "name": "scala",
   "nbconvert_exporter": "script",
   "version": "2.13.8"
  }
 },

On the other hand, what do you think about using the Kernel spec name instead of embedding PID information to the name or display name?

One concern is: is the kernel name unique?; can we say there won't be multiple same kernels running at the same time?

---

Don't know if asking people involved in LSP to update the protocol, so that LSP servers could know about Jupyter kernels started by LSP clients, could be an option here…

I think LSP's responsibility is just to provide some interfaces between the editor(notebook) and language servers and how to connect with Kernel is out of the scope of LSP, I guess.

(kinda off-topic: the notebook LSP spec seems like the notebook content can be evaluated purely with its content (by naively concatenating cells into a script), and assuming there's no need to connect with Kernel).

[tanishiking]

I'm only not sure if there is need in adding it to metals docs.
Maybe it might be left in a issue comment to this feature request?

As @dos65 mentioned, we don't need to merge this PR (I just submit PR because it seems easier to review), I'll turn this into an issue and closing :)

[tanishiking]

#4434