From e966240562a1a7c60bd9032f69d136dc2ebc1eee Mon Sep 17 00:00:00 2001 From: yogeshwaran10 Date: Sat, 24 Jan 2026 02:56:22 +0530 Subject: [PATCH] fix(ui): prevent clearing content filter patterns when editing guardrail The generic provider params update logic loop was unintentionaly overwriting `patterns` and `blocked_words` with empty values because these fields are managed by a separate component ContentFilterManager and not available in the main form values. Changes: - Excluded `patterns` and `blocked_words` from the generic provider params update loop in guardrail_info.tsx - Ensured these fields are only added to the update payload when explicitly handled by the ContentFilterManager logic (detecting changes via `useRef`). - Added a regression test in guardrail_info.test.tsx to verify that patterns are preserved when only the guardrail name is updated. Fixes #19639 --- .../guardrails/guardrail_info.test.tsx | 119 ++++++++++++++++++ .../components/guardrails/guardrail_info.tsx | 5 +- 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/src/components/guardrails/guardrail_info.test.tsx b/ui/litellm-dashboard/src/components/guardrails/guardrail_info.test.tsx index bd2b45d6f3..d532b654f5 100644 --- a/ui/litellm-dashboard/src/components/guardrails/guardrail_info.test.tsx +++ b/ui/litellm-dashboard/src/components/guardrails/guardrail_info.test.tsx @@ -11,6 +11,28 @@ vi.mock("@/components/networking", () => ({ updateGuardrailCall: vi.fn(), })); + +// Mock ContentFilterManager +vi.mock("./content_filter/ContentFilterManager", () => ({ + __esModule: true, + default: ({ onUnsavedChanges, onDataChange, isEditing }: any) => ( +
+ {isEditing && ( + + )} +
+ ), + formatContentFilterDataForAPI: (patterns: any[], blockedWords: any[]) => ({ + patterns, + blocked_words: blockedWords, + }), +})); + describe("Guardrail Info", () => { afterEach(() => { vi.clearAllMocks(); @@ -147,4 +169,101 @@ describe("Guardrail Info", () => { expect(getByText("PII Entity Configuration")).toBeInTheDocument(); }); }); + it("should handle content filter updates correctly", async () => { + // Mock the network responses + vi.mocked(networking.getGuardrailInfo).mockResolvedValue({ + guardrail_id: "123", + guardrail_name: "Content Filter Guardrail", + litellm_params: { + guardrail: "litellm_content_filter", + mode: "pre_call", + default_on: true, + patterns: ["initial_pattern"], + blocked_words: ["initial_word"], + }, + created_at: "2024-01-01T00:00:00Z", + updated_at: "2024-01-01T00:00:00Z", + guardrail_definition_location: "database", + }); + + vi.mocked(networking.getGuardrailUISettings).mockResolvedValue({ + supported_entities: [], + supported_actions: [], + pii_entity_categories: [], + supported_modes: ["pre_call", "post_call"], + }); + + vi.mocked(networking.getGuardrailProviderSpecificParams).mockResolvedValue({}); + vi.mocked(networking.updateGuardrailCall).mockResolvedValue({ status: "success" }); + + const { getByText, getByRole, getAllByRole, getByLabelText } = render( + { }} accessToken="123" isAdmin={true} />, + ); + + await waitFor(() => { + expect(getByText("Settings")).toBeInTheDocument(); + }); + + // Go to Settings tab + fireEvent.click(getByText("Settings")); + + await waitFor(() => { + expect(getByText("Guardrail Settings")).toBeInTheDocument(); + }); + + // Enter Edit Mode + fireEvent.click(getByText("Edit Settings")); + + // Modify Guardrail Name to force an update + const nameInput = getByLabelText("Guardrail Name"); + fireEvent.change(nameInput, { target: { value: "Updated Name" } }); + + // Save with only name change + const saveButton = getByText("Save Changes"); + fireEvent.click(saveButton); + + await waitFor(() => { + expect(networking.updateGuardrailCall).toHaveBeenCalled(); + }); + + // Verify call did NOT include patterns or blocked_words (because no changes) + // updateGuardrailCall(accessToken, guardrailId, updateData) -> index 2 is updateData + const firstCallArgs: any = vi.mocked(networking.updateGuardrailCall).mock.calls[0][2]; + + // Verify attributes that definitely changed + expect(firstCallArgs.guardrail_name).toBe("Updated Name"); + + // litellm_params might be undefined if empty, which is correct. + // If it exists, ensure patterns/blocked_words are not in it. + if (firstCallArgs.litellm_params) { + expect(firstCallArgs.litellm_params.patterns).toBeUndefined(); + expect(firstCallArgs.litellm_params.blocked_words).toBeUndefined(); + } + + // Clear mocks to reset call count + vi.clearAllMocks(); + + // Enter Edit Mode again to make changes + await waitFor(() => { + expect(getByText("Edit Settings")).toBeInTheDocument(); + }); + fireEvent.click(getByText("Edit Settings")); + + // Now modify the values using the mock button + const simulateChangeButton = getByText("Simulate Change"); + fireEvent.click(simulateChangeButton); + + // Save again + fireEvent.click(getByText("Save Changes")); + + await waitFor(() => { + expect(networking.updateGuardrailCall).toHaveBeenCalled(); + }); + + // Verify call INCLUDES patterns and blocked_words + const secondCallArgs: any = vi.mocked(networking.updateGuardrailCall).mock.calls[0][2]; + expect(secondCallArgs.litellm_params).toBeDefined(); + expect(secondCallArgs.litellm_params.patterns).toEqual(["new_pattern"]); + expect(secondCallArgs.litellm_params.blocked_words).toEqual(["new_word"]); + }); }); diff --git a/ui/litellm-dashboard/src/components/guardrails/guardrail_info.tsx b/ui/litellm-dashboard/src/components/guardrails/guardrail_info.tsx index 0d1a205f66..6c91e59d3b 100644 --- a/ui/litellm-dashboard/src/components/guardrails/guardrail_info.tsx +++ b/ui/litellm-dashboard/src/components/guardrails/guardrail_info.tsx @@ -271,7 +271,7 @@ const GuardrailInfoView: React.FC = ({ guardrailId, onClose, } // Only add Content Filter patterns if there are changes - if (guardrailData.litellm_params?.guardrail === "litellm_content_filter") { + if (guardrailData.litellm_params?.guardrail === "litellm_content_filter" && hasUnsavedContentFilterChanges) { const originalPatterns = guardrailData.litellm_params?.patterns || []; const originalBlockedWords = guardrailData.litellm_params?.blocked_words || []; @@ -357,6 +357,9 @@ const GuardrailInfoView: React.FC = ({ guardrailId, onClose, console.log("allowedParams: ", allowedParams); allowedParams.forEach((paramName) => { + if (paramName === "patterns" || paramName === "blocked_words") { + return; + } // Check for both direct parameter name and nested optional_params object let paramValue = values[paramName]; if (paramValue === undefined || paramValue === null || paramValue === "") {