fix(caching): fix duplicate and incorrect index values in /v1/embeddings mixed batches - #41020
fix(caching): fix duplicate and incorrect index values in /v1/embeddings mixed batches#41020kaan6634 wants to merge 3 commits into
Conversation
…ngs mixed batches (BerriAI#41002)
Greptile SummaryThis PR reindexes cached and fresh embedding items after reconstructing mixed batches and adds regression coverage for common cache-hit layouts
Confidence Score: 4/5This PR is not safe to merge until short upstream responses preserve error handling and the explicit repository requirements are satisfied The new bounds check silently converts an upstream cardinality failure into a successful response containing null data entries, while the helper also violates repository typing and immutability requirements Files Needing Attention: litellm/caching/caching_handler.py, tests/local_testing/test_caching_handler.py
|
| Filename | Overview |
|---|---|
| litellm/caching/caching_handler.py | Adds index normalization, but short upstream responses now return None entries and the helper violates typing, immutability, and comment rules |
| tests/local_testing/test_caching_handler.py | Adds useful mixed-batch regressions, although new explanatory comments violate repository guidance and short-response behavior remains untested |
Reviews (1): Last reviewed commit: "fix(caching): fix duplicate and incorrec..." | Re-trigger Greptile
| if item is None and embedding_response.data is not None: | ||
| final_data_list.append(embedding_response.data[idx]) | ||
| for final_idx, item in enumerate(_caching_handler_response.final_embedding_cached_response.data): | ||
| if item is None and embedding_response.data is not None and idx < len(embedding_response.data): |
| return None | ||
|
|
||
| @staticmethod | ||
| def _set_embedding_index(item: Any, index: int) -> Any: |
There was a problem hiding this comment.
Untyped helper violates policy
This helper accepts and returns Any, violating the repository requirement for typed values or caller-side validation before merging
Context Used: CLAUDE.md (source)
| return item | ||
| if hasattr(item, "index"): | ||
| try: | ||
| item.index = index |
There was a problem hiding this comment.
In-place mutation violates policy
The helper mutates objects and dictionaries directly, violating the repository requirement for immutable value flows that must be satisfied before merging
Context Used: CLAUDE.md (source)
| """ | ||
| Helper method to update the index of an embedding item. | ||
| Handles mutable objects (like litellm Embedding), Pydantic models, and dicts. | ||
| """ |
There was a problem hiding this comment.
Disallowed explanatory comments
The new helper and tests add routine explanatory comments, violating the repository comment policy that must be satisfied before merging
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
TLDR
Problem this solves:
/v1/embeddingsrequests returned duplicate or out-of-orderindexvalues indata.data[i].embeddingback toinput[data[i].index]either crashed or mapped vectors to the wrong text.How it solves it:
final_idxin the full batch.Embeddingobjects, Pydantic models, and dicts.User Flow
Before: an application sending mixed cached/fresh embedding batches receives duplicate index numbers
POST /v1/embeddingswith 4 inputs[T1, T2, T3, T4]whereT1andT3are already in cache.[T2, T4]which returns items with relative indices0and1.data[*].indexvalues as[0, 0, 2, 1].0or maps embeddings to wrong inputs.After: the response returns monotonically increasing sequential indices matching input positions
POST /v1/embeddingswith[T1, T2, T3, T4]whereT1andT3are cached.[T2, T4]and reconstructs the full list.data[*].indexvalues as[0, 1, 2, 3].Relevant issues
Fixes #41002
Pre-Submission checklist
tests/local_testing/test_caching_handler.py)