[AI Chat]: Android history setting #26988

fallaciousreasoning · 2024-12-11T21:40:46Z

Resolves brave/brave-browser#42847

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

Go to Leo settings
There should be no History setting
Enable brave://flags/#brave-ai-chat-history
Go to Leo settings
It you should be able to enable/disable the setting
When enabled, conversations should be stored

petemill · 2024-12-11T21:46:15Z

components/ai_chat/core/common/pref_names.h

@@ -15,7 +15,8 @@ namespace ai_chat::prefs {

 inline constexpr char kLastAcceptedDisclaimer[] =
    "brave.ai_chat.last_accepted_disclaimer";
-inline constexpr char kStorageEnabled[] = "brave.ai_chat.storage_enabled";
+inline constexpr char kBraveChatStorageEnabled[] =


we're already in the ai_chat namespace so do we need to add the prefix again? IF we do, perhaps kBraveChat -> kBraveAIChat...

Yeah, I added it because this isn't in the namespace in android so without this its BravePrefs.STORAGE_ENABLED

I went with kBraveChat because that's what everything else is using

That's a shame they were kBraveChat instead of at least kBraveAIChat 🤷 and also it's strange that if anything is used on android then we need to make a globally-unique name, but that's something to sort out another time.

Actually I think we should at least keep this as kBraveAIChatStorageEnabled and then we'll get to renaming the rest. I don't want to introduce more inconsistent Chat references as it sounds like a different feature.

petemill

As discussed, it needs to prompt the user before disabling since disabling deletes all conversations

android/java/res/layout/brave_leo_clear_history_dialog.xml

android/java/org/chromium/chrome/browser/settings/BraveLeoPreferences.java

SergeyZhukovsky

++

fallaciousreasoning · 2024-12-12T23:18:33Z

android/brave_java_resources.gni

@@ -794,6 +794,7 @@ brave_java_resources = [
  "java/res/layout/brave_custom_tabs_toolbar.xml",
  "java/res/layout/brave_dialog_preference.xml",
  "java/res/layout/brave_exit_confirmation.xml",
+  "java/res/layout/brave_leo_clear_history_dialog.xml",


@bridiver would it be okay if we landed this brave_java_resources addition? I'm not an Android expert, so it'll take me a while to work out the proper way of doing this and I'm going on PTO on Monday

this is fine for now

petemill · 2024-12-15T22:09:53Z

components/ai_chat/core/common/pref_names.h

@@ -15,7 +15,8 @@ namespace ai_chat::prefs {

 inline constexpr char kLastAcceptedDisclaimer[] =
    "brave.ai_chat.last_accepted_disclaimer";
-inline constexpr char kStorageEnabled[] = "brave.ai_chat.storage_enabled";
+inline constexpr char kBraveChatStorageEnabled[] =


That's a shame they were kBraveChat instead of at least kBraveAIChat 🤷 and also it's strange that if anything is used on android then we need to make a globally-unique name, but that's something to sort out another time.

Actually I think we should at least keep this as kBraveAIChatStorageEnabled and then we'll get to renaming the rest. I don't want to introduce more inconsistent Chat references as it sounds like a different feature.

deeppandya

LGTM

brave-builds · 2024-12-18T19:33:03Z

Released in v1.75.110

fallaciousreasoning requested review from deeppandya and a team as code owners December 11, 2024 21:40

github-actions bot assigned fallaciousreasoning Dec 11, 2024

fallaciousreasoning requested review from darkdh, SergeyZhukovsky and petemill and removed request for darkdh December 11, 2024 21:41

petemill reviewed Dec 11, 2024

View reviewed changes

petemill requested changes Dec 11, 2024

View reviewed changes

fallaciousreasoning force-pushed the aichat-android-history-toggle branch from 15a58e4 to 8485bfa Compare December 11, 2024 22:49

fallaciousreasoning requested a review from petemill December 12, 2024 00:23

fallaciousreasoning force-pushed the aichat-android-history-toggle branch from 8485bfa to ebdc11a Compare December 12, 2024 00:50

deeppandya reviewed Dec 12, 2024

View reviewed changes

android/java/res/layout/brave_leo_clear_history_dialog.xml Outdated Show resolved Hide resolved

SergeyZhukovsky reviewed Dec 12, 2024

View reviewed changes

android/java/org/chromium/chrome/browser/settings/BraveLeoPreferences.java Outdated Show resolved Hide resolved

SergeyZhukovsky approved these changes Dec 12, 2024

View reviewed changes

fallaciousreasoning added 2 commits December 13, 2024 10:48

[AI Chat]: Android history setting

1059f2c

[AI Chat]: Add confirmation dialog for clearing history

3995017

fallaciousreasoning force-pushed the aichat-android-history-toggle branch from ebdc11a to 00244e8 Compare December 12, 2024 22:27

fallaciousreasoning commented Dec 12, 2024

View reviewed changes

goodov approved these changes Dec 13, 2024

View reviewed changes

[AI Chat]: Changes for PR

a29fae9

fallaciousreasoning force-pushed the aichat-android-history-toggle branch from 00244e8 to a29fae9 Compare December 15, 2024 21:45

petemill approved these changes Dec 15, 2024

View reviewed changes

fallaciousreasoning enabled auto-merge (squash) December 15, 2024 23:07

deeppandya approved these changes Dec 18, 2024

View reviewed changes

fallaciousreasoning merged commit e1a3daa into master Dec 18, 2024
18 checks passed

fallaciousreasoning deleted the aichat-android-history-toggle branch December 18, 2024 18:41

github-actions bot added this to the 1.75.x - Nightly milestone Dec 18, 2024

petemill mentioned this pull request Dec 19, 2024

AI Chat history feature should be disabled by default on Android brave/brave-browser#42928

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AI Chat]: Android history setting #26988

[AI Chat]: Android history setting #26988

fallaciousreasoning commented Dec 11, 2024 •

edited

Loading

petemill Dec 11, 2024

fallaciousreasoning Dec 11, 2024 •

edited

Loading

petemill Dec 15, 2024

petemill left a comment

SergeyZhukovsky left a comment

fallaciousreasoning Dec 12, 2024

bridiver Dec 16, 2024

petemill Dec 15, 2024

deeppandya left a comment

brave-builds commented Dec 18, 2024

[AI Chat]: Android history setting #26988

[AI Chat]: Android history setting #26988

Conversation

fallaciousreasoning commented Dec 11, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

petemill Dec 11, 2024

Choose a reason for hiding this comment

fallaciousreasoning Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

petemill Dec 15, 2024

Choose a reason for hiding this comment

petemill left a comment

Choose a reason for hiding this comment

SergeyZhukovsky left a comment

Choose a reason for hiding this comment

fallaciousreasoning Dec 12, 2024

Choose a reason for hiding this comment

bridiver Dec 16, 2024

Choose a reason for hiding this comment

petemill Dec 15, 2024

Choose a reason for hiding this comment

deeppandya left a comment

Choose a reason for hiding this comment

brave-builds commented Dec 18, 2024

fallaciousreasoning commented Dec 11, 2024 •

edited

Loading

fallaciousreasoning Dec 11, 2024 •

edited

Loading