diff --git a/contextual_orchestrator/model_discovery.py b/contextual_orchestrator/model_discovery.py index a4d28c819..981bf7f4f 100644 --- a/contextual_orchestrator/model_discovery.py +++ b/contextual_orchestrator/model_discovery.py @@ -1034,6 +1034,57 @@ def _parse_openai_compatible(payload: Any, source: ProviderModelSource) -> list[ return _deduplicate_discovered_models(discovered) +def _bytez_meter_price_is_free(meter_price: Any) -> bool: + """Return whether a Bytez ``meterPrice`` names an exact-zero GPU-second rate. + + Bytez prices by GPU-second, e.g. ``"0.0006478333 / sec"`` (see + https://docs.bytez.com/http-reference/list/models.md), not per-token -- + this never feeds ``prompt_price_per_1k``/``completion_price_per_1k``, + only whether the *rate itself* is known to be exactly zero. Parses + through :class:`~decimal.Decimal` to avoid a nonzero rate underflowing to + ``0.0`` in float, matching :func:`_price_per_1k`'s precision handling. A + missing, non-numeric, or malformed value is unknown, not free -- the same + fail-closed default this module uses everywhere else pricing evidence is + incomplete. The sibling ``meter`` field (a GPU-tier name, e.g. + ``"sm-free"``) is not used here: live documentation shows tier names can + contain "free" while their own ``meterPrice`` is nonzero, so tier naming + is not a trustworthy zero-cost signal. + + A string value must match the full documented ``" / "`` shape + -- exactly one ``/`` separating a non-empty rate from a non-empty unit -- + before any part of it is trusted. Reading only the text before the first + ``/`` would let a shape that does not match the documented grammar at all + (a missing unit, e.g. ``"0 /"``, or an extra separator, e.g. + ``"0 / sec / token"``) still read as ``"0"`` and get confidently + classified free; an unexpected shape is itself a signal something about + the row is wrong, so it fails closed instead. The *unit* is deliberately + not required to equal ``"sec"``: a rate of exactly zero cost is zero + regardless of its time unit (``"0 / hour"`` is exactly as free as + ``"0 / sec"``), so this only validates the shape, never the unit name. + """ + if isinstance(meter_price, bool): + return False + if isinstance(meter_price, (int, float)): + try: + return Decimal(str(meter_price)) == 0 + except (ArithmeticError, ValueError): + return False + if not isinstance(meter_price, str): + return False + segments = meter_price.split("/") + if len(segments) != 2: + # Zero or two-or-more "/" characters does not match the documented + # " / " shape -- trust nothing from it, zero included. + return False + rate, unit = (segment.strip() for segment in segments) + if not rate or not unit: + return False + try: + return Decimal(rate) == 0 + except (ArithmeticError, ValueError): + return False + + def _parse_bytez(payload: Any, source: ProviderModelSource) -> list[DiscoveredModel]: rows = payload.get("output") if isinstance(payload, dict) else None discovered: list[DiscoveredModel] = [] @@ -1060,6 +1111,7 @@ def _parse_bytez(payload: Any, source: ProviderModelSource) -> list[DiscoveredMo privacy_policy_urls=_privacy_policy_urls(source, row), # Bytez prices by GPU-second (meterPrice), not per-token; leaving # per-1k pricing unset is more honest than a misleading estimate. + is_free=_bytez_meter_price_is_free(row.get("meterPrice")), ) ) return _deduplicate_discovered_models(discovered) @@ -1459,6 +1511,54 @@ def free_discovered_models(discovered: list[DiscoveredModel]) -> list[Discovered return [model for model in discovered if model.is_free] +def _log_zero_free_serving_contribution( + discovered: list[DiscoveredModel], candidates: list[DiscoveredModel] +) -> None: + """Log one non-fatal diagnostic per account that discovered rows but seeded no + ``orchestrator/free`` serving candidate, naming the coarse reason. + + A hard provider failure (e.g. Bytez's HTTP 500) already gets an explicit + ``model discovery failed account=bytez ...`` line from + :func:`discover_provider_models`. But a provider that discovers rows just fine + and still contributes nothing to the free pool -- OpenRouter's deliberate + ``evidence_only`` exclusion, or a provider with real pricing but no zero-cost + model today -- previously left no comparable trace, making a single-family + pool (e.g. 100% ``nvidia_nim``) look identical whether every other provider + was failing or simply had nothing free to offer. This is derived entirely from + already-discovered rows -- no extra fetch, no behavior change to the returned + candidate list. + + ``credential_name`` here is always the KV credential *name* (e.g. + ``"BYTEZ_API_KEY"``) -- one of the literal strings declared on each + :class:`ProviderModelSource` in ``PROVIDER_MODEL_SOURCES`` and copied + verbatim onto every :class:`DiscoveredModel` at construction + (``_parse_openai_compatible``/``_parse_bytez``). The actual secret + *value* is a distinct local (``api_key`` in + :func:`discover_provider_models`) that is never threaded onto a + ``DiscoveredModel`` and never reaches this function or this log line. + """ + serving_account_names = {model.credential_name for model in candidates} + seen: set[str] = set() + for model in discovered: + if model.credential_name in seen or model.credential_name in serving_account_names: + continue + seen.add(model.credential_name) + account_rows = [m for m in discovered if m.credential_name == model.credential_name] + if all(row.evidence_only for row in account_rows): + reason = "evidence_only" + elif not any(row.is_free for row in account_rows): + reason = "no_free_pricing_reported" + else: + reason = "free_rows_excluded_from_general_pool" + # nosemgrep: python.lang.security.audit.logging.logger-credential-leak.python-logger-credential-disclosure - credential_name is the KV secret's *name* (e.g. "BYTEZ_API_KEY"), never its value; see the docstring above for the exact field/type trace. The rule matches on the word "credential" in the format string, not on any actual secret reaching this call. + _LOGGER.debug( + "free serving pool contribution zero account=%s credential=%s reason=%s", + model.provider_name, + model.credential_name, + reason, + ) + + def general_free_serving_candidates( discovered: list[DiscoveredModel], ) -> list[DiscoveredModel]: @@ -1507,11 +1607,13 @@ def general_free_serving_candidates( count never overstates how many free models the general chat pool could actually serve. """ - return [ + candidates = [ model for model in free_discovered_models(discovered) if is_routable_discovered_model(model) and not _requires_non_text_input(model) ] + _log_zero_free_serving_contribution(discovered, candidates) + return candidates def _currency_is_comparable(currency_code: object, default_currency: object) -> bool: diff --git a/tests/test_model_discovery.py b/tests/test_model_discovery.py index 0b059fd39..66a2087a6 100644 --- a/tests/test_model_discovery.py +++ b/tests/test_model_discovery.py @@ -30,6 +30,7 @@ ProviderDiscoveryError, ProviderModelSource, _MODELS_DEV_FETCH_ATTEMPTS, + _bytez_meter_price_is_free, _deduplicate_discovered_models, _fetch_json, _merge_configured_gateway_metadata, @@ -932,6 +933,78 @@ def test_general_free_serving_candidates_excludes_unroutable_free_models() -> No } +def test_general_free_serving_candidates_logs_zero_contribution_reasons( + caplog: pytest.LogCaptureFixture, +) -> None: + """Accounts that discover rows but seed no free candidate get a reason, not silence. + + OpenRouter's ``evidence_only`` exclusion and OpenAI having no free-tier model + today are both correct behavior, but previously left no diagnostic trace + explaining why they contributed nothing to ``orchestrator/free`` -- unlike a + hard provider failure, which already logs + ``model discovery failed account= ...`` from + :func:`discover_provider_models`. This closes that visibility gap. + """ + openrouter_evidence_only = replace( + DiscoveredModel( + provider_name="openrouter", + model_id="openrouter/some-model", + credential_name="OPENROUTER_API_KEY", + chat_base_url="https://openrouter.ai/api/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("text",), + output_modalities=("text",), + is_free=True, + ), + evidence_only=True, + ) + openai_paid_only = DiscoveredModel( + provider_name="openai", + model_id="openai/gpt-paid", + credential_name="OPENAI_API_KEY", + chat_base_url="https://api.openai.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("text",), + output_modalities=("text",), + is_free=False, + ) + nvidia_serving = DiscoveredModel( + provider_name="nvidia_nim", + model_id="nvidia/free-model", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("text",), + output_modalities=("text",), + is_free=True, + ) + + with caplog.at_level("DEBUG", logger="contextual_orchestrator.model_discovery"): + serving_candidates = general_free_serving_candidates( + [openrouter_evidence_only, openai_paid_only, nvidia_serving] + ) + + assert [model.model_id for model in serving_candidates] == ["nvidia/free-model"] + messages = [record.getMessage() for record in caplog.records] + assert any( + "account=openrouter" in message + and "credential=OPENROUTER_API_KEY" in message + and "reason=evidence_only" in message + for message in messages + ) + assert any( + "account=openai" in message + and "credential=OPENAI_API_KEY" in message + and "reason=no_free_pricing_reported" in message + for message in messages + ) + # The provider that did seed a candidate gets no zero-contribution line. + assert not any("account=nvidia_nim" in message for message in messages) + + def test_general_free_serving_candidates_modality_shapes() -> None: """Explicit three-way modality contract: text-only, image-only, text+image. @@ -1549,6 +1622,97 @@ def urlopen(request, timeout=None): assert discovered[0].capabilities == ("chat",) # Bytez prices by GPU-second, not per-token: no fabricated per-1k estimate. assert discovered[0].prompt_price_per_1k is None + assert discovered[0].completion_price_per_1k is None + # Real, nonzero GPU-second pricing must not be misread as free. + assert discovered[0].is_free is False + + +def test_discover_bytez_marks_zero_meter_price_as_free() -> None: + """A Bytez row whose real ``meterPrice`` rate is exactly zero is free. + + Regression test: ``_parse_bytez`` used to build every ``DiscoveredModel`` + without ever passing ``is_free``, silently defaulting every Bytez model + to ``is_free=False`` regardless of its actual price -- discarding the one + real zero-cost signal Bytez's API does expose (``meterPrice``). This + must stay True without fabricating per-1k pricing (Bytez bills by + GPU-second, not per-token; that omission is intentional and unrelated). + """ + register_credential("BYTEZ_API_KEY", "bytez-secret") + payload = { + "error": None, + "output": [ + {"modelId": "0-hero/Matter-0.1-Slim-7B-C", "task": "chat", "meterPrice": "0 / sec"}, + ], + } + + with patch( + "contextual_orchestrator.model_discovery.urllib.request.urlopen", + return_value=_Response(payload), + ): + discovered = discover_provider_models(BYTEZ_SOURCE) + + assert len(discovered) == 1 + assert discovered[0].is_free is True + # Still no fabricated per-1k estimate: the honest-pricing behavior for + # GPU-second billing is preserved even for a free model. + assert discovered[0].prompt_price_per_1k is None + assert discovered[0].completion_price_per_1k is None + + +def test_discover_bytez_missing_meter_price_stays_unknown_not_free() -> None: + """A Bytez row with no ``meterPrice`` at all is unknown pricing, not free.""" + register_credential("BYTEZ_API_KEY", "bytez-secret") + payload = { + "error": None, + "output": [{"modelId": "0-hero/Matter-0.1-Slim-7B-C", "task": "chat"}], + } + + with patch( + "contextual_orchestrator.model_discovery.urllib.request.urlopen", + return_value=_Response(payload), + ): + discovered = discover_provider_models(BYTEZ_SOURCE) + + assert len(discovered) == 1 + assert discovered[0].is_free is False + + +@pytest.mark.parametrize( + ("meter_price", "expected"), + [ + ("0.0006478333 / sec", False), + ("0 / sec", True), + ("0/sec", True), + ("0.0000 / sec", True), + (0, True), + (0.0, True), + (0.0006, False), + (None, False), + ("", False), + (" ", False), + ("free", False), + (True, False), + (False, False), + ("-0 / sec", True), + # A zero rate is exactly as free regardless of its time unit -- the + # documented grammar constrains shape, not which unit word appears. + ("0 / hour", True), + # Genuinely malformed shapes must fail closed (unknown, not free), + # never trust a numeric-looking prefix pulled out of an unexpected + # overall shape. + ("0 /", False), # missing unit + ("/ sec", False), # missing rate + ("0 / sec / token", False), # extra separator + ("0//sec", False), # extra separator, empty middle segment + ("0", False), # no separator at all -- does not match " / " + ("0 sec", False), # no separator at all, space-joined + (" / ", False), # separator present, both sides empty + ], +) +def test_bytez_meter_price_is_free_classifies_exact_zero_rates( + meter_price: object, expected: bool +) -> None: + assert _bytez_meter_price_is_free(meter_price) is expected def test_discover_bytez_preserves_operator_declared_capabilities() -> None: