Skip to content

feat(ui): point Organization Settings save at /v2/organization/{id} - #32377

Closed
ryan-crabbe-berri wants to merge 3 commits into
litellm_lit_3664_tpm_limit_ui_not_persistedfrom
litellm_lit_3664_org_settings_v2_cutover
Closed

feat(ui): point Organization Settings save at /v2/organization/{id}#32377
ryan-crabbe-berri wants to merge 3 commits into
litellm_lit_3664_tpm_limit_ui_not_persistedfrom
litellm_lit_3664_org_settings_v2_cutover

Conversation

@ryan-crabbe-berri

@ryan-crabbe-berri ryan-crabbe-berri commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

Stacked on #32350 (the PATCH /v2/organization/{organization_id} backend endpoint); this PR points the Admin UI at it. Base branch is litellm_lit_3664_tpm_limit_ui_not_persisted, so review/merge that one first

Linear ticket

Refs LIT-3664

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

UI change, so the proof is the Organization Settings flow against a running proxy. In order:

  1. Go to http://localhost:4000/ui/?page=organizations and open an organization that has a TPM limit and/or metadata set
  2. On the Settings tab, click Edit Settings, clear the TPM (or RPM / Max Budget) field and/or empty the Metadata box, then click Save Changes
  3. Refresh the page. The cleared values stay cleared instead of reverting. In the browser Network tab the save is a PATCH /v2/organization/{organization_id} whose body carries only the changed fields, with the cleared field sent as null (metadata as null)
  4. Edit Settings again, type invalid JSON into the Metadata box, and click Save. Save is blocked with an inline "Metadata must be a valid JSON object" error and no request is sent

Devin e2e QA

Ran the branch locally (PostgreSQL-backed proxy at localhost:4000 with the rebuilt Admin UI) and exercised the Organization Settings save flow through the browser, using the DevTools Network tab to confirm the request URL, the exact request body, and the no-request cases. Full write-up with screenshots and the recorded walkthroughs were shared with the author on Slack. Session: https://app.devin.ai/sessions/fd99cd1fdb5541c688a8ce91ab5f7e5f

Seven cases passed. Clearing Metadata and TPM on a populated org sent PATCH /v2/organization/{id} with only {tpm_limit: null, metadata: null} and the cleared values stayed cleared after a full refresh while the untouched RPM and Max Budget were preserved, which is the LIT-3664 regression. Saving with no edits sent an empty body. Setting Max Budget, TPM, and Metadata on an empty org sent only those fields with the numerics as JSON numbers rather than strings, and they persisted after refresh. Changing only the alias and reset-budget sent only organization_alias and budget_duration. Clearing the models multiselect sent {models: []}. Invalid metadata JSON blocked Save with the inline error and issued no request.

Object-permission path (follow-up, now runtime-verified). Created a vector store (qa-vs-1) and an MCP server (qa_mcp_1) with access group qa_group_a, then on a real org: selecting all three sent only object_permission with the three populated arrays and persisted after a full reload; removing all three sent only object_permission with {vector_stores: [], mcp_servers: [], mcp_access_groups: []} and the cleared state persisted after reload (budget/RPM untouched and preserved). This is the exact clear-and-persist behavior Greptile flagged as a P1 on an earlier commit, confirmed fixed at runtime.

One item to flag. The failing check proxy-infra / Run tests (test_gateway_plus_backend_covers_full_app) is unrelated to this UI diff; it fails because /v2/organization/{organization_id} is missing from the gateway/backend route allowlist, which belongs to the stacked base PR #32350 and should be fixed there.

Type

🐛 Bug Fix

Changes

The Organization Settings save was calling v1 /organization/update, which drops cleared fields, so clearing a limit or the Metadata box reverted on refresh (LIT-3664). This points the save at the deterministic PATCH /v2/organization/{organization_id} endpoint

The payload is built by a pure, typed buildOrganizationUpdateV2Payload that sends only the fields the user actually touched (antd isFieldTouched). An emptied field carries its clear-token (null for numbers, [] for arrays such as models and object permissions, null for metadata), an unchanged field is omitted so the backend leaves it untouched, and a numeric input coerces to number | null rather than an empty string. The request body and response are typed from the generated OpenAPI schema (components["schemas"]["OrganizationUpdateRequestV2"] and LiteLLM_OrganizationTableWithMembers), so the form-to-network path is typed end to end. Invalid metadata JSON blocks Save with an inline error through a Form.Item validator

Tests live next to the module: organizationUpdatePayload.test.ts covers the full set/clear/untouched matrix (limits, metadata set/edit/clear/unchanged/invalid, models, alias, budget duration, nothing-changed, and a bundled multi-change), and an added organization_view.test.tsx case asserts that invalid metadata blocks Save and never calls the update API

