fix(ui): don't overwrite guardrails with empty array on model edit

The edit-model form initialized guardrails to [] and then conditionally
included it in the update payload via `if (values.guardrails)`. Empty
arrays are truthy in JavaScript, so editing any unrelated field (e.g.
temperature) on a model without guardrails would inject guardrails: []
into the model's litellm_params via the PATCH-merge backend.

Mirror the vector_store_ids pattern (commit 91d88737b4):
- Initialize the form value to undefined when there are no stored
  guardrails, so we can distinguish 'untouched' from 'cleared'.
- Only include the field when length > 0; if the user explicitly
  cleared previously-set guardrails, send [] to clear on the backend;
  otherwise delete the key from the payload so PATCH leaves the stored
  value untouched.

Add a regression test to lock in the fixed behavior.

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
This commit is contained in:
Cursor Agent 2026-05-09 19:13:06 +00:00
parent 96f23c2e6f
commit 794dcdbf20
No known key found for this signature in database
2 changed files with 39 additions and 4 deletions

View file

@ -579,6 +579,34 @@ describe("ModelInfoView", () => {
expect(updatePayload.litellm_params).not.toHaveProperty("vector_store_ids");
});
it("should not include guardrails in update payload when model has none and user does not touch the field", async () => {
// Regression: editing a model without guardrails used to inject
// guardrails: [] into litellm_params on every save, because the
// submit handler used `if (values.guardrails)` and an empty array
// is truthy in JavaScript.
const user = userEvent.setup();
render(<ModelInfoView {...DEFAULT_ADMIN_PROPS} />, { wrapper });
await waitFor(() => {
expect(screen.getByRole("button", { name: /edit settings/i })).toBeInTheDocument();
});
await user.click(screen.getByRole("button", { name: /edit settings/i }));
await waitFor(() => {
expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument();
});
await user.click(screen.getByRole("button", { name: /save changes/i }));
await waitFor(() => {
expect(mockModelPatchUpdateCall).toHaveBeenCalled();
});
const updatePayload = mockModelPatchUpdateCall.mock.calls[0][1];
expect(updatePayload.litellm_params).not.toHaveProperty("guardrails");
});
it("should not include input_cost_per_token or output_cost_per_token in update payload when user does not touch cost fields", async () => {
// Regression: editing a model without touching cost fields used to inject
// input_cost_per_token: 0 and output_cost_per_token: 0 into litellm_params,

View file

@ -268,8 +268,13 @@ export default function ModelInfoView({
} else {
delete updatedLitellmParams.litellm_credential_name;
}
if (values.guardrails) {
if (values.guardrails?.length > 0) {
updatedLitellmParams.guardrails = values.guardrails;
} else if (values.guardrails !== undefined) {
// User explicitly cleared previously-set guardrails — send [] to clear on backend
updatedLitellmParams.guardrails = [];
} else {
delete updatedLitellmParams.guardrails;
}
if (values.vector_store_ids?.length > 0) {
updatedLitellmParams.vector_store_ids = values.vector_store_ids;
@ -637,9 +642,11 @@ export default function ModelInfoView({
model_access_group: Array.isArray(localModelData.model_info?.access_groups)
? localModelData.model_info.access_groups
: [],
guardrails: Array.isArray(localModelData.litellm_params?.guardrails)
? localModelData.litellm_params.guardrails
: [],
guardrails:
Array.isArray(localModelData.litellm_params?.guardrails) &&
localModelData.litellm_params.guardrails.length > 0
? localModelData.litellm_params.guardrails
: undefined,
vector_store_ids:
Array.isArray(localModelData.litellm_params?.vector_store_ids) &&
localModelData.litellm_params.vector_store_ids.length > 0