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

Model allowlist and blocklists #446

Merged
merged 4 commits into from
Nov 8, 2023
Merged

Conversation

dlqqq
Copy link
Collaborator

@dlqqq dlqqq commented Nov 8, 2023

Implements model-level allowlist and blocklist traits on the root AiExtension class. These mostly behave as expected:

  • The UI adjusts accordingly; i.e. blocked models are filtered out of the model selection UI.
  • The ConfigManager will automatically set your LM & EM to None if they are forbidden by the provider/model allow/blocklists.

The only unexpected outcome of this PR I'm aware of is that allowed_models is ignored when allowed_providers is set. This is because of the implementation of the provider allowlist established by #415, where it filters out blocked LM & EM providers before passing them to the ConfigManager. This means that if I have allowed_providers=["openai"] and allowed_models=["cohere:medium"], then "cohere:medium" will not be shown in the UI, nor will the ConfigManager allow setting the language model to "cohere:medium".

While this isn't too hard to handle, I lack the time to fully test the interaction between these traits. I've added a note in the helper text to indicate that 1) this is currently undefined behavior and not intentional, and that 2) this is subject to change in a future non-major release.

Next steps

  • Handle edge case of no models being allowed; should stop the server from starting if possible, halting the extension otherwise.
  • We need a better OOP interface for dealing with lists of models.
  • Add more testing and re-enable the extension tests added in Allow to define block and allow lists for providers #415. I had to disable these because they were flaky and dependent on existing config under a user's $JUPYTER_DATA_DIR.

@dlqqq dlqqq added the enhancement New feature or request label Nov 8, 2023
Copy link
Collaborator

@3coins 3coins left a comment

Choose a reason for hiding this comment

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

@dlqqq
Thanks for making these updates. Verified, the allow/blocklist works as expected. Great job on adding this. 🚀

@3coins 3coins merged commit 9f69469 into jupyterlab:main Nov 8, 2023
6 checks passed
@JasonWeill
Copy link
Collaborator

@meeseeksdev please backport to 1.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyter-ai that referenced this pull request Nov 8, 2023
JasonWeill added a commit that referenced this pull request Nov 8, 2023
Co-authored-by: david qiu <david@qiu.dev>
Co-authored-by: Jason Weill <93281816+JasonWeill@users.noreply.github.com>
dbelgrod pushed a commit to dbelgrod/jupyter-ai that referenced this pull request Jun 10, 2024
* implement model allow/blocklist in UI

* skip extension tests due to local flakiness

* implement model allow/blocklists in config manager

* [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>
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.

None yet

4 participants