mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-18 06:26:16 +00:00
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
This commit is contained in:
@@ -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) => (
|
||||
<div data-testid="mock-content-filter-manager">
|
||||
{isEditing && (
|
||||
<button onClick={() => {
|
||||
onUnsavedChanges(true);
|
||||
onDataChange?.(["new_pattern"], ["new_word"]);
|
||||
}}>
|
||||
Simulate Change
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
),
|
||||
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(
|
||||
<GuardrailInfoView guardrailId="123" onClose={() => { }} 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"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -271,7 +271,7 @@ const GuardrailInfoView: React.FC<GuardrailInfoProps> = ({ 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<GuardrailInfoProps> = ({ 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 === "") {
|
||||
|
||||
Reference in New Issue
Block a user