Skip to content

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

Merged
mubashir1osmani merged 4 commits into
litellm_internal_stagingfrom
litellm_fix_store_prompts_toggle_state
Jul 6, 2026
Merged

fix(ui): reflect persisted "Store Prompts in Spend Logs" toggle on load#32145
mubashir1osmani merged 4 commits into
litellm_internal_stagingfrom
litellm_fix_store_prompts_toggle_state

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 4, 2026

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

Resolves LIT-4204

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)

End-to-end recording: toggle ON -> Save -> navigate away -> hard reload stays ON, then OFF -> Save -> hard reload stays OFF

store prompts toggle persistence

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 Bot and others added 2 commits July 4, 2026 19:28
@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

CLAassistant commented Jul 4, 2026

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 all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ryan-crabbe-berri
❌ devin-ai-integration[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Tested end-to-end on a local proxy (/ui/admin-panel -> Logging Settings). Both flows passed.

  • It should persist the toggle ON across a full page reload: passed
  • It should persist the toggle OFF across a full page reload: passed (confirms the sync handles both states, not a hardcoded ON)
Toggle ON persists after a full reload (the fix)
Baseline (OFF) on load ON + Saved (success toast)
baseline off saved

After navigating away and hard-reloading, the switch stays ON (pre-fix it reverted to OFF here):

stays on after reload

Toggle OFF persists after a full reload (edge case)

stays off after reload

Session: https://app.devin.ai/sessions/32b64ceba1494e5991cb45a42c68d9d1

@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 always rendered as OFF on page load, even though the value was correctly persisted in the database. The root cause was the form mounting before proxyConfigData arrived, writing false into the antd form store before the correct server value was available.

  • LoggingSettings.tsx: The Form is now wrapped in an isLoadingConfig ternary so it only mounts after data is available, guaranteeing initialValues is fully populated on first render. The redundant Form.useWatch + manual Switch binding is removed in favour of letting antd's valuePropName=\"checked\" mechanism handle the field.
  • LoggingSettings.test.tsx: Adds an async rerender regression test (loading → data arrives → switch is checked) that directly reproduces the original bug; the superseded "save button disabled during loading" test is replaced by a stricter assertion that the button is absent from the DOM while loading.

Confidence Score: 5/5

Safe to merge — the change is scoped to one UI component, the fix is mechanically sound, and the new test directly reproduces the reported bug scenario.

The conditional render is a minimal, well-understood solution that eliminates the race between antd's form store initialisation and asynchronous data arrival. No backend logic is touched. The removed test is replaced by equivalent or stricter assertions, and the new regression test would fail on the old code.

No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.tsx Replaced fragile key-based form remounting with conditional rendering; Form now mounts only after data loads, so initialValues are always fully populated on mount. Also drops the now-unnecessary Form.useWatch + manual Switch binding.
ui/litellm-dashboard/src/components/Settings/AdminSettings/LoggingSettings/LoggingSettings.test.tsx Replaces the removed "save button disabled during loading" test (button no longer renders at all while loading) with a stronger async rerender test that reproduces the original bug scenario. Coverage is maintained or improved.
ui/litellm-dashboard/eslint-metrics.json Decrements the no-explicit-any count from 1991 to 1990 reflecting the single any type removed from the component.

Reviews (2): Last reviewed commit: "style(ui): prettier-format LoggingSettin..." | Re-trigger Greptile

… an Effect

The persisted store_prompts_in_spend_logs toggle was reflected on load via a
useEffect that pushed proxy-config data into form state. Syncing server state
into local form state with an Effect is an antipattern, and it also clobbers a
user's in-progress edits whenever the config query refetches.

react-query (useProxyConfig) already provides the data and an isLoading flag.
antd applies initialValues once, at mount, so the real fix is to not mount the
Form until the config has loaded. Then initialValues (derived from the query
data) is correct on first render, and the Effect, the JSON.stringify remount
key, and the controlled Switch are all unnecessary; the form stays uncontrolled.
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor

Reworked how the persisted toggle is reflected on load (74ab60f).

The prior approach synced the proxy-config data into form state with a useEffect that called form.setFieldsValue whenever the query data changed. Pushing server state into local form state from an Effect is an antipattern, and it also clobbers a user's in-progress edits any time the config query refetches.

useProxyConfig (react-query) already exposes the data plus an isLoading flag, and antd applies initialValues once, at mount. So rather than seeding the form after mount, the Form now isn't mounted until the config has loaded; a skeleton shows while it loads. Once the data is present the Form mounts with initialValues derived from it, which is correct on the first render. That removed the useEffect, the key={JSON.stringify(initialValues)} remount hack, and the controlled Switch/useWatch, so the form is uncontrolled again

Tests: 14 pass, including should reflect persisted values that arrive after the initial loading render, which is the regression this targets and render-when-ready satisfies. I dropped the now-moot should disable save button while config is loading test (the button no longer renders during load) and folded a button-absence assertion into the skeleton test. eslint is clean; the no-explicit-any metric ratcheted 1991 to 1990 from removing that test

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Thanks — the render-when-ready approach is cleaner, and agreed that pushing server state into form state via an Effect was an antipattern (and would clobber in-progress edits on refetch). Heads up: frontend-lint was failing on 74ab60f purely from a Prettier line-wrap in the retention-period tooltip (unrelated to the eslint metric, which is fine at 1990). Fixed in 18b687d.

@ryan-crabbe-berri

Copy link
Copy Markdown
Contributor

@greptileai

@mubashir1osmani
mubashir1osmani merged commit 9a659b8 into litellm_internal_staging Jul 6, 2026
122 checks passed
@mubashir1osmani
mubashir1osmani deleted the litellm_fix_store_prompts_toggle_state branch July 6, 2026 16:09
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.

4 participants