mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-02 02:11:58 +00:00
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.
This commit is contained in:
parent
07af4a512a
commit
286e28c022
4 changed files with 11 additions and 22 deletions
|
|
@ -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", () => {
|
||||
|
|
|
|||
|
|
@ -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]];
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue