refactor(ui): drive cache settings form from a typed frontend schema - #31939
Conversation
The Cache Settings form was dynamically generated from field metadata shipped by the backend, and read its values back out of the DOM with document.querySelector. That loses type safety and makes client-side validation awkward, which is a poor fit for a form whose shape only changes when a developer edits code. Move the field definitions (name, label, type, default, help text, which redis type they apply to, section, and validation rules) into a typed frontend module and render them through antd Form with controlled state. The GET /cache/settings endpoint is still used to populate current values, and the save/test payload shape sent to POST /cache/settings and /cache/settings/test is unchanged. Per-field validation now lives on each field's antd rules, so an inline error can surface before and on submit; this is where the upcoming Redis URL validation will slot in. The backend's fields output in GET /cache/settings is no longer consumed by the UI, but is left in place since removing it is a separate backend change.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR replaces the dynamically-generated, DOM-querying cache settings form with a typed frontend schema (
Confidence Score: 5/5Frontend-only refactor that is safe to merge; the save and test payloads sent to the backend are unchanged and the new typed schema is well-tested. The change removes all document.querySelector value gathering and replaces it with a typed antd Form. Every field now has explicit validation rules, the two list fields that were previously silently dropping invalid JSON now block submission with inline errors, and the utility functions are covered by pure unit tests plus an integration test suite. No backend contracts are changed. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsFields.ts | New typed field registry for all cache settings; includes port, db, cluster, sentinel, and semantic fields with appropriate validation rules. |
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsUtils.ts | Replaces DOM-querying form value gathering with pure typed functions; boolean fields always emit false rather than undefined, which is tested and intentional. |
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/index.tsx | Main component migrated to antd Form with controlled state; validate() follows standard antd pattern of catching ValidateErrorEntity to surface inline errors while blocking submission. |
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFormField.tsx | New field renderer that consumes CacheField type; correctly sets valuePropName="checked" for Switch controls and delegates validation rules from the field schema. |
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/CacheFieldSection.tsx | Thin layout wrapper that renders nothing when a section has no visible fields; Tailwind class strings are hardcoded literals so purge is safe. |
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/index.test.tsx | New integration tests covering per-type field visibility, inline validation blocking submit, and save payload shape; replaces the deleted CacheFieldRenderer and CacheFieldGroup tests. |
| ui/litellm-dashboard/src/app/(dashboard)/caching/components/cache_settings/cacheSettingsUtils.test.ts | Unit tests for fieldsForSection, buildInitialValues, and buildCachePayload; confirms list field invalid JSON is silently omitted at the utility level (guarded upstream by jsonListRule). |
Reviews (2): Last reviewed commit: "fix(ui): validate numeric cache fields a..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
sentinel_nodes and redis_startup_nodes had no validation rule, so malformed JSON passed validateFields, was caught while building the save payload, and the field was silently omitted; the user's cluster/sentinel config was discarded with no feedback. Add a jsonListRule (same shape as portRule) to both list fields so an invalid value surfaces inline and blocks save.
…he error
The Startup Nodes and Sentinel Nodes help text showed Python-style
single-quoted examples (e.g. [{'host': '127.0.0.1', 'port': '7001'}]),
which the JSON validator correctly rejects, so pasting the example we
display failed. Switch both examples to valid JSON with double quotes and
change the parse-error message to "Must be a valid JSON array (use double
quotes)" so the hint points at the fix. Also add a regression test
asserting a numeric field (Database Index) is included in the save payload.
Numeric fields (Database Index, TTL, Max Connections, Similarity Threshold) rendered as antd InputNumber, which silently coerces non-numeric input to empty. Because the fields are optional, an invalid entry like a full connection URL pasted into Database Index passed validation and was silently dropped from the save payload. Render numeric fields as text inputs with a validation rule (non-negative integer for Database Index and Max Connections, number for TTL and Similarity Threshold), mirroring how Port already works, so invalid input is preserved, flagged inline, and blocks submit instead of vanishing. The save payload still coerces these to real numbers. Adds a regression test for a non-numeric value entered into a numeric field.
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| type: "string", | ||
| section: "connection", | ||
| helpText: | ||
| "Full Redis/Valkey connection URL (e.g. redis://:password@host:6379/1). When set, it takes precedence over Host, Port, Password, and Database Index.", |
There was a problem hiding this comment.
Medium: Redis URL credentials are exposed
This new field explicitly supports embedding the Redis password in the URL, but /cache/settings only redacts password and sentinel_password before returning current_values. A read-only admin can fetch this endpoint and recover the Redis credential from current_values.url; treat url as sensitive end-to-end by redacting or stripping userinfo in the backend response, and avoid rendering credential-bearing URLs as a normal text field.
PR overviewThis pull request refactors the LiteLLM dashboard cache settings UI so the form is driven by a typed frontend schema. The touched cache settings field definitions include support for configuring Redis connection fields, including URL-based configuration. One security issue remains open: Redis credentials can be embedded in the cache URL and then returned through the cache settings endpoint without being redacted. This allows a read-only admin to recover the Redis credential from the rendered or fetched current settings. No issues have been addressed yet, so the PR still needs a backend redaction change and safer UI handling for credential-bearing URLs. Open issues (1)
Fixed/addressed: 0 · PR risk: 7/10 |
2 participants
Relevant issues
Linear ticket
LIT-3996 (UI cache config deficiencies)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
This is a UI-only refactor, so verification is in the browser against a proxy with
STORE_MODEL_IN_DB='True'and a database connected99999, and click Save Changes; confirm an inline error "Port must be an integer between 1 and 65535" shows and nothing is savednot jsoninto Startup Nodes, and click Save Changes; confirm an inline "Must be valid JSON" error shows and nothing is savedType
🧹 Refactoring
Changes
The Cache Settings form used to be generated dynamically from field metadata the backend shipped in
GET /cache/settings, and it read the submitted values back out of the DOM withdocument.querySelector. For a form whose shape only changes when a developer edits code, that loses type safety and makes client-side validation hardThis moves the field definitions into a typed frontend module (
cacheSettingsFields.ts) that mirrors the backendCACHE_SETTINGS_FIELDS, including the recently addedurlanddbfields. Each field declares its name, label, type, default, help text, the redis type it applies to, its section, and optional antd validation rules. The form is now an antdFormwith controlled state;cacheSettingsUtils.tsbuilds the initial values and the save payload as pure, typed functions, so thedocument.querySelectorvalue-gathering is goneThe save and test payloads sent to
POST /cache/settingsandPOST /cache/settings/testare unchanged, and current values are still populated fromGET /cache/settings. Per-field validation lives on each field'srules, so an invalid value blocks submit and surfaces inline before and on save; that is the clean slot where the upcoming Redis URL validation will goThis is a frontend-only change. The
urlanddbbackend handling (field definitions plus url-precedence resolution) rides along with the sibling backend work; nothing here touches the backend. Thefieldsvalue in theGET /cache/settingsresponse is no longer consumed by the UI, but it is left in place since removing it is a separate backend changeCacheFieldRendererandCacheFieldGroup(and their tests) are replaced byCacheFormFieldandCacheFieldSection. Tests cover the fields rendered per redis type, inline validation blocking submit, and the exact save payload shape