mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-02 02:11:58 +00:00
fix(ui): only offer semantic caching for a single Redis node
The redis-semantic backend builds one redis://host:port url for redisvl and has no cluster or sentinel support, so the toggle must not be combinable with those topologies. It is now disabled for Cluster and Sentinel with a short note, and buildCachePayload never sends redis-semantic for them A saved redis-semantic config comes back from GET /cache/settings without a redis_type, so it already loads as a node. The toggle now also turns on from type: redis-semantic, not only from the semantic values Drops the explanatory comments added earlier, per the repository's comment rule Found by the Greptile review on this PR Written with AI assistance (Claude Code); reviewed and verified before pushing
This commit is contained in:
parent
286e28c022
commit
40aa2bf639
4 changed files with 51 additions and 14 deletions
|
|
@ -80,6 +80,20 @@ describe("buildCachePayload", () => {
|
|||
expect(payload.similarity_threshold).toBe(0.9);
|
||||
});
|
||||
|
||||
it.each(["cluster", "sentinel"] as const)(
|
||||
"should never send redis-semantic for %s, which the semantic cache cannot connect to",
|
||||
(redisType) => {
|
||||
const payload = buildCachePayload(
|
||||
redisType,
|
||||
{ similarity_threshold: 0.9, semantic_cache_scope: "end_user" },
|
||||
{ forTesting: false, semanticEnabled: true },
|
||||
);
|
||||
expect(payload.type).toBe("redis");
|
||||
expect(payload).not.toHaveProperty("similarity_threshold");
|
||||
expect(payload).not.toHaveProperty("semantic_cache_scope");
|
||||
},
|
||||
);
|
||||
|
||||
it("should omit semantic fields when semantic caching is disabled, even if they hold values", () => {
|
||||
const payload = buildCachePayload(
|
||||
"node",
|
||||
|
|
@ -98,8 +112,6 @@ describe("buildCachePayload", () => {
|
|||
{ forTesting: false, semanticEnabled: true },
|
||||
);
|
||||
expect(enabled.semantic_cache_scope).toBe("end_user");
|
||||
// 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" },
|
||||
|
|
|
|||
|
|
@ -86,18 +86,18 @@ const saveValueForField = (field: CacheField, raw: CacheFormValue): CacheSavePay
|
|||
return trimmed === "" ? undefined : trimmed;
|
||||
};
|
||||
|
||||
export const supportsSemanticCache = (redisType: RedisType): boolean => redisType === "node";
|
||||
|
||||
export const buildCachePayload = (
|
||||
redisType: RedisType,
|
||||
values: CacheFormValues,
|
||||
{ forTesting, semanticEnabled = false }: { forTesting: boolean; semanticEnabled?: boolean },
|
||||
): CacheSavePayload => {
|
||||
const type = !forTesting && semanticEnabled ? "redis-semantic" : "redis";
|
||||
const semantic = semanticEnabled && supportsSemanticCache(redisType);
|
||||
const type = !forTesting && semantic ? "redis-semantic" : "redis";
|
||||
|
||||
const entries = CACHE_FIELDS.filter((field) => isFieldVisible(field, redisType)).flatMap((field) => {
|
||||
// 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) {
|
||||
if (field.section === "semantic" && !semantic) {
|
||||
return [];
|
||||
}
|
||||
const value = saveValueForField(field, values[field.name]);
|
||||
|
|
|
|||
|
|
@ -93,6 +93,26 @@ describe("CacheSettings", () => {
|
|||
renderSettings();
|
||||
expect(await screen.findByText("Similarity Threshold")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should load a saved redis-semantic config as a node with the toggle on", async () => {
|
||||
getCacheSettingsCall.mockResolvedValue({ current_values: { type: "redis-semantic", host: "localhost" } });
|
||||
renderSettings();
|
||||
expect(await screen.findByText("Similarity Threshold")).toBeInTheDocument();
|
||||
expect(screen.getByText("Node (Single Instance)")).toBeInTheDocument();
|
||||
expect(screen.getByRole("switch")).not.toHaveAttribute("data-disabled");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["cluster", "Startup Nodes"],
|
||||
["sentinel", "Sentinel Nodes"],
|
||||
])("should disable the toggle and hide the semantic fields for %s", async (redisType, topologyField) => {
|
||||
getCacheSettingsCall.mockResolvedValue({ current_values: { redis_type: redisType, similarity_threshold: 0.9 } });
|
||||
renderSettings();
|
||||
expect(await screen.findByText(topologyField)).toBeInTheDocument();
|
||||
expect(screen.getByRole("switch")).toHaveAttribute("data-disabled");
|
||||
expect(screen.getByText(/Semantic caching needs a single Redis node/)).toBeInTheDocument();
|
||||
expect(screen.queryByText("Similarity Threshold")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe("when a field fails inline validation", () => {
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import {
|
|||
CacheFormValues,
|
||||
configuredSecretFields,
|
||||
isFieldVisible,
|
||||
supportsSemanticCache,
|
||||
} from "./cacheSettingsUtils";
|
||||
|
||||
const ADVANCED_SECTIONS = ["ssl", "cacheManagement", "gcp"] as const;
|
||||
|
|
@ -39,6 +40,8 @@ const CacheSettings: React.FC<CacheSettingsProps> = ({ accessToken }) => {
|
|||
const [isSaving, setIsSaving] = useState<boolean>(false);
|
||||
const [configuredSecrets, setConfiguredSecrets] = useState<ReadonlySet<string>>(new Set());
|
||||
const [semanticEnabled, setSemanticEnabled] = useState<boolean>(false);
|
||||
const semanticAvailable = supportsSemanticCache(redisType);
|
||||
const semanticActive = semanticEnabled && semanticAvailable;
|
||||
|
||||
const loadCacheSettings = useCallback(async () => {
|
||||
if (!accessToken) {
|
||||
|
|
@ -50,9 +53,9 @@ const CacheSettings: React.FC<CacheSettingsProps> = ({ accessToken }) => {
|
|||
form.reset(buildInitialValues(currentValues));
|
||||
setConfiguredSecrets(configuredSecretFields(currentValues));
|
||||
setRedisType(toRedisType(currentValues.redis_type));
|
||||
// "semantic" is no longer a redis_type, but existing configs were saved with it.
|
||||
setSemanticEnabled(
|
||||
currentValues.redis_type === "semantic" ||
|
||||
currentValues.type === "redis-semantic" ||
|
||||
currentValues.redis_type === "semantic" ||
|
||||
currentValues.similarity_threshold != null ||
|
||||
currentValues.redis_semantic_cache_embedding_model != null,
|
||||
);
|
||||
|
|
@ -110,7 +113,7 @@ const CacheSettings: React.FC<CacheSettingsProps> = ({ accessToken }) => {
|
|||
try {
|
||||
const result = await testCacheConnectionCall(
|
||||
accessToken,
|
||||
buildCachePayload(redisType, values, { forTesting: true, semanticEnabled }),
|
||||
buildCachePayload(redisType, values, { forTesting: true, semanticEnabled: semanticActive }),
|
||||
);
|
||||
if (result.status === "success") {
|
||||
toast.success("Cache connection test successful!");
|
||||
|
|
@ -138,7 +141,7 @@ const CacheSettings: React.FC<CacheSettingsProps> = ({ accessToken }) => {
|
|||
try {
|
||||
await updateCacheSettingsCall(
|
||||
accessToken,
|
||||
buildCachePayload(redisType, values, { forTesting: false, semanticEnabled }),
|
||||
buildCachePayload(redisType, values, { forTesting: false, semanticEnabled: semanticActive }),
|
||||
);
|
||||
toast.success("Cache settings updated successfully");
|
||||
await loadCacheSettings();
|
||||
|
|
@ -205,15 +208,17 @@ const CacheSettings: React.FC<CacheSettingsProps> = ({ accessToken }) => {
|
|||
|
||||
<div className="pt-4 border-t border-border">
|
||||
<div className="mb-4 flex items-center gap-3">
|
||||
<Switch checked={semanticEnabled} onCheckedChange={setSemanticEnabled} />
|
||||
<Switch checked={semanticActive} disabled={!semanticAvailable} onCheckedChange={setSemanticEnabled} />
|
||||
<div>
|
||||
<span className="text-sm font-medium text-foreground">Enable Semantic Caching</span>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Reuse responses for semantically similar prompts using embedding vectors
|
||||
{semanticAvailable
|
||||
? "Reuse responses for semantically similar prompts using embedding vectors"
|
||||
: "Semantic caching needs a single Redis node, so it is unavailable for Cluster and Sentinel"}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
{semanticEnabled && (
|
||||
{semanticActive && (
|
||||
<CacheFieldSection
|
||||
title="Semantic Configuration"
|
||||
section="semantic"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue