From 286e28c022d551a262e2e3511df4fe4779121b99 Mon Sep 17 00:00:00 2001 From: Rehan Sanjay Date: Sun, 13 Sep 2026 13:10:46 +0530 Subject: [PATCH] fix(ui): omit semantic fields when the toggle is off instead of sending null Sending an explicit null for semantic_cache_scope reaches Cache(semantic_cache_scope=None) on the proxy, and SemanticCacheScope(None) raises. The toggle is off by default, so every plain Redis save persisted the null, failed with a 500 and left the previous cache running; Test Connection failed the same way, and a restart could not rebuild the cache from the row. POST /cache/settings stores the payload as sent (only credentials are merged from the saved row), so leaving the semantic fields out already clears them, and an omitted scope falls back to "key". Found by the Veria review on this PR. Written with AI assistance (Claude Code); reviewed and verified before pushing. --- .../cache_settings/cacheSettingsUtils.test.ts | 18 +++++++----------- .../cache_settings/cacheSettingsUtils.ts | 7 ++++--- .../cache_settings/index.integration.test.tsx | 4 ---- .../_components/cache_settings/index.test.tsx | 4 ---- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts index 86a20f0ba2d..6e63f9e6dad 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.test.ts @@ -51,10 +51,6 @@ describe("buildCachePayload", () => { port: "6379", ssl: false, ssl_check_hostname: false, - // semantic caching is off, so its fields are explicitly cleared - similarity_threshold: null, - redis_semantic_cache_embedding_model: null, - semantic_cache_scope: null, }); expect(payload).not.toHaveProperty("redis_type"); expect(payload).not.toHaveProperty("username"); @@ -84,15 +80,15 @@ describe("buildCachePayload", () => { expect(payload.similarity_threshold).toBe(0.9); }); - it("should send null for semantic fields when semantic caching is disabled, so turning it off clears them", () => { + it("should omit semantic fields when semantic caching is disabled, even if they hold values", () => { const payload = buildCachePayload( "node", - { similarity_threshold: 0.9 }, + { similarity_threshold: 0.9, redis_semantic_cache_embedding_model: "text-embedding-3-small" }, { forTesting: false, semanticEnabled: false }, ); expect(payload.type).toBe("redis"); - expect(payload.similarity_threshold).toBe(null); - expect(payload.redis_semantic_cache_embedding_model).toBe(null); + expect(payload).not.toHaveProperty("similarity_threshold"); + expect(payload).not.toHaveProperty("redis_semantic_cache_embedding_model"); }); it("should send the semantic cache scope only when semantic caching is enabled", () => { @@ -102,14 +98,14 @@ describe("buildCachePayload", () => { { forTesting: false, semanticEnabled: true }, ); expect(enabled.semantic_cache_scope).toBe("end_user"); - // Disabled sends an explicit null rather than omitting the field, so a scope - // configured earlier is cleared on the server instead of silently surviving. + // Disabled must omit the scope rather than send null: the backend rejects a null scope + // when it rebuilds the cache, while an omitted one falls back to its default. const disabled = buildCachePayload( "node", { semantic_cache_scope: "end_user" }, { forTesting: false, semanticEnabled: false }, ); - expect(disabled.semantic_cache_scope).toBe(null); + expect(disabled).not.toHaveProperty("semantic_cache_scope"); }); it("should keep type redis when testing with semantic caching enabled so the test endpoint accepts it", () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts index 1571d31f088..7f850d19e9c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/cacheSettingsUtils.ts @@ -94,10 +94,11 @@ export const buildCachePayload = ( const type = !forTesting && semanticEnabled ? "redis-semantic" : "redis"; const entries = CACHE_FIELDS.filter((field) => isFieldVisible(field, redisType)).flatMap((field) => { - // Semantic fields are always visible now, so clearing the toggle has to send an - // explicit null - omitting them would leave the previous values on the server. + // Semantic fields are always visible now, so leave them out when the toggle is off. The + // backend stores the payload as sent, which already clears them; an explicit null would + // reach Cache(semantic_cache_scope=None) and fail both the save and the connection test. if (field.section === "semantic" && !semanticEnabled) { - return [[field.name, null] as [string, CacheSavePayloadValue]]; + return []; } const value = saveValueForField(field, values[field.name]); return value === undefined ? [] : [[field.name, value] as [string, CacheSavePayloadValue]]; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.integration.test.tsx index 26ff5179e17..0d1da4dc8ce 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.integration.test.tsx @@ -61,10 +61,6 @@ describe("CacheSettings advanced settings round-trip", () => { namespace: "prod-ns", ttl: 300, max_connections: 50, - // semantic caching is off, so its fields are cleared explicitly - similarity_threshold: null, - redis_semantic_cache_embedding_model: null, - semantic_cache_scope: null, }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.test.tsx index 80ab2c86ca2..196154bbaf6 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/caching/_components/cache_settings/index.test.tsx @@ -151,10 +151,6 @@ describe("CacheSettings", () => { port: "6379", ssl: false, ssl_check_hostname: false, - // semantic caching is off, so its fields are cleared explicitly - similarity_threshold: null, - redis_semantic_cache_embedding_model: null, - semantic_cache_scope: null, }), ); });