diff --git a/.pr/README.md b/.pr/README.md new file mode 100644 index 0000000000..cf36399c7b --- /dev/null +++ b/.pr/README.md @@ -0,0 +1,3 @@ +# PR Artifacts + +This directory contains generated PR-only QA artifacts. The PR Artifacts workflow removes it after approval so these files do not enter the final squash merge. diff --git a/.pr/live-e2e/27149471168/live-agent-recording.gif b/.pr/live-e2e/27149471168/live-agent-recording.gif new file mode 100644 index 0000000000..b059f27e78 Binary files /dev/null and b/.pr/live-e2e/27149471168/live-agent-recording.gif differ diff --git a/.pr/live-e2e/27149471168/live-agent-recording.webm b/.pr/live-e2e/27149471168/live-agent-recording.webm new file mode 100644 index 0000000000..69ff026f4c Binary files /dev/null and b/.pr/live-e2e/27149471168/live-agent-recording.webm differ diff --git a/.pr/live-e2e/27149471168/live-agent-response.png b/.pr/live-e2e/27149471168/live-agent-response.png new file mode 100644 index 0000000000..bfdfb18d7f Binary files /dev/null and b/.pr/live-e2e/27149471168/live-agent-response.png differ diff --git a/__tests__/components/conversation-events/chat/event-message-components/critic-result-display.test.tsx b/__tests__/components/conversation-events/chat/event-message-components/critic-result-display.test.tsx new file mode 100644 index 0000000000..5fae4d7085 --- /dev/null +++ b/__tests__/components/conversation-events/chat/event-message-components/critic-result-display.test.tsx @@ -0,0 +1,277 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, it, expect, vi } from "vitest"; +import { renderWithProviders } from "test-utils"; +import { CriticResultDisplay } from "#/components/conversation-events/chat/event-message-components/critic-result-display"; +import type { CriticResult } from "#/types/agent-server/core/base/critic"; + +const mockUseSettings = vi.hoisted(() => + vi.fn(() => ({ + data: { + agent_settings: { + verification: { + enable_iterative_refinement: true, + }, + }, + }, + })), +); + +vi.mock("#/hooks/query/use-settings", () => ({ + useSettings: () => mockUseSettings(), +})); + +const makeCriticResult = ( + overrides: Partial = {}, +): CriticResult => ({ + score: 0.85, + message: null, + metadata: null, + ...overrides, +}); + +beforeEach(() => { + mockUseSettings.mockReturnValue({ + data: { + agent_settings: { + verification: { + enable_iterative_refinement: true, + }, + }, + }, + }); +}); + +describe("CriticResultDisplay", () => { + it("renders score as percentage", () => { + renderWithProviders( + , + ); + + expect(screen.getByText("(72.0%)")).toBeInTheDocument(); + }); + + it("adds an accessible score label to the star rating", () => { + renderWithProviders( + , + ); + + expect(screen.getByLabelText("Score: 72.0%")).toHaveTextContent("★★★★☆"); + }); + + it("renders non-finite scores as 0%", () => { + renderWithProviders( + , + ); + + expect(screen.getByLabelText("Score: 0.0%")).toHaveTextContent("☆☆☆☆☆"); + expect(screen.getByText("(0.0%)")).toBeInTheDocument(); + }); + + it("renders 5 stars for a perfect score", () => { + renderWithProviders( + , + ); + + expect(screen.getByText("★★★★★")).toBeInTheDocument(); + }); + + it("renders 0 stars for a zero score", () => { + renderWithProviders( + , + ); + + expect(screen.getByText("☆☆☆☆☆")).toBeInTheDocument(); + }); + + it("renders green color for high score", () => { + renderWithProviders( + , + ); + + const stars = screen.getByText("★★★★☆"); + expect(stars.className).toContain("text-green-400"); + }); + + it("renders yellow color for medium score", () => { + renderWithProviders( + , + ); + + const stars = screen.getByText("★★★☆☆"); + expect(stars.className).toContain("text-yellow-400"); + }); + + it("renders red color for low score", () => { + renderWithProviders( + , + ); + + const stars = screen.getByText("★☆☆☆☆"); + expect(stars.className).toContain("text-red-400"); + }); + + it("renders label text", () => { + renderWithProviders( + , + ); + + expect( + screen.getByText("CRITIC$SUCCESS_LIKELIHOOD_LABEL"), + ).toBeInTheDocument(); + }); + + it("does not render expand button without features", () => { + renderWithProviders( + , + ); + + expect(screen.queryByLabelText("Expand details")).not.toBeInTheDocument(); + }); + + it("prompts users to enable iterative refinement when it is disabled", () => { + mockUseSettings.mockReturnValue({ + data: { + agent_settings: { + verification: { + enable_iterative_refinement: false, + }, + }, + }, + }); + + renderWithProviders( + , + ); + + expect( + screen.getByTestId("critic-iterative-refinement-hint"), + ).toHaveTextContent("CRITIC$ITERATIVE_REFINEMENT_HINT"); + }); + + it("does not prompt users when iterative refinement is enabled", () => { + renderWithProviders( + , + ); + + expect( + screen.queryByTestId("critic-iterative-refinement-hint"), + ).not.toBeInTheDocument(); + }); + + it("renders expand button when features are present", () => { + const result = makeCriticResult({ + metadata: { + categorized_features: { + agent_behavioral_issues: [ + { + name: "insufficient_testing", + display_name: "Insufficient Testing", + probability: 0.75, + }, + ], + }, + }, + }); + + renderWithProviders(); + + expect(screen.getByLabelText("Expand details")).toBeInTheDocument(); + }); + + it("expands features on click", async () => { + const user = userEvent.setup(); + const result = makeCriticResult({ + metadata: { + categorized_features: { + agent_behavioral_issues: [ + { + name: "insufficient_testing", + display_name: "Insufficient Testing", + probability: 0.75, + }, + ], + }, + }, + }); + + renderWithProviders(); + + expect(screen.queryByText("Insufficient Testing")).not.toBeInTheDocument(); + + await user.click(screen.getByLabelText("Expand details")); + + expect(screen.getByText("Insufficient Testing")).toBeInTheDocument(); + expect(screen.getByText("(75%)")).toBeInTheDocument(); + expect(screen.getByText("CRITIC$POTENTIAL_ISSUES")).toBeInTheDocument(); + }); + + it("collapses features on second click", async () => { + const user = userEvent.setup(); + const result = makeCriticResult({ + metadata: { + categorized_features: { + agent_behavioral_issues: [ + { + name: "loop_behavior", + display_name: "Loop Behavior", + probability: 0.6, + }, + ], + }, + }, + }); + + renderWithProviders(); + + await user.click(screen.getByLabelText("Expand details")); + expect(screen.getByText("Loop Behavior")).toBeInTheDocument(); + + await user.click(screen.getByLabelText("Collapse details")); + expect(screen.queryByText("Loop Behavior")).not.toBeInTheDocument(); + }); + + it("renders multiple categories of features", async () => { + const user = userEvent.setup(); + const result = makeCriticResult({ + metadata: { + categorized_features: { + agent_behavioral_issues: [ + { + name: "incomplete_changes", + display_name: "Incomplete Changes", + probability: 0.8, + }, + ], + infrastructure_issues: [ + { + name: "build_failure", + display_name: "Build Failure", + probability: 0.4, + }, + ], + user_followup_patterns: [ + { + name: "will_ask_refinement", + display_name: "Will Ask Refinement", + probability: 0.55, + }, + ], + }, + }, + }); + + renderWithProviders(); + + await user.click(screen.getByLabelText("Expand details")); + + expect(screen.getByText("CRITIC$POTENTIAL_ISSUES")).toBeInTheDocument(); + expect(screen.getByText("Incomplete Changes")).toBeInTheDocument(); + expect(screen.getByText("CRITIC$INFRASTRUCTURE")).toBeInTheDocument(); + expect(screen.getByText("Build Failure")).toBeInTheDocument(); + expect(screen.getByText("CRITIC$LIKELY_FOLLOWUP")).toBeInTheDocument(); + expect(screen.getByText("Will Ask Refinement")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx b/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx index 6bfc181b03..3c541d821d 100644 --- a/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx +++ b/__tests__/components/features/settings/sdk-settings/schema-field.test.tsx @@ -9,6 +9,11 @@ vi.mock("react-i18next", () => ({ ({ SETTINGS$TOP_P_LABEL: "Top P", SETTINGS$TOP_P_DESCRIPTION: "Controls nucleus sampling.", + SCHEMA$VERIFICATION$CRITIC_API_KEY$HELP_TEXT: + "If OpenHands is selected as your active LLM provider, leave this empty because the Critic API Key is the same as your OpenHands Provider LLM Key, which you can find in the", + SCHEMA$VERIFICATION$CRITIC_API_KEY$HELP_SUFFIX: + "tab of OpenHands Cloud; otherwise, enter a Critic API Key from that page.", + SETTINGS$NAV_API_KEYS: "API Keys", })[key] ?? key, }), })); @@ -67,4 +72,33 @@ describe("SchemaField", () => { expect(screen.getByText("Top P")).toBeInTheDocument(); expect(screen.getByText("Controls nucleus sampling.")).toBeInTheDocument(); }); + + it("renders critic API key guidance as one settings-sized help line", () => { + render( + {}} + />, + ); + + const help = screen.getByTestId("help-link-verification.critic_api_key"); + + expect(help).toHaveTextContent( + "Critic API Key is the same as your OpenHands Provider LLM Key", + ); + expect(help).toHaveTextContent("API Keys"); + expect(help).toHaveClass("text-sm"); + expect(help).toHaveClass("font-normal"); + expect( + screen.queryByText("Server schema description should be replaced."), + ).not.toBeInTheDocument(); + }); }); diff --git a/__tests__/components/features/settings/sdk-settings/sdk-section-page.test.tsx b/__tests__/components/features/settings/sdk-settings/sdk-section-page.test.tsx index f8c006255f..c0fa2f34ff 100644 --- a/__tests__/components/features/settings/sdk-settings/sdk-section-page.test.tsx +++ b/__tests__/components/features/settings/sdk-settings/sdk-section-page.test.tsx @@ -41,6 +41,13 @@ function buildSettings(overrides: Partial = {}): Settings { agent_settings_schema: overrides.agent_settings_schema ?? MOCK_DEFAULT_USER_SETTINGS.agent_settings_schema, + conversation_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.conversation_settings, + ...overrides.conversation_settings, + }, + conversation_settings_schema: + overrides.conversation_settings_schema ?? + MOCK_DEFAULT_USER_SETTINGS.conversation_settings_schema, }; } @@ -157,7 +164,9 @@ describe("SdkSectionPage", () => { ); renderSdkSectionPage({ - sectionKeys: ["llm"], + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], getInitialView: () => "advanced", }); @@ -225,7 +234,9 @@ describe("SdkSectionPage", () => { return ( ( { render(, { wrapper: ({ children }) => ( - {children} + + {children} + ), }); @@ -250,7 +263,9 @@ describe("SdkSectionPage", () => { await userEvent.type(screen.getByTestId("external-state-input"), "a"); await waitFor(() => { - expect(screen.getByTestId("sdk-settings-llm.base_url")).toBeInTheDocument(); + expect( + screen.getByTestId("sdk-settings-llm.base_url"), + ).toBeInTheDocument(); }); }); @@ -305,32 +320,46 @@ describe("SdkSectionPage", () => { const getSettingsSpy = vi .spyOn(SettingsService, "getSettings") .mockImplementation(async () => structuredClone(persistedSettings)); - vi.spyOn(SettingsService, "saveSettings").mockImplementation(async (payload) => { - const agentSettings = payload.agent_settings_diff as Record; - const llmSettings = (agentSettings.llm ?? {}) as Record; + vi.spyOn(SettingsService, "saveSettings").mockImplementation( + async (payload) => { + const agentSettings = payload.agent_settings_diff as Record< + string, + unknown + >; + const llmSettings = (agentSettings.llm ?? {}) as Record< + string, + unknown + >; - persistedSettings = buildSettings({ - agent_settings_schema: schema, - agent_settings: { - llm: { - endpoint: - typeof llmSettings.endpoint === "string" - ? llmSettings.endpoint - : "https://api.example.com", + persistedSettings = buildSettings({ + agent_settings_schema: schema, + agent_settings: { + llm: { + endpoint: + typeof llmSettings.endpoint === "string" + ? llmSettings.endpoint + : "https://api.example.com", + }, }, - }, - }); + }); - return true; + return true; + }, + ); + + renderSdkSectionPage({ + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], }); - renderSdkSectionPage({ sectionKeys: ["llm"] }); - await screen.findByTestId("sdk-section-advanced-toggle"); await userEvent.click(screen.getByTestId("sdk-section-advanced-toggle")); await screen.findByTestId("sdk-settings-llm.api_version"); - const endpointInput = await screen.findByTestId("sdk-settings-llm.endpoint"); + const endpointInput = await screen.findByTestId( + "sdk-settings-llm.endpoint", + ); await userEvent.clear(endpointInput); await userEvent.type(endpointInput, "https://api.changed.example.com"); await userEvent.click(screen.getByTestId("save-button")); @@ -397,32 +426,46 @@ describe("SdkSectionPage", () => { const getSettingsSpy = vi .spyOn(SettingsService, "getSettings") .mockImplementation(async () => structuredClone(persistedSettings)); - vi.spyOn(SettingsService, "saveSettings").mockImplementation(async (payload) => { - const agentSettings = payload.agent_settings_diff as Record; - const llmSettings = (agentSettings.llm ?? {}) as Record; + vi.spyOn(SettingsService, "saveSettings").mockImplementation( + async (payload) => { + const agentSettings = payload.agent_settings_diff as Record< + string, + unknown + >; + const llmSettings = (agentSettings.llm ?? {}) as Record< + string, + unknown + >; - persistedSettings = buildSettings({ - agent_settings_schema: schema, - agent_settings: { - llm: { - endpoint: - typeof llmSettings.endpoint === "string" - ? llmSettings.endpoint - : "https://api.example.com", + persistedSettings = buildSettings({ + agent_settings_schema: schema, + agent_settings: { + llm: { + endpoint: + typeof llmSettings.endpoint === "string" + ? llmSettings.endpoint + : "https://api.example.com", + }, }, - }, - }); + }); - return true; + return true; + }, + ); + + renderSdkSectionPage({ + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], }); - renderSdkSectionPage({ sectionKeys: ["llm"] }); - await screen.findByTestId("sdk-section-all-toggle"); await userEvent.click(screen.getByTestId("sdk-section-all-toggle")); await screen.findByTestId("sdk-settings-llm.timeout"); - const endpointInput = await screen.findByTestId("sdk-settings-llm.endpoint"); + const endpointInput = await screen.findByTestId( + "sdk-settings-llm.endpoint", + ); await userEvent.clear(endpointInput); await userEvent.type(endpointInput, "https://api.changed.example.com"); await userEvent.click(screen.getByTestId("save-button")); @@ -432,26 +475,33 @@ describe("SdkSectionPage", () => { }); await waitFor(() => { - expect(screen.queryByTestId("sdk-settings-llm.timeout")).not.toBeInTheDocument(); + expect( + screen.queryByTestId("sdk-settings-llm.timeout"), + ).not.toBeInTheDocument(); }); }); - - it("shows the advanced toggle when it is forced for a critical-only schema", async () => { - vi.spyOn(SettingsService, "getSettings").mockResolvedValue(buildSavableSettings()); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSavableSettings(), + ); renderSdkSectionPage({ - sectionKeys: ["llm"], + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], forceShowAdvancedView: true, }); await screen.findByTestId("sdk-section-basic-toggle"); - expect(screen.getByTestId("sdk-section-advanced-toggle")).toBeInTheDocument(); - expect(screen.queryByTestId("sdk-section-all-toggle")).not.toBeInTheDocument(); + expect( + screen.getByTestId("sdk-section-advanced-toggle"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("sdk-section-all-toggle"), + ).not.toBeInTheDocument(); }); - it("shows the all toggle instead of an empty advanced tier for minor-only schemas", async () => { const schema: NonNullable = { model_name: "AgentSettings", @@ -501,7 +551,11 @@ describe("SdkSectionPage", () => { }), ); - renderSdkSectionPage({ sectionKeys: ["condenser"] }); + renderSdkSectionPage({ + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["condenser"] }, + ], + }); await screen.findByTestId("sdk-section-basic-toggle"); expect( @@ -553,14 +607,18 @@ describe("SdkSectionPage", () => { buildSettings({ agent_settings_schema: schema, agent_settings: { - "verification.critic_enabled": true, - "verification.critic_server_url": "https://critic.example.com", + verification: { + critic_enabled: true, + critic_server_url: "https://critic.example.com", + }, }, }), ); renderSdkSectionPage({ - sectionKeys: ["verification"], + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["verification"] }, + ], getInitialView: () => "all", }); @@ -579,7 +637,11 @@ describe("SdkSectionPage", () => { "displaySuccessToast", ); - renderSdkSectionPage({ sectionKeys: ["llm"] }); + renderSdkSectionPage({ + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], + }); const endpointInput = await screen.findByTestId( "sdk-settings-llm.endpoint", @@ -593,6 +655,113 @@ describe("SdkSectionPage", () => { }); }); + it("saves dirty fields from multiple settings sources into separate diffs", async () => { + const agentSchema: NonNullable = { + model_name: "AgentSettings", + sections: [ + { + key: "verification", + label: "Verification", + fields: [ + { + key: "verification.critic_enabled", + label: "Enable critic", + section: "verification", + section_label: "Verification", + value_type: "boolean", + default: false, + choices: [], + depends_on: [], + prominence: "critical", + secret: false, + required: false, + }, + ], + }, + ], + }; + const conversationSchema: NonNullable< + Settings["conversation_settings_schema"] + > = { + model_name: "ConversationSettings", + sections: [ + { + key: "verification", + label: "Verification", + fields: [ + { + key: "confirmation_mode", + label: "Confirmation mode", + section: "verification", + section_label: "Verification", + value_type: "boolean", + default: false, + choices: [], + depends_on: [], + prominence: "critical", + secret: false, + required: false, + }, + ], + }, + ], + }; + + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings_schema: agentSchema, + conversation_settings_schema: conversationSchema, + agent_settings: { + verification: { + critic_enabled: false, + }, + }, + conversation_settings: { + confirmation_mode: false, + }, + }), + ); + const saveSettingsSpy = vi + .spyOn(SettingsService, "saveSettings") + .mockResolvedValue(true); + + renderSdkSectionPage({ + settingsSources: [ + { + settingsSource: "conversation_settings", + sectionKeys: ["verification"], + }, + { + settingsSource: "agent_settings", + sectionKeys: ["verification"], + }, + ], + }); + + const confirmationInput = await screen.findByTestId( + "sdk-settings-confirmation_mode", + ); + const criticInput = await screen.findByTestId( + "sdk-settings-verification.critic_enabled", + ); + await userEvent.click(confirmationInput.closest("label")!); + await userEvent.click(criticInput.closest("label")!); + await userEvent.click(screen.getByTestId("save-button")); + + await waitFor(() => { + expect(saveSettingsSpy).toHaveBeenCalledWith({ + conversation_settings_diff: { + confirmation_mode: true, + }, + agent_settings_diff: { + verification: { + critic_enabled: true, + }, + }, + }); + }); + }); + it("shows an error toast when saving settings fails", async () => { vi.spyOn(SettingsService, "getSettings").mockResolvedValue( buildSavableSettings(), @@ -602,7 +771,11 @@ describe("SdkSectionPage", () => { ); const displayErrorToastSpy = vi.spyOn(ToastHandlers, "displayErrorToast"); - renderSdkSectionPage({ sectionKeys: ["llm"] }); + renderSdkSectionPage({ + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], + }); const endpointInput = await screen.findByTestId( "sdk-settings-llm.endpoint", @@ -635,7 +808,11 @@ describe("SdkSectionPage", () => { buildSettings({ agent_settings_schema: malformedSchema }), ); - renderSdkSectionPage({ sectionKeys: ["llm"] }); + renderSdkSectionPage({ + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], + }); expect( await screen.findByText("SETTINGS$SDK_SCHEMA_UNAVAILABLE"), @@ -652,7 +829,9 @@ describe("SdkSectionPage", () => { .mockResolvedValue(true); renderSdkSectionPage({ - sectionKeys: ["llm"], + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], extraDirty: true, buildPayload: (payload) => ({ ...payload, @@ -677,14 +856,18 @@ describe("SdkSectionPage", () => { let latestControl: SdkSectionSaveControl | null = null; renderSdkSectionPage({ - sectionKeys: ["llm"], + settingsSources: [ + { settingsSource: "agent_settings", sectionKeys: ["llm"] }, + ], onSaveControlChange: (control) => { latestControl = control; }, }); // Act — change one field so it becomes dirty. - const endpointInput = await screen.findByTestId("sdk-settings-llm.endpoint"); + const endpointInput = await screen.findByTestId( + "sdk-settings-llm.endpoint", + ); await userEvent.clear(endpointInput); await userEvent.type(endpointInput, "https://new.example.com"); diff --git a/__tests__/routes/verification-settings.test.tsx b/__tests__/routes/verification-settings.test.tsx index 388e38eaf9..ae8f886604 100644 --- a/__tests__/routes/verification-settings.test.tsx +++ b/__tests__/routes/verification-settings.test.tsx @@ -10,10 +10,17 @@ function buildSettings(overrides: Partial = {}): Settings { return { ...MOCK_DEFAULT_USER_SETTINGS, ...overrides, + agent_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.agent_settings, + ...overrides.agent_settings, + }, conversation_settings: { ...MOCK_DEFAULT_USER_SETTINGS.conversation_settings, ...overrides.conversation_settings, }, + agent_settings_schema: + overrides.agent_settings_schema ?? + MOCK_DEFAULT_USER_SETTINGS.agent_settings_schema, conversation_settings_schema: overrides.conversation_settings_schema ?? MOCK_DEFAULT_USER_SETTINGS.conversation_settings_schema, @@ -39,13 +46,185 @@ beforeEach(() => { }); describe("VerificationSettingsScreen", () => { - it("keeps the confirmation controls visible in the basic view", async () => { - vi.spyOn(SettingsService, "getSettings").mockResolvedValue(buildSettings()); + it("renders critic controls in basic view", async () => { + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.agent_settings, + verification: { + critic_enabled: true, + enable_iterative_refinement: false, + }, + }, + }), + ); renderVerificationSettingsScreen(); await screen.findByTestId("verification-settings-screen"); - expect(screen.getByTestId("confirmation-mode-toggle")).toBeInTheDocument(); + // Critical-prominence fields are visible in the basic view + expect( + screen.getByTestId("sdk-settings-verification.critic_enabled"), + ).toBeInTheDocument(); + expect( + screen.getByTestId( + "sdk-settings-verification.enable_iterative_refinement", + ), + ).toBeInTheDocument(); + + // The critic LLM API key is optional because the SDK falls back to the + // active LLM API key when this field is blank. It is still surfaced + // alongside the other critical-prominence fields when the critic is on. + const apiKeyInput = screen.getByTestId( + "sdk-settings-verification.critic_api_key", + ); + expect(apiKeyInput).toBeInTheDocument(); + expect(apiKeyInput).toHaveAttribute("type", "password"); + expect(apiKeyInput).not.toBeRequired(); + + // The accompanying help link points users at OpenHands Cloud, mirroring + // the hint we already show under the LLM provider's API key field. + const helpLink = screen.getByTestId( + "help-link-verification.critic_api_key", + ); + expect(helpLink).toBeInTheDocument(); + expect( + helpLink.querySelector( + 'a[href="https://app.all-hands.dev/settings/api-keys"]', + ), + ).not.toBeNull(); + + // Major-prominence fields (confirmation_mode) are hidden in basic view + expect( + screen.queryByTestId("sdk-settings-confirmation_mode"), + ).not.toBeInTheDocument(); }); -}); \ No newline at end of file + + it("hides the critic API key field when the critic is disabled", async () => { + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.agent_settings, + verification: { + critic_enabled: false, + enable_iterative_refinement: false, + }, + }, + }), + ); + + renderVerificationSettingsScreen(); + + await screen.findByTestId("verification-settings-screen"); + + expect( + screen.queryByTestId("sdk-settings-verification.critic_api_key"), + ).not.toBeInTheDocument(); + }); + + it("shows confirmation controls in the advanced view", async () => { + // Set confirmation_mode to true so inferInitialView picks "advanced" + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + conversation_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.conversation_settings, + confirmation_mode: true, + }, + }), + ); + + renderVerificationSettingsScreen(); + + await screen.findByTestId("verification-settings-screen"); + + // Confirmation mode (major prominence) should be visible + expect( + screen.getByTestId("sdk-settings-confirmation_mode"), + ).toBeInTheDocument(); + // Security analyzer depends on confirmation_mode being true + expect( + screen.getByTestId("sdk-settings-security_analyzer"), + ).toBeInTheDocument(); + }); + + it("deduplicates legacy agent verification fields in favor of conversation settings", async () => { + const legacyAgentSchema = structuredClone( + MOCK_DEFAULT_USER_SETTINGS.agent_settings_schema!, + ); + const verificationSection = legacyAgentSchema.sections.find( + (section) => section.key === "verification", + ); + expect(verificationSection).toBeDefined(); + verificationSection!.fields.push( + { + key: "verification.confirmation_mode", + label: "Legacy confirmation mode", + description: "Legacy agent-owned confirmation mode.", + section: "verification", + section_label: "Verification", + value_type: "boolean", + default: false, + choices: [], + depends_on: [], + prominence: "major", + secret: false, + required: false, + }, + { + key: "verification.security_analyzer", + label: "Legacy security analyzer", + description: "Legacy agent-owned security analyzer.", + section: "verification", + section_label: "Verification", + value_type: "string", + default: "llm", + choices: [ + { label: "llm", value: "llm" }, + { label: "none", value: "none" }, + ], + depends_on: ["verification.confirmation_mode"], + prominence: "major", + secret: false, + required: false, + }, + ); + + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings_schema: legacyAgentSchema, + agent_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.agent_settings, + verification: { + critic_enabled: false, + enable_iterative_refinement: false, + confirmation_mode: true, + security_analyzer: "llm", + }, + }, + conversation_settings: { + ...MOCK_DEFAULT_USER_SETTINGS.conversation_settings, + confirmation_mode: true, + security_analyzer: "llm", + }, + }), + ); + + renderVerificationSettingsScreen(); + + await screen.findByTestId("verification-settings-screen"); + + expect( + screen.getByTestId("sdk-settings-confirmation_mode"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("sdk-settings-security_analyzer"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("sdk-settings-verification.confirmation_mode"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("sdk-settings-verification.security_analyzer"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/utils/sdk-settings-schema.test.ts b/__tests__/utils/sdk-settings-schema.test.ts index 839232a7f8..b530e08e45 100644 --- a/__tests__/utils/sdk-settings-schema.test.ts +++ b/__tests__/utils/sdk-settings-schema.test.ts @@ -77,16 +77,16 @@ const BASE_SETTINGS: Settings = { ], }, { - key: "critic", - label: "Critic", + key: "verification", + label: "Verification", fields: [ { - key: "critic.enabled", + key: "verification.critic_enabled", label: "Enable critic", - section: "critic", - section_label: "Critic", + section: "verification", + section_label: "Verification", value_type: "boolean", - default: false, + default: true, choices: [], depends_on: [], prominence: "critical", @@ -94,17 +94,17 @@ const BASE_SETTINGS: Settings = { required: true, }, { - key: "critic.mode", + key: "verification.critic_mode", label: "Mode", - section: "critic", - section_label: "Critic", + section: "verification", + section_label: "Verification", value_type: "string", default: "finish_and_message", choices: [ { label: "finish_and_message", value: "finish_and_message" }, { label: "all_actions", value: "all_actions" }, ], - depends_on: ["critic.enabled"], + depends_on: ["verification.critic_enabled"], prominence: "minor", secret: false, required: true, @@ -134,15 +134,13 @@ const BASE_SETTINGS: Settings = { }, agent_settings: { agent: "CodeActAgent", - critic: { - mode: "finish_and_message", - enabled: false, - }, llm: { api_key: null, model: "openai/gpt-4o", }, verification: { + critic_enabled: false, + critic_mode: "finish_and_message", confirmation_mode: false, }, condenser: { @@ -155,8 +153,8 @@ const BASE_SETTINGS: Settings = { describe("sdk settings schema helpers", () => { it("builds initial form values from the current settings", () => { expect(buildInitialSettingsFormValues(BASE_SETTINGS)).toEqual({ - "critic.mode": "finish_and_message", - "critic.enabled": false, + "verification.critic_mode": "finish_and_message", + "verification.critic_enabled": false, "llm.api_key": "", "llm.base_url": "", "llm.litellm_extra_body": "{}", @@ -173,10 +171,10 @@ describe("sdk settings schema helpers", () => { ...BASE_SETTINGS, agent_settings: { ...BASE_SETTINGS.agent_settings, - critic: { - ...((BASE_SETTINGS.agent_settings as Record) - .critic as Record), - mode: "all_actions", + verification: { + ...(BASE_SETTINGS.agent_settings as Record) + .verification as Record, + critic_mode: "all_actions", }, }, }; @@ -214,11 +212,13 @@ describe("sdk settings schema helpers", () => { const allSections = getVisibleSettingsSections( BASE_SETTINGS.agent_settings_schema!, - { ...values, "critic.enabled": true }, + { ...values, "verification.critic_enabled": true }, "all", ); - const criticSection = allSections.find((s) => s.key === "critic"); - expect(criticSection?.fields).toHaveLength(2); + const verificationSection = allSections.find( + (s) => s.key === "verification", + ); + expect(verificationSection?.fields).toHaveLength(2); }); it("passes through all fields when excludeKeys is empty", () => { @@ -239,7 +239,7 @@ describe("sdk settings schema helpers", () => { BASE_SETTINGS.agent_settings_schema!, { ...buildInitialSettingsFormValues(BASE_SETTINGS), - "critic.enabled": true, + "verification.critic_enabled": true, "llm.api_key": "new-key", "llm.litellm_extra_body": JSON.stringify( { metadata: { tier: "sample" } }, @@ -248,7 +248,7 @@ describe("sdk settings schema helpers", () => { ), }, { - "critic.enabled": true, + "verification.critic_enabled": true, "llm.api_key": true, "llm.litellm_extra_body": true, "llm.model": false, @@ -256,11 +256,11 @@ describe("sdk settings schema helpers", () => { ); expect(payload).toEqual({ - critic: { enabled: true }, llm: { api_key: "new-key", litellm_extra_body: { metadata: { tier: "sample" } }, }, + verification: { critic_enabled: true }, }); }); @@ -287,8 +287,8 @@ describe("sdk settings schema helpers", () => { }), "llm.model": "anthropic/claude-sonnet-4-20250514", "llm.timeout": "90", - "critic.enabled": true, - "critic.mode": "all_actions", + "verification.critic_enabled": true, + "verification.critic_mode": "all_actions", "llm.litellm_extra_body": JSON.stringify( { metadata: { tier: "sample" } }, null, @@ -299,8 +299,8 @@ describe("sdk settings schema helpers", () => { const dirty = { "llm.model": true, "llm.timeout": true, - "critic.enabled": true, - "critic.mode": true, + "verification.critic_enabled": true, + "verification.critic_mode": true, "llm.litellm_extra_body": true, }; @@ -312,7 +312,7 @@ describe("sdk settings schema helpers", () => { timeout: 30, litellm_extra_body: {}, }, - critic: { enabled: true, mode: "finish_and_message" }, + verification: { critic_enabled: true, critic_mode: "finish_and_message" }, mcp_config: null, }); @@ -324,7 +324,7 @@ describe("sdk settings schema helpers", () => { timeout: 90, litellm_extra_body: {}, }, - critic: { enabled: true, mode: "finish_and_message" }, + verification: { critic_enabled: true, critic_mode: "finish_and_message" }, mcp_config: null, }); @@ -336,7 +336,7 @@ describe("sdk settings schema helpers", () => { timeout: 90, litellm_extra_body: { metadata: { tier: "sample" } }, }, - critic: { enabled: true, mode: "all_actions" }, + verification: { critic_enabled: true, critic_mode: "all_actions" }, }); }); diff --git a/src/components/conversation-events/chat/event-message-components/critic-result-display.tsx b/src/components/conversation-events/chat/event-message-components/critic-result-display.tsx new file mode 100644 index 0000000000..70fff59861 --- /dev/null +++ b/src/components/conversation-events/chat/event-message-components/critic-result-display.tsx @@ -0,0 +1,243 @@ +import React from "react"; +import { useTranslation } from "react-i18next"; +import ArrowDown from "#/icons/angle-down-solid.svg?react"; +import ArrowUp from "#/icons/angle-up-solid.svg?react"; +import { useSettings } from "#/hooks/query/use-settings"; +import { I18nKey } from "#/i18n/declaration"; +import type { + CriticResult, + CriticFeature, + CriticCategorizedFeatures, +} from "#/types/agent-server/core/base/critic"; + +/** + * Normalize potentially malformed runtime scores before rendering. + */ +function normalizeScore(score: number): number { + if (!Number.isFinite(score)) return 0; + return Math.min(1, Math.max(0, score)); +} + +/** + * Convert a normalized score (0-1) to a 5-star rating string. + */ +function getStarRating(normalizedScore: number): { + filled: number; + empty: number; +} { + const filled = Math.round(normalizedScore * 5); + return { filled, empty: 5 - filled }; +} + +/** + * Get the color class for the star rating based on score. + */ +function getScoreColorClass(score: number): string { + if (score >= 0.6) return "text-green-400"; + if (score >= 0.4) return "text-yellow-400"; + return "text-red-400"; +} + +/** + * Get the color class for an issue probability. + */ +function getIssueColorClass(probability: number): string { + if (probability >= 0.7) return "text-red-400 font-semibold"; + if (probability >= 0.5) return "text-yellow-400"; + return "text-neutral-400"; +} + +function isSettingsRecord(value: unknown): value is Record { + return ( + value !== null && + value !== undefined && + typeof value === "object" && + !Array.isArray(value) + ); +} + +function getIterativeRefinementEnabled( + agentSettings: Record | null | undefined, +): boolean | null { + const verification = agentSettings?.verification; + if (!isSettingsRecord(verification)) { + return null; + } + + const value = verification.enable_iterative_refinement; + return typeof value === "boolean" ? value : null; +} + +/** + * Renders a single issue feature with its probability. + */ +function FeatureItem({ feature }: { feature: CriticFeature }) { + const percentage = Math.round(feature.probability * 100); + const colorClass = getIssueColorClass(feature.probability); + + return ( + + {feature.display_name} + ({percentage}%) + + ); +} + +/** + * Renders a category of features (e.g., "Potential Issues", "Infrastructure"). + */ +function FeatureCategory({ + label, + features, +}: { + label: string; + features: CriticFeature[]; +}) { + if (!features || features.length === 0) return null; + + return ( +
+ {label} + {features.map((feature, i) => ( + + {i > 0 && ·} + + + ))} +
+ ); +} + +/** + * Renders the categorized features breakdown. + */ +function FeaturesBreakdown({ + categorized, +}: { + categorized: CriticCategorizedFeatures; +}) { + const { t } = useTranslation(); + + const hasAgentIssues = + categorized.agent_behavioral_issues && + categorized.agent_behavioral_issues.length > 0; + const hasUserPatterns = + categorized.user_followup_patterns && + categorized.user_followup_patterns.length > 0; + const hasInfra = + categorized.infrastructure_issues && + categorized.infrastructure_issues.length > 0; + const hasOther = categorized.other && categorized.other.length > 0; + + if (!hasAgentIssues && !hasUserPatterns && !hasInfra && !hasOther) { + return null; + } + + return ( +
+ {hasAgentIssues && ( + + )} + {hasInfra && ( + + )} + {hasUserPatterns && ( + + )} + {hasOther && ( + + )} +
+ ); +} + +interface CriticResultDisplayProps { + criticResult: CriticResult; +} + +/** + * Displays a critic evaluation result with star rating, score percentage, + * and expandable categorized feature breakdown. + */ +export function CriticResultDisplay({ + criticResult, +}: CriticResultDisplayProps) { + const { t } = useTranslation(); + const { data: settings } = useSettings(); + const [expanded, setExpanded] = React.useState(false); + + const normalizedScore = normalizeScore(criticResult.score); + const { filled, empty } = getStarRating(normalizedScore); + const colorClass = getScoreColorClass(normalizedScore); + const percentage = (normalizedScore * 100).toFixed(1); + const iterativeRefinementEnabled = getIterativeRefinementEnabled( + settings?.agent_settings as Record | null | undefined, + ); + const showIterativeRefinementHint = iterativeRefinementEnabled === false; + + const categorized = criticResult.metadata?.categorized_features; + const hasDetails = + categorized != null && + ((categorized.agent_behavioral_issues ?? []).length > 0 || + (categorized.user_followup_patterns ?? []).length > 0 || + (categorized.infrastructure_issues ?? []).length > 0 || + (categorized.other ?? []).length > 0); + + return ( +
+
+ + {t(I18nKey.CRITIC$SUCCESS_LIKELIHOOD_LABEL)} + + + {"★".repeat(filled)} + {"☆".repeat(empty)} + + ({percentage}%) + + {hasDetails && ( + + )} +
+ + {expanded && hasDetails && ( + + )} + + {showIterativeRefinementHint && ( +

+ {t(I18nKey.CRITIC$ITERATIVE_REFINEMENT_HINT)} +

+ )} +
+ ); +} diff --git a/src/components/conversation-events/chat/event-message-components/finish-event-message.tsx b/src/components/conversation-events/chat/event-message-components/finish-event-message.tsx index 40bed72e99..6cfee059d0 100644 --- a/src/components/conversation-events/chat/event-message-components/finish-event-message.tsx +++ b/src/components/conversation-events/chat/event-message-components/finish-event-message.tsx @@ -2,6 +2,7 @@ import { ActionEvent } from "#/types/agent-server/core"; import { FinishAction } from "#/types/agent-server/core/base/action"; import { ChatMessage } from "../../../features/chat/chat-message"; import { getEventContent } from "../event-content-helpers/get-event-content"; +import { CriticResultDisplay } from "./critic-result-display"; interface FinishEventMessageProps { event: ActionEvent; @@ -19,10 +20,15 @@ export function FinishEventMessage({ : String(eventContent.details); return ( - + <> + + {event.critic_result != null && ( + + )} + ); } diff --git a/src/components/conversation-events/chat/event-message-components/index.ts b/src/components/conversation-events/chat/event-message-components/index.ts index 1acf8c9371..6644066fc8 100644 --- a/src/components/conversation-events/chat/event-message-components/index.ts +++ b/src/components/conversation-events/chat/event-message-components/index.ts @@ -6,3 +6,4 @@ export { GenericEventMessageWrapper } from "./generic-event-message-wrapper"; export { ThoughtEventMessage } from "./thought-event-message"; export { HookExecutionEventMessage } from "./hook-execution-event-message"; export { EventGroup } from "./event-group"; +export { CriticResultDisplay } from "./critic-result-display"; diff --git a/src/components/conversation-events/chat/event-message-components/user-assistant-event-message.tsx b/src/components/conversation-events/chat/event-message-components/user-assistant-event-message.tsx index 5727ddc513..33233ac7f9 100644 --- a/src/components/conversation-events/chat/event-message-components/user-assistant-event-message.tsx +++ b/src/components/conversation-events/chat/event-message-components/user-assistant-event-message.tsx @@ -4,6 +4,7 @@ import { ChatMessage } from "../../../features/chat/chat-message"; import { ImageCarousel } from "../../../features/images/image-carousel"; import { ConversationConfirmationButtons } from "#/components/shared/buttons/conversation-confirmation-buttons"; import { parseMessageFromEvent } from "../event-content-helpers/parse-message-from-event"; +import { CriticResultDisplay } from "./critic-result-display"; interface UserAssistantEventMessageProps { event: MessageEvent; @@ -28,15 +29,20 @@ export function UserAssistantEventMessage({ } return ( - - {imageUrls.length > 0 && ( - + <> + + {imageUrls.length > 0 && ( + + )} + {isLastMessage && } + + {event.source === "agent" && event.critic_result != null && ( + )} - {isLastMessage && } - + ); } diff --git a/src/components/features/settings/sdk-settings/schema-field.tsx b/src/components/features/settings/sdk-settings/schema-field.tsx index f69e0b8dff..9661bb002c 100644 --- a/src/components/features/settings/sdk-settings/schema-field.tsx +++ b/src/components/features/settings/sdk-settings/schema-field.tsx @@ -25,15 +25,43 @@ import { // --------------------------------------------------------------------------- export const FIELD_HELP_LINKS: Record< string, - { textKey: string; linkTextKey: string; href: string } + { + textKey: string; + linkTextKey: string; + href: string; + /** Skip rendering the schema description separately when the help text already includes it. */ + hideDescription?: boolean; + /** Optional trailing copy rendered after the link (e.g. " tab of OpenHands Cloud."). */ + suffixKey?: string; + } > = { "llm.api_key": { textKey: "SCHEMA$LLM$API_KEY$HELP_TEXT", linkTextKey: "SCHEMA$LLM$API_KEY$HELP_LINK_TEXT", href: "https://docs.openhands.dev/usage/local-setup#getting-an-api-key", }, + // Mirror the hint shown under the LLM provider's API key field when + // OpenHands is selected as the active provider; the SDK reuses that active + // LLM key when the critic key is empty. + "verification.critic_api_key": { + textKey: "SCHEMA$VERIFICATION$CRITIC_API_KEY$HELP_TEXT", + linkTextKey: "SETTINGS$NAV_API_KEYS", + suffixKey: "SCHEMA$VERIFICATION$CRITIC_API_KEY$HELP_SUFFIX", + href: "https://app.all-hands.dev/settings/api-keys", + hideDescription: true, + }, }; +/** + * Field keys that should span the full settings grid (both columns on xl + * screens) instead of sharing a row with the next field. Used for inputs + * whose label + value + help link need horizontal room so they don't + * sit awkwardly opposite a single toggle. + */ +export const FIELD_FULL_WIDTH_KEYS: ReadonlySet = new Set([ + "verification.critic_api_key", +]); + function FieldHelp({ field }: { field: SettingsFieldSchema }) { const { t } = useTranslation("openhands"); const helpLink = FIELD_HELP_LINKS[field.key]; @@ -45,7 +73,7 @@ function FieldHelp({ field }: { field: SettingsFieldSchema }) { return ( <> - {description ? ( + {description && !helpLink?.hideDescription ? ( {description} @@ -56,6 +84,7 @@ function FieldHelp({ field }: { field: SettingsFieldSchema }) { text={t(helpLink.textKey)} linkText={t(helpLink.linkTextKey)} href={helpLink.href} + suffix={helpLink.suffixKey ? ` ${t(helpLink.suffixKey)}` : undefined} size="settings" linkColor="white" /> diff --git a/src/components/features/settings/sdk-settings/sdk-section-page.tsx b/src/components/features/settings/sdk-settings/sdk-section-page.tsx index 02e98e3bf1..a7746a0b11 100644 --- a/src/components/features/settings/sdk-settings/sdk-section-page.tsx +++ b/src/components/features/settings/sdk-settings/sdk-section-page.tsx @@ -31,7 +31,7 @@ import { type SettingsValueSource, type SettingsView, } from "#/utils/sdk-settings-schema"; -import { SchemaField } from "./schema-field"; +import { FIELD_FULL_WIDTH_KEYS, SchemaField } from "./schema-field"; import { ViewToggle } from "./view-toggle"; const EMPTY_EXCLUDE_KEYS = new Set(); @@ -48,6 +48,12 @@ const getLessDetailedView = ( ): SettingsView => VIEW_ORDER[nextView] < VIEW_ORDER[currentView] ? nextView : currentView; +const getMoreDetailedView = ( + currentView: SettingsView, + nextView: SettingsView, +): SettingsView => + VIEW_ORDER[nextView] > VIEW_ORDER[currentView] ? nextView : currentView; + const normalizeView = ( view: SettingsView, { @@ -77,6 +83,11 @@ const normalizeView = ( return "basic"; }; +const PAYLOAD_DIFF_KEY: Record = { + agent_settings: "agent_settings_diff", + conversation_settings: "conversation_settings_diff", +}; + const getSchemaUnavailableMessage = ( error: unknown, fallbackMessage: string, @@ -96,6 +107,15 @@ const getSchemaUnavailableMessage = ( return fallbackMessage; }; +export interface SettingsSourceConfig { + /** Which schema/values bucket on `settings` this source pulls from. */ + settingsSource: SettingsValueSource; + /** Section keys (e.g. ["llm"]) within that schema to render. */ + sectionKeys: string[]; + /** Field keys to skip (rendered elsewhere by the caller). */ + excludeKeys?: Set; +} + export interface SdkSectionHeaderProps { values: SettingsFormValues; isDisabled: boolean; @@ -103,6 +123,10 @@ export interface SdkSectionHeaderProps { onChange: (key: string, value: string | boolean) => void; } +interface ResolvedSource extends SettingsSourceConfig { + filteredSchema: SettingsSchema | null; +} + /** * Snapshot of the page's save state, surfaced to the parent so it can * render its own Save/Next button (e.g. in onboarding) when @@ -130,19 +154,24 @@ export interface SdkSectionSaveControl { } /** - * A generic SDK-schema–driven settings page that renders fields - * from one or more schema sections. + * A generic SDK-schema-driven settings page that renders fields from one or + * more schema sections. * - * @param sectionKeys - which schema section(s) this page owns (e.g. ["condenser"]) - * @param excludeKeys - field keys to skip (rendered elsewhere by the caller) - * @param header - optional render prop receiving shared state to render above fields - * @param testId - data-testid for the page wrapper + * The `settingsSources` array specifies which schema(s)/section(s) the page + * owns. The page tracks values/dirty state per source, renders sections from + * each source in order (filtered by the schema's `prominence` field for the + * selected view), and emits a combined save payload like + * `{ conversation_settings_diff: {...}, agent_settings_diff: {...} }` --- + * including only the keys for sources that actually have dirty changes. + * + * @param settingsSources one or more schemas to render fields from + * @param header render prop above the fields (receives unified state) + * @param buildPayload customize the save payload before submission + * @param testId data-testid on the page wrapper */ export function SdkSectionPage({ - sectionKeys, - excludeKeys = EMPTY_EXCLUDE_KEYS, + settingsSources, scope = "personal", - settingsSource = "agent_settings", header, extraDirty = false, buildPayload, @@ -157,15 +186,18 @@ export function SdkSectionPage({ onSaveControlChange, testId = "sdk-section-settings-screen", }: { - sectionKeys: string[]; - excludeKeys?: Set; + settingsSources: SettingsSourceConfig[]; scope?: SettingsScope; - settingsSource?: SettingsValueSource; header?: (props: SdkSectionHeaderProps) => React.ReactNode; extraDirty?: boolean; + /** + * Customize the save payload. Receives the wrapped default payload (e.g. + * `{ agent_settings_diff: { llm: { model: "gpt-4" } } }`) plus the unified + * form context. Return the payload to actually send. + */ buildPayload?: ( - payload: ReturnType, + defaultPayload: Record, context: { values: SettingsFormValues; dirty: SettingsDirtyState; @@ -188,17 +220,7 @@ export function SdkSectionPage({ * particular default (e.g. onboarding pre-filling OpenHands/Opus). */ initialValueOverrides?: SettingsFormValues; - /** - * When true, the Save button container is rendered inline (no - * sticky positioning, no contrasting `bg-base` band) so the page - * can be dropped into a modal/card without a hard footer break. - */ embedded?: boolean; - /** - * Suppress the built-in Save Changes button entirely. Pair with - * {@link onSaveControlChange} to drive saving from a parent-rendered - * action (e.g. an onboarding "Next" button). - */ hideSaveButton?: boolean; /** Suppress the default success toast after save completes. */ suppressSuccessToast?: boolean; @@ -220,112 +242,171 @@ export function SdkSectionPage({ const conversationSchemaQuery = useConversationSettingsSchema( settings?.conversation_settings_schema, ); - const activeSchemaQuery = - settingsSource === "conversation_settings" - ? conversationSchemaQuery - : agentSchemaQuery; - const schema = activeSchemaQuery.data; - const isSchemaLoading = activeSchemaQuery.isLoading; const isReadOnly = false; - const [view, setView] = React.useState("basic"); - const [values, setValues] = React.useState({}); - const [dirty, setDirty] = React.useState({}); - const hasHydratedViewRef = React.useRef(false); - - const sectionKeysSignature = React.useMemo( - () => JSON.stringify(sectionKeys), - [sectionKeys], - ); - const stableSectionKeys = React.useMemo( - () => JSON.parse(sectionKeysSignature) as string[], - [sectionKeysSignature], + const sourcesSignature = React.useMemo( + () => + JSON.stringify( + settingsSources.map((s) => ({ + source: s.settingsSource, + sectionKeys: s.sectionKeys, + excludeKeys: s.excludeKeys ? Array.from(s.excludeKeys).sort() : null, + })), + ), + [settingsSources], ); - // Build a filtered schema containing only the requested sections. - // `isValidSettingsSchema` guards against truthy-but-malformed schema - // responses (e.g. when the deployment is pointed at a host that does - // not serve `/api/settings/agent-schema` and returns an SPA shell - // that parses into an object without a `sections` array). Without - // the guard, `schema.sections.filter(...)` would throw and React - // Router would escalate the crash to a full-screen error. - const filteredSchema = React.useMemo(() => { - if (!isValidSettingsSchema(schema)) return null; - const sectionSet = new Set(stableSectionKeys); - return { - ...schema, - sections: schema.sections.filter((s) => sectionSet.has(s.key)), - }; - }, [schema, stableSectionKeys]); + const resolvedSourceConfigs = React.useMemo(() => { + const parsed = JSON.parse(sourcesSignature) as Array<{ + source: SettingsValueSource; + sectionKeys: string[]; + excludeKeys: string[] | null; + }>; + return parsed.map((p) => ({ + settingsSource: p.source, + sectionKeys: p.sectionKeys, + excludeKeys: p.excludeKeys ? new Set(p.excludeKeys) : undefined, + })); + }, [sourcesSignature]); + + const getSchemaForSource = React.useCallback( + (source: SettingsValueSource) => + source === "conversation_settings" + ? conversationSchemaQuery.data + : agentSchemaQuery.data, + [agentSchemaQuery.data, conversationSchemaQuery.data], + ); + + const isSchemaLoading = resolvedSourceConfigs.some((src) => + src.settingsSource === "conversation_settings" + ? conversationSchemaQuery.isLoading + : agentSchemaQuery.isLoading, + ); + + const resolvedSources = React.useMemo( + () => + resolvedSourceConfigs.map((src) => { + const schema = getSchemaForSource(src.settingsSource); + if (!isValidSettingsSchema(schema)) { + return { ...src, filteredSchema: null }; + } + const sectionSet = new Set(src.sectionKeys); + const filteredSchema: SettingsSchema = { + ...schema, + sections: schema.sections.filter((s) => sectionSet.has(s.key)), + }; + return { ...src, filteredSchema }; + }), + [resolvedSourceConfigs, getSchemaForSource], + ); const showAdvanced = - forceShowAdvancedView || hasAdvancedSettings(filteredSchema); - const showAll = allowAllView && hasMinorSettings(filteredSchema); - const schemaUnavailableMessage = React.useMemo( - () => - getSchemaUnavailableMessage( - activeSchemaQuery.error, - t(I18nKey.SETTINGS$SDK_SCHEMA_UNAVAILABLE), - ), - [activeSchemaQuery.error, t], - ); + forceShowAdvancedView || + resolvedSources.some((src) => hasAdvancedSettings(src.filteredSchema)); + const showAll = + allowAllView && + resolvedSources.some((src) => hasMinorSettings(src.filteredSchema)); + + const schemaUnavailableMessage = React.useMemo(() => { + const firstError = resolvedSourceConfigs.reduce( + (err, src) => + err ?? + (src.settingsSource === "conversation_settings" + ? conversationSchemaQuery.error + : agentSchemaQuery.error), + null, + ); + return getSchemaUnavailableMessage( + firstError, + t(I18nKey.SETTINGS$SDK_SCHEMA_UNAVAILABLE), + ); + }, [ + resolvedSourceConfigs, + agentSchemaQuery.error, + conversationSchemaQuery.error, + t, + ]); const overridesSignature = React.useMemo( () => (initialValueOverrides ? JSON.stringify(initialValueOverrides) : ""), [initialValueOverrides], ); - const initialValues = React.useMemo(() => { - if (!settings || !filteredSchema) return null; - const base = buildInitialSettingsFormValues( - settings, - filteredSchema, - settingsSource, - ); - if (!initialValueOverrides) return base; - return { ...base, ...initialValueOverrides }; - // overridesSignature keeps the memo reactive without depending on - // a (potentially recreated) object reference each render. - }, [settings, filteredSchema, settingsSource, overridesSignature]); + const [view, setView] = React.useState("basic"); + const [valuesBySource, setValuesBySource] = React.useState< + Partial> + >({}); + const [dirtyBySource, setDirtyBySource] = React.useState< + Partial> + >({}); + const hasHydratedViewRef = React.useRef(false); + + const initialValuesBySource = React.useMemo + > | null>(() => { + if (!settings) return null; + const result: Partial> = {}; + for (const src of resolvedSources) { + if (!src.filteredSchema) return null; + result[src.settingsSource] = { + ...(result[src.settingsSource] ?? {}), + ...buildInitialSettingsFormValues( + settings, + src.filteredSchema, + src.settingsSource, + ), + }; + } + if (initialValueOverrides) { + const firstSource = resolvedSources[0]?.settingsSource; + if (firstSource && result[firstSource]) { + result[firstSource] = { + ...result[firstSource], + ...initialValueOverrides, + }; + } + } + return result; + }, [settings, resolvedSources, overridesSignature]); const initialView = React.useMemo(() => { - if (!settings || !filteredSchema) return null; - - const resolvedInitialView = getInitialView - ? getInitialView(settings, filteredSchema) - : inferInitialView(settings, filteredSchema, settingsSource); - - return normalizeView(resolvedInitialView, { showAdvanced, showAll }); - }, [ - settings, - filteredSchema, - getInitialView, - settingsSource, - showAdvanced, - showAll, - ]); + if (!settings) return null; + let result: SettingsView | null = null; + for (const src of resolvedSources) { + if (!src.filteredSchema) return null; + const perSource = getInitialView + ? getInitialView(settings, src.filteredSchema) + : inferInitialView(settings, src.filteredSchema, src.settingsSource); + result = result ? getMoreDetailedView(result, perSource) : perSource; + } + if (!result) return null; + return normalizeView(result, { showAdvanced, showAll }); + }, [settings, resolvedSources, getInitialView, showAdvanced, showAll]); React.useEffect(() => { hasHydratedViewRef.current = false; setView("basic"); - setValues({}); - setDirty({}); - }, [scope, settingsSource, sectionKeysSignature]); + setValuesBySource({}); + setDirtyBySource({}); + }, [scope, sourcesSignature]); React.useEffect(() => { - if (!initialValues || !initialView) return; + if (!initialValuesBySource || !initialView) return; - setValues(initialValues); - // Override-supplied keys are pre-populated for the user, so mark - // them dirty up-front; otherwise the Save button stays disabled - // until the user touches a field, defeating the point of the - // override. - const overrideDirty: SettingsDirtyState = initialValueOverrides - ? Object.fromEntries( + setValuesBySource(initialValuesBySource); + if (initialValueOverrides) { + const firstSource = resolvedSources[0]?.settingsSource; + if (firstSource) { + const overrideDirty: SettingsDirtyState = Object.fromEntries( Object.keys(initialValueOverrides).map((key) => [key, true]), - ) - : {}; - setDirty(overrideDirty); + ); + setDirtyBySource({ [firstSource]: overrideDirty }); + } else { + setDirtyBySource({}); + } + } else { + setDirtyBySource({}); + } setView((currentView) => { if (!hasHydratedViewRef.current) { hasHydratedViewRef.current = true; @@ -334,27 +415,60 @@ export function SdkSectionPage({ return getLessDetailedView(currentView, initialView); }); - // initialValueOverrides is intentionally tracked via - // overridesSignature on initialValues; including the object ref - // here would re-fire the effect every render. - }, [initialValues, initialView]); + }, [initialValuesBySource, initialView]); - const visibleSections = React.useMemo(() => { - if (!filteredSchema) return []; - return getVisibleSettingsSections( - filteredSchema, - values, - view, - excludeKeys, - ); - }, [filteredSchema, values, view, excludeKeys]); + const fieldKeyToSource = React.useMemo(() => { + const map = new Map(); + for (const src of resolvedSources) { + if (src.filteredSchema) { + for (const section of src.filteredSchema.sections) { + for (const field of section.fields) { + if (!map.has(field.key)) { + map.set(field.key, src.settingsSource); + } + } + } + } + } + return map; + }, [resolvedSources]); + + const flatValues = React.useMemo(() => { + const merged: SettingsFormValues = {}; + for (const src of resolvedSources) { + Object.assign(merged, valuesBySource[src.settingsSource] ?? {}); + } + return merged; + }, [resolvedSources, valuesBySource]); + + const flatDirty = React.useMemo(() => { + const merged: SettingsDirtyState = {}; + for (const src of resolvedSources) { + Object.assign(merged, dirtyBySource[src.settingsSource] ?? {}); + } + return merged; + }, [resolvedSources, dirtyBySource]); const handleFieldChange = React.useCallback( (fieldKey: string, nextValue: string | boolean) => { - setValues((prev) => ({ ...prev, [fieldKey]: nextValue })); - setDirty((prev) => ({ ...prev, [fieldKey]: true })); + const sourceKey = fieldKeyToSource.get(fieldKey); + if (!sourceKey) return; + setValuesBySource((prev) => ({ + ...prev, + [sourceKey]: { + ...(prev[sourceKey] ?? {}), + [fieldKey]: nextValue, + }, + })); + setDirtyBySource((prev) => ({ + ...prev, + [sourceKey]: { + ...(prev[sourceKey] ?? {}), + [fieldKey]: true, + }, + })); }, - [], + [fieldKeyToSource], ); const handleError = React.useCallback( @@ -365,9 +479,6 @@ export function SdkSectionPage({ [t], ); - // Stable save callback so `onSaveControlChange` can hand a single - // function reference to the parent across renders. The latest - // closure is kept up to date via `handleSaveRef`. const handleSaveRef = React.useRef<() => void>(() => {}); const stableSave = React.useCallback(() => { handleSaveRef.current(); @@ -385,24 +496,39 @@ export function SdkSectionPage({ ); const handleSave = () => { - if (!filteredSchema || isReadOnly) return; + if (isReadOnly) return; + if (resolvedSources.some((src) => !src.filteredSchema)) return; let payload: Record; try { - const basePayload = buildSdkSettingsPayloadForView( - filteredSchema, - values, - dirty, - view, - ); - let defaultPayload: Record; - if (settingsSource === "conversation_settings") { - defaultPayload = { conversation_settings_diff: basePayload }; - } else { - defaultPayload = { agent_settings_diff: basePayload }; + const defaultPayload: Record = {}; + for (const src of resolvedSources) { + const schema = src.filteredSchema!; + const sourceValues = valuesBySource[src.settingsSource] ?? {}; + const sourceDirty = dirtyBySource[src.settingsSource] ?? {}; + const diff = buildSdkSettingsPayloadForView( + schema, + sourceValues, + sourceDirty, + view, + ); + if (Object.keys(diff).length > 0) { + const diffKey = PAYLOAD_DIFF_KEY[src.settingsSource]; + defaultPayload[diffKey] = { + ...((defaultPayload[diffKey] as + | Record + | undefined) ?? {}), + ...diff, + }; + } } + payload = buildPayload - ? buildPayload(basePayload, { values, dirty, view }) + ? buildPayload(defaultPayload, { + values: flatValues, + dirty: flatDirty, + view, + }) : defaultPayload; } catch (error) { displayErrorToast( @@ -419,7 +545,7 @@ export function SdkSectionPage({ if (!suppressSuccessToast) { displaySuccessToast(t(I18nKey.SETTINGS$SAVED_WARNING)); } - setDirty({}); + setDirtyBySource({}); onSaveSuccess?.(); }, }); @@ -429,30 +555,36 @@ export function SdkSectionPage({ // Dirty-only (NOT view-filtered): we must never inject defaults for // non-visible fields here, or a custom save flow would reset fields the // user never touched. `buildSdkSettingsPayloadForView` is reserved for the - // built-in full-replace save above. - buildDirtyPayloadRef.current = () => - filteredSchema - ? buildSdkSettingsPayload(filteredSchema, values, dirty) - : {}; + // built-in full-replace save above. With multiple sources, we merge each + // source's nested payload at the top level so single-source consumers + // (e.g. `LlmSettingsLocalView`) keep reading `.llm` etc. unchanged. + buildDirtyPayloadRef.current = () => { + const merged: Record = {}; + for (const src of resolvedSources) { + if (!src.filteredSchema) continue; + const sourceValues = valuesBySource[src.settingsSource] ?? {}; + const sourceDirty = dirtyBySource[src.settingsSource] ?? {}; + Object.assign( + merged, + buildSdkSettingsPayload(src.filteredSchema, sourceValues, sourceDirty), + ); + } + return merged; + }; - // Surface save state to the parent. Hooks must run before any - // conditional early-returns below, so this lives here rather than - // alongside the JSX. The dependency list deliberately excludes - // `stableSave` (it never changes) and `onSaveControlChange` (we - // tolerate ref-instability of the callback to avoid spamming the - // parent on every render). - const saveControlIsDirty = Object.keys(dirty).length > 0 || extraDirty; + const isDirty = Object.keys(flatDirty).length > 0; + const saveControlIsDirty = isDirty || extraDirty; React.useEffect(() => { if (!onSaveControlChange) return; onSaveControlChange({ save: stableSave, isSaving: isPending, isDirty: saveControlIsDirty, - values, + values: flatValues, view, getDirtyPayload: stableGetDirtyPayload, }); - }, [isPending, saveControlIsDirty, values, view]); + }, [isPending, saveControlIsDirty, flatValues, view]); // Keep existing form content visible during background refetches to avoid // flashing the full skeleton (notably during onboarding Next transitions). @@ -461,7 +593,11 @@ export function SdkSectionPage({ return ; } - if (!filteredSchema || filteredSchema.sections.length === 0) { + const hasAnyVisibleSection = resolvedSources.some( + (src) => src.filteredSchema && src.filteredSchema.sections.length > 0, + ); + + if (!hasAnyVisibleSection) { return ( {schemaUnavailableMessage} @@ -469,7 +605,9 @@ export function SdkSectionPage({ ); } - if (Object.keys(values).length === 0) return ; + if (Object.keys(flatValues).length === 0) { + return ; + } // Scrolling is owned by the settings shell (or onboarding wrapper), not a // nested scroll region. Save actions are inline after the last field. @@ -494,32 +632,50 @@ export function SdkSectionPage({
{header?.({ - values, + values: flatValues, isDisabled: isReadOnly, view, onChange: handleFieldChange, })} - {visibleSections.map((section, sectionIndex) => ( -
-
- {section.fields.map((field) => ( - - handleFieldChange(field.key, nextValue) - } - /> - ))} -
-
- ))} + {resolvedSources.map((src) => { + if (!src.filteredSchema) return null; + const sourceValues = valuesBySource[src.settingsSource] ?? {}; + const visibleSections = getVisibleSettingsSections( + src.filteredSchema, + { ...flatValues, ...sourceValues }, + view, + src.excludeKeys ?? EMPTY_EXCLUDE_KEYS, + ); + return visibleSections.map((section) => ( +
+
+ {section.fields.map((field) => ( +
+ + handleFieldChange(field.key, nextValue) + } + /> +
+ ))} +
+
+ )); + })} {!isReadOnly && !hideSaveButton ? (
@@ -527,9 +683,7 @@ export function SdkSectionPage({ testId="save-button" type="button" variant="primary" - isDisabled={ - isPending || (Object.keys(dirty).length === 0 && !extraDirty) - } + isDisabled={isPending || (!isDirty && !extraDirty)} onClick={handleSave} > {isPending diff --git a/src/i18n/translation.json b/src/i18n/translation.json index b7a1f350d3..ee8510b883 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -4555,6 +4555,74 @@ "uk": "Режим підтвердження", "ca": "Mode de confirmació" }, + "SCHEMA$VERIFICATION$CRITIC_API_KEY$DESCRIPTION": { + "en": "If OpenHands is selected as your active LLM provider, leave this empty; the critic reuses that provider's key. Otherwise, provide a critic API key from the link below.", + "ja": "批評者がLLMを呼び出すために使用する任意のAPIキー。空欄の場合、批評者はLLM APIキーを再利用します。", + "zh-CN": "批评器调用其 LLM 所使用的可选 API 密钥。留空时，批评器会复用 LLM API 密钥。", + "zh-TW": "批評器呼叫其 LLM 所使用的選用 API 金鑰。留空時，批評器會重用 LLM API 金鑰。", + "ko-KR": "비평가가 LLM을 호출할 때 사용하는 선택적 API 키입니다. 비워 두면 비평가는 LLM API 키를 다시 사용합니다.", + "no": "Valgfri API-nøkkel kritikeren bruker for å kalle LLM-en. Hvis den er tom, gjenbruker kritikeren LLM-API-nøkkelen.", + "it": "Chiave API facoltativa che il critico usa per chiamare il suo LLM. Se vuota, il critico riutilizza la chiave API LLM.", + "pt": "Chave de API opcional que o crítico usa para chamar seu LLM. Se ficar em branco, o crítico reutiliza a chave de API do LLM.", + "es": "Clave de API opcional que el crítico usa para llamar a su LLM. Si está vacía, el crítico reutiliza la clave de API del LLM.", + "ar": "مفتاح API اختياري يستخدمه الناقد لاستدعاء نموذج اللغة الكبير. إذا تُرك فارغًا، فسيعيد الناقد استخدام مفتاح API الخاص بنموذج اللغة الكبير.", + "fr": "Clé API facultative que le critique utilise pour appeler son LLM. Si elle est vide, le critique réutilise la clé API du LLM.", + "tr": "Eleştirmenin LLM'sini çağırmak için kullandığı isteğe bağlı API anahtarı. Boş bırakılırsa eleştirmen LLM API anahtarını yeniden kullanır.", + "de": "Optionaler API-Schlüssel, den der Kritiker verwendet, um sein LLM aufzurufen. Bleibt er leer, verwendet der Kritiker den LLM-API-Schlüssel erneut.", + "uk": "Необов'язковий ключ API, який критик використовує для виклику свого LLM. Якщо залишити поле порожнім, критик повторно використає ключ API LLM.", + "ca": "Clau d'API opcional que el crític utilitza per cridar el seu LLM. Si es deixa en blanc, el crític reutilitza la clau d'API de l'LLM." + }, + "SCHEMA$VERIFICATION$CRITIC_API_KEY$HELP_TEXT": { + "en": "If OpenHands is selected as your active LLM provider, leave this empty because the Critic API Key is the same as your OpenHands Provider LLM Key, which you can find in the", + "ja": "アクティブなLLMプロバイダーとしてOpenHandsを選択している場合、この項目は空欄のままにしてください。Critic API Key は OpenHands Provider LLM Key と同じキーで、", + "zh-CN": "如果已选择 OpenHands 作为当前使用的 LLM 提供商，请将此处留空，因为 Critic API Key 和 OpenHands Provider LLM Key 是同一个密钥，可在", + "zh-TW": "如果已選擇 OpenHands 作為目前使用的 LLM 提供者，請將此處留空，因為 Critic API Key 和 OpenHands Provider LLM Key 是同一個金鑰，可在", + "ko-KR": "OpenHands를 활성 LLM 제공자로 선택한 경우 이 필드를 비워 두세요. Critic API Key는 OpenHands Provider LLM Key와 같은 키이며,", + "no": "Hvis OpenHands er valgt som aktiv LLM-leverandør, lar du dette stå tomt fordi Critic API Key er den samme som OpenHands Provider LLM Key, som du finner i", + "it": "Se OpenHands è selezionato come provider LLM attivo, lascia vuoto questo campo perché Critic API Key è la stessa chiave di OpenHands Provider LLM Key, che puoi trovare nella", + "pt": "Se o OpenHands estiver selecionado como provedor LLM ativo, deixe este campo em branco porque a Critic API Key é a mesma chave que a OpenHands Provider LLM Key, que você encontra na", + "es": "Si OpenHands está seleccionado como proveedor LLM activo, deja esto vacío porque Critic API Key es la misma clave que OpenHands Provider LLM Key, que puedes encontrar en la", + "ar": "إذا كان OpenHands محددًا كمزوّد LLM النشط، فاترك هذا الحقل فارغًا لأن Critic API Key هو نفس مفتاح OpenHands Provider LLM Key، ويمكنك العثور عليه في", + "fr": "Si OpenHands est sélectionné comme fournisseur LLM actif, laissez ce champ vide, car Critic API Key est la même clé que OpenHands Provider LLM Key, que vous pouvez trouver dans", + "tr": "Etkin LLM sağlayıcısı olarak OpenHands seçiliyse burayı boş bırakın; Critic API Key, OpenHands Provider LLM Key ile aynı anahtardır ve", + "de": "Wenn OpenHands als aktiver LLM-Anbieter ausgewählt ist, lassen Sie dieses Feld leer, da der Critic API Key derselbe Schlüssel wie der OpenHands Provider LLM Key ist, den Sie im", + "uk": "Якщо OpenHands вибрано як активного постачальника LLM, залиште це поле порожнім, оскільки Critic API Key є тим самим ключем, що й OpenHands Provider LLM Key, який можна знайти у", + "ca": "Si OpenHands està seleccionat com a proveïdor LLM actiu, deixeu aquest camp buit perquè Critic API Key és la mateixa clau que OpenHands Provider LLM Key, que podeu trobar a la" + }, + "SCHEMA$VERIFICATION$CRITIC_API_KEY$HELP_SUFFIX": { + "en": "tab of OpenHands Cloud; otherwise, enter a Critic API Key from that page.", + "ja": "タブで確認できます。それ以外の場合は、そのページの Critic API Key を入力してください。", + "zh-CN": "标签页中找到；否则，请从该页面输入 Critic API Key。", + "zh-TW": "標籤頁中找到；否則，請從該頁面輸入 Critic API Key。", + "ko-KR": "탭에서 찾을 수 있습니다. 그렇지 않으면 해당 페이지의 Critic API Key를 입력하세요.", + "no": "-fanen i OpenHands Cloud; ellers skriver du inn en Critic API Key fra den siden.", + "it": "scheda di OpenHands Cloud; altrimenti inserisci una Critic API Key da quella pagina.", + "pt": "guia do OpenHands Cloud; caso contrário, insira uma Critic API Key dessa página.", + "es": "pestaña de OpenHands Cloud; de lo contrario, introduce una Critic API Key desde esa página.", + "ar": "علامة التبويب في OpenHands Cloud؛ وإلا فأدخل Critic API Key من تلك الصفحة.", + "fr": "l'onglet d'OpenHands Cloud ; sinon, saisissez une Critic API Key depuis cette page.", + "tr": "sekmesinde bulabilirsiniz; aksi takdirde o sayfadan bir Critic API Key girin.", + "de": "Tab von OpenHands Cloud finden; andernfalls geben Sie einen Critic API Key von dieser Seite ein.", + "uk": "вкладці OpenHands Cloud; інакше введіть Critic API Key з цієї сторінки.", + "ca": "pestanya d'OpenHands Cloud; si no, introduïu una Critic API Key des d'aquesta pàgina." + }, + "SCHEMA$VERIFICATION$CRITIC_API_KEY$LABEL": { + "en": "Critic API Key", + "ja": "批評者APIキー", + "zh-CN": "批评器 API 密钥", + "zh-TW": "批評器 API 金鑰", + "ko-KR": "비평가 API 키", + "no": "API-nøkkel for kritiker", + "it": "Chiave API del critico", + "pt": "Chave de API do crítico", + "es": "Clave de API del crítico", + "ar": "مفتاح API للناقد", + "fr": "Clé API du critique", + "tr": "Eleştirmen API Anahtarı", + "de": "Kritiker-API-Schlüssel", + "uk": "Ключ API критика", + "ca": "Clau d'API del crític" + }, "SCHEMA$VERIFICATION$CRITIC_ENABLED$DESCRIPTION": { "en": "Enable critic evaluation for the agent.", "ja": "エージェントの批評評価を有効にする。", @@ -11084,44 +11152,44 @@ "ca": "Les claus d'API us permeten autenticar-vos amb l'API d'OpenHands de manera programàtica. Manteniu les vostres claus d'API segures; qualsevol persona amb la vostra clau d'API pot accedir al vostre compte. Per obtenir més informació sobre com fer servir l'API, consulteu la nostra documentació de l'API." }, "SETTINGS$OPENHANDS_API_KEY_HELP": { - "en": "You can find your OpenHands API Key in the API Keys tab of OpenHands Cloud.", - "ja": "OpenHands APIキーはOpenHands CloudのAPIキータブで確認できます。", - "zh-CN": "您可以在OpenHands Cloud的API密钥标签页中找到您的OpenHands API密钥。", - "zh-TW": "您可以在OpenHands Cloud的API密鑰標籤頁中找到您的OpenHands API密鑰。", - "ko-KR": "OpenHands API 키는 OpenHands Cloud의 API 키 탭에서 찾을 수 있습니다.", - "no": "Du kan finne din OpenHands API-nøkkel i API-nøkler-fanen i OpenHands Cloud.", - "it": "Puoi trovare la tua chiave API OpenHands nella scheda Chiavi API di OpenHands Cloud.", - "pt": "Você pode encontrar sua chave de API OpenHands na guia Chaves de API do OpenHands Cloud.", - "es": "Puede encontrar su clave API de OpenHands en la pestaña Claves API de OpenHands Cloud.", - "ar": "يمكنك العثور على مفتاح API الخاص بـ OpenHands في علامة التبويب مفاتيح API في OpenHands Cloud.", - "fr": "Vous pouvez trouver votre clé API OpenHands dans l'onglet Clés API d'OpenHands Cloud.", - "tr": "OpenHands API Anahtarınızı OpenHands Cloud'un API Anahtarları sekmesinde bulabilirsiniz.", - "de": "Sie finden Ihren OpenHands API-Schlüssel im Tab API-Schlüssel von OpenHands Cloud.", - "uk": "Ви можете знайти свій ключ API OpenHands у вкладці Ключі API OpenHands Cloud.", - "ca": "Podeu trobar la vostra clau d'API d'OpenHands a la pestanya Claus d'API d'OpenHands Cloud." + "en": "You can find your OpenHands Provider LLM Key in the API Keys tab of OpenHands Cloud.", + "ja": "OpenHands Provider LLM Key はOpenHands CloudのAPIキータブで確認できます。", + "zh-CN": "您可以在OpenHands Cloud的API密钥标签页中找到您的 OpenHands Provider LLM Key。", + "zh-TW": "您可以在OpenHands Cloud的API密鑰標籤頁中找到您的 OpenHands Provider LLM Key。", + "ko-KR": "OpenHands Provider LLM Key는 OpenHands Cloud의 API 키 탭에서 찾을 수 있습니다.", + "no": "Du kan finne din OpenHands Provider LLM Key i API-nøkler-fanen i OpenHands Cloud.", + "it": "Puoi trovare la tua OpenHands Provider LLM Key nella scheda Chiavi API di OpenHands Cloud.", + "pt": "Você pode encontrar sua OpenHands Provider LLM Key na guia Chaves de API do OpenHands Cloud.", + "es": "Puede encontrar su OpenHands Provider LLM Key en la pestaña Claves API de OpenHands Cloud.", + "ar": "يمكنك العثور على OpenHands Provider LLM Key في علامة التبويب مفاتيح API في OpenHands Cloud.", + "fr": "Vous pouvez trouver votre OpenHands Provider LLM Key dans l'onglet Clés API d'OpenHands Cloud.", + "tr": "OpenHands Provider LLM Key'inizi OpenHands Cloud'un API Anahtarları sekmesinde bulabilirsiniz.", + "de": "Sie finden Ihren OpenHands Provider LLM Key im Tab API-Schlüssel von OpenHands Cloud.", + "uk": "Ви можете знайти свій OpenHands Provider LLM Key у вкладці Ключі API OpenHands Cloud.", + "ca": "Podeu trobar la vostra OpenHands Provider LLM Key a la pestanya Claus d'API d'OpenHands Cloud." }, "SETTINGS$OPENHANDS_API_KEY_HELP_TEXT": { - "en": "You can find your OpenHands API Key in the", - "ja": "OpenHands APIキーは", + "en": "You can find your OpenHands Provider LLM Key in the", + "ja": "OpenHands Provider LLM Key は", "zh-CN": "您可以在", "zh-TW": "您可以在", - "ko-KR": "OpenHands API 키는", - "no": "Du kan finne din OpenHands API-nøkkel i", - "it": "Puoi trovare la tua chiave API OpenHands nella", - "pt": "Você pode encontrar sua chave de API OpenHands na", - "es": "Puede encontrar su clave API de OpenHands en la", - "ar": "يمكنك العثور على مفتاح API الخاص بـ OpenHands في", - "fr": "Vous pouvez trouver votre clé API OpenHands dans", - "tr": "OpenHands API Anahtarınızı", - "de": "Sie finden Ihren OpenHands API-Schlüssel im", - "uk": "Ви можете знайти свій ключ API OpenHands у", - "ca": "Podeu trobar la vostra clau d'API d'OpenHands a la" + "ko-KR": "OpenHands Provider LLM Key는", + "no": "Du kan finne din OpenHands Provider LLM Key i", + "it": "Puoi trovare la tua OpenHands Provider LLM Key nella", + "pt": "Você pode encontrar sua OpenHands Provider LLM Key na", + "es": "Puede encontrar su OpenHands Provider LLM Key en la", + "ar": "يمكنك العثور على OpenHands Provider LLM Key في", + "fr": "Vous pouvez trouver votre OpenHands Provider LLM Key dans", + "tr": "OpenHands Provider LLM Key'inizi", + "de": "Sie finden Ihren OpenHands Provider LLM Key im", + "uk": "Ви можете знайти свій OpenHands Provider LLM Key у", + "ca": "Podeu trobar la vostra OpenHands Provider LLM Key a la" }, "SETTINGS$OPENHANDS_API_KEY_HELP_SUFFIX": { "en": "tab of OpenHands Cloud.", "ja": "タブで確認できます。", - "zh-CN": "标签页中找到您的OpenHands API密钥。", - "zh-TW": "標籤頁中找到您的OpenHands API密鑰。", + "zh-CN": "标签页中找到您的 OpenHands Provider LLM Key。", + "zh-TW": "標籤頁中找到您的 OpenHands Provider LLM Key。", "ko-KR": "탭에서 찾을 수 있습니다.", "no": "-fanen i OpenHands Cloud.", "it": "scheda di OpenHands Cloud.", @@ -29689,5 +29757,192 @@ "ca": "Desat, però aquest backend encara no pot fer servir aquestes credencials: s'aplicaran quan admeti credencials de fitxer ACP.", "tr": "Kaydedildi, ancak bu arka uç bu kimlik bilgilerini henüz kullanamıyor — ACP dosya kimlik bilgilerini desteklediğinde geçerli olacaklar.", "uk": "Збережено, але цей бекенд поки не може використовувати ці облікові дані — вони застосуються, щойно він підтримає файлові облікові дані ACP." + }, + "SETTINGS$AGENT": { + "en": "Agent", + "ja": "エージェント", + "zh-CN": "代理", + "zh-TW": "代理", + "ko-KR": "에이전트", + "no": "Agent", + "it": "Agente", + "pt": "Agente", + "es": "Agente", + "ar": "وكيل", + "fr": "Agent", + "tr": "Ajan", + "de": "Agent", + "uk": "Агент", + "ca": "Agent" + }, + "CONVERSATION$CONFIRM_DELETE_OLDER_TITLE": { + "en": "Delete older conversations", + "ja": "古い会話を削除", + "zh-CN": "删除较早的会话", + "zh-TW": "刪除較舊的對話", + "ko-KR": "이전 대화 삭제", + "no": "Slett eldre samtaler", + "ar": "حذف المحادثات الأقدم", + "de": "Ältere Unterhaltungen löschen", + "fr": "Supprimer les anciennes conversations", + "it": "Elimina conversazioni più vecchie", + "pt": "Excluir conversas mais antigas", + "es": "Eliminar conversaciones anteriores", + "ca": "Esborra les converses anteriors", + "tr": "Eski sohbetleri sil", + "uk": "Видалити старіші розмови" + }, + "CONVERSATION$CONFIRM_DELETE_OLDER_DESC": { + "en": "Are you sure you want to delete {{count}} older conversations? This action cannot be undone.", + "ja": "{{count}} 件の古い会話を削除してもよろしいですか？この操作は取り消せません。", + "zh-CN": "确定要删除 {{count}} 条较早的会话吗？此操作无法撤销。", + "zh-TW": "確定要刪除 {{count}} 則較舊的對話嗎？此動作無法復原。", + "ko-KR": "이전 대화 {{count}}개를 삭제하시겠습니까? 이 작업은 되돌릴 수 없습니다.", + "no": "Er du sikker på at du vil slette {{count}} eldre samtaler? Denne handlingen kan ikke angres.", + "ar": "هل أنت متأكد من حذف {{count}} محادثة أقدم؟ لا يمكن التراجع عن هذا الإجراء.", + "de": "Möchten Sie wirklich {{count}} ältere Unterhaltungen löschen? Diese Aktion kann nicht rückgängig gemacht werden.", + "fr": "Voulez-vous vraiment supprimer {{count}} anciennes conversations ? Cette action est irréversible.", + "it": "Sei sicuro di voler eliminare {{count}} conversazioni più vecchie? Questa azione non può essere annullata.", + "pt": "Tem certeza de que deseja excluir {{count}} conversas mais antigas? Esta ação não pode ser desfeita.", + "es": "¿Seguro que quieres eliminar {{count}} conversaciones anteriores? Esta acción no se puede deshacer.", + "ca": "Segur que voleu esborrar {{count}} converses anteriors? Aquesta acció no es pot desfer.", + "tr": "{{count}} eski sohbeti silmek istediğinize emin misiniz? Bu işlem geri alınamaz.", + "uk": "Видалити {{count}} старіших розмов? Цю дію не можна скасувати." + }, + "HOOKS_MODAL$HOOK_COUNT": { + "en": "{{count}} hook(s)", + "ja": "{{count}}個のフック", + "zh-CN": "{{count}}个钩子", + "zh-TW": "{{count}}個鉤子", + "ko-KR": "{{count}}개 훅", + "no": "{{count}} krok", + "ar": "{{count}} خطاف", + "de": "{{count}} Hook", + "fr": "{{count}} hook", + "it": "{{count}} hook", + "pt": "{{count}} hook", + "es": "{{count}} hook", + "tr": "{{count}} kanca", + "uk": "{{count}} хук", + "ca": "{{count}} hook(s)" + }, + "ONBOARDING$AGENT_COMING_SOON": { + "en": "Support for other agents coming soon!", + "ja": "他のエージェントのサポートは近日公開予定です！", + "zh-CN": "更多智能体支持即将上线！", + "zh-TW": "更多代理支援即將推出！", + "ko-KR": "다른 에이전트 지원이 곧 추가됩니다!", + "no": "Støtte for andre agenter kommer snart!", + "ar": "دعم وكلاء آخرين قريبًا!", + "de": "Unterstützung für weitere Agenten folgt in Kürze!", + "fr": "Prise en charge d'autres agents bientôt disponible !", + "it": "Supporto per altri agenti in arrivo!", + "pt": "Suporte a outros agentes em breve!", + "es": "¡Pronto se admitirán más agentes!", + "ca": "Aviat hi haurà compatibilitat amb més agents!", + "tr": "Diğer araçlar için destek yakında geliyor!", + "uk": "Підтримка інших агентів незабаром!" + }, + "CRITIC$SUCCESS_LIKELIHOOD_LABEL": { + "en": "Critic: agent success likelihood", + "ja": "クリティック: エージェント成功確率", + "zh-CN": "评估: 代理成功概率", + "zh-TW": "評估: 代理成功機率", + "ko-KR": "평가: 에이전트 성공 가능성", + "no": "Kritiker: sannsynlighet for agentsuksess", + "it": "Critico: probabilità di successo dell'agente", + "pt": "Crítico: probabilidade de sucesso do agente", + "es": "Crítico: probabilidad de éxito del agente", + "ar": "الناقد: احتمالية نجاح الوكيل", + "fr": "Critique : probabilité de succès de l'agent", + "tr": "Eleştirmen: ajan başarı olasılığı", + "de": "Kritiker: Erfolgswahrscheinlichkeit des Agenten", + "uk": "Критик: ймовірність успіху агента", + "ca": "Crític: probabilitat d'èxit de l'agent" + }, + "CRITIC$ITERATIVE_REFINEMENT_HINT": { + "en": "Want the agent to automatically fix potential issues? Enable iterative refinement in settings.", + "ja": "エージェントに潜在的な問題を自動修正させますか？設定で反復改善を有効にしてください。", + "zh-CN": "想让代理自动修复可能出现的问题？请在设置里开启迭代优化。", + "zh-TW": "想讓代理自動修復可能出現的問題？請在設定中開啟迭代最佳化。", + "ko-KR": "에이전트가 잠재적인 문제를 자동으로 수정하게 할까요? 설정에서 반복 개선을 켜세요.", + "no": "Vil du at agenten automatisk skal fikse mulige problemer? Slå på iterativ forbedring i innstillingene.", + "it": "Vuoi che l'agente corregga automaticamente i potenziali problemi? Attiva il perfezionamento iterativo nelle impostazioni.", + "pt": "Quer que o agente corrija automaticamente possíveis problemas? Ative o refinamento iterativo nas configurações.", + "es": "¿Quieres que el agente corrija automáticamente posibles problemas? Activa el refinamiento iterativo en la configuración.", + "ar": "هل تريد أن يصلح الوكيل المشكلات المحتملة تلقائيًا؟ فعّل التحسين التكراري من الإعدادات.", + "fr": "Vous voulez que l'agent corrige automatiquement les problèmes possibles ? Activez l'affinage itératif dans les paramètres.", + "tr": "Aracının olası sorunları otomatik olarak düzeltmesini ister misiniz? Ayarlardan yinelemeli iyileştirmeyi etkinleştirin.", + "de": "Soll der Agent mögliche Probleme automatisch beheben? Aktiviere iterative Verfeinerung in den Einstellungen.", + "uk": "Хочете, щоб агент автоматично виправляв можливі проблеми? Увімкніть ітеративне вдосконалення в налаштуваннях.", + "ca": "Vols que l'agent corregeixi automàticament possibles problemes? Activa el refinament iteratiu a la configuració." + }, + "CRITIC$POTENTIAL_ISSUES": { + "en": "Potential Issues:", + "ja": "潜在的な問題:", + "zh-CN": "潜在问题:", + "zh-TW": "潛在問題:", + "ko-KR": "잠재적 문제:", + "no": "Potensielle problemer:", + "it": "Problemi potenziali:", + "pt": "Problemas potenciais:", + "es": "Problemas potenciales:", + "ar": "مشاكل محتملة:", + "fr": "Problèmes potentiels :", + "tr": "Olası sorunlar:", + "de": "Mögliche Probleme:", + "uk": "Потенційні проблеми:", + "ca": "Problemes potencials:" + }, + "CRITIC$INFRASTRUCTURE": { + "en": "Infrastructure:", + "ja": "インフラストラクチャ:", + "zh-CN": "基础设施:", + "zh-TW": "基礎設施:", + "ko-KR": "인프라:", + "no": "Infrastruktur:", + "it": "Infrastruttura:", + "pt": "Infraestrutura:", + "es": "Infraestructura:", + "ar": "البنية التحتية:", + "fr": "Infrastructure :", + "tr": "Altyapı:", + "de": "Infrastruktur:", + "uk": "Інфраструктура:", + "ca": "Infraestructura:" + }, + "CRITIC$LIKELY_FOLLOWUP": { + "en": "Likely Follow-up:", + "ja": "予想されるフォローアップ:", + "zh-CN": "可能的后续操作:", + "zh-TW": "可能的後續操作:", + "ko-KR": "예상 후속 조치:", + "no": "Sannsynlig oppfølging:", + "it": "Probabile seguito:", + "pt": "Provável acompanhamento:", + "es": "Probable seguimiento:", + "ar": "متابعة محتملة:", + "fr": "Suivi probable :", + "tr": "Olası takip:", + "de": "Wahrscheinliche Nachverfolgung:", + "uk": "Ймовірне продовження:", + "ca": "Probable seguiment:" + }, + "CRITIC$OTHER": { + "en": "Other:", + "ja": "その他:", + "zh-CN": "其他:", + "zh-TW": "其他:", + "ko-KR": "기타:", + "no": "Annet:", + "it": "Altro:", + "pt": "Outros:", + "es": "Otros:", + "ar": "أخرى:", + "fr": "Autre :", + "tr": "Diğer:", + "de": "Sonstiges:", + "uk": "Інше:", + "ca": "Altres:" } } diff --git a/src/mocks/settings-handlers.ts b/src/mocks/settings-handlers.ts index 964af34fc0..f1a3491ff2 100644 --- a/src/mocks/settings-handlers.ts +++ b/src/mocks/settings-handlers.ts @@ -150,42 +150,140 @@ const MOCK_AGENT_SETTINGS_SCHEMA: NonNullable< ], }, { - key: "critic", - label: "Critic", + key: "verification", + label: "Verification", fields: [ { + key: "verification.critic_enabled", + label: "Enable Critic", description: "Enable an additional critic pass to review the agent's work.", - - key: "critic.enabled", - label: "Enable critic", - section: "critic", - section_label: "Critic", + section: "verification", + section_label: "Verification", value_type: "boolean", default: false, choices: [], depends_on: [], prominence: "critical", secret: false, - required: true, + required: false, }, { + key: "verification.critic_mode", + label: "Critic Mode", description: "Choose when the critic should review and intervene.", - - key: "critic.mode", - label: "Mode", - section: "critic", - section_label: "Critic", + section: "verification", + section_label: "Verification", value_type: "string", default: "finish_and_message", choices: [ - { label: "finish_and_message", value: "finish_and_message" }, + { + label: "finish_and_message", + value: "finish_and_message", + }, { label: "all_actions", value: "all_actions" }, ], - depends_on: ["critic.enabled"], + depends_on: ["verification.critic_enabled"], + prominence: "major", + secret: false, + required: false, + }, + { + key: "verification.enable_iterative_refinement", + label: "Enable Iterative Refinement", + description: + "Let the critic send the agent back to refine its work when issues are found.", + section: "verification", + section_label: "Verification", + value_type: "boolean", + default: false, + choices: [], + depends_on: ["verification.critic_enabled"], + prominence: "critical", + secret: false, + required: false, + }, + // Rendered as a full-width row (see FIELD_FULL_WIDTH_KEYS) below the + // two critical-prominence toggles so the input + OpenHands Cloud help + // link have room to breathe. + { + key: "verification.critic_api_key", + label: "Critic API Key", + description: + "If OpenHands is selected as your active LLM provider, leave this empty; the critic reuses the OpenHands Provider LLM Key.", + section: "verification", + section_label: "Verification", + value_type: "string", + default: null, + choices: [], + depends_on: ["verification.critic_enabled"], + prominence: "critical", + secret: true, + required: false, + }, + { + key: "verification.critic_threshold", + label: "Critic Threshold", + description: + "Critic success threshold used for iterative refinement.", + section: "verification", + section_label: "Verification", + value_type: "number", + default: 0.6, + choices: [], + depends_on: [ + "verification.critic_enabled", + "verification.enable_iterative_refinement", + ], prominence: "minor", secret: false, - required: true, + required: false, + }, + { + key: "verification.max_refinement_iterations", + label: "Max Refinement Iterations", + description: + "Maximum number of refinement attempts after critic feedback.", + section: "verification", + section_label: "Verification", + value_type: "integer", + default: 3, + choices: [], + depends_on: [ + "verification.critic_enabled", + "verification.enable_iterative_refinement", + ], + prominence: "minor", + secret: false, + required: false, + }, + { + key: "verification.critic_server_url", + label: "Critic Server URL", + description: "Override the critic service URL.", + section: "verification", + section_label: "Verification", + value_type: "string", + default: null, + choices: [], + depends_on: ["verification.critic_enabled"], + prominence: "minor", + secret: false, + required: false, + }, + { + key: "verification.critic_model_name", + label: "Critic Model Name", + description: "Override the critic model name.", + section: "verification", + section_label: "Verification", + value_type: "string", + default: null, + choices: [], + depends_on: ["verification.critic_enabled"], + prominence: "minor", + secret: false, + required: false, }, ], }, @@ -305,9 +403,9 @@ export const MOCK_DEFAULT_USER_SETTINGS: Settings = { agent_settings_schema: MOCK_AGENT_SETTINGS_SCHEMA, agent_settings: { ...DEFAULT_AGENT_SETTINGS, - critic: { - mode: "finish_and_message", - enabled: false, + verification: { + critic_enabled: false, + enable_iterative_refinement: false, }, llm: { ...(llmDefaults ?? {}), diff --git a/src/routes/condenser-settings.tsx b/src/routes/condenser-settings.tsx index 3747d73cc2..b81d52d862 100644 --- a/src/routes/condenser-settings.tsx +++ b/src/routes/condenser-settings.tsx @@ -3,7 +3,9 @@ import { SdkSectionPage } from "#/components/features/settings/sdk-settings/sdk- function CondenserSettingsScreen() { return ( ); diff --git a/src/routes/llm-settings.tsx b/src/routes/llm-settings.tsx index f45c01de4f..5bf21bd0e7 100644 --- a/src/routes/llm-settings.tsx +++ b/src/routes/llm-settings.tsx @@ -273,14 +273,18 @@ export function LlmSettingsScreen({ const buildPayload = React.useCallback( ( - basePayload: Record, + defaultPayload: Record, context: { values: Record; view: SettingsView; }, ) => { - // basePayload is a nested dict (e.g. {llm: {model: "gpt-4"}}) - const agentSettings = structuredClone(basePayload); + // defaultPayload is the wrapped diff (e.g. + // `{ agent_settings_diff: { llm: { model: "gpt-4" } } }`); we only + // mutate the inner `llm` object below. + const agentSettings = structuredClone( + (defaultPayload.agent_settings_diff as Record) ?? {}, + ); const llm = (agentSettings.llm ?? {}) as Record; @@ -301,8 +305,13 @@ export function LlmSettingsScreen({ return ( void; - onSecurityAnalyzerChange: (value: string | null) => void; - renderTopContent?: () => React.ReactNode; -}) { - const { t } = useTranslation("openhands"); - - const securityAnalyzerItems = React.useMemo( - () => [ - { - key: "llm", - label: t(I18nKey.SETTINGS$SECURITY_ANALYZER_LLM_DEFAULT), - }, - { - key: "none", - label: t(I18nKey.SETTINGS$SECURITY_ANALYZER_NONE), - }, - ], - [t], - ); - - const showSecurityAnalyzer = confirmationMode; - - return ( -
- {renderTopContent?.()} - -
-
- - {t(I18nKey.SETTINGS_FORM$ENABLE_CONFIRMATION_MODE_LABEL)} - -

- {t(I18nKey.SETTINGS$CONFIRMATION_MODE_TOOLTIP)} -

-
- - {showSecurityAnalyzer ? ( -
- - onSecurityAnalyzerChange(key ? String(key) : null) - } - /> -

- {t(I18nKey.SETTINGS$SECURITY_ANALYZER_DESCRIPTION)} -

-
- ) : null} -
-
- ); -} - export function VerificationSettingsScreen({ scope = "personal", renderTopContent, @@ -108,87 +20,21 @@ export function VerificationSettingsScreen({ renderTopContent?: () => React.ReactNode; testId?: string; }) { - const { data: settings } = useSettings(scope); - const [confirmationMode, setConfirmationMode] = React.useState( - DEFAULT_SETTINGS.confirmation_mode, - ); - const [securityAnalyzer, setSecurityAnalyzer] = React.useState( - DEFAULT_SETTINGS.security_analyzer, - ); - const [confirmationModeDirty, setConfirmationModeDirty] = - React.useState(false); - const [securityAnalyzerDirty, setSecurityAnalyzerDirty] = - React.useState(false); - - React.useEffect(() => { - setConfirmationMode( - settings?.confirmation_mode ?? DEFAULT_SETTINGS.confirmation_mode, - ); - setSecurityAnalyzer( - settings?.security_analyzer ?? DEFAULT_SETTINGS.security_analyzer, - ); - setConfirmationModeDirty(false); - setSecurityAnalyzerDirty(false); - }, [settings?.confirmation_mode, settings?.security_analyzer]); - - const buildHeader = React.useCallback( - ({ isDisabled }: SdkSectionHeaderProps) => ( - { - setConfirmationMode(value); - setConfirmationModeDirty(true); - }} - onSecurityAnalyzerChange={(value) => { - setSecurityAnalyzer(value); - setSecurityAnalyzerDirty(true); - }} - renderTopContent={renderTopContent} - /> - ), - [confirmationMode, renderTopContent, securityAnalyzer], - ); - - const buildPayload = React.useCallback( - (basePayload: Record) => { - const payload = { ...basePayload }; - - if (confirmationModeDirty) { - payload.confirmation_mode = confirmationMode; - } - if ( - securityAnalyzerDirty || - (confirmationMode && settings?.security_analyzer !== securityAnalyzer) - ) { - payload.security_analyzer = securityAnalyzer; - } - - return { conversation_settings_diff: payload }; - }, - [ - confirmationMode, - confirmationModeDirty, - securityAnalyzer, - securityAnalyzerDirty, - settings?.security_analyzer, - ], - ); - return ( { - setConfirmationModeDirty(false); - setSecurityAnalyzerDirty(false); - }} + settingsSources={[ + { + settingsSource: "conversation_settings", + sectionKeys: ["verification"], + }, + { + settingsSource: "agent_settings", + sectionKeys: ["verification"], + excludeKeys: CONVERSATION_OWNED_AGENT_VERIFICATION_FIELD_KEYS, + }, + ]} + header={renderTopContent ? () => renderTopContent() : undefined} testId={testId} /> ); diff --git a/src/services/settings.ts b/src/services/settings.ts index 4671a2174d..bf6da29352 100644 --- a/src/services/settings.ts +++ b/src/services/settings.ts @@ -46,6 +46,10 @@ export const DEFAULT_SETTINGS: Settings = { enabled: true, max_size: 240, }, + verification: { + critic_enabled: false, + enable_iterative_refinement: false, + }, enable_sub_agents: false, mcp_config: { sse_servers: [], diff --git a/src/types/agent-server/core/base/critic.ts b/src/types/agent-server/core/base/critic.ts new file mode 100644 index 0000000000..da60cea06f --- /dev/null +++ b/src/types/agent-server/core/base/critic.ts @@ -0,0 +1,50 @@ +/** + * A single feature detected by the critic (e.g., "Insufficient Testing"). + */ +export interface CriticFeature { + /** Internal feature name (e.g., "insufficient_testing") */ + name: string; + /** Human-readable display name (e.g., "Insufficient Testing") */ + display_name: string; + /** Probability of this feature being present (0-1) */ + probability: number; +} + +/** + * Categorized features from the critic evaluation. + */ +export interface CriticCategorizedFeatures { + /** Agent behavioral issues (e.g., insufficient testing, loop behavior) */ + agent_behavioral_issues?: CriticFeature[]; + /** Likely user follow-up patterns */ + user_followup_patterns?: CriticFeature[]; + /** Infrastructure-related issues */ + infrastructure_issues?: CriticFeature[]; + /** Other uncategorized metrics */ + other?: CriticFeature[]; +} + +/** + * Metadata from a critic evaluation, including categorized features + * and event IDs for reproducibility. + */ +export interface CriticMetadata { + categorized_features?: CriticCategorizedFeatures; + event_ids?: string[]; + [key: string]: unknown; +} + +/** + * Result of a critic evaluation on an agent's actions. + * + * The critic predicts the probability that the agent has successfully + * completed the task. + */ +export interface CriticResult { + /** Predicted probability of success (0-1) */ + score: number; + /** Optional message explaining the score */ + message: string | null; + /** Optional metadata with categorized features and event IDs */ + metadata: CriticMetadata | null; +} diff --git a/src/types/agent-server/core/base/index.ts b/src/types/agent-server/core/base/index.ts index 07292f3ed7..6053554e30 100644 --- a/src/types/agent-server/core/base/index.ts +++ b/src/types/agent-server/core/base/index.ts @@ -2,5 +2,6 @@ export * from "./action"; export * from "./base"; export * from "./common"; +export * from "./critic"; export * from "./event"; export * from "./observation"; diff --git a/src/types/agent-server/core/events/action-event.ts b/src/types/agent-server/core/events/action-event.ts index fd2408b5d6..03d9153e4e 100644 --- a/src/types/agent-server/core/events/action-event.ts +++ b/src/types/agent-server/core/events/action-event.ts @@ -1,5 +1,6 @@ import { Action } from "../base/action"; import { EventID, ToolCallID, SecurityRisk, TextContent } from "../base/common"; +import { CriticResult } from "../base/critic"; import { BaseEvent, ChatCompletionMessageToolCall, @@ -59,6 +60,11 @@ export interface ActionEvent extends BaseEvent { */ security_risk: SecurityRisk; + /** + * Optional critic evaluation of this action and preceding history. + */ + critic_result?: CriticResult | null; + /** * Optional LLM-generated summary used to label the tool call in the UI. */ diff --git a/src/types/agent-server/core/events/message-event.ts b/src/types/agent-server/core/events/message-event.ts index be89631645..954284439d 100644 --- a/src/types/agent-server/core/events/message-event.ts +++ b/src/types/agent-server/core/events/message-event.ts @@ -1,4 +1,5 @@ import { TextContent } from "../base/common"; +import { CriticResult } from "../base/critic"; import { BaseEvent, Message } from "../base/event"; export interface MessageEvent extends BaseEvent { @@ -16,4 +17,9 @@ export interface MessageEvent extends BaseEvent { * List of content added by agent context */ extended_content: TextContent[]; + + /** + * Optional critic evaluation of the agent's work at this point. + */ + critic_result?: CriticResult | null; } diff --git a/tests/e2e/live/real-agent-server-conversation.spec.ts b/tests/e2e/live/real-agent-server-conversation.spec.ts index 761b4aa1aa..85f9946d3c 100644 --- a/tests/e2e/live/real-agent-server-conversation.spec.ts +++ b/tests/e2e/live/real-agent-server-conversation.spec.ts @@ -3,8 +3,9 @@ import { test, type APIRequestContext } from "@playwright/test"; import { BACKEND_URL, clickButtonByTestId, - clickButtonByTestIdOrText, configureLiveAgentServer, + createLiveConversation, + deleteLiveLlmProfile, dismissAnalyticsModal, enableLiveE2EFlags, EXPECTED_BASH_COMMAND, @@ -13,15 +14,15 @@ import { expandVisibleEventDetails, fillChatInput, getLiveArtifactMask, - getConversationIdFromURL, getOptionalConversationIdFromURL, guardAgainstPostHogRequests, hasLiveLLMConfig, missingLiveLLMConfigMessage, - openCreatedConversation, routeBackendSessionApiKey, sessionApiKey, waitForAgentReply, + waitForCriticResultDisplay, + waitForCriticResultEvent, waitForNonUserMessageText, waitForSuccessfulBashObservation, waitForTestId, @@ -81,10 +82,12 @@ test.describe("live Agent Server terminal conversation", () => { } await cleanupKnownConversations(request); + await deleteLiveLlmProfile(request); }); test.afterAll(async ({ request }) => { await cleanupKnownConversations(request); + await deleteLiveLlmProfile(request); }); test("runs a real LLM-backed Agent Server terminal conversation through the UI", async ({ @@ -94,19 +97,15 @@ test.describe("live Agent Server terminal conversation", () => { test.skip(!hasLiveLLMConfig, missingLiveLLMConfigMessage); await configureLiveAgentServer(request); + const conversationId = await createLiveConversation(request); + createdConversationIds.add(conversationId); await routeBackendSessionApiKey(page); const postHogGuard = await guardAgainstPostHogRequests(page); - await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.goto(`/conversations/${conversationId}`, { + waitUntil: "domcontentloaded", + }); await dismissAnalyticsModal(page); - await clickButtonByTestIdOrText( - page, - "launch-new-conversation-button", - "New Conversation", - ); - await openCreatedConversation(page); - const conversationId = getConversationIdFromURL(page); - createdConversationIds.add(conversationId); await waitForTestId(page, "app-route"); await waitForTestId(page, "chat-interface"); await waitForTestId(page, "interactive-chat-box"); @@ -139,4 +138,56 @@ test.describe("live Agent Server terminal conversation", () => { await postHogGuard.expectNoRequests(); }); + + test("renders critic evaluation results for a real LLM-backed conversation", async ({ + page, + request, + }, testInfo) => { + test.skip(!hasLiveLLMConfig, missingLiveLLMConfigMessage); + test.setTimeout(240_000); + + await configureLiveAgentServer(request, { enableCritic: true }); + const conversationId = await createLiveConversation(request, { + enableCritic: true, + }); + createdConversationIds.add(conversationId); + await routeBackendSessionApiKey(page); + const postHogGuard = await guardAgainstPostHogRequests(page); + + await page.goto(`/conversations/${conversationId}`, { + waitUntil: "domcontentloaded", + }); + await dismissAnalyticsModal(page); + await waitForTestId(page, "app-route"); + await waitForTestId(page, "chat-interface"); + await waitForTestId(page, "interactive-chat-box"); + + await fillChatInput( + page, + [ + "Use the terminal/bash tool exactly once.", + `Run this exact command: ${EXPECTED_BASH_COMMAND}`, + `After the command succeeds, reply with exactly this token and then finish: ${EXPECTED_REPLY_TOKEN}`, + "Do not use any other tools. Do not add any other text in the final reply.", + ].join("\n"), + ); + await clickButtonByTestId(page, "submit-button"); + + await waitForAgentReply(page); + await waitForSuccessfulBashObservation(request, conversationId); + await waitForCriticResultEvent(request, conversationId); + await waitForCriticResultDisplay(page); + + const screenshotPath = testInfo.outputPath("live-critic-result.png"); + await page.getByTestId("chat-interface").screenshot({ + path: screenshotPath, + mask: getLiveArtifactMask(page), + }); + await testInfo.attach("live-critic-result", { + path: screenshotPath, + contentType: "image/png", + }); + + await postHogGuard.expectNoRequests(); + }); }); diff --git a/tests/e2e/live/utils/agent-server-conversation.ts b/tests/e2e/live/utils/agent-server-conversation.ts index bcc3418503..9001abe27e 100644 --- a/tests/e2e/live/utils/agent-server-conversation.ts +++ b/tests/e2e/live/utils/agent-server-conversation.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { expect, type APIRequestContext, @@ -41,6 +42,12 @@ const llmModel = : openAIKey?.trim() ? "openai/gpt-5.4-mini" : "anthropic/claude-haiku-4-5-20251001"); +const DEFAULT_AGENT_TOOLS = [ + { name: "terminal", params: {} }, + { name: "file_editor", params: {} }, + { name: "task_tracker", params: {} }, + { name: "canvas_ui", params: {} }, +]; export const sessionApiKey = firstNonEmpty( process.env.LIVE_E2E_SESSION_API_KEY, process.env.LOCAL_BACKEND_API_KEY, @@ -50,10 +57,15 @@ if (!sessionApiKey) { throw new Error("LIVE_E2E_SESSION_API_KEY must be set for live E2E."); } +const LIVE_LLM_PROFILE_NAME = `live-e2e-${sessionApiKey.slice(0, 8)}`; export const hasLiveLLMConfig = Boolean(llmApiKey); export const missingLiveLLMConfigMessage = "Set LIVE_E2E_LLM_API_KEY, OPENAI_API_KEY, ANTHROPIC_API_KEY, or LLM_API_KEY to run live E2E."; +interface ConfigureLiveAgentServerOptions { + enableCritic?: boolean; +} + function escapeRegExp(value: string) { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } @@ -89,21 +101,15 @@ export function getLiveArtifactMask(page: Page): Locator[] { ]; } -export async function configureLiveAgentServer(request: APIRequestContext) { +export async function configureLiveAgentServer( + request: APIRequestContext, + options: ConfigureLiveAgentServerOptions = {}, +) { if (!llmApiKey.trim()) { throw new Error(missingLiveLLMConfigMessage); } - const llmSettings: Record = { - model: llmModel, - api_key: llmApiKey, - extended_thinking_budget: 1024, - max_output_tokens: 2048, - temperature: 0, - }; - if (llmBaseUrl) { - llmSettings.base_url = llmBaseUrl; - } + await ensureLiveLlmProfile(request); const settingsResponse = await request.patch(`${BACKEND_URL}/api/settings`, { headers: { @@ -111,10 +117,11 @@ export async function configureLiveAgentServer(request: APIRequestContext) { }, data: { agent_settings_diff: { - llm: llmSettings, + llm: buildLiveLlmSettings(), condenser: { enabled: false, }, + verification: buildLiveVerificationSettings(options), }, conversation_settings_diff: { confirmation_mode: false, @@ -128,6 +135,139 @@ export async function configureLiveAgentServer(request: APIRequestContext) { ).toBeTruthy(); } +async function ensureLiveLlmProfile(request: APIRequestContext) { + const profileUrl = `${BACKEND_URL}/api/profiles/${encodeURIComponent(LIVE_LLM_PROFILE_NAME)}`; + const headers = { + "X-Session-API-Key": sessionApiKey, + }; + + await deleteLiveLlmProfile(request); + + const saveResponse = await request.post(profileUrl, { + headers: { + ...headers, + "Content-Type": "application/json", + }, + data: { + llm: buildLiveLlmSettings(), + include_secrets: true, + }, + }); + expect( + saveResponse.ok(), + `POST /api/profiles/${LIVE_LLM_PROFILE_NAME} failed with ${saveResponse.status()}; response body omitted because live LLM credentials are configured in this request.`, + ).toBeTruthy(); + + const activateResponse = await request.post(`${profileUrl}/activate`, { + headers, + }); + expect( + activateResponse.ok(), + `POST /api/profiles/${LIVE_LLM_PROFILE_NAME}/activate failed with ${activateResponse.status()}; response body omitted because live LLM credentials are configured in this request.`, + ).toBeTruthy(); +} + +export async function deleteLiveLlmProfile(request: APIRequestContext) { + await request.delete( + `${BACKEND_URL}/api/profiles/${encodeURIComponent(LIVE_LLM_PROFILE_NAME)}`, + { + headers: { + "X-Session-API-Key": sessionApiKey, + }, + }, + ); +} + +function buildLiveLlmSettings(): Record { + const llmSettings: Record = { + model: llmModel, + api_key: llmApiKey, + extended_thinking_budget: 1024, + max_output_tokens: 2048, + temperature: 0, + }; + if (llmBaseUrl) { + llmSettings.base_url = llmBaseUrl; + } + return llmSettings; +} + +function buildLiveVerificationSettings( + options: ConfigureLiveAgentServerOptions = {}, +): Record { + const verificationSettings: Record = + options.enableCritic + ? { + critic_enabled: true, + critic_mode: "finish_and_message", + enable_iterative_refinement: false, + critic_api_key: llmApiKey, + critic_model_name: llmModel, + } + : { + critic_enabled: false, + enable_iterative_refinement: false, + }; + if (options.enableCritic && llmBaseUrl) { + verificationSettings.critic_server_url = llmBaseUrl; + } + return verificationSettings; +} + +export async function createLiveConversation( + request: APIRequestContext, + options: ConfigureLiveAgentServerOptions = {}, +) { + if (!llmApiKey.trim()) { + throw new Error(missingLiveLLMConfigMessage); + } + + const conversationId = randomUUID(); + const response = await request.post(`${BACKEND_URL}/api/conversations`, { + headers: { + "X-Session-API-Key": sessionApiKey, + }, + data: { + conversation_id: conversationId, + workspace: { + kind: "LocalWorkspace", + working_dir: `workspace/project/${conversationId}`, + }, + worktree: true, + max_iterations: 6, + stuck_detection: true, + autotitle: true, + confirmation_policy: { + kind: "NeverConfirm", + }, + agent_settings: { + agent_kind: "openhands", + llm: buildLiveLlmSettings(), + condenser: { + enabled: false, + }, + verification: buildLiveVerificationSettings(options), + tools: DEFAULT_AGENT_TOOLS, + }, + tool_module_qualnames: { + canvas_ui: "canvas_ui_tool", + }, + }, + }); + + expect( + response.ok(), + `POST /api/conversations failed with ${response.status()}; response body omitted because live LLM credentials are configured in this request.`, + ).toBeTruthy(); + + const body = (await response.json()) as { id?: unknown }; + expect( + typeof body.id === "string" && body.id.length > 0, + "POST /api/conversations did not return a conversation id.", + ).toBe(true); + return body.id as string; +} + export async function enableLiveE2EFlags(page: Page) { await page.addInitScript(() => { window.localStorage.setItem("analytics-consent", "false"); @@ -458,6 +598,20 @@ function isSuccessfulBashObservation(event: unknown) { ); } +function hasCriticResult(event: unknown) { + if (!event || typeof event !== "object") { + return false; + } + + const criticResult = (event as { critic_result?: unknown }).critic_result; + if (!criticResult || typeof criticResult !== "object") { + return false; + } + + const score = (criticResult as { score?: unknown }).score; + return typeof score === "number" && Number.isFinite(score); +} + export async function waitForSuccessfulBashObservation( request: APIRequestContext, conversationId: string, @@ -489,3 +643,51 @@ export async function waitForSuccessfulBashObservation( ) .toBe(true); } + +export async function waitForCriticResultEvent( + request: APIRequestContext, + conversationId: string, +) { + await expect + .poll( + async () => { + const response = await request.get( + `${BACKEND_URL}/api/conversations/${encodeURIComponent(conversationId)}/events/search`, + { + headers: { + "X-Session-API-Key": sessionApiKey, + }, + params: { + limit: "100", + sort_order: "TIMESTAMP_DESC", + }, + }, + ); + + if (!response.ok()) { + return false; + } + + const body = (await response.json()) as { items?: unknown[] }; + return body.items?.some(hasCriticResult) ?? false; + }, + { timeout: 180_000 }, + ) + .toBe(true); +} + +export async function waitForCriticResultDisplay(page: Page) { + await expect + .poll( + async () => + page + .evaluate(() => + document.body.textContent?.includes( + "Critic: agent success likelihood", + ), + ) + .catch(() => false), + { timeout: 120_000 }, + ) + .toBe(true); +} diff --git a/tests/e2e/mock-llm/mock-llm-ui-regressions.spec.ts b/tests/e2e/mock-llm/mock-llm-ui-regressions.spec.ts index 7042363c76..7ed7971cf1 100644 --- a/tests/e2e/mock-llm/mock-llm-ui-regressions.spec.ts +++ b/tests/e2e/mock-llm/mock-llm-ui-regressions.spec.ts @@ -11,10 +11,11 @@ */ import test, { expect, type Page, type Request } from "@playwright/test"; -import { - seedLocalStorage, - routeSessionApiKey, -} from "./utils/mock-llm-helpers"; +import type { ActionEvent, MessageEvent } from "#/types/agent-server/core"; +import { SecurityRisk } from "#/types/agent-server/core"; +import type { FinishAction } from "#/types/agent-server/core/base/action"; +import type { CriticResult } from "#/types/agent-server/core/base/critic"; +import { seedLocalStorage, routeSessionApiKey } from "./utils/mock-llm-helpers"; test.describe.configure({ mode: "serial" }); @@ -25,6 +26,7 @@ test.describe.configure({ mode: "serial" }); // ─── pagination helpers ────────────────────────────────────────────── const PAGINATION_CONVERSATION_ID = "pagination-e2e"; +const CRITIC_CONVERSATION_ID = "critic-rendering-e2e"; const PAGE_SIZE = 50; const PAGINATION_EVENT_COUNT = 100; const PAGINATION_BASE_TIME = Date.UTC(2026, 4, 13, 0, 0, 0); @@ -44,10 +46,9 @@ interface PaginationEvent { }; } -function createPaginationEvent( - index: number, - prefix: string, -): PaginationEvent { +type CriticEvent = MessageEvent | ActionEvent; + +function createPaginationEvent(index: number, prefix: string): PaginationEvent { return { id: `${prefix.toLowerCase().replaceAll(" ", "-")}-${index}`, timestamp: timestampForEvent(index), @@ -102,6 +103,127 @@ function buildMockConversation() { }; } +function buildCriticConversation() { + return { + id: CRITIC_CONVERSATION_ID, + conversation_id: CRITIC_CONVERSATION_ID, + status: "STOPPED", + execution_status: "stopped", + created_at: timestampForEvent(1), + updated_at: timestampForEvent(4), + title: "Critic rendering test", + }; +} + +function buildCriticResult(score: number, eventId: string): CriticResult { + return { + score, + message: null, + metadata: { + categorized_features: { + agent_behavioral_issues: [ + { + name: "incomplete_changes", + display_name: "Incomplete Changes", + probability: 0.74, + }, + ], + infrastructure_issues: [ + { + name: "test_environment", + display_name: "Test Environment", + probability: 0.52, + }, + ], + user_followup_patterns: [ + { + name: "asks_for_tests", + display_name: "Asks for tests", + probability: 0.33, + }, + ], + }, + event_ids: [eventId], + }, + }; +} + +function createCriticEvents(): CriticEvent[] { + return [ + { + id: "critic-user-message", + timestamp: timestampForEvent(1), + source: "user", + llm_message: { + role: "user", + content: [{ type: "text", text: "Please do the task." }], + }, + activated_microagents: [], + extended_content: [], + }, + { + id: "critic-agent-message", + timestamp: timestampForEvent(2), + source: "agent", + llm_message: { + role: "assistant", + content: [ + { + type: "text", + text: "The requested change has been implemented.", + }, + ], + }, + activated_microagents: [], + extended_content: [], + critic_result: buildCriticResult(0.82, "critic-agent-message"), + }, + { + id: "critic-finish-action", + timestamp: timestampForEvent(3), + source: "agent", + thought: [], + thinking_blocks: [], + action: { + kind: "FinishAction", + message: "Finished with critic evaluation.", + }, + tool_name: "finish", + tool_call_id: "call_critic_finish", + tool_call: { + id: "call_critic_finish", + type: "function", + function: { + name: "finish", + arguments: JSON.stringify({ + message: "Finished with critic evaluation.", + }), + }, + }, + llm_response_id: "response_critic_finish", + security_risk: SecurityRisk.UNKNOWN, + critic_result: buildCriticResult(0.64, "critic-finish-action"), + }, + ]; +} + +function searchCriticEvents( + events: CriticEvent[], + searchParams: URLSearchParams, +) { + const limit = Number(searchParams.get("limit") ?? "100"); + const sortOrder = searchParams.get("sort_order"); + const sorted = [...events].sort((a, b) => + sortOrder === "TIMESTAMP_DESC" + ? b.timestamp.localeCompare(a.timestamp) + : a.timestamp.localeCompare(b.timestamp), + ); + return { + items: sorted.slice(0, limit), + next_page_id: null, + }; +} + /** Intercept conversation lookup + event search for pagination tests. */ async function routePaginationConversation(page: Page) { const allEvents = createAllPaginationEvents("Pagination message"); @@ -111,30 +233,27 @@ async function routePaginationConversation(page: Page) { // Use a regex to match the query-param form of the URL. The glob `?` // character is a single-char wildcard in Playwright, so a regex is // more reliable for matching literal query strings. - await page.route( - /\/api\/conversations\?/, - async (route, req) => { - if (req.method() !== "GET") { - await route.fallback(); - return; - } - const url = new URL(req.url()); - // Axios serializes array params as `ids[]=a&ids[]=b` (bracket notation). - const ids = [ - ...url.searchParams.getAll("ids"), - ...url.searchParams.getAll("ids[]"), - ]; - if (ids.includes(PAGINATION_CONVERSATION_ID)) { - await route.fulfill({ - status: 200, - contentType: "application/json", - body: JSON.stringify([buildMockConversation()]), - }); - } else { - await route.fallback(); - } - }, - ); + await page.route(/\/api\/conversations\?/, async (route, req) => { + if (req.method() !== "GET") { + await route.fallback(); + return; + } + const url = new URL(req.url()); + // Axios serializes array params as `ids[]=a&ids[]=b` (bracket notation). + const ids = [ + ...url.searchParams.getAll("ids"), + ...url.searchParams.getAll("ids[]"), + ]; + if (ids.includes(PAGINATION_CONVERSATION_ID)) { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([buildMockConversation()]), + }); + } else { + await route.fallback(); + } + }); // Stub the event search endpoint with the synthetic paginated events. // Older-events requests (those with timestamp__lt) are delayed slightly so @@ -164,6 +283,50 @@ async function routePaginationConversation(page: Page) { ); } +/** Intercept conversation lookup + event search for critic rendering tests. */ +async function routeCriticConversation(page: Page) { + const criticEvents = createCriticEvents(); + + await page.route(/\/api\/conversations\?/, async (route, req) => { + if (req.method() !== "GET") { + await route.fallback(); + return; + } + const url = new URL(req.url()); + const ids = [ + ...url.searchParams.getAll("ids"), + ...url.searchParams.getAll("ids[]"), + ]; + if (ids.includes(CRITIC_CONVERSATION_ID)) { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([buildCriticConversation()]), + }); + } else { + await route.fallback(); + } + }); + + await page.route( + `**/api/conversations/${CRITIC_CONVERSATION_ID}/events/search**`, + async (route, req) => { + if (req.method() !== "GET") { + await route.fallback(); + return; + } + const url = new URL(req.url()); + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify( + searchCriticEvents(criticEvents, url.searchParams), + ), + }); + }, + ); +} + async function getChatScroller(page: Page) { const chatInterface = page.getByTestId("chat-interface"); await expect(chatInterface).toBeVisible({ timeout: 15_000 }); @@ -175,11 +338,9 @@ async function getChatScroller(page: Page) { async function waitForScrollableConversation(page: Page) { const scroller = await getChatScroller(page); await expect - .poll( - () => - scroller.evaluate((el) => el.scrollHeight > el.clientHeight), - { timeout: 15_000 }, - ) + .poll(() => scroller.evaluate((el) => el.scrollHeight > el.clientHeight), { + timeout: 15_000, + }) .toBe(true); return scroller; } @@ -209,9 +370,9 @@ test.describe("UI regressions", () => { await routeSessionApiKey(page); await page.goto("/", { waitUntil: "domcontentloaded" }); - await expect( - page.locator("[data-agent-server-ui]").first(), - ).toBeVisible({ timeout: 15_000 }); + await expect(page.locator("[data-agent-server-ui]").first()).toBeVisible({ + timeout: 15_000, + }); const layout = page.getByTestId("root-layout"); await expect(layout).toBeVisible(); @@ -235,6 +396,32 @@ test.describe("UI regressions", () => { expect(outsideStyles.backgroundColor).not.toBe(insideBackground); }); + // ── critic result rendering ────────────────────────────────────── + + test("renders critic results on agent messages and finish actions", async ({ + page, + }) => { + await routeSessionApiKey(page); + await routeCriticConversation(page); + + await page.goto(`/conversations/${CRITIC_CONVERSATION_ID}`, { + waitUntil: "domcontentloaded", + }); + + const criticLabels = page.getByText("Critic: agent success likelihood"); + await expect(criticLabels).toHaveCount(2, { timeout: 15_000 }); + await expect(page.locator('[aria-label="Score: 82.0%"]')).toBeVisible(); + await expect(page.locator('[aria-label="Score: 64.0%"]')).toBeVisible(); + + await page.getByLabel("Expand details").first().click(); + await expect(page.getByText("Potential Issues:")).toBeVisible(); + await expect(page.getByText("Incomplete Changes")).toBeVisible(); + await expect(page.getByText("Infrastructure:")).toBeVisible(); + await expect(page.getByText("Test Environment")).toBeVisible(); + await expect(page.getByText("Likely Follow-up:")).toBeVisible(); + await expect(page.getByText("Asks for tests")).toBeVisible(); + }); + // ── event pagination on scroll-up ──────────────────────────────── test("loads older events when scrolling up", async ({ page }) => { @@ -356,9 +543,7 @@ test.describe("UI regressions", () => { const openButton = page.getByTestId("open-workspace-button"); await expect(openButton).toBeEnabled({ timeout: 15_000 }); await openButton.click(); - await expect( - page.getByTestId("open-workspace-dialog-body"), - ).toBeVisible(); + await expect(page.getByTestId("open-workspace-dialog-body")).toBeVisible(); const dropdown = page.getByTestId("workspace-dropdown"); await expect(dropdown).toBeEnabled({ timeout: 15_000 }); @@ -391,9 +576,7 @@ test.describe("UI regressions", () => { const reopenButton = page.getByTestId("open-workspace-button"); await expect(reopenButton).toBeEnabled({ timeout: 15_000 }); await reopenButton.click(); - await expect( - page.getByTestId("open-workspace-dialog-body"), - ).toBeVisible(); + await expect(page.getByTestId("open-workspace-dialog-body")).toBeVisible(); const restored = page.getByTestId("workspace-dropdown"); await expect(restored).toBeEnabled({ timeout: 15_000 }); @@ -442,9 +625,7 @@ test.describe("UI regressions", () => { const openButton = page.getByTestId("open-workspace-button"); await expect(openButton).toBeEnabled({ timeout: 15_000 }); await openButton.click(); - await expect( - page.getByTestId("open-workspace-dialog-body"), - ).toBeVisible(); + await expect(page.getByTestId("open-workspace-dialog-body")).toBeVisible(); const dropdown = page.getByTestId("workspace-dropdown"); await expect(dropdown).toBeEnabled({ timeout: 15_000 }); diff --git a/tests/e2e/snapshots/settings-verification.snapshot.spec.ts b/tests/e2e/snapshots/settings-verification.snapshot.spec.ts index 73b828172d..a96f4fc6fd 100644 --- a/tests/e2e/snapshots/settings-verification.snapshot.spec.ts +++ b/tests/e2e/snapshots/settings-verification.snapshot.spec.ts @@ -6,14 +6,21 @@ import { seedLocalStorage } from "./support/seed-local-storage"; * - /settings/verification (Confirmation Mode toggle + Security Analyzer) * - /settings/condenser (Schema-driven condenser form) * + * The verification page is now fully schema-driven (no hand-written header + * for the confirmation-mode toggle), and `confirmation_mode` is a + * `prominence: "major"` field so it lives in the Advanced/All views, not + * Basic. Each verification test therefore switches to the "All" view + * before snapshotting so both the critic-related and confirmation-mode + * controls are on screen. + * * MSW provides the default settings (confirmation_mode: false) so the first * verification snapshot shows the toggle in the OFF position with a dimmed - * Save Changes button. Toggling it ON reveals the Security Analyzer dropdown + * Save Changes button. Toggling it ON reveals the Security Analyzer dropdown * and enables Save Changes — captured in the second snapshot. * - * Three snapshots, not four: there is no separate "dirty" snapshot because - * clicking the toggle IS the dirty action — the "ON" snapshot already captures - * the dirty/enabled-Save-Changes state. + * There is no separate "dirty" snapshot for confirmation mode because clicking + * the toggle IS the dirty action — the "ON" snapshot already captures the + * dirty/enabled-Save-Changes state. */ async function dismissConsentModal(page: Page) { @@ -31,15 +38,41 @@ test.describe("Settings – Verification & Condenser Visual Snapshots", () => { test.setTimeout(60_000); /** - * Helper: wait for the verification page to be ready by checking for the - * "Enable Confirmation Mode" label text (visible once settings are loaded). - * The underlying checkbox is `hidden` in the DOM (a styled toggle pattern), - * so we cannot use toBeVisible() on the checkbox itself. + * Helper: wait for the schema-driven verification page to be ready, then + * switch to the "All" view so `confirmation_mode` (a `major`-prominence + * field) is rendered alongside the `critic_enabled` toggle. + * + * Readiness signal: the "Enable Critic" label is the first critical- + * prominence field and is always visible once the schema is loaded, + * regardless of view. The underlying checkbox is `hidden` in the DOM + * (styled toggle pattern), so we assert on the label text instead. */ async function waitForVerificationPage(page: Page) { await expect( - page.getByText("Enable Confirmation Mode"), - ).toBeVisible({ timeout: 10_000 }); + page.getByTestId("sdk-settings-verification.critic_enabled"), + ).toBeAttached({ + timeout: 10_000, + }); + await page.getByTestId("sdk-section-all-toggle").click(); + // The visible label is "Confirmation Mode" — the schema's raw "Confirmation + // mode" goes through the i18n translation table (SCHEMA$CONFIRMATION_MODE$LABEL). + await expect(page.getByText("Confirmation Mode")).toBeVisible({ + timeout: 5_000, + }); + } + + async function ensureCriticEnabled(page: Page) { + const apiKeyInput = page.getByTestId( + "sdk-settings-verification.critic_api_key", + ); + if (!(await apiKeyInput.isVisible())) { + await page + .locator( + `label:has([data-testid="sdk-settings-verification.critic_enabled"])`, + ) + .click(); + } + await expect(apiKeyInput).toBeVisible({ timeout: 5_000 }); } test("verification settings with confirmation mode OFF (default)", async ({ @@ -52,15 +85,39 @@ test.describe("Settings – Verification & Condenser Visual Snapshots", () => { await waitForVerificationPage(page); - // Security Analyzer combobox must NOT be present when toggle is off. - // HeroUI Autocomplete does not forward data-testid; use role + label. + // Security Analyzer combobox must NOT be present when confirmation_mode + // is off (it depends_on the toggle). HeroUI Autocomplete does not + // forward data-testid, so match by accessible role + label (case + // insensitive since the schema label is "Security analyzer"). await expect( - page.getByRole("combobox", { name: /Security Analyzer/ }), + page.getByRole("combobox", { name: /security analyzer/i }), ).toHaveCount(0); + const rootLayout = page.getByTestId("root-layout"); + await expect(rootLayout).toHaveScreenshot("verification-settings-off.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); + }); + + test("verification settings with critic enabled shows API key guidance", async ({ + page, + }) => { + await setupMocks(page); + await page.goto("/settings/verification"); + await dismissConsentModal(page); + await waitForVerificationPage(page); + + await ensureCriticEnabled(page); + await expect( + page.getByText( + /Critic API Key is the same as your OpenHands Provider LLM Key/i, + ), + ).toBeVisible(); + const rootLayout = page.getByTestId("root-layout"); await expect(rootLayout).toHaveScreenshot( - "verification-settings-off.png", + "verification-settings-critic-enabled.png", { animations: "disabled", maxDiffPixelRatio: 0.01 }, ); }); @@ -73,25 +130,24 @@ test.describe("Settings – Verification & Condenser Visual Snapshots", () => { await dismissConsentModal(page); await waitForVerificationPage(page); - // The underlying is `hidden`; clicking the visible - // label that wraps it activates the form control through standard HTML - // label–control association. + // The schema-rendered SettingsSwitch's underlying + // is `hidden`; clicking the visible label that wraps it activates the + // form control through standard HTML label–control association. The + // testId now comes from SchemaField's `sdk-settings-${field.key}` scheme. await page - .locator(`label:has([data-testid="confirmation-mode-toggle"])`) + .locator(`label:has([data-testid="sdk-settings-confirmation_mode"])`) .click(); // Security Analyzer dropdown should now appear. - // The HeroUI Autocomplete component does not forward data-testid to the DOM; - // match by accessible role + label instead. await expect( - page.getByRole("combobox", { name: /Security Analyzer/ }), + page.getByRole("combobox", { name: /security analyzer/i }), ).toBeVisible({ timeout: 5_000 }); const rootLayout = page.getByTestId("root-layout"); - await expect(rootLayout).toHaveScreenshot( - "verification-settings-on.png", - { animations: "disabled", maxDiffPixelRatio: 0.01 }, - ); + await expect(rootLayout).toHaveScreenshot("verification-settings-on.png", { + animations: "disabled", + maxDiffPixelRatio: 0.01, + }); }); test("condenser settings page renders schema form", async ({ page }) => { @@ -100,16 +156,13 @@ test.describe("Settings – Verification & Condenser Visual Snapshots", () => { await dismissConsentModal(page); await page.waitForLoadState("networkidle"); - // Wait for the condenser form to render: the schema provides - // "Enable default condenser" as the first field label - await expect( - page.getByText("Enable default condenser"), - ).toBeVisible({ timeout: 15_000 }); - // The wrapper div with data-testid should be present once the form renders + await expect(page.getByTestId("condenser-settings-screen")).toBeAttached({ + timeout: 5_000, + }); await expect( - page.getByTestId("condenser-settings-screen"), - ).toBeAttached({ timeout: 5_000 }); + page.getByText(/Enable (default condenser|Memory Condensation)/i), + ).toBeVisible({ timeout: 15_000 }); const rootLayout = page.getByTestId("root-layout"); await expect(rootLayout).toHaveScreenshot("condenser-settings.png", {