Skip to content

fix: isolate embedding cache by model and prefix - #213

Open
Daniel Peng (original4422) wants to merge 1 commit into
braintrustdata:mainfrom
original4422:fix/embedding-cache-configuration
Open

fix: isolate embedding cache by model and prefix#213
Daniel Peng (original4422) wants to merge 1 commit into
braintrustdata:mainfrom
original4422:fix/embedding-cache-configuration

Conversation

@original4422

Copy link
Copy Markdown

Summary

  • key the class-level embedding cache by model, prefix, and normalized input
  • share the same cache-key logic between sync and async embedding paths
  • add deterministic sync and async regressions for configuration isolation and same-configuration cache hits

Root cause

EmbeddingSimilarity used only the normalized input as its class-level cache key even though the embedding request also depends on model and prefix. 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-files
  • python -m build
  • python -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

Copilot AI lite review requested due to automatic review settings August 19, 2026 19:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 EmbeddingSimilarity and 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.

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.

EmbeddingSimilarity class-level embedding cache ignores model and prefix — cross-model cache poisoning

2 participants