Cuts the Admin UI's Organization Settings save over to the deterministic v2 endpoint so clearing a limit or the Metadata box persists instead of reverting

A pure, typed buildOrganizationUpdateV2Payload value-diffs the form against a server-seeded baseline and sends only changed fields, with an emptied field carrying its clear-token (null for numbers, [] for models, null for metadata). The request body and response are typed from the generated OpenAPI schema, and invalid metadata JSON now blocks Save with an inline error via a Form.Item validator

Refs LIT-3664
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes LIT-3664 by pointing the Organization Settings save at the new PATCH /v2/organization/{id} endpoint instead of /organization/update, which was silently dropping cleared fields and causing reverts on refresh.

  • Introduces buildOrganizationUpdateV2Payload, a pure typed function that value-diffs the form against its server-seeded baseline and sends only touched fields, with clear-tokens (null for numbers/metadata, [] for arrays) when a user empties a field.
  • Adds an Ant Design Form.Item validator that blocks Save on invalid metadata JSON and shows an inline error, with the request/response path fully typed from the generated OpenAPI schema.
  • Tests cover the full set/clear/untouched matrix in unit form (organizationUpdatePayload.test.ts) and two integration cases in organization_view.test.tsx (invalid metadata blocks submit, alias change sends only the changed field).

Confidence Score: 5/5

Safe to merge once the stacked base PR lands; all changed paths are UI-only and scoped to the Organization Settings save flow.

The fix is well-contained: a new pure payload builder with exhaustive unit tests, a correctly typed network call using generated OpenAPI types, and form logic that correctly relies on Ant Design Form.Item to track touched fields. The object_permission bundling is intentional and correct. No auth changes, no backend logic, no schema migrations.

No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/organization/organizationUpdatePayload.ts New pure module with typed payload builder, metadata parser, and numeric coercion helpers; logic is correct and well-isolated.
ui/litellm-dashboard/src/components/organization/organization_view.tsx Switches save from V1 organizationUpdateCall to V2 PATCH; replaces manual setFieldValue wiring with Form.Item-controlled fields and adds metadata JSON validator.
ui/litellm-dashboard/src/components/networking.tsx Adds organizationUpdateV2Call typed against generated OpenAPI schema; uses encodeURIComponent on the org ID and re-throws for caller error handling.
ui/litellm-dashboard/src/components/organization/organizationUpdatePayload.test.ts Comprehensive unit tests covering all set/clear/untouched matrix cases for the payload builder, including the multi-change bundling test.
ui/litellm-dashboard/src/components/organization/organization_view.test.tsx Adds two new integration tests: one asserting invalid metadata blocks Save without calling the API, another verifying only the changed field is sent to the V2 endpoint.

Reviews (3): Last reviewed commit: "refactor(ui): derive org-settings save f..." | Re-trigger Greptile

Comment thread ui/litellm-dashboard/src/components/organization/organization_view.tsx Outdated
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…eline

Addresses review findings on the Organization Settings v2 save

object_permission is now built by the pure value-diff, so it is only sent when something in it actually changed, and clearing all MCP servers or access groups sends [] instead of silently keeping the stale value that previously reverted on refresh. The save-time baseline is snapshotted when edit mode opens rather than recomputed from live query data at submit, so a background refetch can no longer make an untouched field look changed. Adds object_permission cases to the pure payload tests and a happy-path component test proving Save sends only the changed fields to the v2 endpoint

Refs LIT-3664
@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

@greptileai re review

…a value-diff

Replaces the hand-rolled baseline + deepEqual value-diff with antd's own touched tracking: the save now sends a field only when form.isFieldTouched(name) is true, coerced at the boundary. Also un-wires the three custom selectors (ModelSelect, VectorStoreSelector, MCPServerSelector) so Form.Item injects value/onChange; the manual value={getFieldValue}/onChange={setFieldValue} bypassed antd's binding and broke touched tracking for models, vector stores, and MCP. Deletes OrgSettingsBaseline, buildOrgSettingsBaseline, deepEqual, buildObjectPermissionUpdate, and the editBaseline snapshot, so the builder is a single object-literal delta. Matches the Edit Model Settings form's isFieldTouched pattern

Refs LIT-3664
@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

@greptileai re review

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor Author

Closing in favor of rebuilding this form on the react-hook-form + zod + shadcn pattern, stacked on #32350. The hand-rolled value-diff here predates the form kit from #34170; the replacement derives the same minimal PATCH payload from RHF dirty tracking, so this antd wiring would be deleted right after merging anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant