fix(discovery): set is_free from Bytez's real meterPrice signal

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 ``"<rate> / <unit>"`` 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
+        # "<rate> / <unit>" 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:

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=<name> ...`` 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 "<rate> / <unit>"
+        ("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: