Follow-up to #383

[dlqqq]

cc @brichet @andrii-i @erkin98

Hey, thanks for working on PR #383.

I just got a chance to review it, and had a couple of comments:

1. I think it makes sense to make NotebookAttachment.mimetype a required string attribute that defaults to "application/x-ipynb+json". In python/jupyterlab-chat/jupyterlab_chat/models.py:

    mimetype: str = "application/x-ipynb+json"
    """
    The mime type of the notebook. Defaults to "application/x-ipynb+json".
    """

2. I don't think we should make NotebookAttachment a subclass of FileAttachment. The "file" type basically refers to any file that is not a notebook currently, and it has a mimetype field. The interface is already sufficiently general. There are benefits to keeping the two interfaces separate. a) The types may continue to diverge. b) the frontend can distinguish between the two types by using the attachment.type key (see discriminated unions, TS docs). If we make FileAttachment.type be a general string instead of a hard-coded string literal, this would not be possible.

[erkin98]

Agreed on both points. Should we point in new issue?

[brichet]

Thanks for your feedback @dlqqq, I also agree with both points.