diff --git a/routstr/algorithm.py b/routstr/algorithm.py index fbc5388e..44cbddae 100644 --- a/routstr/algorithm.py +++ b/routstr/algorithm.py @@ -169,14 +169,27 @@ def create_model_mappings( base_url = str(getattr(upstream, "base_url", "") or "").lower() return f"{provider_type}|{base_url}" + def get_effective_forwarded_model_id(model: "Model") -> str | None: + """Ignore legacy self-aliases so they keep the model's base identity.""" + forwarded_model_id = model.forwarded_model_id + if forwarded_model_id == model.id: + return None + return forwarded_model_id + def _add_candidate( alias: str, model: "Model", provider: "BaseUpstreamProvider" ) -> None: - """Add candidate model/provider for an alias.""" + """Add one candidate per model/provider identity for an alias.""" alias_lower = alias.lower() - if alias_lower not in candidates: - candidates[alias_lower] = [] - candidates[alias_lower].append((model, provider)) + alias_candidates = candidates.setdefault(alias_lower, []) + provider_identity = get_provider_identity(provider) + if any( + existing_model.id.lower() == model.id.lower() + and get_provider_identity(existing_provider) == provider_identity + for existing_model, existing_provider in alias_candidates + ): + return + alias_candidates.append((model, provider)) def process_provider_models( upstream: "BaseUpstreamProvider", is_openrouter: bool = False @@ -208,12 +221,14 @@ def create_model_mappings( # Add to unique models base_id = get_base_model_id(model_to_use.id) - unique_key = model_to_use.forwarded_model_id or base_id + forwarded_model_id = get_effective_forwarded_model_id(model_to_use) + unique_key = forwarded_model_id or base_id if not is_openrouter or unique_key not in unique_models: unique_model = model_to_use.copy( update={ "id": base_id, "upstream_provider_id": upstream.provider_type, + "forwarded_model_id": forwarded_model_id, } ) unique_models[unique_key] = unique_model @@ -231,9 +246,9 @@ def create_model_mappings( if prefixed_id not in aliases: aliases.append(prefixed_id) - # Register forwarded_model_id as a routable alias - if model_to_use.forwarded_model_id and model_to_use.forwarded_model_id not in aliases: - aliases.append(model_to_use.forwarded_model_id) + # Register a distinct forwarded_model_id as a routable alias. + if forwarded_model_id and forwarded_model_id not in aliases: + aliases.append(forwarded_model_id) # Try to set each alias for alias in aliases: @@ -283,7 +298,8 @@ def create_model_mappings( continue base_id = get_base_model_id(model_to_use.id) - unique_key = model_to_use.forwarded_model_id or base_id + forwarded_model_id = get_effective_forwarded_model_id(model_to_use) + unique_key = forwarded_model_id or base_id is_openrouter = ( getattr(upstream_for_override, "base_url", "") == "https://openrouter.ai/api/v1" @@ -293,6 +309,7 @@ def create_model_mappings( update={ "id": base_id, "upstream_provider_id": upstream_for_override.provider_type, + "forwarded_model_id": forwarded_model_id, } ) unique_models[unique_key] = unique_model @@ -321,9 +338,9 @@ def create_model_mappings( if prefixed_id not in aliases: aliases.append(prefixed_id) - # Register forwarded_model_id as a routable alias - if model_to_use.forwarded_model_id and model_to_use.forwarded_model_id not in aliases: - aliases.append(model_to_use.forwarded_model_id) + # Register a distinct forwarded_model_id as a routable alias. + if forwarded_model_id and forwarded_model_id not in aliases: + aliases.append(forwarded_model_id) for alias in aliases: _add_candidate(alias, model_to_use, upstream_for_override) @@ -342,10 +359,8 @@ def create_model_mappings( forwarded_model_ids, the one whose forwarded_model_id equals the requested alias wins. """ - if ( - model.forwarded_model_id - and model.forwarded_model_id.lower() == alias - ): + forwarded_model_id = get_effective_forwarded_model_id(model) + if forwarded_model_id and forwarded_model_id.lower() == alias: return 5 if ( diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 66a1d288..e2403774 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -499,6 +499,12 @@ class ModelCreate(BaseModel): forwarded_model_id: str | None = None +def _normalize_forwarded_model_id(value: str | None) -> str | None: + if value is None: + return None + return value.strip() or None + + @admin_router.post( "/api/upstream-providers/{provider_id}/models", dependencies=[Depends(require_admin_api)], @@ -539,7 +545,10 @@ async def upsert_provider_model( json.dumps(payload.alias_ids) if payload.alias_ids else None ) existing_row.enabled = payload.enabled - existing_row.forwarded_model_id = payload.forwarded_model_id or payload.id + if "forwarded_model_id" in payload.model_fields_set: + existing_row.forwarded_model_id = _normalize_forwarded_model_id( + payload.forwarded_model_id + ) session.add(existing_row) await session.commit() @@ -572,7 +581,9 @@ async def upsert_provider_model( ), upstream_provider_id=provider_pk, enabled=payload.enabled, - forwarded_model_id=payload.forwarded_model_id or payload.id, + forwarded_model_id=_normalize_forwarded_model_id( + payload.forwarded_model_id + ), ) session.add(row) await session.commit() @@ -705,6 +716,10 @@ async def batch_override_provider_models( json.dumps(model_data.alias_ids) if model_data.alias_ids else None ) existing_row.enabled = model_data.enabled + if "forwarded_model_id" in model_data.model_fields_set: + existing_row.forwarded_model_id = _normalize_forwarded_model_id( + model_data.forwarded_model_id + ) session.add(existing_row) else: # Create new @@ -735,6 +750,9 @@ async def batch_override_provider_models( ), upstream_provider_id=provider_pk, enabled=model_data.enabled, + forwarded_model_id=_normalize_forwarded_model_id( + model_data.forwarded_model_id + ), ) session.add(row) diff --git a/routstr/payment/models.py b/routstr/payment/models.py index 17e8ac64..ba88834d 100644 --- a/routstr/payment/models.py +++ b/routstr/payment/models.py @@ -271,7 +271,7 @@ def _row_to_model( upstream_provider_id=row.upstream_provider_id, canonical_slug=getattr(row, "canonical_slug", None), alias_ids=json.loads(row.alias_ids) if row.alias_ids else None, - forwarded_model_id=getattr(row, "forwarded_model_id", None) or row.id, + forwarded_model_id=getattr(row, "forwarded_model_id", None), ) if apply_provider_fee: diff --git a/tests/integration/test_admin_forwarded_model_id.py b/tests/integration/test_admin_forwarded_model_id.py index 609dc18a..c0c3a4b3 100644 --- a/tests/integration/test_admin_forwarded_model_id.py +++ b/tests/integration/test_admin_forwarded_model_id.py @@ -111,6 +111,7 @@ async def test_admin_create_without_forwarded_model_id_preserves_null( ) assert response.status_code == 200 + assert response.json()["forwarded_model_id"] is None row = await integration_session.get(ModelRow, (MODEL_ID, provider.id)) assert row is not None assert row.forwarded_model_id is None @@ -118,14 +119,19 @@ async def test_admin_create_without_forwarded_model_id_preserves_null( @pytest.mark.integration @pytest.mark.asyncio +@pytest.mark.parametrize( + "forwarded_model_id", [None, "", " "], ids=["null", "empty", "blank"] +) async def test_admin_update_can_clear_forwarded_model_id( integration_client: AsyncClient, integration_session: AsyncSession, + forwarded_model_id: str | None, ) -> None: - """An explicit JSON null must remove an existing upstream alias.""" + """An explicit null or blank value must remove an existing upstream alias.""" provider = await _create_provider( integration_session, base_url="https://issue-639-clear.example/v1" ) + assert provider.id is not None row = _model_row(provider.id, forwarded_model_id="upstream-deepseek-chat") integration_session.add(row) await integration_session.commit() @@ -135,9 +141,159 @@ async def test_admin_update_can_clear_forwarded_model_id( response = await integration_client.post( f"/admin/api/upstream-providers/{provider.id}/models", headers=_admin_headers(), - json=_model_payload(forwarded_model_id=None, include_alias=True), + json=_model_payload( + forwarded_model_id=forwarded_model_id, include_alias=True + ), + ) + + assert response.status_code == 200 + assert response.json()["forwarded_model_id"] is None + await integration_session.refresh(row) + assert row.forwarded_model_id is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_update_without_forwarded_model_id_preserves_alias( + integration_client: AsyncClient, + integration_session: AsyncSession, +) -> None: + """Omitting the alias must not overwrite an existing value.""" + provider = await _create_provider( + integration_session, base_url="https://issue-639-preserve.example/v1" + ) + assert provider.id is not None + row = _model_row(provider.id, forwarded_model_id="upstream-deepseek-chat") + integration_session.add(row) + await integration_session.commit() + await reinitialize_upstreams() + + with patch("routstr.payment.models.sats_usd_price", return_value=1e-6): + response = await integration_client.post( + f"/admin/api/upstream-providers/{provider.id}/models", + headers=_admin_headers(), + json=_model_payload(forwarded_model_id=None, include_alias=False), + ) + + assert response.status_code == 200 + assert response.json()["forwarded_model_id"] == "upstream-deepseek-chat" + await integration_session.refresh(row) + assert row.forwarded_model_id == "upstream-deepseek-chat" + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_create_serializes_forwarded_model_id( + integration_client: AsyncClient, + integration_session: AsyncSession, +) -> None: + """A configured alias must remain visible in the admin response.""" + provider = await _create_provider( + integration_session, base_url="https://issue-639-serialize.example/v1" + ) + await reinitialize_upstreams() + + with patch("routstr.payment.models.sats_usd_price", return_value=1e-6): + response = await integration_client.post( + f"/admin/api/upstream-providers/{provider.id}/models", + headers=_admin_headers(), + json=_model_payload( + forwarded_model_id="upstream-deepseek-chat", include_alias=True + ), + ) + + assert response.status_code == 200 + assert response.json()["forwarded_model_id"] == "upstream-deepseek-chat" + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_batch_create_preserves_forwarded_model_id( + integration_client: AsyncClient, + integration_session: AsyncSession, +) -> None: + """Batch creation must persist a distinct upstream alias.""" + provider = await _create_provider( + integration_session, base_url="https://issue-639-batch-create.example/v1" + ) + await reinitialize_upstreams() + + response = await integration_client.post( + f"/admin/api/upstream-providers/{provider.id}/batch-override", + headers=_admin_headers(), + json={ + "models": [ + _model_payload( + forwarded_model_id="upstream-deepseek-chat", include_alias=True + ) + ] + }, + ) + + assert response.status_code == 200 + row = await integration_session.get(ModelRow, (MODEL_ID, provider.id)) + assert row is not None + assert row.forwarded_model_id == "upstream-deepseek-chat" + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_batch_update_can_clear_forwarded_model_id( + integration_client: AsyncClient, + integration_session: AsyncSession, +) -> None: + """Batch overrides must persist an explicit null alias too.""" + provider = await _create_provider( + integration_session, base_url="https://issue-639-batch-clear.example/v1" + ) + assert provider.id is not None + row = _model_row(provider.id, forwarded_model_id="upstream-deepseek-chat") + integration_session.add(row) + await integration_session.commit() + await reinitialize_upstreams() + + with patch("routstr.payment.models.sats_usd_price", return_value=1e-6): + response = await integration_client.post( + f"/admin/api/upstream-providers/{provider.id}/batch-override", + headers=_admin_headers(), + json={ + "models": [ + _model_payload(forwarded_model_id=None, include_alias=True) + ] + }, ) assert response.status_code == 200 await integration_session.refresh(row) assert row.forwarded_model_id is None + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_admin_batch_update_without_forwarded_model_id_preserves_alias( + integration_client: AsyncClient, + integration_session: AsyncSession, +) -> None: + """Batch updates must preserve aliases when the field is omitted.""" + provider = await _create_provider( + integration_session, base_url="https://issue-639-batch-preserve.example/v1" + ) + assert provider.id is not None + row = _model_row(provider.id, forwarded_model_id="upstream-deepseek-chat") + integration_session.add(row) + await integration_session.commit() + await reinitialize_upstreams() + + response = await integration_client.post( + f"/admin/api/upstream-providers/{provider.id}/batch-override", + headers=_admin_headers(), + json={ + "models": [ + _model_payload(forwarded_model_id=None, include_alias=False) + ] + }, + ) + + assert response.status_code == 200 + await integration_session.refresh(row) + assert row.forwarded_model_id == "upstream-deepseek-chat" diff --git a/tests/unit/test_algorithm.py b/tests/unit/test_algorithm.py index 92452cc9..aa151662 100644 --- a/tests/unit/test_algorithm.py +++ b/tests/unit/test_algorithm.py @@ -245,10 +245,16 @@ def test_create_model_mappings_does_not_split_self_alias_from_base_identity( "provider-b", "https://provider-b.example/v1", db_id=2, - models=[create_test_model(model_id)], + models=[ + create_test_model( + model_id, prompt_price=0.0001, completion_price=0.0001 + ) + ], ) - admin_saved_model = create_test_model(model_id) + admin_saved_model = create_test_model( + model_id, prompt_price=1.0, completion_price=1.0 + ) admin_saved_model.forwarded_model_id = model_id override_row = SimpleNamespace(id=model_id, upstream_provider_id=1, enabled=True) @@ -257,7 +263,7 @@ def test_create_model_mappings_does_not_split_self_alias_from_base_identity( monkeypatch.setattr("routstr.payment.models._row_to_model", fake_row_to_model) - _, _, unique_models = create_model_mappings( + _, provider_map, unique_models = create_model_mappings( upstreams=[provider_a, provider_b], overrides_by_key={(model_id, 1): (override_row, 1.0)}, disabled_model_keys=set(), @@ -267,6 +273,40 @@ def test_create_model_mappings_does_not_split_self_alias_from_base_identity( model.forwarded_model_id or model.id for model in unique_models.values() ) assert advertised_ids == ["deepseek-chat"] + assert provider_map[model_id][0][1] is provider_b + + +def test_create_model_mappings_preserves_case_only_forwarded_alias( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A case-only alias is distinct and must not be normalized as a self-alias.""" + model_id = "deepseek/deepseek-chat" + case_only_alias = "DeepSeek/DeepSeek-Chat" + provider = create_test_provider( + "provider-a", + "https://provider-a.example/v1", + db_id=1, + models=[create_test_model(model_id)], + ) + + override_model = create_test_model(model_id) + override_model.forwarded_model_id = case_only_alias + override_row = SimpleNamespace(id=model_id, upstream_provider_id=1, enabled=True) + + def fake_row_to_model(*args, **kwargs) -> Model: # type: ignore[no-untyped-def] + return override_model + + monkeypatch.setattr("routstr.payment.models._row_to_model", fake_row_to_model) + + _, provider_map, unique_models = create_model_mappings( + upstreams=[provider], + overrides_by_key={(model_id, 1): (override_row, 1.0)}, + disabled_model_keys=set(), + ) + + assert list(unique_models) == [case_only_alias] + assert unique_models[case_only_alias].forwarded_model_id == case_only_alias + assert len(provider_map[case_only_alias.lower()]) == 1 def test_create_model_mappings_disables_only_matching_provider() -> None: diff --git a/ui/components/add-provider-model-dialog.tsx b/ui/components/add-provider-model-dialog.tsx index f3d37abd..bd4d6b64 100644 --- a/ui/components/add-provider-model-dialog.tsx +++ b/ui/components/add-provider-model-dialog.tsx @@ -187,7 +187,7 @@ export function AddProviderModelDialog({ : '', canonical_slug: initialData.canonical_slug || '', alias_ids_raw: listToString(initialData.alias_ids), - forwarded_model_id: initialData.forwarded_model_id || initialData.id, + forwarded_model_id: initialData.forwarded_model_id || '', upstream_provider_id: typeof initialData.upstream_provider_id === 'string' ? initialData.upstream_provider_id @@ -293,7 +293,7 @@ export function AddProviderModelDialog({ ); form.setValue('canonical_slug', model.canonical_slug || ''); form.setValue('alias_ids_raw', listToString(model.alias_ids)); - form.setValue('forwarded_model_id', model.forwarded_model_id || model.id); + form.setValue('forwarded_model_id', model.forwarded_model_id || ''); form.setValue( 'upstream_provider_id', typeof model.upstream_provider_id === 'string' @@ -403,7 +403,7 @@ export function AddProviderModelDialog({ canonical_slug: data.canonical_slug?.trim() || null, alias_ids: listFromString(data.alias_ids_raw || ''), enabled: data.enabled, - forwarded_model_id: data.forwarded_model_id?.trim() || data.id, + forwarded_model_id: data.forwarded_model_id?.trim() || null, }; if (isEdit) { @@ -578,7 +578,8 @@ export function AddProviderModelDialog({ Alternate ID that clients can use to reference this - model. Defaults to the model's own ID. + model. Leave blank to use the model's own ID or + reset an existing alias.