AI Chat: Conversation API implementation #23067

petemill · 2024-04-14T23:36:33Z

This requires the folllowing AI Chat feature parameter to be enabled (disabled by default): conversation_api. It can be enabled by CLI via --enable-features=AIChat:conversation_api\/true

It is default disabled until the server side is ready and this implementation is verified with it.

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:

petemill · 2024-04-30T18:55:53Z

components/ai_chat/core/browser/engine/engine_consumer_conversation_api_unittest.cc

+    {"role": "user", "type": "userText", "content": "Hello World"},
+    {"role": "user", "type": "requestRewrite", "content": "Use a funny tone"}


@evq this is one area that I don't think is covered by the existing events - rewriting text in place. This is what I came up with, let me know if you want to change the event format

bcaller · 2024-05-01T22:06:49Z

components/ai_chat/core/browser/engine/conversation_api_client.cc

+        trigger:
+          "Triggered by user sending a prompt."
+        data:
+          "Will generate a text that attempts to match the user gave it"


Not entirely clear.

I've pushed an improvement:

semantics { sender: "AI Chat" description: "This is used to communicate with Brave's AI Conversation API" "on behalf of the user interacting with different browser AI" "features." trigger: "Triggered by user interactions such as submitting an AI Chat" "conversation message, or requesting a text rewrite." data: "Conversational messages input by the user as well as associated" "content or user text to be rewritten. Can contain PII." destination: WEBSITE }

thypon · 2024-05-01T22:46:20Z

components/api_request_helper/api_request_helper.cc

      request->headers.SetHeader(entry.first, entry.second);
    }
  }

