Skip to content

fix(caching): fix duplicate and incorrect index values in /v1/embeddings mixed batches - #41020

Open
kaan6634 wants to merge 3 commits into
BerriAI:mainfrom
kaan6634:fix/embeddings-mixed-cache-duplicate-index
Open

fix(caching): fix duplicate and incorrect index values in /v1/embeddings mixed batches#41020
kaan6634 wants to merge 3 commits into
BerriAI:mainfrom
kaan6634:fix/embeddings-mixed-cache-duplicate-index

Conversation

@kaan6634

Copy link
Copy Markdown

TLDR

Problem this solves:

  • Mixed cached and uncached /v1/embeddings requests returned duplicate or out-of-order index values in data.
  • Downstream OpenAI clients mapping data[i].embedding back to input[data[i].index] either crashed or mapped vectors to the wrong text.

How it solves it:

  • Re-indexes each item in the combined embedding response to its sequential position final_idx in the full batch.
  • Safely updates index on mutable Embedding objects, Pydantic models, and dicts.
  • Added 3 regression tests covering interleaved, uncached-first, and standard zero-indexed API returns.

User Flow

Before: an application sending mixed cached/fresh embedding batches receives duplicate index numbers

  1. A client sends POST /v1/embeddings with 4 inputs [T1, T2, T3, T4] where T1 and T3 are already in cache.
  2. The proxy makes an upstream call for [T2, T4] which returns items with relative indices 0 and 1.
  3. The response returns with data[*].index values as [0, 0, 2, 1].
  4. The client fails validation due to duplicate index 0 or maps embeddings to wrong inputs.

After: the response returns monotonically increasing sequential indices matching input positions

  1. The client sends the same POST /v1/embeddings with [T1, T2, T3, T4] where T1 and T3 are cached.
  2. The proxy fetches [T2, T4] and reconstructs the full list.
  3. The combined response returns data[*].index values as [0, 1, 2, 3].
  4. The client correctly maps every embedding to its original input text.

Relevant issues

Fixes #41002

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally (tests/local_testing/test_caching_handler.py)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem

@kaan6634
kaan6634 requested a review from a team September 13, 2026 22:36
@codspeed-hq

codspeed-hq Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing kaan6634:fix/embeddings-mixed-cache-duplicate-index (ee53971) with main (30f33a9)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reindexes cached and fresh embedding items after reconstructing mixed batches and adds regression coverage for common cache-hit layouts

  • Normalizes each returned embedding index to its full-batch position
  • Supports mutable embedding objects, model-copy APIs, and dictionaries
  • Adds mixed, uncached-first, and zero-based upstream response tests
  • Introduces malformed output when an upstream response contains too few embeddings

Confidence Score: 4/5

This 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

Important Files Changed

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

Comment thread litellm/caching/caching_handler.py Outdated
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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Malformed short responses

If upstream returns too few embeddings, this branch appends None, returning invalid data instead of invoking existing error handling

Comment thread litellm/caching/caching_handler.py Outdated
return None

@staticmethod
def _set_embedding_index(item: Any, index: int) -> Any:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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)

Comment thread litellm/caching/caching_handler.py Outdated
return item
if hasattr(item, "index"):
try:
item.index = index

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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)

Comment thread litellm/caching/caching_handler.py Outdated
Comment on lines +624 to +627
"""
Helper method to update the index of an embedding item.
Handles mutable objects (like litellm Embedding), Pydantic models, and dicts.
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

@CLAassistant

CLAassistant commented Sep 13, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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]: /v1/embeddings returns duplicate "index" values when a batch mixes cached and uncached inputs

2 participants