Skip to content

fix(caching): forward metadata to embedding in valkey semantic sync path - #32494

Open
devin-ai-integration[bot] wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_fix_valkey_semantic_sync_metadata_32324
Open

fix(caching): forward metadata to embedding in valkey semantic sync path#32494
devin-ai-integration[bot] wants to merge 1 commit into
litellm_internal_stagingfrom
litellm_fix_valkey_semantic_sync_metadata_32324

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Relevant issues

Fixes #32324

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 received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

In-process caching logic, so proof is a regression test on the sync set_cache/get_cache paths that spies the embedding call and asserts the request metadata is forwarded. Captured before/after on this branch.

Before (on litellm_internal_staging): the sync path calls self._get_embedding(prompt) with no metadata, so the spy sees None:

FAILED tests/test_litellm/caching/test_valkey_semantic_cache.py::test_set_cache_passes_metadata_to_get_embedding
FAILED tests/test_litellm/caching/test_valkey_semantic_cache.py::test_get_cache_passes_metadata_to_get_embedding

After:

tests/test_litellm/caching/test_valkey_semantic_cache.py::test_set_cache_passes_metadata_to_get_embedding PASSED
tests/test_litellm/caching/test_valkey_semantic_cache.py::test_get_cache_passes_metadata_to_get_embedding PASSED
33 passed

Type

🐛 Bug Fix

Changes

The async crash originally reported in #32324 (_get_async_embedding receiving **kwargs such as cache_key) was already fixed by #32295, which changed the two async embedding calls to pass metadata=kwargs.get("metadata"). The synchronous set_cache and get_cache paths in valkey_semantic_cache.py were left calling self._get_embedding(prompt) with no metadata at all, so the embedding request lost the request metadata (used for embedding spend attribution and per-deployment routing). This diverged from both the async valkey path and RedisSemanticCache, whose sync path already passes metadata=kwargs.get("metadata").

This aligns the two sync calls with the async valkey path and Redis:

embedding = self._get_embedding(prompt, metadata=kwargs.get("metadata"))

_get_embedding already accepts an optional metadata argument, so no signature change is needed.

Added test_set_cache_passes_metadata_to_get_embedding and test_get_cache_passes_metadata_to_get_embedding, mirroring the existing async metadata-forwarding tests. Both fail on the pre-fix sync calls and pass after.

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

@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 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a metadata forwarding gap in the synchronous set_cache and get_cache paths of ValkeySemanticCache, where _get_embedding was called without the request metadata that controls spend attribution and per-deployment routing.

  • Fix: Both sync embedding calls now pass metadata=kwargs.get("metadata"), matching the existing async valkey path and RedisSemanticCache sync path.
  • Tests: Two new regression tests mirror the existing async metadata-forwarding tests, using spy functions to assert the embedding call receives the correct metadata dict.

Confidence Score: 5/5

The change is safe to merge — two isolated one-line fixes that bring the sync embedding calls in line with an already-correct async path.

The diff is minimal and mechanically consistent: the sync calls now pass exactly the same metadata=kwargs.get("metadata") argument that the async calls already used. The fix is verified by two new tests that spy on _get_embedding and would fail if the argument were missing, matching the pattern of the pre-existing async tests. No existing tests were modified, no mock behavior was weakened, and no real network calls are made.

No files require special attention.

Important Files Changed

Filename Overview
litellm/caching/valkey_semantic_cache.py Two one-line fixes: sync set_cache and get_cache now pass metadata=kwargs.get("metadata") to _get_embedding, matching the async paths and RedisSemanticCache.
tests/test_litellm/caching/test_valkey_semantic_cache.py Two new regression tests use spy functions to verify metadata is forwarded in the sync paths, mirroring the existing async test pattern. No real network calls, no existing tests modified.

Reviews (1): Last reviewed commit: "fix(caching): forward metadata to embedd..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 30 untouched benchmarks


Comparing litellm_fix_valkey_semantic_sync_metadata_32324 (7416000) with litellm_internal_staging (cd6e8cd)

Open in CodSpeed

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.

[Bug]: Valkey semantic cache failed as two self._get_async_embedding() call in valkey_semantic_cache.py forward **kwargs instead of metadata

1 participant