From 9ac9e1ab097279d1d0e9674d5f10f95b13d5f9ba Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Mon, 8 Jun 2026 01:29:36 +0200
Subject: [PATCH] fix(usage): drop redundant GatewayProvider.embed_batch
 override (round 3)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round-3 claude-review finding:

- 🟡 Double _ensure_bearer() on gateway.embed_batch(). Round 1 made
  OpenAIProvider.embed_batch() delegate to embed_batch_with_usage(); because
  GatewayProvider overrode both embed_batch() and embed_batch_with_usage() (each
  calling _ensure_bearer), gateway.embed_batch() refreshed the bearer twice
  (the second a cache-hit no-op). Remove the now-redundant embed_batch()
  override: OpenAI's embed_batch() routes through embed_batch_with_usage(),
  which the gateway still overrides, so the bearer refreshes exactly once on
  every path. The remaining two overrides (embed + embed_batch_with_usage) cover
  all four entrypoints; documented the topology.

- 🟢 Added test_gateway_embed_batch_ensures_bearer_once locking in the single
  refresh.

Cohere token-fallback (🟢 nit) is already covered by
test_bedrock_with_usage_estimates_when_token_count_absent.

Deck #67.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../embedding/gateway_client.py               | 19 ++++++------
 tests/unit/providers/test_gateway_provider.py | 31 +++++++++++++++++++
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/nextcloud_mcp_server/embedding/gateway_client.py b/nextcloud_mcp_server/embedding/gateway_client.py
index cc624486..f372c7f8 100644
--- a/nextcloud_mcp_server/embedding/gateway_client.py
+++ b/nextcloud_mcp_server/embedding/gateway_client.py
@@ -217,22 +217,21 @@ class GatewayProvider(OpenAIProvider):
                 exc,
             )
 
+    # Bearer-refresh override topology. OpenAIProvider routes embed_batch(),
+    # embed_with_usage() and embed_batch_with_usage() all through
+    # embed_batch_with_usage(); only embed() (single) is self-contained. So we
+    # override exactly two methods to refresh the bearer exactly once on every
+    # path: embed() (its own entrypoint) and embed_batch_with_usage() (the
+    # shared funnel for the other three). Overriding embed_batch() as well would
+    # double-call _ensure_bearer() (override → super().embed_batch() →
+    # self.embed_batch_with_usage() → override again).
+
     async def embed(self, text: str) -> list[float]:
         await self._ensure_bearer()
         return await super().embed(text)
 
-    async def embed_batch(self, texts: list[str]) -> list[list[float]]:
-        await self._ensure_bearer()
-        return await super().embed_batch(texts)
-
     async def embed_batch_with_usage(
         self, texts: list[str]
     ) -> tuple[list[list[float]], int]:
-        # Only the batch usage-variant is overridden: OpenAIProvider's
-        # embed_with_usage() routes through embed_batch_with_usage(), so a
-        # single embed_with_usage() call already lands here and refreshes the
-        # bearer exactly once (overriding both would double-ensure). This
-        # differs from embed()/embed_batch() above, where the single embed() is
-        # self-contained and therefore needs its own override.
         await self._ensure_bearer()
         return await super().embed_batch_with_usage(texts)
diff --git a/tests/unit/providers/test_gateway_provider.py b/tests/unit/providers/test_gateway_provider.py
index 3a834358..e7f5ad95 100644
--- a/tests/unit/providers/test_gateway_provider.py
+++ b/tests/unit/providers/test_gateway_provider.py
@@ -393,6 +393,37 @@ async def test_gateway_embed_batch_with_usage_forwards_after_bearer(monkeypatch)
     assert ensured["n"] == 1
 
 
+async def test_gateway_embed_batch_ensures_bearer_once(monkeypatch):
+    """embed_batch() has no override: it routes through the inherited OpenAI
+    embed_batch() → embed_batch_with_usage() (overridden), so the bearer is
+    refreshed exactly once — not twice."""
+    # https mock host (never contacted — the OpenAI client is patched below).
+    provider = GatewayProvider(
+        base_url="https://gw:8083/v1", embedding_model="mistral/mistral-embed"
+    )
+    ensured = {"n": 0}
+
+    async def _ensure_bearer():
+        ensured["n"] += 1
+
+    monkeypatch.setattr(provider, "_ensure_bearer", _ensure_bearer)
+
+    item = MagicMock()
+    item.embedding = [0.1, 0.2]
+    item.index = 0
+    response = MagicMock()
+    response.data = [item]
+    response.usage = MagicMock(total_tokens=4)
+    monkeypatch.setattr(
+        provider.client.embeddings, "create", AsyncMock(return_value=response)
+    )
+
+    embeddings = await provider.embed_batch(["x"])
+
+    assert embeddings == [[0.1, 0.2]]
+    assert ensured["n"] == 1  # not 2 — embed_batch() must not double-refresh
+
+
 async def test_detect_dimension_with_bare_base_url_hits_v1_models(monkeypatch):
     """End-to-end of the fix: a bare-origin base_url still resolves the
     dimension because discovery lands on /v1/models."""