Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguish between completion and chat models #711

Merged

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Apr 2, 2024

Closes #669

  • Splits the model selector for completion and chat. API keys are the same between chat/completion.
  • Adds a button to open the Inline Completion Settings
  • Adds a warning when completion model is selected but the jupyter-ai inline completion provider is not enabled

The ability to open the settings for incline completion was preserved; when a model is selected, but the inline completer is not enabled a warning icon will be shown.

Settings button Settings button with a warning
image image

When the completion plugin is disabled a different warning text will be shown: "A completer model is selected, but the completion provider plugin is not available"; I deem this preferable to hiding the input box as this will be easier to debug for users.

The warning icon is cleared as soon as the completion provider is enabled in settings:

warning-clearing

UI in initial version for reference

Splits the model/embedding model selectors and API keys UI to a new component. API keys are the same between chat/completion but having them in this component makes it convenient for the user to setup the API keys in whichever form they are editing.

image

image

I would love to put that within JupyterLab Inline Completer settings, appending to the fragment indicated by red box below:

image

I tried doing so but due to limitation of the form registry it is not straightforward in this case yet, but I can give it another try if you are interested.

For now I am thinking about using some kind of warnings or adding a button opening the Inline Completer settings:

image

What do you all think?

@krassowski krassowski added the enhancement New feature or request label Apr 2, 2024
@krassowski krassowski force-pushed the separate-completer-and-chat-settings branch from 28e4f07 to edcf55a Compare April 2, 2024 15:33
@krassowski krassowski marked this pull request as ready for review April 3, 2024 15:01
@krassowski krassowski force-pushed the separate-completer-and-chat-settings branch from aca2beb to caa6fa8 Compare April 5, 2024 08:16
@krassowski
Copy link
Member Author

Kicking CI because GitHub Actions had an outage this morning and some jobs are still marked as pending.

@krassowski

This comment was marked as outdated.

@krassowski krassowski force-pushed the separate-completer-and-chat-settings branch from 006a302 to 578f63c Compare April 10, 2024 19:26
@krassowski krassowski requested review from JasonWeill and dlqqq April 11, 2024 11:47
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@krassowski Thank you for working on this! This clearly took a lot of effort and we appreciate your contribution. 🤗

I left some detailed feedback below. In addition, I would like to make the following high-level suggestions:

  • Because these are language models, I don't think we need a separate chat_models property on BaseProvider to indicate which models served by a provider support chat; that is, we already assume that all of the models served by a provider support chat. We should remove chat_models from the BaseProvider class and the ListProviders API response models as well.
  • There are a lot of changes to critical components of the frontend. I'm not able to confidently assert that the changes proposed in this PR preserve the current functionality of Jupyter AI's frontend. Therefore, I don't think this PR can be accepted as-is without further revision in how the frontend capability is implemented here.
  • To reduce the number of changes, can we instead have the inline completer model selection be part of the existing ChatSettings component? IOW, users should be able to select the completions model from the existing chat settings UI provided by Jupyter AI's side panel. It can be another dropdown underneath the embedding model dropdown.
    • I don't understand the motivation behind separating Jupyter AI's configuration into two distinct areas within JupyterLab; this can be confusing for users. We use our own config UI precisely to avoid known issues with JupyterLab's existing Settings UI, some of which you encountered.

Let me know your thoughts on the above. Note that it may be some time before I can review this again because I'm really busy at work; I appreciate your patience in the meantime.

packages/jupyter-ai/jupyter_ai/config/config_schema.json Outdated Show resolved Hide resolved
packages/jupyter-ai/jupyter_ai/config_manager.py Outdated Show resolved Hide resolved
packages/jupyter-ai/jupyter_ai/config_manager.py Outdated Show resolved Hide resolved
packages/jupyter-ai/jupyter_ai/config_manager.py Outdated Show resolved Hide resolved
@krassowski
Copy link
Member Author

