fix(obs): stand down when litellm's routing table cannot be read

Greptile P2 on #523, and it is right: the previous commit warned and then kept
recording with a five-provider fallback, which marks groq, deepseek, xai and
~50 others as native. litellm still sends those over the OpenAI client, so both
that client and inference_call record them. A warning does not prevent the bad
metric data — it just annotates it.

Both halves of the suggestion are taken.

**A stable source.** The list is read from `litellm.constants`, where it is
defined (constants.py:789), instead of the `litellm` top level, which is an
incidental re-export. litellm declares no `__all__` at all, so the top-level
name carries no stability promise even informally, which is what pyright was
objecting to in the first place. Verified the two are the same object.

**Stand down when it is unknown.** `_over_openai_client` now returns None for
"cannot tell", and `_split_model`'s second element is `bool | None`. None is
deliberately not collapsed into False, because treating an unknown provider as
native is exactly what double-counts it. `inference_call` returns the null
recorder on None, so no record is started at all.

That choice is deliberate in one direction: standing down also drops the native
providers (anthropic, bedrock, vertex_ai) that nothing else records, so the
fallback loses real data. It is still the right trade. A doubled token or cost
figure is silent and gets believed; a gap is visible, and this one is warned
about by name. The path is also effectively unreachable — litellm is a hard
dependency and the constant has been there for many releases — so the
conservative branch costs nothing in practice.

Verified end to end: with `litellm.constants` blocked and a working sgp-obs
stub present, every model resolves to None and zero records are started.

Tests: 35 in this file, covering both the None contract and the property that
actually protects the data — no sgp-obs record is created when routing is
unknown. `./scripts/lint` (ruff, pyright 0 errors, import) and
`./scripts/check-slim-deps` clean; 1714 repo-wide.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: stephen-wang24

src/agentex/lib/core/adapters/llm/_genai_metrics.py

@@ -53,51 +53,46 @@
 _openai_client_providers: Any = _OPENAI_CLIENT_PROVIDERS_UNRESOLVED
 
 
-def _over_openai_client(provider: str) -> bool:
-    """Would the OpenAI client instrumentor already have recorded this call?
+def _openai_client_provider_set() -> frozenset[str] | None:
+    """Providers litellm dispatches over the ``openai`` client, or None if unknowable.
 
-    Answered from litellm's own ``openai_compatible_providers`` rather than a list of
-    our own, because that list is what litellm actually routes on and it grows every
-    release (54 entries as of 1.87.0: groq, deepseek, xai, fireworks_ai, ...).
+    Imported from ``litellm.constants``, which is where the list is defined, rather
+    than from the ``litellm`` top level, which is an incidental re-export: litellm
+    declares no ``__all__``, so a type checker treats the top-level name as private
+    and it carries no stability promise even informally.
     """
     global _openai_client_providers
     if _openai_client_providers is _OPENAI_CLIENT_PROVIDERS_UNRESOLVED:
-        compatible: Any = None
         try:
-            import litellm
-
-            # Read via getattr: the attribute is not in litellm's __all__, so it is
-            # not a promised export and a future release may rename or drop it.
-            compatible = getattr(litellm, "openai_compatible_providers", None)
-        except Exception:  # pragma: no cover - litellm is a hard dependency
-            compatible = None
+            from litellm.constants import openai_compatible_providers
 
-        if compatible:
             _openai_client_providers = (
-                frozenset(compatible) | _EXTRA_OPENAI_CLIENT_PROVIDERS
+                frozenset(openai_compatible_providers) | _EXTRA_OPENAI_CLIENT_PROVIDERS
             )
-        else:
-            # Degrading quietly here would re-introduce the double counting this
-            # function exists to prevent: every openai-compatible provider would look
-            # native again and be recorded twice. Say so rather than drift.
+        except Exception:  # pragma: no cover - litellm is a hard dependency
             logger.warning(
-                "litellm no longer exposes openai_compatible_providers, so GenAI "
-                "metrics can only recognise %d providers as reaching the model over "
-                "the OpenAI client. Calls to openai-compatible providers such as "
-                "groq or deepseek may now be counted twice, once here and once by "
-                "the OpenAI client instrumentor.",
-                len(_EXTRA_OPENAI_CLIENT_PROVIDERS),
+                "litellm.constants.openai_compatible_providers is unavailable, so "
+                "GenAI metrics cannot tell which calls the OpenAI client instrumentor "
+                "already records. Recording anyway would double-count every "
+                "openai-compatible provider, so litellm gateway metrics are off for "
+                "this process."
             )
-            _openai_client_providers = _EXTRA_OPENAI_CLIENT_PROVIDERS
-    return provider in _openai_client_providers
-
-# sgp-obs is an optional install (see ``sgp_obs_setup``), and Python does NOT cache a
-# FAILED import, so importing inside ``inference_call`` re-walked sys.path on every
-# single model call for the majority of agents that do not have it. Measured on
-# 0.27.0b2 in a venv with five sys.path entries (a container image has more): 62us per
-# attempt, which took the gateway's own per-call overhead from 12us to 84us. Resolved
-# once, to the module or to None -- the shape ``base_acp_server`` already uses for
-# ``sgp_obs.context``, for the same reason.
+            _openai_client_providers = None
+    return _openai_client_providers
+
+
+def _over_openai_client(provider: str) -> bool | None:
+    """Would the OpenAI client instrumentor already have recorded this call?
+
+    None means "cannot tell", which is NOT the same as False and must not collapse
+    into it: treating an unknown provider as native is what double-counts it.
+    """
+    known = _openai_client_provider_set()
+    if known is None:
+        return None
+    return provider in known
+
+
 _GENAI_UNRESOLVED = object()
 _genai_module: Any = _GENAI_UNRESOLVED
 
@@ -126,7 +121,7 @@ def _genai() -> Any | None:
     return _genai_module
 
 
-def _split_model(model: str) -> tuple[str, bool]:
+def _split_model(model: str) -> tuple[str, bool | None]:
     """``(provider, goes_out_over_the_openai_client)`` for a litellm model string.
 
     ``"litellm_proxy/anthropic/claude-sonnet-4"`` -> ``("anthropic", True)``
@@ -153,6 +148,10 @@ def _split_model(model: str) -> tuple[str, bool]:
     routing instruction, so the vendor underneath it is the interesting label — and the
     one thing the OpenAI client instrumentor cannot report, since from inside that
     client the call is simply "openai".
+
+    A second element of None means the routing table itself could not be read, so
+    whether this call is already recorded elsewhere is unknown. Callers must stand down
+    rather than guess; see :func:`inference_call`.
     """
     proxied = model.startswith(_PROXY_PREFIX)
     rest = model[len(_PROXY_PREFIX):] if proxied else model
@@ -246,6 +245,13 @@ def inference_call(kwargs: dict[str, Any], args: tuple[Any, ...] = ()) -> Any:
     try:
         model = resolve_model(args, kwargs)
         vendor, over_openai_client = _split_model(model)
+        if over_openai_client is None:
+            # The routing table could not be read, so we cannot tell whether the
+            # OpenAI client instrumentor is already recording this call. Recording
+            # would double-count every openai-compatible provider, and a doubled
+            # token or cost figure is worse than a missing one: the gap is visible
+            # and warned about, the doubling is silent and gets believed.
+            return _NULL_CALL
         return genai.call(
             provider=vendor,
             operation=genai.CHAT,

src/agentex/lib/core/adapters/llm/tests/test_genai_metrics.py

@@ -119,31 +119,80 @@ def test_the_proxy_vendor_survives_resolution(self):
     def test_the_provider_list_agrees_with_litellm(self):
         """Pinned to litellm's own list rather than a copy of it, because the copy
         would go stale every release."""
-        import litellm
+        import litellm.constants
 
         from agentex.lib.core.adapters.llm._genai_metrics import _over_openai_client
 
-        for provider in list(getattr(litellm, "openai_compatible_providers", []))[:20]:
+        for provider in list(litellm.constants.openai_compatible_providers)[:20]:
             assert _over_openai_client(provider), provider
         for provider in ("anthropic", "bedrock", "vertex_ai", "gemini"):
             assert not _over_openai_client(provider), provider
 
-    def test_losing_litellms_list_is_not_silent(self, monkeypatch, caplog):
-        """`openai_compatible_providers` is not in litellm's __all__, so it is read
-        defensively. But degrading quietly would re-introduce the double counting this
-        whole function exists to prevent, so the fallback has to be audible."""
-        import litellm
+    def test_an_unknown_routing_table_stands_down_rather_than_guessing(
+        self, monkeypatch, caplog
+    ):
+        """If the routing table cannot be read we do not know whether the OpenAI client
+        instrumentor is already recording a call. Recording anyway would double-count
+        every openai-compatible provider, and a doubled token or cost figure is worse
+        than a missing one: the gap is visible and warned about, the doubling is silent
+        and gets believed. So the recorder stands down entirely."""
+        import builtins
 
-        monkeypatch.delattr(litellm, "openai_compatible_providers", raising=False)
+        real_import = builtins.__import__
+
+        def no_constants(name, *args, **kwargs):
+            if name == "litellm.constants":
+                raise ImportError("litellm.constants is gone")
+            return real_import(name, *args, **kwargs)
+
+        monkeypatch.setattr(builtins, "__import__", no_constants)
         _genai_metrics._reset_for_tests()
 
         with caplog.at_level("WARNING", logger=_genai_metrics.logger.name):
-            # The five explicit ones still work; the ~50 from litellm no longer do.
-            assert _genai_metrics._over_openai_client("azure") is True
-            assert _genai_metrics._over_openai_client("groq") is False
-        assert any("openai_compatible_providers" in r.message for r in caplog.records), [
-            r.message for r in caplog.records
-        ]
+            assert _genai_metrics._over_openai_client("groq") is None
+            assert _genai_metrics._over_openai_client("anthropic") is None
+            assert _split_model("groq/llama3-8b-8192") == ("groq", None)
+
+        assert any(
+            "openai_compatible_providers is unavailable" in r.message
+            for r in caplog.records
+        ), [r.message for r in caplog.records]
+
+    def test_a_real_sgp_obs_is_not_started_when_routing_is_unknown(self, monkeypatch):
+        """The property that actually protects the data: no record is started at all,
+        rather than one started with a guessed transport."""
+        import sys
+        import builtins
+
+        started = []
+
+        class _Genai:
+            CHAT = "chat"
+            OPENAI_SPEC = "openai"
+            OPENAI = "openai"
+
+            @staticmethod
+            def call(**kwargs):
+                started.append(kwargs)
+                return object()
+
+        module = type(sys)("sgp_obs.metrics")
+        module.genai = _Genai
+        monkeypatch.setitem(sys.modules, "sgp_obs", type(sys)("sgp_obs"))
+        monkeypatch.setitem(sys.modules, "sgp_obs.metrics", module)
+
+        real_import = builtins.__import__
+
+        def no_constants(name, *args, **kwargs):
+            if name == "litellm.constants":
+                raise ImportError("litellm.constants is gone")
+            return real_import(name, *args, **kwargs)
+
+        monkeypatch.setattr(builtins, "__import__", no_constants)
+        _genai_metrics._reset_for_tests()
+
+        assert inference_call({"model": "groq/llama3-8b-8192"}) is _genai_metrics._NULL_CALL
+        assert started == [], "a record was started with an unknown routing table"
 
     def test_an_unresolvable_model_does_not_spam_stdout(self):
         """litellm prints a red "Provider List" banner to STDOUT (not logging, so it