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.

Sign up for GitHub

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

Jump to bottom

[AI Rewriter]: Introduce placeholder UI behind a flag #23688

Merged
merged 5 commits into from
May 27, 2024
Merged

[AI Rewriter]: Introduce placeholder UI behind a flag #23688

merged 5 commits into from
May 27, 2024

Conversation

fallaciousreasoning
Copy link
Contributor

Resolves brave/brave-browser#38357

The UI is behind the brave://flags/#brave-ai-rewriter flag

Screencast.from.2024-05-16.14-10-27.webm

Note: The UI is not wired up to Leo at all, and has no functionality beyond showing/hiding the dialog.

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:

Test Plan:

@fallaciousreasoning fallaciousreasoning requested review from a team as code owners May 16, 2024 02:11
@github-actions github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build CI/run-upstream-tests Run upstream unit and browser tests on Linux and Windows (otherwise only on Linux) labels May 16, 2024
sangwoo108
sangwoo108 reviewed May 16, 2024
View reviewed changes
renderer/brave_content_renderer_client.cc Outdated Show resolved Hide resolved
components/ai_rewriter/resources/page/components/NoContent.tsx Show resolved Hide resolved
components/ai_rewriter/renderer/ai_rewriter_agent.cc Show resolved Hide resolved
components/ai_rewriter/renderer/BUILD.gn Outdated Show resolved Hide resolved
chromium_src/chrome/browser/ui/webui/chrome_untrusted_web_ui_configs.cc Outdated Show resolved Hide resolved
browser/ui/ai_rewriter/ai_rewriter_dialog_delegate.cc Show resolved Hide resolved
components/ai_rewriter/common/mojom/ai_rewriter.mojom Outdated Show resolved Hide resolved
browser/ui/ai_rewriter/ai_rewriter_dialog_delegate.cc Show resolved Hide resolved
browser/ui/ai_rewriter/ai_rewriter_dialog_delegate.h Outdated Show resolved Hide resolved
browser/ui/ai_rewriter/ai_rewriter_dialog_delegate.h Outdated Show resolved Hide resolved
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

sangwoo108
sangwoo108 approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@sangwoo108 sangwoo108 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

components/ai_rewriter/common/BUILD.gn Outdated Show resolved Hide resolved
components/ai_rewriter/common/BUILD.gn Outdated Show resolved Hide resolved
components/ai_chat/core/common/mojom/BUILD.gn Show resolved Hide resolved
browser/ui/webui/ai_rewriter/ai_rewriter_ui.cc Outdated Show resolved Hide resolved
browser/ui/ai_rewriter/ai_rewriter_dialog_delegate.cc Show resolved Hide resolved
chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc Show resolved Hide resolved
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

bsclifton
bsclifton approved these changes May 20, 2024
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

string reviewers ++

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

goodov
goodov approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@goodov goodov left a comment

Choose a reason for hiding this comment

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

lgtm with few things to fix.

make sure to add unconditional buildflags dep wherever you have if (enable_ai_rewriter) { ... }.

resources/resource_ids.spec Outdated Show resolved Hide resolved
browser/about_flags.cc Outdated Show resolved Hide resolved
browser/sources.gni Outdated Show resolved Hide resolved
renderer/sources.gni Show resolved Hide resolved
renderer/BUILD.gn Show resolved Hide resolved
@petemill petemill requested a review from yrliou May 22, 2024 16:31
goodov
goodov reviewed May 23, 2024
View reviewed changes
browser/sources.gni Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

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

Here is my review of the pull request:

Description

This PR adds a new AI-powered text rewriting feature called "AI Rewriter". It introduces a new UI accessible at chrome://rewriter that allows rewriting selected text using AI models. The feature is enabled on desktop platforms (Windows, Mac, Linux) if the brave://flags/#brave-ai-chat flag is enabled.

Changes

Changes

  • Adds new AI Rewriter component:
    • Implements common, renderer and resources code for the AI Rewriter feature under brave/components/ai_rewriter
    • Hooks up the feature in various places (about_flags.cc, brave_content_browser_client.cc, brave_web_ui_controller_factory.cc, etc)
    • Exposes AI Rewriter UI at chrome://rewriter URL
  • brave/browser/ui:
    • Implements AIRewriterDialogDelegate to show the rewriter dialog
  • brave/browser/ui/webui:
    • Implements AIRewriterUI web UI controller for chrome://rewriter
  • brave/chromium_src:
    • Hooks up "Rewrite" context menu option in RenderViewContextMenu when text is selected
    • Injects AIRewriterAgent in renderer to communicate bounds of selected text
  • brave/components/constants:
    • Defines chrome://rewriter URL constants
  • brave/renderer:
    • Links against new ai_rewriter/renderer component
  • brave/components/resources:
    • Adds resources for AI Rewriter UI
  • brave/ui/webui/resources:
    • Adds some new icons used by the Rewriter UI

Overall, this is a significant new feature addition that touches browser, renderer, and resources code.

Security Hotspots

None found. The new code does not appear to introduce any major security risks. The main untrusted data appears to be the user-selected text passed from the renderer to the browser for rewriting, but this seems to be handled safely.

Let me know if you have any other questions!

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@fallaciousreasoning fallaciousreasoning merged commit 0bcd5f8 into master May 27, 2024
19 checks passed
@fallaciousreasoning fallaciousreasoning deleted the ai-dialog branch May 27, 2024 22:28
@github-actions github-actions bot added this to the 1.68.x - Nightly milestone May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goodov goodov goodov approved these changes

@github-actions github-actions[bot] github-actions left review comments

@bsclifton bsclifton bsclifton approved these changes

@sangwoo108 sangwoo108 sangwoo108 approved these changes

@petemill petemill Awaiting requested review from petemill

@nullhook nullhook Awaiting requested review from nullhook

@yrliou yrliou Awaiting requested review from yrliou

Assignees

@thypon thypon

@iefremov iefremov

@bcaller bcaller

@goodov goodov

@fallaciousreasoning fallaciousreasoning

Labels
CI/run-upstream-tests Run upstream unit and browser tests on Linux and Windows (otherwise only on Linux) CI/storybook-url Deploy storybook and provide a unique URL for each build needs-security-review puLL-Merge
Projects
None yet
Milestone
1.68.x - Nightly
8 participants
@fallaciousreasoning @brave-builds @bsclifton @sangwoo108 @goodov @thypon @iefremov @bcaller