Skip to content

fix(ui): reflect persisted "Store Prompts in Spend Logs" toggle on load - #32143

Closed
devin-ai-integration[bot] wants to merge 2 commits into
litellm_internal_stagingfrom
devin/1783193286-fix-store-prompts-toggle
Closed

fix(ui): reflect persisted "Store Prompts in Spend Logs" toggle on load#32143
devin-ai-integration[bot] wants to merge 2 commits into
litellm_internal_stagingfrom
devin/1783193286-fix-store-prompts-toggle

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Relevant issues

Admin Settings → Logging Settings → "Store Prompts in Spend Logs" toggle appeared to revert to OFF. The value was correctly persisted to the DB (verified via /config/update + /config/list), but the switch rendered as OFF whenever the config loaded asynchronously (fresh page load / navigating back to the tab).

Linear ticket

Pre-Submission checklist

  • 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 requested a Greptile review

Screenshots / Proof of Fix

Backend was already persisting correctly; the bug was purely in the UI. Reproduced live on a local proxy (/ui/admin-panel → Logging Settings) with store_prompts_in_spend_logs=true in the DB.

Before (DB = true, switch renders OFF on load):

before

After (DB = true, switch renders ON on load):

after

DB round-trip confirming persistence both directions:

$ curl -s "$PROXY/config/list?config_type=general_settings" -H "Authorization: Bearer sk-1234" | jq '... store_prompts_in_spend_logs'
# after saving ON  -> [True]
# after saving OFF -> [False]

Type

🐛 Bug Fix

Changes

Root cause: the form was reset via key={JSON.stringify(initialValues)} while reusing the same useForm instance. On first render proxyConfigData is undefined, so the form mounted with store_prompts_in_spend_logs: false and wrote that into the (preserved) form store. When the config arrived and the key changed, antd re-mounts the Form element but keeps the persisted store, so the newer initialValues (true) were not re-applied — the Switch, driven by Form.useWatch, kept showing false.

Fix in LoggingSettings.tsx:

  • Sync the form from server data with form.setFieldsValue(initialValues) in a useEffect keyed on proxyConfigData (re-applies on initial load and after post-save refetch), instead of the fragile key-remount.
  • Let Form.Item own the switch (name + valuePropName="checked"); drop the manual Form.useWatch + controlled checked/onChange.
- const storePromptsValue = Form.useWatch("store_prompts_in_spend_logs", form);
+ useEffect(() => {
+   if (proxyConfigData) form.setFieldsValue(initialValues);
+ }, [form, proxyConfigData, initialValues]);

- <Form key={proxyConfigData ? JSON.stringify(initialValues) : "loading"} ...>
+ <Form form={form} layout="vertical" onFinish={handleFormSubmit} initialValues={initialValues}>
-   <Switch checked={storePromptsValue ?? false} onChange={(c) => form.setFieldValue(...)} />
+   <Switch />

Added a regression test (LoggingSettings.test.tsx) that renders in the loading state first, then rerenders with field_value: true, and asserts the switch is checked — this fails on the old code and passes with the fix.

Link to Devin session: https://app.devin.ai/sessions/32b64ceba1494e5991cb45a42c68d9d1

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a UI-only bug where the "Store Prompts in Spend Logs" toggle incorrectly showed as OFF on page load even when the backend had it set to true. The root cause was that antd's Form persists its internal store across key-driven remounts, so the initialValues prop had no effect after the async config arrived.

  • LoggingSettings.tsx: Removes key-based remount and Form.useWatch; adds a useEffect that calls form.setFieldsValue(initialValues) whenever proxyConfigData changes, reliably syncing the form to server state after the async load resolves.
  • LoggingSettings.test.tsx: Adds a new test that starts in the loading state then rerenders with real config data, asserting the switch becomes checked — this test fails on the old code and passes with the fix.

Confidence Score: 5/5

Small, focused UI fix with a targeted regression test; no backend or auth changes.

The change touches only two files in a React component: it replaces one pattern for syncing async data into an antd form with a more correct one, and adds a test that directly reproduces the bug. The logic is straightforward — form.setFieldsValue inside a useEffect keyed on the server data is the standard antd pattern for this scenario. The initialValues prop on Form is now only used as the mount-time default (its documented scope), and explicit setFieldsValue handles subsequent async updates. No security, auth, or data-path logic is touched.

No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx Replaces fragile key-remount strategy with a useEffect + setFieldsValue pattern so persisted toggle values from the DB are correctly reflected on load; drops the now-unnecessary Form.useWatch.
ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx Adds a regression test that renders in the loading state first and then rerenders with real data, asserting the switch reflects the persisted value — this test would have failed on the old code.

Reviews (1): Last reviewed commit: "fix(ui): reflect persisted store_prompts..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Superseded by #32145 (rebranched to a litellm_ prefix so enterprise CI runs). Same changes.

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