fix(dspy): add opt-in model_id to Embedder to disambiguate callable-model cache keys

[detail-app]

Warning

GitHub issue creation failed

Detail attempted to publish this bug to GitHub, but the issue could not be created. This fix PR was created without that issue, and missing tracker references are shown as Unknown issue.

You can review and merge this PR normally. Please review your tracker integration settings before the next publish run.

Detail bug report: View on Detail

📝 Changes Description

This MR/PR contains the following changes:

• Bug: When dspy.Embedder is backed by a callable model with caching=True (the default), the @request_cache cache key in dspy/clients/cache.py::_transform_value collapses distinct callable models to the same value. The source branch (<callable_source:{inspect.getsource(value)}>) is byte-identical across instances/checkpoints of the same class (the documented dspy.Embedder(model.encode) pattern — e.g. two SentenceTransformer checkpoints), and the fallback branch (<callable:{value.__name__ or 'lambda'}>) collapses callable instances / functools.partials to <callable:lambda>. Two different configured models then silently return each other's cached embeddings (their underlying callable is never invoked), including across separate processes via the default on-disk cache (~/.dspy_cache).
• Fix: Add an opt-in, cross-process-safe model_id: str | None parameter to dspy.Embedder. When set, model_id is folded into the @request_cache request dict (and thus the cache key) but is never forwarded to the callable model nor to litellm. Distinct model_id values never collide; the same model_id reuses the cache across processes (preserving the on-disk reuse benefit). When None (the default), it is omitted from the key entirely, preserving existing cache keys — so this is an opt-in mitigation; the residual default-path limitation is documented in the Embedder/KNN/KNNFewShot docstrings. dspy/clients/cache.py is intentionally left unchanged (an id()-based mitigation was rejected because it would break on-disk reuse and not fix the cross-process case).

Closes Unknown issue

✅ Contributor Checklist

• Pre-Commit checks are passing (locally and remotely)
• Title of your PR / MR corresponds to the required format
• Commit message follows required format {label}(dspy): {message}

⚠️ Warnings

• This is an opt-in mitigation: users must pass a stable model_id (e.g. the checkpoint name) when switching callable models of the same class while sharing a cache namespace. The default path (no model_id) is unchanged to avoid silently invalidating existing on-disk caches; the docstrings of Embedder, KNN, and KNNFewShot now document this guidance.
• Testing: unit tests, lint, and format all pass. Added 10 tests to tests/clients/test_embedding.py covering: the buggy _transform_value branches still collapse distinct callables (premise guard); model_id disambiguating the source branch (bound methods) and the fallback branch (callable instances); same-model_id on-disk reuse across a simulated fresh process; distinct-model_id preventing cross-cache collision; model_id=None preserving existing cache keys; model_id not being forwarded to the callable nor to litellm; per-call model_id overriding the instance-level one; and async-parity via acall. The full existing tests/clients, tests/predict, tests/teleprompt, and tests/retrievers suites pass (512 passed, 81 skipped), as does the CI-mirror command (1263 passed, 252 skipped, 2 xfailed).
• End-to-end verification against the real SentenceTransformer (installed sentence-transformers==6.0.1): confirmed two real checkpoints (all-MiniLM-L6-v2 vs paraphrase-MiniLM-L6-v2) provably share a dspy.cache.cache_key without model_id and are disambiguated with distinct model_ids; verified in-process (each embedder returns its own ground-truth vectors vs a colliding baseline), cross-process via a shared on-disk cache (a second fresh process with a distinct model_id computes its own vectors and does not read the first's), and cross-process reuse with the same model_id (the second process hits the on-disk cache with encode call count 0). KNN end-to-end with a model_id-tagged Embedder retrieves correct neighbors and reuses the on-disk cache on a second run. KNNFewShot initialization with a real model_id-tagged Embedder works; the KNNFewShot.compile path was not exercised end-to-end because its existing repo test (_test_knn_few_shot_compile) is a disabled TODO unrelated to this cache-key bug.
• AI disclosure: authored by Detail: Automatic Fixes.

---

Automatic Fixes PRs can be configured here.

[greptile-apps]

Greptile Summary

This PR adds an opt-in Embedder.model_id cache-key discriminator for callable embedding models and documents its use in KNN workflows.

• Threads the discriminator through synchronous and asynchronous cached embedding paths without forwarding it to the underlying model.
• Preserves existing cache keys when no discriminator is configured.
• Adds coverage for callable-key collisions, disk-cache reuse, provider isolation, and asynchronous behavior.
• Introduces a backward-compatibility conflict for custom callables that previously received model_id through the documented forwarded-keyword interface.

Confidence Score: 4/5

The PR should not merge until the newly reserved keyword no longer breaks custom embedding callables that previously accepted model_id through Embedder forwarding.

The cache-key flow itself is consistent and well tested, but the public passthrough contract changes for both constructor and per-call keyword arguments, causing realistic existing callable models to fail or behave differently.

Files Needing Attention: dspy/clients/embedding.py

Important Files Changed

Filename	Overview
dspy/clients/embedding.py	Adds the cache-key discriminator across sync and async embedding paths, but reserving the formerly forwarded model_id keyword breaks compatible custom callables.
tests/clients/test_embedding.py	Adds focused coverage for both callable-key transformation branches, cache disambiguation and reuse, non-forwarding, overrides, and async parity.
dspy/predict/knn.py	Documents how KNN users can assign checkpoint-level identifiers to callable embedders.
dspy/teleprompt/knn_fewshot.py	Documents the same callable-cache guidance for KNNFewShot users.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Embedder call] --> B[Resolve instance or per-call model_id]
    B --> C{Caching enabled?}
    C -- Yes --> D[Include model_id in DSPy cache request]
    D --> E{Cache hit?}
    E -- Yes --> F[Return cached embeddings]
    E -- No --> G[Strip model_id]
    C -- No --> G
    G --> H[Callable model or LiteLLM]
    H --> I[Return embeddings]

Loading

_{Reviews (1): Last reviewed commit: "fix(dspy): add opt-in model_id to Embedd..." | Re-trigger Greptile}

[greptile-apps]

Reserved Keyword Breaks Callables

If an existing custom embedding callable accepts model_id as a construction-time or per-call keyword, this change now consumes that value as cache metadata instead of forwarding it as before. A callable that requires the argument will fail with TypeError, while one that uses it for model routing may silently select different behavior. Use a cache-specific parameter name or preserve an explicit way to forward a model argument named model_id.

Knowledge Base Used: Language model and embedding clients