Repository: vscode
Issue: Support showing Pull Request comments on notebook cells
Labels: feature-request,comments,notebook

<!-- ⚠️⚠️ Do Not Delete This! feature_request_template ⚠️⚠️ -->
<!-- Please read our Rules of Conduct: https://opensource.microsoft.com/codeofconduct/ -->
<!-- Please search existing issues to avoid creating duplicates. -->

VSCode has a comment API for code review comments - this is used e.g. by the GitHub Pull Request extension to show comments from pull requests inside VSCode.

VSCode also supports a Notebook Editor, e.g. for ipynb files. This editor does support comments on cells, which was added in #144850.

The two do not work very well together though: When you leave a comment on a GitHub pull request, it is attached to a file URI  and line/column range (even for notebook files, like ipynb). The Notebook Editor will not render those, because it only renders comments attached to the notebook cell URI (ignoring the range). Example: [PR with a comment on a line of a code cell in an ipynb file](https://github.com/rehmsen/notebook_test/pull/1) - this comment is rendered in VSCode's comment pnael, but not in the Notebook editor:
<img width="1380" alt="image" src="https://github.com/microsoft/vscode/assets/29040/4962adea-c15c-46cc-9749-c42d657ff64b">

I would like them to also show up in the notebook editor. 

Here are some ideas from https://github.com/microsoft/vscode/issues/144850#issuecomment-2115453397 how one might make that happen:

> I found it a bit tricky to work with that it expect comments to be attached to the cell URI, not the file URI. The issue with that is that the cell URI is dynamically created during deserializing ([from the file URI changing the scheme and adding a cell fragment](https://github.com/microsoft/vscode/blob/88f27ceb0c235a4e84f3e4271f39305961f150bd/src/vs/workbench/services/notebook/common/notebookDocumentService.ts#L45) which is basically an encoded [increasing number](https://github.com/microsoft/vscode/blob/462a2a7c8fcfa063773202dc3c32e6dac67356ee/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts#L297)), but VSCode also has a comments panel, where the comment should ideally be shown even before you open the file. At that point the cell URI is unknown.
>
> I was wondering how you deal with that, and tried to get Github Pull Request comments to show up in the NotebookEditor for ipynb files, which does not seem to work: They are only shown when I reopen the ipynb file in a TextEditor.
>
> I have a workaround, which starts creating the comments with the file URI, and the line number in the raw JSON ipynb file, and then upon opening the notebook editor, recreates the CommentThread with the cell URI (and cell-relative line number) as it becomes known. I need to recreate because the URI of a CommentThread is actually readonly - this might cause all kinds of issues with keeping view state, like whether comments are collapsed and any non-saved replies.
>
> Instead, I wonder if we could not leave CommentThreads on the file URI, and when we fetch the comments for a notebook in [getNotebookComments](https://github.com/microsoft/vscode/blob/462a2a7c8fcfa063773202dc3c32e6dac67356ee/src/vs/workbench/api/browser/mainThreadComments.ts#L446), derive the appropriate cell id and cell-line-number from the raw file URI and line number. This would require some sort of source map between the raw file numbers and the structure of the notebook, and as VSCode hands off deserialization to extensible [NotebookSerializers](https://github.com/microsoft/vscode/blob/462a2a7c8fcfa063773202dc3c32e6dac67356ee/src/vscode-dts/vscode.d.ts#L14956), that API would also need to allow providing a source map.
>
> As an example, NotebookData could get a new optional field
> 
> ```
> interface NotebookData {
>    /* Maps a line number in a raw notebook file to a (cell, cell line number) pair in the deserialized notebook model. */
>    lineMap?: Map<number, {cellId: string, cellLine: number}>;
>  }
>  ```
> If a NotebookSerializer fills that, then it is used to map any file URI comments for notebook files to the corresponding cell locations in the notebook editor. The inverse transformation may be needed when creating a new comment on a cell.
> 
> One issue is that I believe right now cells do have a stable ID (beyond this increasing integer which is ephemeral) - maybe that could be added for this purpose (or we would have to make the assumption that we can reference the deserialized cells by their index in the serialized source). Another issue might be comments on cells that were not in the serialized source. We can probably work through these issues.
>
> What do you think about this proposal? Is there some simpler approach that I am overlooking? I would be happy to contribute the changes, but would like to align on the design direction first.