Repository navigation
fix: isolate embedding cache by model and prefix - #213
Abhijeet Prasad (AbhiPrasad) merged 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness issue in the Python EmbeddingSimilarity scorer where its class-level embedding cache could be inadvertently shared across different configurations, leading to cross-model / cross-prefix cache poisoning. It aligns with the library’s goal of providing reliable, deterministic evaluators by ensuring cached embedding results are only reused when the embedding request configuration matches.
Changes:
- Introduces a shared cache-key function for
EmbeddingSimilarityand keys the cache by(model, prefix, normalized_input). - Applies the same cache-keying behavior to both sync and async embedding paths.
- Adds regression tests to confirm cache isolation across models/prefixes and cache hits within the same configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| py/autoevals/string.py | Fixes EmbeddingSimilarity cache keying to prevent cross-configuration reuse and shares key logic between sync/async paths. |
| py/autoevals/test_embeddings.py | Adds deterministic regression tests (sync + async) and a per-test cache reset fixture to validate isolation and same-config cache hits. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Hi David Elner (@delner), when you have a chance, would you mind taking a look at this cache-key fix and letting me know if any changes are needed? Thank you! |
|
Hi Abhijeet Prasad (@AbhiPrasad), when you have a chance, could you please review this cache-isolation fix and let me know whether it's ready to merge? I reran the sync/async regression tests in the locked Python environment; both pass. Thank you! |
|
Hi Abhijeet Prasad (@AbhiPrasad), when you have a chance, could you please take a look at this model/prefix embedding-cache isolation fix and let me know if any changes are needed? Thank you! |
|
I'm taking a look Daniel Peng (@original4422) ! |
211d5da
into
braintrustdata:main
Braintrust eval report
|
Summary
Root cause
EmbeddingSimilarityused only the normalized input as its class-level cache key even though the embedding request also depends onmodelandprefix. Instances with different configurations could therefore reuse vectors produced by another model or prefixed input.Validation
pytest py/autoevals/test_embeddings.py -k 'cache_isolated' -q(2 passed)pytest py/autoevals/test_values.py py/autoevals/test_json.py -q(8 passed)pre-commit run --all-filespython -m buildpython -m twine check dist/*The full local suite completed with 72 passed and 17 integration failures because OpenAI/Braintrust API credentials are unavailable locally; those failures stop at client initialization and do not exercise this cache change.
Fixes #211