mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-08-09 11:04:36 +00:00
fix: preserve nullable forwarded model aliases
This commit is contained in:
+31
-16
@@ -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 (
|
||||
|
||||
+20
-2
@@ -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)
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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({
|
||||
</FormControl>
|
||||
<FormDescription>
|
||||
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.
|
||||
</FormDescription>
|
||||
<FormMessage />
|
||||
</FormItem>
|
||||
|
||||
Reference in New Issue
Block a user