krassowski commented Apr 16, 2024

Thank you for the review @dlqqq!

Because these are language models, I don't think we need a separate chat_models property on BaseProvider to indicate which models served by a provider support chat; that is, we already assume that all of the models served by a provider support chat

Many models fine-tune for code completion do not support chat and will output gibberish if used in chat.

Right now, retrieval-augmented generation is not supported for inline completions. It is also unclear whether we can or should implement support for this capability in the future. Given this, we should remove the completions_embeddings_provider_id field here in this PR.
[...]
we don't have any plans for completion embedding models.

This is good to know that you do not have plans for it. However, let me point out that state of the art completion software does use RAGs, see for example https://githubnext.com/projects/copilot-view/ and https://tabby.tabbyml.com/blog/#release-v030---retrieval-augmented-code-completion-. I am not sure if jupyter-ai should support this in the future, but if a user issues a /learn command in chat, they may expect the learned documentation to be used for code completions. @ellisonbg do you think you will consider supporting this in the future?

To reduce the number of changes, can we instead have the inline completer model selection be part of the existing ChatSettings component? IOW, users should be able to select the completions model from the existing chat settings UI provided by Jupyter AI's side panel. It can be another dropdown underneath the embedding model dropdown.

This will be problematic if a user wants to disable the chat but still use the completer. This is also problematic in Jupyter Notebook where chat is not supported so users have no way to configure a model.

I don't understand the motivation behind separating Jupyter AI's configuration into two distinct areas within JupyterLab; this can be confusing for users.

The aim is to have the inline completer configuration in JupyterLab's existing Settings UI for a few reasons:

  • this makes it easier for user to find the settings
  • to enable using the inline completer with chat plugin disabled
  • there are settings which have to be applied across all inline completion providers, such as the delay and timeout, thus having both settings which apply across providers, and settings of individual providers in a single place makes it easier to use

Since putting this in the red box as indicated in the screenshot above would require changes in JupyterLab, in the meantime I am happy with putting the dropdown (s) in chat and reevaluate later.

We use our own config UI precisely to avoid known issues with JupyterLab's existing Settings UI, some of which you encountered.

There is no issue moving the entire jupyter-ai settings panel to the existing JupyterLab's Settings UI because you can swap the entire renderer. With changes in this PR it would be in fact pretty easy to achieve. I think the cog icon could open an appropriate page in the JupyterLab Settings UI. If there however is an issue I would highly appreciate if you could report this upstream in JupyterLab repository or link to it.

@krassowski
Copy link
Member Author

krassowski commented Apr 17, 2024

To reduce the number of changes, can we instead have the inline completer model selection be part of the existing ChatSettings component? IOW, users should be able to select the completions model from the existing chat settings UI provided by Jupyter AI's side panel. It can be another dropdown underneath the embedding model dropdown.

Done in 575f3f4

The ability to open the settings for incline completion was preserved; when a model is selected, but the inline completer is not enabled a warning icon will be shown.

Settings button Settings button with a warning
image image

When the completion plugin is disabled a different warning text will be shown: "A completer model is selected, but the completion provider plugin is not available"; I deem this preferable to hiding the input box as this will be easier to debug for users.

The warning icon is cleared as soon as the completion provider is enabled in settings:

warning-clearing

as the team has no plans to support it :(
Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.
@krassowski krassowski requested a review from dlqqq April 18, 2024 11:50
@krassowski krassowski force-pushed the separate-completer-and-chat-settings branch from 0edb11a to 434a991 Compare April 25, 2024 12:43
@krassowski krassowski force-pushed the separate-completer-and-chat-settings branch from 434a991 to c410441 Compare April 25, 2024 13:30
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@krassowski Thank you for updating the PR with my suggestions! I love the new code, and the "inline completions" dropdown seems very natural to have in the settings UI. I have some additional suggestions below, but they are minor compared to the ones I left in my last review.

Can you also address this comment from the last review? #711 (comment)

packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
Comment on lines +520 to +525
title={
'A completer model is selected, but ' +
(props.provider === null
? 'the completion provider plugin is not available.'
: 'the inline completion provider is not enabled in the settings: click to open settings.')
}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of relying on alt text (which takes >1000ms to appear on my Chrome browser), can you use a MUI Tooltip to contain this message?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 18e57f4.

@dlqqq
Copy link
Member

dlqqq commented Apr 29, 2024

@krassowski Let me address some of your concerns in your reply:

This is good to know that you do not have plans for it. However, let me point out that state of the art completion software does use RAGs, see for example https://githubnext.com/projects/copilot-view/ and https://tabby.tabbyml.com/blog/#release-v030---retrieval-augmented-code-completion-. I am not sure if jupyter-ai should support this in the future, but if a user issues a /learn command in chat, they may expect the learned documentation to be used for code completions.

Sorry, that certainly wasn't the intent of my message! Please allow me to clarify a few points:

  • We are certainly open to implementing RAG for inline completions; I was indicating the fact that Jupyter AI does not support it today, and that having an additional field for "completion embeddings" which does nothing would be confusing for users.

  • If we do move forward with RAG for inline completions, it's not obvious why a separate embedding model should be configured by the user. From my personal experience, the differences in behavior between embedding models are hard to notice, and it's unclear why a user would want a different embedding model for chat and for completions.

  • Adding another optional embedding model also introduces new engineering difficulties; specifically, we would need to maintain two vector stores in the backend, since the embeddings produced by one model are not generally compatible with embeddings produced by another.

The aim is to have the inline completer configuration in JupyterLab's existing Settings UI for a few reasons:

There is no issue moving the entire jupyter-ai settings panel to the existing JupyterLab's Settings UI because you can swap the entire renderer.

This context is super helpful, thank you! Rendering settings as a main area widget could allow us to put more advanced configuration in the Settings UI. However, this would have to be done in a future major release.

This will be problematic if a user wants to disable the chat but still use the completer. This is also problematic in Jupyter Notebook where chat is not supported so users have no way to configure a model.

This is a valid concern. In a future major release, we could maybe split off the completions code into its own JupyterLab extension. I'd like to take advantage of our existing monorepo setup to give users more flexibility by allowing users to simply install the features they want, separate from the chat panel. So in the future, perhaps Notebook users could simply install a jupyter-ai-completions package that has no dependency on jupyter-ai.

However, given the issues previously mentioned, I think we should stick to keeping the inline completions settings within Jupyter AI settings for now.

krassowski added 11 commits May 2, 2024 16:23
and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.
using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.
from `completerIsEnabled` to `isCompleterEnabled`
Copy link
Member Author

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you for the review @dlqqq! I believe I made all requested changes.

packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
packages/jupyter-ai/src/components/chat-settings.tsx Outdated Show resolved Hide resolved
Comment on lines +520 to +525
title={
'A completer model is selected, but ' +
(props.provider === null
? 'the completion provider plugin is not available.'
: 'the inline completion provider is not enabled in the settings: click to open settings.')
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 18e57f4.

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@krassowski Awesome work, thank you for addressing my feedback!

@dlqqq dlqqq merged commit cf20800 into jupyterlab:main May 3, 2024
8 checks passed
@krassowski
Copy link
Member Author

Amazing, thank you! Is there a release planned any time soon?

srdas pushed a commit to srdas/jupyter-ai that referenced this pull request May 7, 2024
* Distinguish between completion and chat models

* Fix tests

* Shorten the tab name, move settings button

Lint

* Implement the completion model selection in chat UI

* Improve docstring

* Call `_validate_lm_em_id` only once, add typing annotations

* Remove embeddings provider for completions

as the team has no plans to support it :(

* Use type alias to reduce changeset/make review easier

Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.

* Rename `_validate_lm_em_id` to `_validate_model_ids`

* Rename `LLMHandlerMixin` to `CompletionsModelMixin`

and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.

* Rename "Chat LM" to "LM"; add title attribute; note

using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.

* Rename heading "Completer model" → "Inline completions model"

* Move `UseSignal` down to `CompleterSettingsButton` implementation

* Rename the label in the select to "Inline completion model"

* Disable selection when completer is not enabled

* Remove use of `UseSignal`, tweak naming of `useState`

from `completerIsEnabled` to `isCompleterEnabled`

* Use mui tooltips

* Fix use of `jai_config_manager`

* Fix tests
3coins added a commit that referenced this pull request May 8, 2024
* save chat history to jupyter lab root dir

The /export command was saving  chat history in the pwd, i.e., the directory from where JupyterLab is launched rather than the JupyterLab root. For JupyterLab sessions, where the root directory is different from the launch directory, users will not be able to see the exported chat history file in JupyterLab. This is now rectified.

* learn arxiv tex files (#742)

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Streamlined code for learning arxiv files

(1) removed temp dir handling
{2) extracted only tex files
(3) Moved imports into the `arxiv_to_text` function
{4) improved tar file processing

* update learn for arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* removed extra imports

* Fix /learn in 2.14.0 (#747)

* accumulate filepaths

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update learn for arxiv files

Redoing code after the PR 747 made changes to the same file.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Improved code for arxiv files

Improvements to  PR 742:
(i) removed extra `arxiv.Client` call
(ii) removed unnecessary `try/catch`
(iii) moved `datetime` import outside `arxiv_to_text` function

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Saves arxiv to root, better exception handling.

* Added arxiv feature to docs.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>

* Distinguish between completion and chat models (#711)

* Distinguish between completion and chat models

* Fix tests

* Shorten the tab name, move settings button

Lint

* Implement the completion model selection in chat UI

* Improve docstring

* Call `_validate_lm_em_id` only once, add typing annotations

* Remove embeddings provider for completions

as the team has no plans to support it :(

* Use type alias to reduce changeset/make review easier

Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.

* Rename `_validate_lm_em_id` to `_validate_model_ids`

* Rename `LLMHandlerMixin` to `CompletionsModelMixin`

and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.

* Rename "Chat LM" to "LM"; add title attribute; note

using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.

* Rename heading "Completer model" → "Inline completions model"

* Move `UseSignal` down to `CompleterSettingsButton` implementation

* Rename the label in the select to "Inline completion model"

* Disable selection when completer is not enabled

* Remove use of `UseSignal`, tweak naming of `useState`

from `completerIsEnabled` to `isCompleterEnabled`

* Use mui tooltips

* Fix use of `jai_config_manager`

* Fix tests

* Fix `unsupported_slash_commands` default (#768)

* Updates to /export command

(1) Save chat history file to Jupyter root directory, not cwd, in markdown format
(2) Add time stamps to the saved files
(3) Enable the `/export <filename>` option, else use default file name.
(4) Remove code to increment file numbers for multiple chat histories, given that the time stamps are now being used.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update export function

(1) Use argparse to extract filename if the option is used
(2) Remove the old function to get the chat file name and streamline the code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Titan embedding model v2 (#778)

* Add Titan embeddning model v2

Included the new embedding model, released recently on Amazon Bedrock.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* refactor /export code

Remove if then and replace with a single inline expression

* Refactor export code reprise

Updated some variable names

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>
Co-authored-by: Michał Krassowski <[email protected]>
srdas added a commit to srdas/jupyter-ai that referenced this pull request May 9, 2024
* save chat history to jupyter lab root dir

The /export command was saving  chat history in the pwd, i.e., the directory from where JupyterLab is launched rather than the JupyterLab root. For JupyterLab sessions, where the root directory is different from the launch directory, users will not be able to see the exported chat history file in JupyterLab. This is now rectified.

* learn arxiv tex files (jupyterlab#742)

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Streamlined code for learning arxiv files

(1) removed temp dir handling
{2) extracted only tex files
(3) Moved imports into the `arxiv_to_text` function
{4) improved tar file processing

* update learn for arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* removed extra imports

* Fix /learn in 2.14.0 (jupyterlab#747)

* accumulate filepaths

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update learn for arxiv files

Redoing code after the PR 747 made changes to the same file.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Improved code for arxiv files

Improvements to  PR 742:
(i) removed extra `arxiv.Client` call
(ii) removed unnecessary `try/catch`
(iii) moved `datetime` import outside `arxiv_to_text` function

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Saves arxiv to root, better exception handling.

* Added arxiv feature to docs.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>

* Distinguish between completion and chat models (jupyterlab#711)

* Distinguish between completion and chat models

* Fix tests

* Shorten the tab name, move settings button

Lint

* Implement the completion model selection in chat UI

* Improve docstring

* Call `_validate_lm_em_id` only once, add typing annotations

* Remove embeddings provider for completions

as the team has no plans to support it :(

* Use type alias to reduce changeset/make review easier

Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.

* Rename `_validate_lm_em_id` to `_validate_model_ids`

* Rename `LLMHandlerMixin` to `CompletionsModelMixin`

and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.

* Rename "Chat LM" to "LM"; add title attribute; note

using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.

* Rename heading "Completer model" → "Inline completions model"

* Move `UseSignal` down to `CompleterSettingsButton` implementation

* Rename the label in the select to "Inline completion model"

* Disable selection when completer is not enabled

* Remove use of `UseSignal`, tweak naming of `useState`

from `completerIsEnabled` to `isCompleterEnabled`

* Use mui tooltips

* Fix use of `jai_config_manager`

* Fix tests

* Fix `unsupported_slash_commands` default (jupyterlab#768)

* Updates to /export command

(1) Save chat history file to Jupyter root directory, not cwd, in markdown format
(2) Add time stamps to the saved files
(3) Enable the `/export <filename>` option, else use default file name.
(4) Remove code to increment file numbers for multiple chat histories, given that the time stamps are now being used.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update export function

(1) Use argparse to extract filename if the option is used
(2) Remove the old function to get the chat file name and streamline the code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Titan embedding model v2 (jupyterlab#778)

* Add Titan embeddning model v2

Included the new embedding model, released recently on Amazon Bedrock.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* refactor /export code

Remove if then and replace with a single inline expression

* Refactor export code reprise

Updated some variable names

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>
Co-authored-by: Michał Krassowski <[email protected]>
srdas added a commit that referenced this pull request May 9, 2024
* save chat history to jupyter lab root dir

The /export command was saving  chat history in the pwd, i.e., the directory from where JupyterLab is launched rather than the JupyterLab root. For JupyterLab sessions, where the root directory is different from the launch directory, users will not be able to see the exported chat history file in JupyterLab. This is now rectified.

* learn arxiv tex files (#742)

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Streamlined code for learning arxiv files

(1) removed temp dir handling
{2) extracted only tex files
(3) Moved imports into the `arxiv_to_text` function
{4) improved tar file processing

* update learn for arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* removed extra imports

* Fix /learn in 2.14.0 (#747)

* accumulate filepaths

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------



* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update learn for arxiv files

Redoing code after the PR 747 made changes to the same file.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Improved code for arxiv files

Improvements to  PR 742:
(i) removed extra `arxiv.Client` call
(ii) removed unnecessary `try/catch`
(iii) moved `datetime` import outside `arxiv_to_text` function

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Saves arxiv to root, better exception handling.

* Added arxiv feature to docs.

---------





* Distinguish between completion and chat models (#711)

* Distinguish between completion and chat models

* Fix tests

* Shorten the tab name, move settings button

Lint

* Implement the completion model selection in chat UI

* Improve docstring

* Call `_validate_lm_em_id` only once, add typing annotations

* Remove embeddings provider for completions

as the team has no plans to support it :(

* Use type alias to reduce changeset/make review easier

Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.

* Rename `_validate_lm_em_id` to `_validate_model_ids`

* Rename `LLMHandlerMixin` to `CompletionsModelMixin`

and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.

* Rename "Chat LM" to "LM"; add title attribute; note

using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.

* Rename heading "Completer model" → "Inline completions model"

* Move `UseSignal` down to `CompleterSettingsButton` implementation

* Rename the label in the select to "Inline completion model"

* Disable selection when completer is not enabled

* Remove use of `UseSignal`, tweak naming of `useState`

from `completerIsEnabled` to `isCompleterEnabled`

* Use mui tooltips

* Fix use of `jai_config_manager`

* Fix tests

* Fix `unsupported_slash_commands` default (#768)

* Updates to /export command

(1) Save chat history file to Jupyter root directory, not cwd, in markdown format
(2) Add time stamps to the saved files
(3) Enable the `/export <filename>` option, else use default file name.
(4) Remove code to increment file numbers for multiple chat histories, given that the time stamps are now being used.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update export function

(1) Use argparse to extract filename if the option is used
(2) Remove the old function to get the chat file name and streamline the code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Titan embedding model v2 (#778)

* Add Titan embeddning model v2

Included the new embedding model, released recently on Amazon Bedrock.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------



* refactor /export code

Remove if then and replace with a single inline expression

* Refactor export code reprise

Updated some variable names

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>
Co-authored-by: Michał Krassowski <[email protected]>
@dlqqq
Copy link
Member

dlqqq commented May 9, 2024

@krassowski Hey Mike, we are planning a minor release to occur either this afternoon or tomorrow morning. Apologies for the delay, I just got back after taking some time off.

@yuhon0528
Copy link

@krassowski Will it also be better if we can set something like default_completion_model and default_chat_model when running jupyter lab --config using config.json?
Previously, we can only do something like:

{
    "AiExtension": {
        "default_language_model": "Self-Hosted:deepseek-coder-6.7b-instruct",
        "allowed_providers": "Self-Hosted",
        "model_parameters": {
            "Self-Hosted:deepseek-coder-6.7b-base": {},
            "Self-Hosted:deepseek-coder-6.7b-instruct": {}
        },
        "default_api_keys":{}
    }
}

It may be better to modify the extension.py also. Thank you.

@krassowski krassowski deleted the separate-completer-and-chat-settings branch May 10, 2024 08:35
@krassowski
Copy link
Member Author

@dlqqq welcome back and thank you! @yuhon0528 this is a good idea, I opened #781 to track this.

Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
* Distinguish between completion and chat models

* Fix tests

* Shorten the tab name, move settings button

Lint

* Implement the completion model selection in chat UI

* Improve docstring

* Call `_validate_lm_em_id` only once, add typing annotations

* Remove embeddings provider for completions

as the team has no plans to support it :(

* Use type alias to reduce changeset/make review easier

Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.

* Rename `_validate_lm_em_id` to `_validate_model_ids`

* Rename `LLMHandlerMixin` to `CompletionsModelMixin`

and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.

* Rename "Chat LM" to "LM"; add title attribute; note

using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.

* Rename heading "Completer model" → "Inline completions model"

* Move `UseSignal` down to `CompleterSettingsButton` implementation

* Rename the label in the select to "Inline completion model"

* Disable selection when completer is not enabled

* Remove use of `UseSignal`, tweak naming of `useState`

from `completerIsEnabled` to `isCompleterEnabled`

* Use mui tooltips

* Fix use of `jai_config_manager`

* Fix tests
Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
* save chat history to jupyter lab root dir

The /export command was saving  chat history in the pwd, i.e., the directory from where JupyterLab is launched rather than the JupyterLab root. For JupyterLab sessions, where the root directory is different from the launch directory, users will not be able to see the exported chat history file in JupyterLab. This is now rectified.

* learn arxiv tex files (jupyterlab#742)

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* learn_arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Streamlined code for learning arxiv files

(1) removed temp dir handling
{2) extracted only tex files
(3) Moved imports into the `arxiv_to_text` function
{4) improved tar file processing

* update learn for arxiv

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* removed extra imports

* Fix /learn in 2.14.0 (jupyterlab#747)

* accumulate filepaths

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* learn arxiv tex files

* Created a new option remote or -r. Example: /learn -r arxiv <arxiv-id>
* Approach: downloads the tar file for the entire paper into downloads_temp. Then, unzips and collects all .tex files in the tar file and concatenates them. Different authors use various approaches. Some have the entire paper in one tex file, whereas others may have separate tex files for each section, so we need to collect all the tex file into a single file and then hand off to the splitter, embedder. After completion, remove the temp directory. Return a properly formatted error if package arxiv needs to be installed.
* Handle two types of errors: (i) package arxiv not installed. (ii) User enters a wrong paper id.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update learn for arxiv files

Redoing code after the PR 747 made changes to the same file.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Improved code for arxiv files

Improvements to  PR 742:
(i) removed extra `arxiv.Client` call
(ii) removed unnecessary `try/catch`
(iii) moved `datetime` import outside `arxiv_to_text` function

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Saves arxiv to root, better exception handling.

* Added arxiv feature to docs.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>

* Distinguish between completion and chat models (jupyterlab#711)

* Distinguish between completion and chat models

* Fix tests

* Shorten the tab name, move settings button

Lint

* Implement the completion model selection in chat UI

* Improve docstring

* Call `_validate_lm_em_id` only once, add typing annotations

* Remove embeddings provider for completions

as the team has no plans to support it :(

* Use type alias to reduce changeset/make review easier

Without this change prettier reformats the plugin with an extra
indentation, which leads to bad changeset display on GitHub.

* Rename `_validate_lm_em_id` to `_validate_model_ids`

* Rename `LLMHandlerMixin` to `CompletionsModelMixin`

and rename the file from `llm_mixin` to `model_mixin` fro consistency.
Of note, the file name does not need `completions_` prefix as the file
is in `completions/` subdirectory.

* Rename "Chat LM" to "LM"; add title attribute; note

using the title attribute because getting the icon to show up nicely
(getting they nice grey color and positioning as it gets in buttons,
compared to just plain black) was not trivial; I think the icon might
be the way to go in the future but I would postpone it to another PR.

That said, I still think it should say "Chat LM" because it has no
effect on magics nor completions.

* Rename heading "Completer model" → "Inline completions model"

* Move `UseSignal` down to `CompleterSettingsButton` implementation

* Rename the label in the select to "Inline completion model"

* Disable selection when completer is not enabled

* Remove use of `UseSignal`, tweak naming of `useState`

from `completerIsEnabled` to `isCompleterEnabled`

* Use mui tooltips

* Fix use of `jai_config_manager`

* Fix tests

* Fix `unsupported_slash_commands` default (jupyterlab#768)

* Updates to /export command

(1) Save chat history file to Jupyter root directory, not cwd, in markdown format
(2) Add time stamps to the saved files
(3) Enable the `/export <filename>` option, else use default file name.
(4) Remove code to increment file numbers for multiple chat histories, given that the time stamps are now being used.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update export function

(1) Use argparse to extract filename if the option is used
(2) Remove the old function to get the chat file name and streamline the code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Titan embedding model v2 (jupyterlab#778)

* Add Titan embeddning model v2

Included the new embedding model, released recently on Amazon Bedrock.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* refactor /export code

Remove if then and replace with a single inline expression

* Refactor export code reprise

Updated some variable names

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: michaelchia <[email protected]>
Co-authored-by: Piyush Jain <[email protected]>
Co-authored-by: Michał Krassowski <[email protected]>
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.

Separate providers for inline completion
3 participants