-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: enable grounding features for Vertex AI #6777
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||||
| import { useCallback } from "react" | ||||||||
| import { Checkbox } from "vscrui" | ||||||||
| import { VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" | ||||||||
|
|
||||||||
| import { type ProviderSettings, VERTEX_REGIONS } from "@roo-code/types" | ||||||||
|
|
@@ -91,6 +92,28 @@ export const Vertex = ({ apiConfiguration, setApiConfigurationField }: VertexPro | |||||||
| </SelectContent> | ||||||||
| </Select> | ||||||||
| </div> | ||||||||
|
|
||||||||
| <div className="mt-6"> | ||||||||
|
||||||||
| <div className="mt-6"> | |
| {/* Note: Grounding features only work with Gemini models on Vertex AI, not Claude models */} | |
| <div className="mt-6"> |
This could help future developers understand when these options are actually functional.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,61 +1,229 @@ | ||
| // Tests for VERTEX_REGIONS "global" region handling | ||
| import { render, screen } from "@testing-library/react" | ||
| import userEvent from "@testing-library/user-event" | ||
| import { Vertex } from "../Vertex" | ||
| import type { ProviderSettings } from "@roo-code/types" | ||
| import { VERTEX_REGIONS } from "@roo-code/types" | ||
|
|
||
| import { describe, it, expect } from "vitest" | ||
| import { VERTEX_REGIONS } from "../../../../../../packages/types/src/providers/vertex" | ||
| vi.mock("@vscode/webview-ui-toolkit/react", () => ({ | ||
| VSCodeTextField: ({ children, value, onInput, type }: any) => ( | ||
| <div> | ||
| {children} | ||
| <input type={type} value={value} onChange={(e) => onInput(e)} /> | ||
| </div> | ||
| ), | ||
| VSCodeLink: ({ children, href }: any) => <a href={href}>{children}</a>, | ||
| })) | ||
|
|
||
| describe("VERTEX_REGIONS", () => { | ||
| it('should include the "global" region as the first entry', () => { | ||
| expect(VERTEX_REGIONS[0]).toEqual({ value: "global", label: "global" }) | ||
| vi.mock("vscrui", () => ({ | ||
| Checkbox: ({ children, checked, onChange, "data-testid": testId }: any) => ( | ||
| <label data-testid={testId}> | ||
| <input type="checkbox" checked={checked} onChange={(e) => onChange(e.target.checked)} /> | ||
| {children} | ||
| </label> | ||
| ), | ||
| })) | ||
|
|
||
| vi.mock("@src/i18n/TranslationContext", () => ({ | ||
| useAppTranslation: () => ({ t: (key: string) => key }), | ||
| })) | ||
|
|
||
| vi.mock("@src/components/ui", () => ({ | ||
| Select: ({ children, value, onValueChange }: any) => ( | ||
| <div data-value={value} data-onvaluechange={onValueChange}> | ||
| {children} | ||
| </div> | ||
| ), | ||
| SelectContent: ({ children }: any) => <div>{children}</div>, | ||
| SelectItem: ({ children, value }: any) => <div data-value={value}>{children}</div>, | ||
| SelectTrigger: ({ children }: any) => <div>{children}</div>, | ||
| SelectValue: ({ placeholder }: any) => <div>{placeholder}</div>, | ||
| })) | ||
|
|
||
| describe("Vertex", () => { | ||
| const defaultApiConfiguration: ProviderSettings = { | ||
| vertexKeyFile: "", | ||
| vertexJsonCredentials: "", | ||
| vertexProjectId: "", | ||
| vertexRegion: "", | ||
| enableUrlContext: false, | ||
| enableGrounding: false, | ||
| } | ||
|
|
||
| const mockSetApiConfigurationField = vi.fn() | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| it('should contain "global" region exactly once', () => { | ||
| const globalRegions = VERTEX_REGIONS.filter((r: { value: string; label: string }) => r.value === "global") | ||
| expect(globalRegions).toHaveLength(1) | ||
| describe("VERTEX_REGIONS", () => { | ||
| it('should include the "global" region as the first entry', () => { | ||
| expect(VERTEX_REGIONS[0]).toEqual({ value: "global", label: "global" }) | ||
| }) | ||
|
|
||
| it('should contain "global" region exactly once', () => { | ||
| const globalRegions = VERTEX_REGIONS.filter((r: { value: string; label: string }) => r.value === "global") | ||
| expect(globalRegions).toHaveLength(1) | ||
| }) | ||
|
|
||
| it('should contain all expected regions including "global"', () => { | ||
| // The expected list is the imported VERTEX_REGIONS itself | ||
| expect(VERTEX_REGIONS).toEqual([ | ||
| { value: "global", label: "global" }, | ||
| { value: "us-central1", label: "us-central1" }, | ||
| { value: "us-east1", label: "us-east1" }, | ||
| { value: "us-east4", label: "us-east4" }, | ||
| { value: "us-east5", label: "us-east5" }, | ||
| { value: "us-west1", label: "us-west1" }, | ||
| { value: "us-west2", label: "us-west2" }, | ||
| { value: "us-west3", label: "us-west3" }, | ||
| { value: "us-west4", label: "us-west4" }, | ||
| { value: "northamerica-northeast1", label: "northamerica-northeast1" }, | ||
| { value: "northamerica-northeast2", label: "northamerica-northeast2" }, | ||
| { value: "southamerica-east1", label: "southamerica-east1" }, | ||
| { value: "europe-west1", label: "europe-west1" }, | ||
| { value: "europe-west2", label: "europe-west2" }, | ||
| { value: "europe-west3", label: "europe-west3" }, | ||
| { value: "europe-west4", label: "europe-west4" }, | ||
| { value: "europe-west6", label: "europe-west6" }, | ||
| { value: "europe-central2", label: "europe-central2" }, | ||
| { value: "asia-east1", label: "asia-east1" }, | ||
| { value: "asia-east2", label: "asia-east2" }, | ||
| { value: "asia-northeast1", label: "asia-northeast1" }, | ||
| { value: "asia-northeast2", label: "asia-northeast2" }, | ||
| { value: "asia-northeast3", label: "asia-northeast3" }, | ||
| { value: "asia-south1", label: "asia-south1" }, | ||
| { value: "asia-south2", label: "asia-south2" }, | ||
| { value: "asia-southeast1", label: "asia-southeast1" }, | ||
| { value: "asia-southeast2", label: "asia-southeast2" }, | ||
| { value: "australia-southeast1", label: "australia-southeast1" }, | ||
| { value: "australia-southeast2", label: "australia-southeast2" }, | ||
| { value: "me-west1", label: "me-west1" }, | ||
| { value: "me-central1", label: "me-central1" }, | ||
| { value: "africa-south1", label: "africa-south1" }, | ||
| ]) | ||
| }) | ||
|
|
||
| it('should contain "asia-east1" region exactly once', () => { | ||
| const asiaEast1Regions = VERTEX_REGIONS.filter( | ||
| (r: { value: string; label: string }) => r.value === "asia-east1" && r.label === "asia-east1", | ||
| ) | ||
| expect(asiaEast1Regions).toHaveLength(1) | ||
| expect(asiaEast1Regions[0]).toEqual({ value: "asia-east1", label: "asia-east1" }) | ||
| }) | ||
| }) | ||
|
|
||
| it('should contain all expected regions including "global"', () => { | ||
| // The expected list is the imported VERTEX_REGIONS itself | ||
| expect(VERTEX_REGIONS).toEqual([ | ||
| { value: "global", label: "global" }, | ||
| { value: "us-central1", label: "us-central1" }, | ||
| { value: "us-east1", label: "us-east1" }, | ||
| { value: "us-east4", label: "us-east4" }, | ||
| { value: "us-east5", label: "us-east5" }, | ||
| { value: "us-west1", label: "us-west1" }, | ||
| { value: "us-west2", label: "us-west2" }, | ||
| { value: "us-west3", label: "us-west3" }, | ||
| { value: "us-west4", label: "us-west4" }, | ||
| { value: "northamerica-northeast1", label: "northamerica-northeast1" }, | ||
| { value: "northamerica-northeast2", label: "northamerica-northeast2" }, | ||
| { value: "southamerica-east1", label: "southamerica-east1" }, | ||
| { value: "europe-west1", label: "europe-west1" }, | ||
| { value: "europe-west2", label: "europe-west2" }, | ||
| { value: "europe-west3", label: "europe-west3" }, | ||
| { value: "europe-west4", label: "europe-west4" }, | ||
| { value: "europe-west6", label: "europe-west6" }, | ||
| { value: "europe-central2", label: "europe-central2" }, | ||
| { value: "asia-east1", label: "asia-east1" }, | ||
| { value: "asia-east2", label: "asia-east2" }, | ||
| { value: "asia-northeast1", label: "asia-northeast1" }, | ||
| { value: "asia-northeast2", label: "asia-northeast2" }, | ||
| { value: "asia-northeast3", label: "asia-northeast3" }, | ||
| { value: "asia-south1", label: "asia-south1" }, | ||
| { value: "asia-south2", label: "asia-south2" }, | ||
| { value: "asia-southeast1", label: "asia-southeast1" }, | ||
| { value: "asia-southeast2", label: "asia-southeast2" }, | ||
| { value: "australia-southeast1", label: "australia-southeast1" }, | ||
| { value: "australia-southeast2", label: "australia-southeast2" }, | ||
| { value: "me-west1", label: "me-west1" }, | ||
| { value: "me-central1", label: "me-central1" }, | ||
| { value: "africa-south1", label: "africa-south1" }, | ||
| ]) | ||
| describe("URL Context Checkbox", () => { | ||
| it("should render URL context checkbox unchecked by default", () => { | ||
| render( | ||
| <Vertex | ||
| apiConfiguration={defaultApiConfiguration} | ||
| setApiConfigurationField={mockSetApiConfigurationField} | ||
| />, | ||
| ) | ||
|
|
||
| const urlContextCheckbox = screen.getByTestId("checkbox-url-context") | ||
| const checkbox = urlContextCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
| expect(checkbox.checked).toBe(false) | ||
| }) | ||
|
|
||
| it("should render URL context checkbox checked when enableUrlContext is true", () => { | ||
| const apiConfiguration = { ...defaultApiConfiguration, enableUrlContext: true } | ||
| render( | ||
| <Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={mockSetApiConfigurationField} />, | ||
| ) | ||
|
|
||
| const urlContextCheckbox = screen.getByTestId("checkbox-url-context") | ||
| const checkbox = urlContextCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
| expect(checkbox.checked).toBe(true) | ||
| }) | ||
|
|
||
| it("should call setApiConfigurationField with correct parameters when URL context checkbox is toggled", async () => { | ||
| const user = userEvent.setup() | ||
| render( | ||
| <Vertex | ||
| apiConfiguration={defaultApiConfiguration} | ||
| setApiConfigurationField={mockSetApiConfigurationField} | ||
| />, | ||
| ) | ||
|
|
||
| const urlContextCheckbox = screen.getByTestId("checkbox-url-context") | ||
| const checkbox = urlContextCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
|
|
||
| await user.click(checkbox) | ||
|
|
||
| expect(mockSetApiConfigurationField).toHaveBeenCalledWith("enableUrlContext", true) | ||
| }) | ||
| }) | ||
|
|
||
| it('should contain "asia-east1" region exactly once', () => { | ||
| const asiaEast1Regions = VERTEX_REGIONS.filter( | ||
| (r: { value: string; label: string }) => r.value === "asia-east1" && r.label === "asia-east1", | ||
| ) | ||
| expect(asiaEast1Regions).toHaveLength(1) | ||
| expect(asiaEast1Regions[0]).toEqual({ value: "asia-east1", label: "asia-east1" }) | ||
| describe("Grounding with Google Search Checkbox", () => { | ||
| it("should render grounding search checkbox unchecked by default", () => { | ||
| render( | ||
| <Vertex | ||
| apiConfiguration={defaultApiConfiguration} | ||
| setApiConfigurationField={mockSetApiConfigurationField} | ||
| />, | ||
| ) | ||
|
|
||
| const groundingCheckbox = screen.getByTestId("checkbox-grounding-search") | ||
| const checkbox = groundingCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
| expect(checkbox.checked).toBe(false) | ||
| }) | ||
|
|
||
| it("should render grounding search checkbox checked when enableGrounding is true", () => { | ||
| const apiConfiguration = { ...defaultApiConfiguration, enableGrounding: true } | ||
| render( | ||
| <Vertex apiConfiguration={apiConfiguration} setApiConfigurationField={mockSetApiConfigurationField} />, | ||
| ) | ||
|
|
||
| const groundingCheckbox = screen.getByTestId("checkbox-grounding-search") | ||
| const checkbox = groundingCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
| expect(checkbox.checked).toBe(true) | ||
| }) | ||
|
|
||
| it("should call setApiConfigurationField with correct parameters when grounding search checkbox is toggled", async () => { | ||
| const user = userEvent.setup() | ||
| render( | ||
| <Vertex | ||
| apiConfiguration={defaultApiConfiguration} | ||
| setApiConfigurationField={mockSetApiConfigurationField} | ||
| />, | ||
| ) | ||
|
|
||
| const groundingCheckbox = screen.getByTestId("checkbox-grounding-search") | ||
| const checkbox = groundingCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
|
|
||
| await user.click(checkbox) | ||
|
|
||
| expect(mockSetApiConfigurationField).toHaveBeenCalledWith("enableGrounding", true) | ||
| }) | ||
| }) | ||
|
|
||
| describe("Both checkboxes interaction", () => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great test coverage for the UI! Would it be valuable to add a test that verifies these options work correctly when using Gemini models (like "gemini-2.5-flash") on Vertex? Since grounding is Gemini-specific, it might be good to have a test that validates the integration with actual Gemini model IDs. |
||
| it("should be able to toggle both checkboxes independently", async () => { | ||
| const user = userEvent.setup() | ||
| render( | ||
| <Vertex | ||
| apiConfiguration={defaultApiConfiguration} | ||
| setApiConfigurationField={mockSetApiConfigurationField} | ||
| />, | ||
| ) | ||
|
|
||
| const urlContextCheckbox = screen.getByTestId("checkbox-url-context") | ||
| const urlCheckbox = urlContextCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
|
|
||
| const groundingCheckbox = screen.getByTestId("checkbox-grounding-search") | ||
| const groundCheckbox = groundingCheckbox.querySelector("input[type='checkbox']") as HTMLInputElement | ||
|
|
||
| // Toggle URL context | ||
| await user.click(urlCheckbox) | ||
| expect(mockSetApiConfigurationField).toHaveBeenCalledWith("enableUrlContext", true) | ||
|
|
||
| // Toggle grounding | ||
| await user.click(groundCheckbox) | ||
| expect(mockSetApiConfigurationField).toHaveBeenCalledWith("enableGrounding", true) | ||
|
|
||
| // Both should have been called | ||
| expect(mockSetApiConfigurationField).toHaveBeenCalledTimes(2) | ||
| }) | ||
| }) | ||
| }) | ||
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.
Is it intentional that these grounding options aren't wrapped in a
!fromWelcomeViewcondition like they are in the Gemini component (line 77 in Gemini.tsx)? Just curious if we want to show these options in the welcome view for Vertex when we hide them for Gemini.