+  if (!payload.empty()) {
+    DVLOG(4) << "Payload type " << payload_content_type << ":";
+    DVLOG(4) << payload;


are we logging the page content on the command line here? Mightn't these contain PII?

Perhaps but this is debug builds only and verbose flag level 4. Is that acceptable?

components/ai_chat/core/browser/engine/engine_consumer_llama.h

petemill · 2024-05-02T05:27:41Z

I've pushed a commit that enables delta updates for in-progress chat messages as per server counterpart at https://github.com/brave/aichat/pull/182/commits/3919d31036c1058bebd0514cfdbaf9c8bca51269

yrliou

I'm still looking at engine_consumer_conversation_api.cc and its unittest, but figure I should post comments for others I have locally first.

components/ai_chat/core/browser/engine/engine_consumer_llama.h

components/ai_chat/core/browser/engine/engine_consumer_llama.cc

components/ai_chat/core/browser/engine/engine_consumer.h

components/ai_chat/core/browser/conversation_driver.cc

components/ai_chat/core/browser/engine/engine_consumer_conversation_api.h

components/ai_chat/core/browser/engine/conversation_api_client.cc

components/ai_chat/core/browser/engine/engine_consumer_conversation_api.cc

github-actions · 2024-05-07T16:29:48Z

[puLL-Merge] - brave/brave-core@23067

Description

This PR makes changes to support the new Brave Conversation API for AI Chat features. It adds a new EngineConsumerConversationAPI class that converts between AI Chat's conversation actions and data model to the Conversation API's request/response format. A new ConversationAPIClient class is added to perform the actual HTTP requests to the Conversation API endpoint.

Changes

Changes

ai_chat_render_view_context_menu_browsertest.cc:

Adds a new MockEngineConsumer for testing the rewrite functionality
Updates tests to use the new mock instead of the previous MockRemoteCompletionClient

conversation_api_client.cc, conversation_api_client.h:

Adds a new ConversationAPIClient class to perform HTTP requests to the Conversation API
Handles converting ConversationEvent structs to the expected JSON request format
Parses streaming and non-streaming API responses and invokes callbacks

conversation_api_client_unittest.cc:

Adds unit tests for the ConversationAPIClient request building and header handling

conversation_driver.cc:

Conditionally uses the new EngineConsumerConversationAPI if the feature flag is enabled
Handles streaming responses that provide delta text by appending to the existing assistant response

engine_consumer.h:

Adds a new virtual SupportsDeltaTextResponses() method to indicate streaming response format

engine_consumer_conversation_api.cc, engine_consumer_conversation_api.h:

Implements the EngineConsumer interface by converting to/from ConversationEvent
Uses the ConversationAPIClient to perform the actual HTTP requests

engine_consumer_conversation_api_unittest.cc:

Adds unit tests for the event building logic in EngineConsumerConversationAPI

features.cc, features.h:

Adds a new feature flag kConversationAPIEnabled to control usage of the new API

api_request_helper.cc:

Adds more debug logging when performing requests

Security Hotspots

High - conversation_api_client.cc:
- User provided prompts and conversation history are sent to a remote API. Need to ensure this data is properly sanitized and handled securely.
- Secrets like premium user credentials are attached to requests. Ensure these are not logged or leaked.
Medium - engine_consumer_conversation_api.cc:
- Parses responses from the remote API which could potentially contain malicious content. Response handling should be robust.
Low - tests:
- Some tests compare JSON strings, which could lead to flaky tests if formatting changes. Consider comparing parsed JSON instead.

Let me know if you have any other questions!

yrliou · 2024-05-07T20:50:38Z

components/ai_chat/core/browser/engine/conversation_api_client_unittest.cc

+  std::optional<CredentialCacheEntry> credential_ = std::nullopt;
+};
+
+TEST_F(ConversationAPIUnitTest, PremiumHeaders) {


PerformRequest_PremiumHeaders and add the same prefix for other test cases.

Modified the test names (and now squashed all follow-ups now). Thanks for the review!

Also support delta updates

yrliou · 2024-05-07T22:43:46Z

components/ai_chat/core/browser/engine/conversation_api_client_unittest.cc

+  testing::Mock::VerifyAndClearExpectations(credential_manager_.get());
+}
+
+TEST_F(ConversationAPIUnitTest, FailNoConversationEvents) {


Please add PerformRequest preix to this one too.

github-actions bot assigned petemill Apr 14, 2024

petemill mentioned this pull request Apr 15, 2024

AI Chat: Use conversation API brave/brave-browser#37575

Closed

petemill force-pushed the ai-chat-conversation-api branch 2 times, most recently from d62a722 to 76116fa Compare April 15, 2024 02:29

petemill force-pushed the ai-chat-conversation-api branch from 76116fa to df35431 Compare April 30, 2024 07:10

petemill marked this pull request as ready for review April 30, 2024 07:10

petemill force-pushed the ai-chat-conversation-api branch 2 times, most recently from 3e2a4a4 to 8f2b644 Compare April 30, 2024 18:53

petemill requested review from yrliou, Brandon-T, nullhook and evq April 30, 2024 18:54

github-actions bot added the puLL-Merge label Apr 30, 2024

petemill commented Apr 30, 2024

View reviewed changes

petemill requested a review from LorenzoMinto May 1, 2024 15:56

bcaller reviewed May 1, 2024

View reviewed changes

thypon reviewed May 1, 2024

View reviewed changes

yrliou reviewed May 1, 2024

View reviewed changes

components/ai_chat/core/browser/engine/engine_consumer_llama.h Outdated Show resolved Hide resolved

yrliou reviewed May 3, 2024

View reviewed changes

components/ai_chat/core/browser/engine/engine_consumer_conversation_api.cc Outdated Show resolved Hide resolved

APIRequestHelper: DVLOG for request headers and payload

0a5d83d

petemill force-pushed the ai-chat-conversation-api branch from 65cc514 to d98e697 Compare May 7, 2024 06:49

yrliou approved these changes May 7, 2024

View reviewed changes

AI Chat: Conversation API implementation

d0ab661

Also support delta updates

petemill force-pushed the ai-chat-conversation-api branch from d97b659 to d0ab661 Compare May 7, 2024 21:09

petemill enabled auto-merge May 7, 2024 21:10

thypon approved these changes May 7, 2024

View reviewed changes

yrliou reviewed May 7, 2024

View reviewed changes

petemill merged commit 897a22c into master May 7, 2024
19 checks passed

petemill deleted the ai-chat-conversation-api branch May 7, 2024 22:50

github-actions bot added this to the 1.67.x - Nightly milestone May 7, 2024

petemill mentioned this pull request May 8, 2024

AI Chat search events from Conversation API #23501

Merged

24 tasks

petemill mentioned this pull request May 22, 2024

AI Chat: enable conversation API as engine provider brave/brave-browser#38476

Closed

kjozwiak mentioned this pull request Jun 24, 2024

AI Chat: Use conversation API - follow up to #37575 brave/brave-browser#39331

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AI Chat: Conversation API implementation #23067

AI Chat: Conversation API implementation #23067

petemill commented Apr 14, 2024 •

edited

Loading

petemill Apr 30, 2024

bcaller May 1, 2024

petemill May 2, 2024

thypon May 1, 2024

petemill May 2, 2024

petemill commented May 2, 2024

yrliou left a comment

github-actions bot commented May 7, 2024

Changes

yrliou May 7, 2024 •

edited

Loading

petemill May 7, 2024

yrliou May 7, 2024

		{"role": "user", "type": "userText", "content": "Hello World"},
		{"role": "user", "type": "requestRewrite", "content": "Use a funny tone"}

AI Chat: Conversation API implementation #23067

AI Chat: Conversation API implementation #23067

Conversation

petemill commented Apr 14, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

petemill Apr 30, 2024

Choose a reason for hiding this comment

bcaller May 1, 2024

Choose a reason for hiding this comment

petemill May 2, 2024

Choose a reason for hiding this comment

thypon May 1, 2024

Choose a reason for hiding this comment

petemill May 2, 2024

Choose a reason for hiding this comment

petemill commented May 2, 2024

yrliou left a comment

Choose a reason for hiding this comment

github-actions bot commented May 7, 2024

Description

Changes

Security Hotspots

yrliou May 7, 2024 • edited Loading

Choose a reason for hiding this comment

petemill May 7, 2024

Choose a reason for hiding this comment

yrliou May 7, 2024

Choose a reason for hiding this comment

petemill commented Apr 14, 2024 •

edited

Loading

yrliou May 7, 2024 •

edited

Loading