-
Notifications
You must be signed in to change notification settings - Fork 895
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]: Cleanup SiteInfo - now the presence/absence of the struct indicates whether content association is possible #27243
base: master
Are you sure you want to change the base?
Conversation
metadata_->associated_content->content_type == | ||
mojom::ContentType::VideoTranscript); | ||
} else { | ||
// TODO: Confirm with @petemill if this is the correct behavior |
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.
@petemill should I delete the content here instead?
// Note: We don't create a new AssociatedContent object here unless one | ||
// doesn't exist. If we generate one with a new UUID the deserializer | ||
// breaks. | ||
// TODO: Confirm with @petemill if I should try and fix this. |
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 was an interesting one - presumably its because we were generating the AssociatedContent once and then changing the detail on it before, so the Id would never change.
I think a slightly better approach here would be to update the Id for the ArchiveContent when we do this, but I wasn't really sure of the implications.
A Storybook has been deployed to preview UI for the latest push |
[puLL-Merge] - brave/brave-core@27243 Here's my review of the pull request: DescriptionThis PR refactors the ChangesChanges
sequenceDiagram
participant UI as User Interface
participant CH as ConversationHandler
participant DB as AIChatDatabase
participant AC as AssociatedContent
UI->>CH: Request conversation state
CH->>DB: Fetch conversation data
DB->>CH: Return conversation data
CH->>AC: Create AssociatedContent (if applicable)
CH->>UI: Return state with AssociatedContent
UI->>UI: Update UI based on AssociatedContent
Possible Issues
Security HotspotsNo significant security hotspots were identified in this change. This sequence diagram illustrates the flow of data with the new |
Resolves brave/brave-browser#43293
The changes are mostly to
ai_chat.mojom
- everything else is just adapting to the new shape of the APISubmitter 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 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:
N/A - no user facing changes