-
Notifications
You must be signed in to change notification settings - Fork 885
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
AI Chat: introduce freemium model concept #21398
Conversation
f565bc7
to
28d69f8
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
28d69f8
to
eaf4886
Compare
A Storybook has been deployed to preview UI for the latest push |
eaf4886
to
7838ad5
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
…ich prompts to use a basic (free) model as secondary action
If true, certain freemium models are available to non-premium users. If false, those models are premium-only.
f4fa11c
to
b7be646
Compare
IDS_SETTINGS_APPEARANCE_SETTINGS_GET_MORE_THEMES}, | ||
{"appearanceBraveDefaultImagesOptionLabel", | ||
IDS_SETTINGS_APPEARANCE_SETTINGS_BRAVE_DEFAULT_IMAGES_OPTION_LABEL}, | ||
{"importExtensions", IDS_SETTINGS_IMPORT_EXTENSIONS_CHECKBOX}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This formatting must have been not done for cr121 @mkarolin? npm run format
is producing this - happy to keep in this PR as I hope to merge today
{"braveLeoAssistantModelSelectionLabel", | ||
IDS_SETTINGS_LEO_ASSISTANT_MODEL_SELECTION_LABEL}, | ||
{"braveLeoModelCategory-chat", IDS_CHAT_UI_MODEL_CATEGORY_CHAT}, | ||
{"braveLeoModelSubtitle-chat-basic", IDS_CHAT_UI_CHAT_BASIC_SUBTITLE}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only line I actually changed in this file for this PR (aside from the formatting) - key changed from "-default" to "-basic" suffix.
403ca6f
to
60f62d1
Compare
… maintains iOS compat
A Storybook has been deployed to preview UI for the latest push |
@@ -132,28 +135,29 @@ ConversationDriver::~ConversationDriver() = default; | |||
void ConversationDriver::ChangeModel(const std::string& model_key) { | |||
DCHECK(!model_key.empty()); | |||
// Check that the key exists | |||
if (kAllModels.find(model_key) == kAllModels.end()) { | |||
auto* new_model = GetModel(model_key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: i know you like writing auto
but i can't infer the type by reading the code here.
there's a general guidance on this somewhere but briefly:
- write auto if type is relatively obvious
- write auto if it ocassionally needs to be inferred
InitEngine(); | ||
} | ||
|
||
const mojom::Model& ConversationDriver::GetCurrentModel() { | ||
return kAllModels.find(model_key_)->second; | ||
auto* model = GetModel(model_key_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an example of it being relatively obvious from the above line, so this works
[puLL-Merge] - brave/brave-core@21398 DescriptionThis PR contains multiple changes across different files related to ai_chat and browser settings. The motivation seems to be to enhance and refactor the AI chat models, including UI adjustments and introducing freemium models for non-premium users. There's also an emphasis on code robustness and a slight shift in how model names are handled. ChangesChanges
Security HotspotsNo specific security hotspots have been detected in these changes. The PR mainly touches UI components, localized string definitions, and internal model management for the AI chat feature. The refactoring of model access does not seem to present direct vulnerabilities, but it is worth noting that access control changes should always be double-checked to ensure they do not introduce unintended permissions or access. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ string reviewers
A Storybook has been deployed to preview UI for the latest push |
* AI Chat: introduce freemium model concept * AI Chat: Add mixtral model * AI Chat: premium's default also becomes leo-expanded * AI Chat: non-premium users on freemium models get rate limit error which prompts to use a basic (free) model as secondary action * fix format from chromium upgrade * AI Chat: introduce "is_freemium_available" boolean feature param If true, certain freemium models are available to non-premium users. If false, those models are premium-only. * AI Chat: migrate default model key pref from "chat-default" to "chat-basic" * AI Chat: reduce claude page content character length to avoid token length mismatch more often * AI Chat: model menu uses model display name instead of API name
Verification
|
example | example | example |
---|---|---|
Claude Instant
- PASSED
Steps:
- installed
1.63.124
- launched Brave using
--env-leo=staging --env-ai-chat.bsg=dev --env-ai-chat-premium.bsg=dev --enable-logging=stderr
- clicked on
Leo
in the sidebar - clicked on
Accept and begin
- loaded
vox.com
- clicked on
...
- chose
Claude Instant
- loaded an article
- clicked on
Summarize this page
- clicked on
Suggest questions...
example | example | example |
---|---|---|
llama2-13b
- PASSED
example | example | example |
---|---|---|
Add Griffin feature flag for setting Llama-13b
(actually Mixtral
) as the default: brave/brave-browser#34721 - PASSED
Steps:
- installed
1.63.124
- launched Brave using
--env-leo=staging --env-ai-chat.bsg=dev --env-ai-chat-premium.bsg=dev --enable-logging=stderr
- clicked on
Leo
in the sidebar - clicked on
Accept and begin
- loaded
wired.com
- loaded an article
- clicked on
Summarize this page
- clicked on
Suggest questions...
- opened
brave://settings/leo-assistant
Confirmed Mixtral
as the non-Premium default
example | example | example | example |
---|---|---|---|
1st launch | 2nd launch | default |
---|---|---|
New Premium-enabled indicator: brave/brave-browser#34718 - PASSED
) * AI Chat: introduce freemium model concept (#21398) * AI Chat: introduce freemium model concept * AI Chat: Add mixtral model * AI Chat: premium's default also becomes leo-expanded * AI Chat: non-premium users on freemium models get rate limit error which prompts to use a basic (free) model as secondary action * fix format from chromium upgrade * AI Chat: introduce "is_freemium_available" boolean feature param If true, certain freemium models are available to non-premium users. If false, those models are premium-only. * AI Chat: migrate default model key pref from "chat-default" to "chat-basic" * AI Chat: reduce claude page content character length to avoid token length mismatch more often * AI Chat: model menu uses model display name instead of API name * fix GetModels * format
Related to brave/brave-core#21398 Blocked on deploying brave/aichat-ops#359 to prod This sets the default model for 25% of free users to `chat-basic`, which corresponds to [Llama 3 8b](https://github.com/brave/brave-core/blob/c839e1676031a3d155b04b9dbfe49e11b3b8601b/components/ai_chat/core/browser/model_service.cc#L154). The intention is to start this to progressively roll out this change (25%, 50%, 75%, 100%), so we can ensure our single instance of Llama 3 can handle the increase in traffic. Based on these [estimations](https://artificialanalysis.ai/?models_selected=llama-3-1-instruct-8b%2Cmixtral-8x7b-instruct), one instance of llama 3 (one gpu) should have higher token throughput than our single instance of mixtral (four gpus), so we expect this to work with two llama instances which will be added in brave/aichat-ops#359. cc @petemill @LorenzoMinto Note: * Chromium version 122.0.6261.57 was selected because that was the first chromium version when 1.63.x was released (see https://bravesoftware.slack.com/archives/C04PX1BUN/p1708629893634639), which is when brave/brave-core#21398 went in. * I have included all platforms because we want to make this change across all platforms, however this does differ from the BraveAIChatEnabledStudy which applies only to desktop --------- Co-authored-by: Nick von Pentz <nvonpentz@users.noreply.github.com> Co-authored-by: Pete Miller <miller.pete@gmail.com> Co-authored-by: Aleksey Khoroshilov <akhoroshilov@brave.com>
Resolves:
Don't allow freemium models to be selected when griffin flag isn't pointing at a freemium model (or other flag)
Testable via
--enable-features=AIChat:is_freemium_available\/false\/default_model\/chat-basic
Modify rate limiting message text for freemium models with non-premium user: "Switch to free model"
Remove concept of separate premium user default model or make it the same model (made it the same model)
Migrate pref values:
chat-default
tochat-basic
for llama2-13bSubmitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: