diff --git a/routstr/upstream/base.py b/routstr/upstream/base.py index 4215426d..bc267dc2 100644 --- a/routstr/upstream/base.py +++ b/routstr/upstream/base.py @@ -203,14 +203,26 @@ class BaseUpstreamProvider: already reported its own provider (e.g. OpenRouter returns ``"provider": "Fireworks"``), otherwise just ``""`` for direct upstreams. + + Idempotent: re-stamping an already-stamped payload must not nest the + prefix repeatedly (e.g. never ``"anthropic:anthropic"``). This matters + because streaming paths can apply the field more than once per chunk. """ if not isinstance(response_json, dict): return + provider_type = (self.provider_type or "").strip() existing = response_json.get("provider") - if isinstance(existing, str) and existing.strip(): - response_json["provider"] = f"{self.provider_type}:{existing.strip()}" - else: - response_json["provider"] = self.provider_type + existing_str = existing.strip() if isinstance(existing, str) else "" + if not existing_str: + response_json["provider"] = provider_type + return + # Already stamped by a previous pass — leave it untouched. + if existing_str == provider_type or existing_str.startswith( + f"{provider_type}:" + ): + response_json["provider"] = existing_str + return + response_json["provider"] = f"{provider_type}:{existing_str}" def inject_cost_metadata( self, diff --git a/routstr/upstream/openrouter.py b/routstr/upstream/openrouter.py index 2cbc34de..36932d22 100644 --- a/routstr/upstream/openrouter.py +++ b/routstr/upstream/openrouter.py @@ -18,6 +18,33 @@ class OpenRouterUpstreamProvider(BaseUpstreamProvider): supports_anthropic_messages = True litellm_provider_prefix = "openrouter/" + def _apply_provider_field(self, response_json: object) -> None: + """Stamp the ``provider`` field for OpenRouter responses. + + OpenRouter is a router, not the real serving provider, so a bare + ``"openrouter"`` value carries no useful information. Rules: + + - Real upstream sub-provider (e.g. ``"GMICloud"``) -> ``"openrouter:GMICloud"``. + - Missing sub-provider, or one that merely echoes ``"openrouter"`` -> + ``"unknown"``. + - Idempotent: re-stamping never produces ``"openrouter:openrouter:..."``; + the ``openrouter:`` prefix appears at most once. + """ + if not isinstance(response_json, dict): + return + provider_type = (self.provider_type or "").strip() + existing = response_json.get("provider") + sub = existing.strip() if isinstance(existing, str) else "" + # Strip any already-applied "openrouter:" prefixes (idempotency). + prefix = f"{provider_type}:" + while sub.lower().startswith(prefix.lower()): + sub = sub[len(prefix) :].strip() + # No real sub-provider, or it just echoes our own router name. + if not sub or sub.lower() == provider_type.lower(): + response_json["provider"] = "unknown" + return + response_json["provider"] = f"{provider_type}:{sub}" + def __init__(self, api_key: str, provider_fee: float = 1.06): """Initialize OpenRouter provider with API key. diff --git a/tests/unit/test_provider_field_injection.py b/tests/unit/test_provider_field_injection.py index 93658a94..bf2813e0 100644 --- a/tests/unit/test_provider_field_injection.py +++ b/tests/unit/test_provider_field_injection.py @@ -32,11 +32,39 @@ def test_apply_provider_field_openrouter_passthrough() -> None: def test_apply_provider_field_openrouter_no_upstream_provider() -> None: - """If OpenRouter omits the provider field, fall back to provider_type.""" + """If OpenRouter omits the provider field, the real serving provider is + unknown — a bare ``openrouter`` value carries no information.""" p = _make_provider(OpenRouterUpstreamProvider, "openrouter") data: dict = {"id": "gen-abc"} p._apply_provider_field(data) - assert data["provider"] == "openrouter" + assert data["provider"] == "unknown" + + +def test_apply_provider_field_openrouter_echoes_router_name() -> None: + """If OpenRouter reports its own name as the provider, treat as unknown.""" + p = _make_provider(OpenRouterUpstreamProvider, "openrouter") + data: dict = {"provider": "openrouter"} + p._apply_provider_field(data) + assert data["provider"] == "unknown" + + +def test_apply_provider_field_openrouter_idempotent_no_double_prefix() -> None: + """Re-stamping must never nest the prefix: openrouter only once.""" + p = _make_provider(OpenRouterUpstreamProvider, "openrouter") + data: dict = {"provider": "GMICloud"} + p._apply_provider_field(data) + assert data["provider"] == "openrouter:GMICloud" + # Second pass (e.g. streaming) keeps a single prefix. + p._apply_provider_field(data) + assert data["provider"] == "openrouter:GMICloud" + + +def test_apply_provider_field_openrouter_collapses_existing_double_prefix() -> None: + """A pre-existing double prefix is collapsed to a single one.""" + p = _make_provider(OpenRouterUpstreamProvider, "openrouter") + data: dict = {"provider": "openrouter:openrouter:GMICloud"} + p._apply_provider_field(data) + assert data["provider"] == "openrouter:GMICloud" def test_apply_provider_field_strips_whitespace() -> None: @@ -50,28 +78,24 @@ def test_apply_provider_field_blank_upstream_treated_as_missing() -> None: p = _make_provider(OpenRouterUpstreamProvider, "openrouter") data: dict = {"provider": " "} p._apply_provider_field(data) - assert data["provider"] == "openrouter" + assert data["provider"] == "unknown" def test_apply_provider_field_non_string_upstream_treated_as_missing() -> None: p = _make_provider(OpenRouterUpstreamProvider, "openrouter") data: dict = {"provider": 42} p._apply_provider_field(data) - assert data["provider"] == "openrouter" + assert data["provider"] == "unknown" def test_apply_provider_field_idempotent_for_direct_upstream() -> None: - """Calling twice on a direct upstream payload should keep the same - value, not nest the prefix repeatedly.""" + """Calling twice on a direct upstream payload keeps the same value and + never nests the prefix (no ``anthropic:anthropic``).""" p = _make_provider(AnthropicUpstreamProvider, "anthropic") data: dict = {} p._apply_provider_field(data) p._apply_provider_field(data) - assert data["provider"] == "anthropic:anthropic" - # Document current (deliberate) behavior: second pass treats the - # first-pass value as an upstream-reported provider. Callers should - # only invoke this once per chunk — guarded via the - # ``"provider" not in data`` checks in streaming paths. + assert data["provider"] == "anthropic" def test_apply_provider_field_ignores_non_dict() -> None: