fix: AnswerRelevancy silently ignores embedding_model (both sync and async) - #209
Open
Rehan (spacesheepinternet) wants to merge 1 commit into
Conversation
…rRelevancy AnswerRelevancy passed model=self.embedding_model to EmbeddingSimilarity's eval()/eval_async() call, but EmbeddingSimilarity only reads `model` in its constructor (into extra_args); at eval time the kwarg lands in **kwargs of _run_eval_sync/_run_eval_async and is silently dropped. As a result the embedding_model parameter had no effect in either the sync or async path and the default embedding model was always used. Completes the partial fix from braintrustdata#164 (which corrected model -> embedding_model at the call site but left the parameter unforwarded) and takes the same approach as braintrustdata#167, adding regression tests for both paths. Fixes braintrustdata#166 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #166.
The bug is broader than #166 describes:
embedding_modelis ignored in both the sync and async paths, not just sync.AnswerRelevancypasses the model at eval time:but
EmbeddingSimilarityonly readsmodelin its constructor (stored intoself.extra_args). At eval time the kwarg lands in the**kwargsof_run_eval_sync/_run_eval_asyncand is never used — so everyAnswerRelevancyrun silently embeds with the default model, regardless of what the caller configured.#164 fixed half of this (the sync path was passing
self.model— the judge LLM — instead ofself.embedding_model), but the corrected value still went to the call site, where it's dropped. #167 had the right approach (constructor, not call site) but no test coverage; this PR completes it.Fix: pass
model=self.embedding_modelto theEmbeddingSimilarityconstructor in both paths.embedding_model=Noneremains safe (the constructor falls back toget_default_embedding_model()).Tests: two regression tests (sync + async) following the pattern of
test_answer_correctness_uses_custom_embedding_model— respx captures themodelfield actually sent to/v1/embeddings. Onmainboth fail with['text-embedding-ada-002', ...]; with the fix both pass. The tests pinmodel="gpt-4o-mini"for question generation so the mocked chat-completions endpoint is used (gpt-5-class defaults route through the Responses API).Found while preparing to use
AnswerRelevancywith a custom embedding model for a RAG eval harness.