diff --git a/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.stories.tsx b/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.stories.tsx index 3cadee56ec..126007fa41 100644 --- a/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.stories.tsx +++ b/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.stories.tsx @@ -68,7 +68,7 @@ export default meta; type Story = StoryObj; export const Default: Story = { - play: async ({ canvasElement, args }) => { + play: async ({ canvasElement }) => { const canvas = within(canvasElement); const gpt4oInput = await canvas.findByRole("spinbutton", { name: /GPT-4o compaction threshold/i, @@ -78,23 +78,44 @@ export const Default: Story = { expect(canvas.getByText("Claude Sonnet")).toBeInTheDocument(); expect(canvas.queryByText("GPT-3.5 (Disabled)")).not.toBeInTheDocument(); - await userEvent.type(gpt4oInput, "100"); + // No footer visible when nothing is dirty expect( - canvas.getByText( - "⚠ Setting 100% will disable auto-compaction for this model.", - ), - ).toBeInTheDocument(); - await userEvent.clear(gpt4oInput); - await userEvent.type(gpt4oInput, "95"); + canvas.queryByRole("button", { name: /Save/i }), + ).not.toBeInTheDocument(); - const saveButtons = canvas.getAllByRole("button", { name: "Save" }); + // Type a value to make the footer appear + await userEvent.type(gpt4oInput, "95"); await waitFor(() => { - expect(saveButtons[0]).toBeEnabled(); + expect( + canvas.getByRole("button", { name: /Save 1 change/i }), + ).toBeInTheDocument(); + }); + }, +}; + +export const SaveAll: Story = { + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const gpt4oInput = await canvas.findByRole("spinbutton", { + name: /GPT-4o compaction threshold/i, + }); + const claudeInput = await canvas.findByRole("spinbutton", { + name: /Claude Sonnet compaction threshold/i, }); - await userEvent.click(saveButtons[0]); + // Edit both models + await userEvent.type(gpt4oInput, "95"); + await userEvent.type(claudeInput, "50"); + + // Footer should show "Save 2 changes" + const saveButton = await canvas.findByRole("button", { + name: /Save 2 changes/i, + }); + await userEvent.click(saveButton); + await waitFor(() => { expect(args.onSaveThreshold).toHaveBeenCalledWith("model-1", 95); + expect(args.onSaveThreshold).toHaveBeenCalledWith("model-2", 50); }); }, }; @@ -118,7 +139,12 @@ export const WithOverrides: Story = { expect(gpt4oInput).toHaveValue(90); expect(claudeInput).toHaveValue(50); - const resetButtons = canvas.getAllByRole("button", { name: "Reset" }); + // Reset buttons should be visible for both overridden models + const resetButtons = canvas.getAllByRole("button", { + name: /Reset .+ to default/i, + }); + expect(resetButtons).toHaveLength(2); + await userEvent.click(resetButtons[0]); await waitFor(() => { expect(args.onResetThreshold).toHaveBeenCalledWith("model-1"); @@ -126,12 +152,124 @@ export const WithOverrides: Story = { }, }; +export const CancelChanges: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const gpt4oInput = await canvas.findByRole("spinbutton", { + name: /GPT-4o compaction threshold/i, + }); + + await userEvent.type(gpt4oInput, "42"); + const cancelButton = await canvas.findByRole("button", { name: /Cancel/i }); + await userEvent.click(cancelButton); + + // Footer should disappear after cancel + await waitFor(() => { + expect( + canvas.queryByRole("button", { name: /Save/i }), + ).not.toBeInTheDocument(); + }); + + // Input should be cleared back to empty (no override) + expect(gpt4oInput).toHaveValue(null); + }, +}; + +export const InvalidDraftShowsFooter: Story = { + name: "Invalid Draft Shows Footer", + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const gpt4oInput = await canvas.findByRole("spinbutton", { + name: /GPT-4o compaction threshold/i, + }); + + // Type an out-of-range value (number inputs reject non-numeric chars) + await userEvent.type(gpt4oInput, "150"); + + // Input should be marked invalid + await waitFor(() => { + expect(gpt4oInput).toHaveAttribute("aria-invalid", "true"); + }); + + // Cancel button should be visible so user can discard the edit + expect(canvas.getByRole("button", { name: /Cancel/i })).toBeInTheDocument(); + + // Save button should NOT be visible (nothing valid to save) + expect( + canvas.queryByRole("button", { name: /Save/i }), + ).not.toBeInTheDocument(); + }, +}; + +export const DisableCompactionWarning: Story = { + name: "100% Disable Compaction Warning", + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const gpt4oInput = await canvas.findByRole("spinbutton", { + name: /GPT-4o compaction threshold/i, + }); + + await userEvent.type(gpt4oInput, "100"); + + // sr-only warning should be in the DOM for screen readers + await waitFor(() => { + expect( + canvas.getByText( + "Setting 100% will disable auto-compaction for this model.", + ), + ).toBeInTheDocument(); + }); + }, +}; + export const Loading: Story = { args: { isThresholdsLoading: true, }, }; +export const PartialSaveFailure: Story = { + name: "Partial Save Failure", + args: { + onSaveThreshold: fn(async (modelConfigId: string) => { + if (modelConfigId === "model-2") { + throw new globalThis.Error("Network error"); + } + }), + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const gpt4oInput = await canvas.findByRole("spinbutton", { + name: /GPT-4o compaction threshold/i, + }); + const claudeInput = await canvas.findByRole("spinbutton", { + name: /Claude Sonnet compaction threshold/i, + }); + + await userEvent.type(gpt4oInput, "90"); + await userEvent.type(claudeInput, "55"); + + const saveButton = await canvas.findByRole("button", { + name: /Save 2 changes/i, + }); + await userEvent.click(saveButton); + + await waitFor(() => { + expect(args.onSaveThreshold).toHaveBeenCalledWith("model-1", 90); + expect(args.onSaveThreshold).toHaveBeenCalledWith("model-2", 55); + }); + + // model-2 should show an error, footer should still be visible + // with Save showing "Save 1 change" for the failed row + await waitFor(() => { + expect(canvas.getByText("Network error")).toBeInTheDocument(); + expect( + canvas.getByRole("button", { name: /Save 1 change/i }), + ).toBeInTheDocument(); + }); + }, +}; + export const ErrorState: Story = { name: "Error", args: { diff --git a/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.tsx b/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.tsx index bcd8883ebe..db10b72404 100644 --- a/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.tsx +++ b/site/src/pages/AgentsPage/components/UserCompactionThresholdSettings.tsx @@ -1,9 +1,25 @@ +import { RotateCcwIcon } from "lucide-react"; import { type FC, useState } from "react"; import { getErrorMessage } from "#/api/errors"; import type * as TypesGen from "#/api/typesGenerated"; import { Button } from "#/components/Button/Button"; import { Input } from "#/components/Input/Input"; import { Spinner } from "#/components/Spinner/Spinner"; +import { + Table, + TableBody, + TableCell, + TableFooter, + TableHead, + TableHeader, + TableRow, +} from "#/components/Table/Table"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "#/components/Tooltip/Tooltip"; +import { cn } from "#/utils/cn"; interface UserCompactionThresholdSettingsProps { modelConfigs: readonly TypesGen.ChatModelConfig[]; @@ -49,6 +65,16 @@ export const UserCompactionThresholdSettings: FC< const [rowErrors, setRowErrors] = useState>({}); const [pendingModels, setPendingModels] = useState>(new Set()); + const enabledModelConfigs = modelConfigs.filter((config) => config.enabled); + const overridesByModelID = new Map( + (thresholds ?? []).map( + (threshold: TypesGen.UserChatCompactionThreshold) => [ + threshold.model_config_id, + threshold.threshold_percent, + ], + ), + ); + const clearDraft = (modelConfigID: string) => { setDrafts((currentDrafts) => { const nextDrafts = { ...currentDrafts }; @@ -68,39 +94,21 @@ export const UserCompactionThresholdSettings: FC< }); }; - const handleSave = (modelConfigId: string, thresholdPercent: number) => { - clearRowError(modelConfigId); - setPendingModels((currentPendingModels) => - new Set(currentPendingModels).add(modelConfigId), - ); - onSaveThreshold(modelConfigId, thresholdPercent) - .then(() => { - clearDraft(modelConfigId); - clearRowError(modelConfigId); - }) - .catch((error: unknown) => { - setRowErrors((currentErrors) => ({ - ...currentErrors, - [modelConfigId]: getErrorMessage( - error, - "Failed to save compaction threshold.", - ), - })); - }) - .finally(() => { - setPendingModels((currentPendingModels) => { - const nextPendingModels = new Set(currentPendingModels); - nextPendingModels.delete(modelConfigId); - return nextPendingModels; - }); - }); + const addPending = (id: string) => { + setPendingModels((pending) => new Set(pending).add(id)); + }; + + const removePending = (id: string) => { + setPendingModels((pending) => { + const next = new Set(pending); + next.delete(id); + return next; + }); }; const handleReset = (modelConfigId: string) => { clearRowError(modelConfigId); - setPendingModels((currentPendingModels) => - new Set(currentPendingModels).add(modelConfigId), - ); + addPending(modelConfigId); onResetThreshold(modelConfigId) .then(() => { clearDraft(modelConfigId); @@ -116,23 +124,57 @@ export const UserCompactionThresholdSettings: FC< })); }) .finally(() => { - setPendingModels((currentPendingModels) => { - const nextPendingModels = new Set(currentPendingModels); - nextPendingModels.delete(modelConfigId); - return nextPendingModels; - }); + removePending(modelConfigId); }); }; - const enabledModelConfigs = modelConfigs.filter((config) => config.enabled); - const overridesByModelID = new Map( - (thresholds ?? []).map( - (threshold: TypesGen.UserChatCompactionThreshold) => [ - threshold.model_config_id, - threshold.threshold_percent, - ], - ), - ); + // Compute dirty rows: rows where the user has typed a valid value + // that differs from the current server-side override. + const dirtyRows: Array<{ modelConfigId: string; value: number }> = []; + for (const modelConfig of enabledModelConfigs) { + const draft = drafts[modelConfig.id]; + if (draft === undefined) continue; + const parsed = parseThresholdDraft(draft); + if (parsed === null) continue; + const existingOverride = overridesByModelID.get(modelConfig.id); + if (parsed === existingOverride) continue; + dirtyRows.push({ modelConfigId: modelConfig.id, value: parsed }); + } + + const handleSaveAll = () => { + const saves = dirtyRows.map(({ modelConfigId, value }) => { + clearRowError(modelConfigId); + addPending(modelConfigId); + return onSaveThreshold(modelConfigId, value) + .then(() => { + clearDraft(modelConfigId); + clearRowError(modelConfigId); + }) + .catch((error: unknown) => { + setRowErrors((currentErrors) => ({ + ...currentErrors, + [modelConfigId]: getErrorMessage( + error, + "Failed to save compaction threshold.", + ), + })); + }) + .finally(() => { + removePending(modelConfigId); + }); + }); + void Promise.allSettled(saves); + }; + + const handleCancelAll = () => { + setDrafts({}); + setRowErrors({}); + }; + + const hasAnyPending = pendingModels.size > 0; + const hasAnyErrors = Object.keys(rowErrors).length > 0; + const hasAnyDrafts = Object.keys(drafts).length > 0; + if (isThresholdsLoading) { return (
@@ -198,107 +240,165 @@ export const UserCompactionThresholdSettings: FC< models before compaction thresholds can be set.

) : ( -
- {enabledModelConfigs.map((modelConfig) => { - const existingOverride = overridesByModelID.get(modelConfig.id); - const hasOverride = overridesByModelID.has(modelConfig.id); - const draftValue = - drafts[modelConfig.id] ?? - (existingOverride !== undefined ? String(existingOverride) : ""); - const parsedDraftValue = parseThresholdDraft(draftValue); - const isThisModelMutating = pendingModels.has(modelConfig.id); - const isSaveDisabled = - draftValue.length === 0 || - parsedDraftValue === null || - parsedDraftValue === existingOverride || - isThisModelMutating; + + + + Model + Default + Threshold + + + + {enabledModelConfigs.map((modelConfig) => { + const existingOverride = overridesByModelID.get(modelConfig.id); + const hasOverride = overridesByModelID.has(modelConfig.id); + const draftValue = + drafts[modelConfig.id] ?? + (existingOverride !== undefined + ? String(existingOverride) + : ""); + const parsedDraftValue = parseThresholdDraft(draftValue); + const isThisModelMutating = pendingModels.has(modelConfig.id); + const isInvalid = + draftValue.length > 0 && parsedDraftValue === null; + // Only warn when user-typed, not when loaded from + // the server. + const isDraftDisablingCompaction = + draftValue === "100" && drafts[modelConfig.id] !== undefined; + const rowError = rowErrors[modelConfig.id]; + const modelName = modelConfig.display_name || modelConfig.model; - return ( -
-
-
- - {modelConfig.display_name || modelConfig.model} - - - System default:{" "} - - {modelConfig.compression_threshold}% + return ( + + + {modelName} + {rowError && ( +

+ {rowError} +

+ )} + + + {modelConfig.compression_threshold}% + + +
+ + + { + setDrafts((currentDrafts) => ({ + ...currentDrafts, + [modelConfig.id]: event.target.value, + })); + clearRowError(modelConfig.id); + }} + disabled={isThisModelMutating} + /> + + {(isInvalid || isDraftDisablingCompaction) && ( + + {isInvalid + ? "Enter a whole number between 0 and 100." + : "Setting 100% will disable auto-compaction for this model."} + + )} + + % + + + + + {hasOverride && ( + + Reset to default ( + {modelConfig.compression_threshold}%) + + )} + +
+ {isInvalid && ( + + Enter a whole number between 0 and 100. - -
-
- { - setDrafts((currentDrafts) => ({ - ...currentDrafts, - [modelConfig.id]: event.target.value, - })); - clearRowError(modelConfig.id); - }} - disabled={isThisModelMutating} - /> - % + )} + {isDraftDisablingCompaction && ( + + Setting 100% will disable auto-compaction for this + model. + + )} + + + ); + })} + + {(dirtyRows.length > 0 || hasAnyErrors || hasAnyDrafts) && ( + + + +
- {hasOverride && ( + {dirtyRows.length > 0 && ( )}
-
- {draftValue.length > 0 && parsedDraftValue === null && ( -

- Enter a whole number between 0 and 100. -

- )} - {rowErrors[modelConfig.id] && ( -

- {rowErrors[modelConfig.id]} -

- )} - {draftValue === "100" && ( -

- ⚠ Setting 100% will disable auto-compaction for this model. -

- )} -
- ); - })} -
+ + + + )} +
)}
);