Skip to content

Commit 898e58e

Browse files
authored
Merge branch 'main' into codex/fix-dart-inheritors-query
2 parents 749d857 + 565f156 commit 898e58e

2 files changed

Lines changed: 149 additions & 7 deletions

File tree

code_review_graph/embeddings.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -369,9 +369,10 @@ class OpenAIEmbeddingProvider(EmbeddingProvider):
369369
from different backends (e.g. real OpenAI vs. an OpenAI-compatible
370370
gateway that ships different weights under the same model name).
371371
372-
Dimension is detected from the first response and frozen; switching the
373-
``model`` in the environment also changes ``provider.name`` and triggers
374-
re-embed via the same isolation key.
372+
When no dimension is explicitly requested, it is detected from the first
373+
response and retained as local metadata. Switching the ``model`` in the
374+
environment also changes ``provider.name`` and triggers re-embed via the
375+
same isolation key.
375376
"""
376377

377378
_DEFAULT_BATCH_SIZE = 100
@@ -392,6 +393,7 @@ def __init__(
392393
self._api_key = api_key
393394
self._base_url = base_url.rstrip("/")
394395
self._model = model
396+
self._requested_dimension = dimension
395397
self._dimension = dimension
396398
self._timeout = timeout
397399
self._batch_size = batch_size or self._DEFAULT_BATCH_SIZE
@@ -445,10 +447,13 @@ def _call_api(self, texts: list[str]) -> list[list[float]]:
445447
import urllib.request
446448

447449
body: dict[str, Any] = {"model": self._model, "input": texts}
448-
# OpenAI v3 models (text-embedding-3-*) support dimension reduction;
449-
# only forward the param when the user explicitly pinned one.
450-
if self._dimension is not None:
451-
body["dimensions"] = self._dimension
450+
# Forward only a dimension explicitly requested by the user. The
451+
# model name may be an Azure deployment or gateway alias, so it cannot
452+
# tell us whether the endpoint accepts dimension reduction. A dimension
453+
# learned from a response is local metadata and must never leak into a
454+
# later request.
455+
if self._requested_dimension is not None:
456+
body["dimensions"] = self._requested_dimension
452457

453458
payload = _json.dumps(body).encode("utf-8")
454459
req = urllib.request.Request(

tests/test_embeddings.py

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import json
44
import os
5+
import threading
56
import time
7+
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
68
from unittest.mock import MagicMock, patch
79

810
import pytest
@@ -585,6 +587,47 @@ def _make_openai_response(vectors: list[list[float]]) -> MagicMock:
585587
return mock
586588

587589

590+
@pytest.fixture
591+
def openai_loopback_server():
592+
"""Serve deterministic OpenAI-compatible embeddings on loopback."""
593+
payloads: list[dict] = []
594+
595+
class Handler(BaseHTTPRequestHandler):
596+
def do_POST(self):
597+
size = int(self.headers["Content-Length"])
598+
payload = json.loads(self.rfile.read(size))
599+
payloads.append(payload)
600+
601+
dimension = payload.get("dimensions", 7)
602+
response = {
603+
"data": [
604+
{"embedding": [0.1] * dimension, "index": index}
605+
for index, _text in enumerate(payload["input"])
606+
],
607+
"model": payload["model"],
608+
}
609+
body = json.dumps(response).encode("utf-8")
610+
self.send_response(200)
611+
self.send_header("Content-Type", "application/json")
612+
self.send_header("Content-Length", str(len(body)))
613+
self.end_headers()
614+
self.wfile.write(body)
615+
616+
def log_message(self, _format, *_args):
617+
return
618+
619+
server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
620+
thread = threading.Thread(target=server.serve_forever, daemon=True)
621+
thread.start()
622+
try:
623+
host, port = server.server_address
624+
yield f"http://{host}:{port}/v1", payloads
625+
finally:
626+
server.shutdown()
627+
server.server_close()
628+
thread.join(timeout=5)
629+
630+
588631
class TestIsLocalhostUrl:
589632
"""Ensure localhost detection is robust against subdomain tricks."""
590633

@@ -679,6 +722,100 @@ def test_explicit_dimension_forwarded_in_payload(self):
679722
payload = json.loads(mock_urlopen.call_args[0][0].data.decode("utf-8"))
680723
assert payload["dimensions"] == 256
681724

725+
def test_loopback_auto_learned_dimension_is_never_resent(
726+
self, openai_loopback_server,
727+
):
728+
base_url, payloads = openai_loopback_server
729+
provider = OpenAIEmbeddingProvider(
730+
api_key="k",
731+
base_url=base_url,
732+
model="text-embedding-3-small",
733+
)
734+
735+
assert len(provider.embed_query("first")) == 7
736+
assert provider.dimension == 7
737+
assert len(provider.embed_query("second")) == 7
738+
739+
assert len(payloads) == 2
740+
assert all("dimensions" not in payload for payload in payloads)
741+
742+
def test_loopback_explicit_dimension_is_sent_for_custom_model_alias(
743+
self, openai_loopback_server,
744+
):
745+
base_url, payloads = openai_loopback_server
746+
provider = OpenAIEmbeddingProvider(
747+
api_key="k",
748+
base_url=base_url,
749+
model="azure-production-deployment",
750+
dimension=4,
751+
)
752+
753+
assert len(provider.embed_query("custom alias")) == 4
754+
assert payloads == [{
755+
"model": "azure-production-deployment",
756+
"input": ["custom alias"],
757+
"dimensions": 4,
758+
}]
759+
760+
def test_auto_learned_dimension_omitted_for_non_v3_models(self):
761+
# Many OpenAI-compatible providers (SiliconFlow, Cohere, voyage-3,
762+
# custom vLLM gateways) reject the `dimensions` body field with
763+
# HTTP 400. The provider auto-learns dimension from the first
764+
# response and would otherwise forward it on every subsequent call.
765+
p = OpenAIEmbeddingProvider(
766+
api_key="k", base_url="http://localhost:3000/v1",
767+
model="BAAI/bge-m3",
768+
)
769+
with patch(
770+
"urllib.request.urlopen",
771+
return_value=_make_openai_response([[0.1] * 1024]),
772+
) as mock_urlopen:
773+
vec = p.embed_query("x")
774+
assert len(vec) == 1024
775+
assert p.dimension == 1024
776+
777+
# Second call would have auto-forwarded dimensions before the fix.
778+
with patch(
779+
"urllib.request.urlopen",
780+
return_value=_make_openai_response([[0.1] * 1024]),
781+
) as mock_urlopen:
782+
p.embed_query("y")
783+
payload = json.loads(mock_urlopen.call_args[0][0].data.decode("utf-8"))
784+
assert "dimensions" not in payload, (
785+
f"non-v3 model {p._model!r} should not send `dimensions`; "
786+
f"got payload keys: {list(payload)}"
787+
)
788+
789+
def test_explicit_dimension_forwarded_for_non_v3_models(self):
790+
# Explicit requests must not be inferred from the model name. OpenAI-
791+
# compatible gateways can expose dimension-capable models under
792+
# arbitrary aliases.
793+
p = OpenAIEmbeddingProvider(
794+
api_key="k", base_url="http://localhost:3000/v1",
795+
model="BAAI/bge-m3", dimension=1024,
796+
)
797+
with patch(
798+
"urllib.request.urlopen",
799+
return_value=_make_openai_response([[0.1] * 1024]),
800+
) as mock_urlopen:
801+
p.embed_query("x")
802+
payload = json.loads(mock_urlopen.call_args[0][0].data.decode("utf-8"))
803+
assert payload["dimensions"] == 1024
804+
805+
def test_explicit_dimension_forwarded_for_v3_models(self):
806+
# Regression guard: v3 models must still honor the pinned dimension.
807+
p = OpenAIEmbeddingProvider(
808+
api_key="k", base_url="http://localhost:3000/v1",
809+
model="text-embedding-3-large", dimension=512,
810+
)
811+
with patch(
812+
"urllib.request.urlopen",
813+
return_value=_make_openai_response([[0.1] * 512]),
814+
) as mock_urlopen:
815+
p.embed_query("x")
816+
payload = json.loads(mock_urlopen.call_args[0][0].data.decode("utf-8"))
817+
assert payload["dimensions"] == 512
818+
682819
def test_base_url_trailing_slash_stripped(self):
683820
p = OpenAIEmbeddingProvider(
684821
api_key="k", base_url="http://localhost:3000/v1/", model="m",

0 commit comments

Comments
 (0)