diff --git a/CHANGELOG.md b/CHANGELOG.md index 322e8e279..b45576189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,71 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) production-default gate returns false on junk reports and on `measurement_status=estimated`. Access-list scope is a real ablation factor, not a duplicate label. +- `free_discovered_models()` no longer admits a zero-priced model that + declares a non-text input modality (e.g. NVIDIA NIM's + `meta/llama-3.2-90b-vision-instruct`, whose Models.dev evidence reports + `cost: 0/0` and, misleadingly for this exact deployment, `tool_call: true`) + into the general-purpose `orchestrator/free` pool. That pool serves + arbitrary request shapes -- including Strix's tool/function-calling + requests -- without knowing in advance which capability a request needs, so + a free model whose only price-evidenced identity requires an extra input + modality is reserved for a caller that explicitly needs it, not a general + worker. Fixes the required Strix Security Scan failure reproduced in + `ContextualWisdomLab/.github` PR #1198 (run 33325907333, job 99295892400), + where every one of 3 independent scan attempts exhausted the pool against + this exact agent with an identical HTTP 400 `invalid_request_error`. A + model excluded here remains fully discovered and price-evidenced; it is + only withheld from the free/tool-calling default pool. +- Devin's review on PR #933 found the fix above incomplete: `free_discovered_models()` + itself is now pure price-based inventory again (every zero-priced model + counts, restoring correct `--free-only`, `free_tier_count`, and free-tier + data-privacy totals), and a new, separately named selector, + `general_free_serving_candidates()`, carries the modality-based exclusion + for composing the blind `orchestrator/free` pool specifically. More + importantly, that exclusion previously lived only inside the one function + this PR touched: `_auto_discover_runtime_agents` (`--auto-discover-model-agents`) + and `provider_bootstrap._active_agent_from_discovered` (used by both + `bootstrap_provider_runtime` and, through it, + `provider_catalog_bootstrap.bootstrap_provider_catalog_runtime`) tag an + agent `cost:free` directly from raw price evidence and never consulted it, + so the incident model could still reach a live `cost:free` agent through + either path. The fix is now a single choke point, `TaskOrchestrator._is_free_agent`, + which every `orchestrator/free` selection path shares: an agent whose tags + declare a non-text input modality is never treated as free-pool eligible + there, regardless of which code built it or how old that agent-pool row is + -- while `cost:free` keeps meaning "honest zero price" everywhere else, + preserving `provider_catalog_store.py`'s durable `is_free` round trip. + Verified against three explicit modality fixtures (text-only, vision-only, + and text+image); evaluated and rejected narrowing the exclusion to spare a + model that "also declares text" (Devin's other suggestion) because the + incident model itself declares both `text` and `image` per Models.dev, so + that narrowing would have silently re-admitted it. +- Devin's review on the fix above found it had overshot: `TaskOrchestrator._is_free_agent` + is also the predicate `_capability_agents` uses for every explicit + capability-scoped free route (`/v1/audio/transcriptions`, `/v1/videos`, + image, speech, rerank), where a free agent's non-text `input:` + tag is the capability's own expected shape, not a surprise -- so the + shared choke point made those genuinely free agents unreachable through + their own free route. `_is_free_agent` now reverts to plain, modality-blind + price evidence; a new, stricter `_is_general_free_agent` (price-blind + `_is_free_agent` plus the non-text-input exclusion) carries the exclusion + only at capability-blind general-chat call sites (`proxy_completion`, + `_orchestrated_provider_completion`, `route_once`, `conduct`, + `list_openai_models`'s advertising check, and `server.py`'s + capability-agnostic `_require_pool_model` branch). The discovery-time and + selection-time "what counts as non-text input" classification is now one + shared predicate, `chat_capability.requires_non_text_input`, so + `model_discovery._requires_non_text_input` and + `orchestrator._agent_requires_non_text_input` cannot drift apart. +- Devin's next review pass found `general_free_serving_candidates()` still + overcounted: it admitted a zero-priced, text-input catalog row regardless + of whether it could ever actually become a serving agent, so an + `evidence_only` row (`agent_from_discovered` refuses to build an agent from + one) or a free non-chat-capable model (e.g. an embedding-only deployment) + both inflated `general_free_serving_count`. The selector now also requires + `is_routable_discovered_model` -- the same predicate `_auto_discover_runtime_agents` + and `provider_bootstrap` already require before promoting a discovered row + to an ordinary chat agent. ### Changed diff --git a/contextual_orchestrator/__main__.py b/contextual_orchestrator/__main__.py index f1c92fbad..a55f97e74 100644 --- a/contextual_orchestrator/__main__.py +++ b/contextual_orchestrator/__main__.py @@ -21,6 +21,7 @@ configured_gateway_source, discover_all_models, free_discovered_models, + general_free_serving_candidates, is_routable_discovered_model, refresh_price_book, select_bootstrap_discovered_agents, @@ -312,7 +313,13 @@ def _discover_models_command(argv: list[str]) -> None: privacy_assessments = [] if args.analyze_privacy_policies: discovered, privacy_assessments = analyze_discovered_privacy_policies(discovered) + # free_tier_count and general_free_serving_count are always computed over + # the complete `discovered` population, independent of --free-only (which + # only filters `reported`, the per-model listing below): both answer a + # global "how many, out of everything found" question, matching each + # other's population by design rather than "reported"'s row-level filter. free_models = free_discovered_models(discovered) + general_free_serving_models = general_free_serving_candidates(discovered) reported = free_models if args.free_only else discovered price_book = PriceBook(InMemoryConfigStore()) priced_count = refresh_price_book(reported, price_book) @@ -342,6 +349,7 @@ def _discover_models_command(argv: list[str]) -> None: report = { "discovered_count": len(reported), "free_tier_count": len(free_models), + "general_free_serving_count": len(general_free_serving_models), "free_data_privacy": { status: sum(1 for model in free_models if ( "supported" if model.supports_zero_data_retention is True else diff --git a/contextual_orchestrator/chat_capability.py b/contextual_orchestrator/chat_capability.py index 1dad07967..f10c1ba63 100644 --- a/contextual_orchestrator/chat_capability.py +++ b/contextual_orchestrator/chat_capability.py @@ -97,6 +97,31 @@ def is_general_chat_agent_model_id(model_id: str) -> bool: ) +def requires_non_text_input(input_modalities: Iterable[str]) -> bool: + """Return whether declared input-modality evidence includes non-text input. + + A model whose provider/catalog architecture evidence declares an input + modality other than ``"text"`` (e.g. ``image``, ``audio``, ``video``) is a + specialized multimodal deployment: a caller cannot use it for an arbitrary + request without knowing in advance that the request must carry that extra + modality. Absence of modality evidence is not evidence of a multimodal + requirement, so an empty ``input_modalities`` iterable never triggers this. + + Shared by ``model_discovery._requires_non_text_input`` (reading + ``DiscoveredModel.input_modalities`` directly) and + ``orchestrator.TaskOrchestrator._agent_requires_non_text_input`` (reading + an agent's persisted ``input:`` tags, with the ``input:`` prefix + already stripped by the caller) so the "what counts as non-text" reading + lives in exactly one place -- the two representations of the same catalog + evidence can never drift on this question independently of each other. + """ + return any( + modality.strip().casefold() != "text" + for modality in input_modalities + if isinstance(modality, str) and modality.strip() + ) + + def is_general_chat_candidate( model_id: str, *, diff --git a/contextual_orchestrator/model_discovery.py b/contextual_orchestrator/model_discovery.py index d23d3125b..5cf896e34 100644 --- a/contextual_orchestrator/model_discovery.py +++ b/contextual_orchestrator/model_discovery.py @@ -26,7 +26,11 @@ from typing import TYPE_CHECKING, Any, Literal, Mapping from urllib.parse import quote, urlsplit, urlunsplit -from .chat_capability import is_general_chat_agent_model_id, is_general_chat_candidate +from .chat_capability import ( + is_general_chat_agent_model_id, + is_general_chat_candidate, + requires_non_text_input, +) from .credentials import get_credential from .orchestrator import ModelAgent, ModelClient @@ -1304,11 +1308,116 @@ def agent_from_discovered(discovered: DiscoveredModel, *, priority: int = 0) -> ) +def _requires_non_text_input(discovered: DiscoveredModel) -> bool: + """Return whether catalog evidence shows this model needs non-text input. + + A model whose provider/catalog architecture evidence declares an input + modality other than ``"text"`` (e.g. ``image``, ``audio``, ``video``) is a + specialized multimodal deployment: a caller cannot use it for an arbitrary + request without knowing in advance that the request must carry that extra + modality. Absence of modality evidence is not evidence of a multimodal + requirement, so an empty ``input_modalities`` tuple never triggers this. + + This is a deliberately conservative reading: catalog fields such as + Models.dev's ``modalities.input`` document *supported* inputs, not which + ones a given request must supply, so a model that lists ``text`` next to + ``image`` still trips this check. ContextualWisdomLab/.github#1198's + incident model (NVIDIA NIM's ``meta/llama-3.2-90b-vision-instruct``) is + exactly that shape -- Models.dev reports its inputs as ``text`` *and* + ``image`` -- yet NIM's live deployment rejected a plain tool-calling + request against it three times in a row. With no reliable per-deployment + tool-calling signal available (see :func:`general_free_serving_candidates` + for the incident writeup), treating "declares any non-text input" as + disqualifying for *blind* serving is the only evidence-based reading that + actually keeps that incident fixed; a model believed to also serve plain + text requests just fine can still be reached through a pool that is not + modality-blind (see :func:`general_free_serving_candidates`'s docstring). + + Delegates the actual classification to + ``chat_capability.requires_non_text_input``, the single evidence-based + rule shared with ``orchestrator.TaskOrchestrator._agent_requires_non_text_input`` + (which reads an agent's persisted ``input:`` tags instead of + ``DiscoveredModel`` directly) so the two representations of the same + catalog evidence cannot drift on this question independently of each + other. + """ + return requires_non_text_input(discovered.input_modalities) + + def free_discovered_models(discovered: list[DiscoveredModel]) -> list[DiscoveredModel]: - """Return models whose provider metadata identifies zero-cost inference.""" + """Return the complete zero-cost model inventory (price evidence only). + + This is pure price-based inventory: every model whose structured + provider/catalog pricing evidence is entirely zero, regardless of input + or output modality. Reporting surfaces that answer "is this model free" + -- the ``discover-models`` CLI's ``--free-only`` report, ``free_tier_count``, + and the free-tier data-privacy totals -- need this complete inventory, not + a servable subset. + + Fitness for the general-purpose *blind* serving pool (``orchestrator/free``) + is a stricter, separate question: see :func:`general_free_serving_candidates`. + An earlier revision of this function conflated the two, which silently + undercounted genuinely free models that are simply unsuited to + capability-blind serving in every "is this model free" report. + """ return [model for model in discovered if model.is_free] +def general_free_serving_candidates( + discovered: list[DiscoveredModel], +) -> list[DiscoveredModel]: + """Return free models fit for the general-purpose blind serving pool. + + A zero price alone does not certify fitness for arbitrary callers: the + free pool (``orchestrator/free``) serves every role and request shape -- + including tool/function-calling requests -- without knowing in advance + which capability a given request will need. Provider pricing can be + reliably zero on a model that only a caller who already knows to supply + an extra input modality (e.g. an image) could ever use meaningfully; + :func:`_requires_non_text_input` excludes exactly those rows here, using + catalog evidence discovery already records, not a per-model name rule. + Such a model remains fully discovered, fully counted in + :func:`free_discovered_models`'s price-based inventory, and eligible for + a pool that is not modality-blind (e.g. one built for vision/multimodal + tasks) -- it is only withheld from *this* general-purpose free selector. + + Reproduces ContextualWisdomLab/.github#1198's required Strix Security Scan + failure (run 33325907333, job 99295892400): NVIDIA NIM's free + ``meta/llama-3.2-90b-vision-instruct`` passed every existing chat-capability + check, yet NIM's live deployment rejected Strix's tool-calling request + against it with a definitive HTTP 400 three independent times in a row -- + because the free pool had no other candidate to fail over to, this one + vision-input model alone exhausted the whole tool-calling pool. + + This is the selector every runtime pool-construction path must apply + before treating a discovered model as eligible for blind free serving + (e.g. tagging an agent ``cost:free`` in a context where that tag alone + drives general-chat ``orchestrator/free`` routing). + ``TaskOrchestrator._is_general_free_agent`` additionally re-checks an + agent's persisted ``input:`` tags at selection time -- but only + for the capability-blind general chat pool, never for a capability-scoped + free route (``_capability_agents``), where that same tag is the expected + shape, not a surprise -- so a durable agent-pool row written by an older + build, before this exclusion existed, cannot bypass it either. + + Zero price and text-only input still are not enough on their own: an + ``evidence_only`` catalog row can never become a serving agent at all + (:func:`agent_from_discovered` refuses to build one), and a free + non-chat-capable model (e.g. an embedding-only deployment) is not a + general chat candidate either. :func:`is_routable_discovered_model` -- + the same predicate ``_auto_discover_runtime_agents`` and + ``provider_bootstrap`` already require before promoting a discovered row + to an ordinary chat agent -- excludes both here too, so this selector's + count never overstates how many free models the general chat pool could + actually serve. + """ + return [ + model + for model in free_discovered_models(discovered) + if is_routable_discovered_model(model) and not _requires_non_text_input(model) + ] + + def _currency_is_comparable(currency_code: object, default_currency: object) -> bool: """Return whether two ISO-style currency codes can be compared directly.""" return ( diff --git a/contextual_orchestrator/orchestrator.py b/contextual_orchestrator/orchestrator.py index bac15f936..8f012c9f7 100644 --- a/contextual_orchestrator/orchestrator.py +++ b/contextual_orchestrator/orchestrator.py @@ -38,6 +38,7 @@ from .chat_capability import ( is_chat_compatible_model_id, is_general_chat_candidate, + requires_non_text_input, ) from .conventions import require_object_name from .credentials import NotConfigured, get_credential @@ -3666,7 +3667,7 @@ def proxy_completion( { candidate.id for candidate in self.agents - if self._is_free_agent(candidate) and self._zdr_agent_allowed(candidate) + if self._is_general_free_agent(candidate) and self._zdr_agent_allowed(candidate) } if requested_model == self.FREE_MODEL else ( @@ -4001,7 +4002,7 @@ def _orchestrated_provider_completion( { candidate.id for candidate in self.agents - if self._is_free_agent(candidate) and self._zdr_agent_allowed(candidate) + if self._is_general_free_agent(candidate) and self._zdr_agent_allowed(candidate) } if free_only else ( @@ -5175,7 +5176,7 @@ def route_once( free_ids = { candidate.id for candidate in self.agents - if self._is_free_agent(candidate) and self._zdr_agent_allowed(candidate) + if self._is_general_free_agent(candidate) and self._zdr_agent_allowed(candidate) } allowed_agent_ids = free_ids if free_only else None @@ -5364,7 +5365,7 @@ def conduct( free_ids = { candidate.id for candidate in self.agents - if self._is_free_agent(candidate) and self._zdr_agent_allowed(candidate) + if self._is_general_free_agent(candidate) and self._zdr_agent_allowed(candidate) } requested_agent = self._requested_agent(model_name) judge_agent_ids = ( @@ -5688,6 +5689,19 @@ def _ranked_agents( 3. Inside one logical model group, measured ledgers refine member order (:meth:`_measured_member_order`: judged quality first, then successful responses per second). + + ``free_only`` selects between the two ``FREE_MODEL`` eligibility + predicates using ``chat_only`` as the scope signal: ``chat_only=True`` + (every caller except ``_capability_agents``) means the request shape + is not yet known, so :meth:`_is_general_free_agent` applies the + blind-serving modality exclusion; ``chat_only=False`` means a + specific, already-known capability was requested (only + ``_capability_agents`` passes this), so the plain price-only + :meth:`_is_free_agent` applies instead -- a non-text ``input:`` + modality there is the capability's own expected shape, not a + surprise. This is a deliberate reuse of an existing, audited signal + (``chat_only`` already means "the caller does not know which + capability will be needed"), not a new implicit distinction. """ source = self.agents if candidate_pool is None else list(candidate_pool) candidates = [ @@ -5695,7 +5709,10 @@ def _ranked_agents( for agent in source if not agent.disabled and self._zdr_agent_allowed(agent) - if (not free_only or self._is_free_agent(agent)) + if ( + not free_only + or (self._is_general_free_agent(agent) if chat_only else self._is_free_agent(agent)) + ) and (not chat_only or _is_general_chat_agent(agent)) and all(tag in agent.tags for tag in required_tags) ] @@ -5835,14 +5852,64 @@ def _routers_forget_members(self, member_ids: set[str]) -> None: for router in self._routing_ledgers(): router.forget_members(member_ids) + @staticmethod + def _agent_requires_non_text_input(agent: ModelAgent) -> bool: + """Return whether an agent's discovery-derived tags declare non-text input. + + ``ModelAgent`` carries no dedicated modality field; every discovery + pathway (``model_discovery.agent_from_discovered``, + ``provider_bootstrap.serving_tags_for_discovered``) instead records + each declared input modality as an ``input:`` tag. Delegates + the actual "what counts as non-text" classification to + ``chat_capability.requires_non_text_input``, the single evidence-based + rule shared with ``model_discovery._requires_non_text_input`` (which + reads ``DiscoveredModel.input_modalities`` directly) so the two + representations of the same catalog evidence cannot drift on this + question independently of each other. + """ + return requires_non_text_input( + tag[len("input:"):] for tag in agent.tags if tag.startswith("input:") + ) + def _is_free_agent(self, agent: ModelAgent) -> bool: - """Return true only for explicitly zero-priced configured models.""" + """Return true only for explicitly zero-priced configured models. + + Price-only, deliberately blind to modality: this predicate backs + every ``FREE_MODEL`` selection path, including capability-scoped + media routes (``_capability_agents`` -> ``/v1/audio/transcriptions``, + ``/v1/videos``, image, speech, rerank) where an agent's non-text + ``input:`` tag is exactly the modality the request is + already asking for, not a surprise -- excluding it there would make a + genuinely free transcription/video/image agent unreachable through + its own capability's free route. See :meth:`_is_general_free_agent` + for the stricter, blind-general-chat variant. + """ if "cost:free" in agent.tags or self.price_per_million.get(agent.id) == 0: return True return self.price_per_million.get(agent.model) == 0 and sum( candidate.model == agent.model for candidate in self.candidates ) == 1 + def _is_general_free_agent(self, agent: ModelAgent) -> bool: + """Return true only for zero-priced models fit for *blind* free serving. + + Zero price alone does not certify fitness for the general-purpose + ``orchestrator/free`` chat pool: that pool serves every role and + request shape -- including tool-calling requests -- without knowing + in advance which capability a request will need. An agent whose tags + declare a non-text input modality (e.g. a vision-input deployment) is + therefore never treated as free *here*, even when it is honestly + tagged ``cost:free`` for price inventory purposes and for its own + capability-scoped free route (see :meth:`_is_free_agent`, and + ``contextual_orchestrator.model_discovery.general_free_serving_candidates`` + for the equivalent discovery-time selector and its incident writeup). + This is the single choke point every *general chat* ``FREE_MODEL`` + selection path shares -- including an agent row loaded from a durable + pool store that was written before this exclusion existed, or one + activated by a pool-construction path this repository adds later. + """ + return self._is_free_agent(agent) and not self._agent_requires_non_text_input(agent) + # --- semantic-affinity evidence (cosine similarity; no keyword lists) --- @staticmethod @@ -6077,6 +6144,10 @@ def _capability_agents(self, capability: str, model_name: str | None = None) -> ranked = [ agent for agent in self._ranked_agents( + # chat_only=False signals a known, explicit capability request + # (not a blind general-chat one), so free_only here uses + # _ranked_agents' price-only _is_free_agent branch: a capable + # agent's own non-text input tag is expected, not disqualifying. "", capability, free_only=free_only, chat_only=False ) if not agent.disabled @@ -6954,7 +7025,7 @@ def list_openai_models(self) -> dict[str, Any]: "created": created, "owned_by": "contextual-orchestrator", }) - if any(self._is_free_agent(agent) for agent in self.agents): + if any(self._is_general_free_agent(agent) for agent in self.agents): data.append({ "id": self.FREE_MODEL, "object": "model", diff --git a/contextual_orchestrator/server.py b/contextual_orchestrator/server.py index 37dae77f2..6f7bd022f 100644 --- a/contextual_orchestrator/server.py +++ b/contextual_orchestrator/server.py @@ -2361,7 +2361,7 @@ def _require_pool_model( if any(zdr_allowed(agent) for agent in agents): return model_name raise RequestError(400, "invalid_model", "no enabled model is available") - if any(zdr_allowed(agent) and orchestrator._is_free_agent(agent) for agent in agents): + if any(zdr_allowed(agent) and orchestrator._is_general_free_agent(agent) for agent in agents): return model_name raise RequestError(400, "invalid_model", "no enabled zero-cost model is available") try: @@ -2370,6 +2370,12 @@ def _require_pool_model( capability_agents = [] capability_agents = [agent for agent in capability_agents if zdr_allowed(agent)] if model_name == TaskOrchestrator.FREE_MODEL: + # Deliberately the price-only ``_is_free_agent`` here, not the + # blind-general-chat ``_is_general_free_agent``: capability_agents + # is already scoped to required_capability, so a non-text + # ``input:`` tag (e.g. a transcription agent's + # ``input:audio``) is the expected shape for this exact route, not + # a surprise the caller needs protecting from. capability_agents = [ agent for agent in capability_agents diff --git a/fuzz/fuzz_agent_config.py b/fuzz/fuzz_agent_config.py index 3a70bec85..e681d6e53 100644 --- a/fuzz/fuzz_agent_config.py +++ b/fuzz/fuzz_agent_config.py @@ -22,6 +22,7 @@ def one_input(data: bytes) -> None: + """Feed one Atheris-fuzzed byte string through the agent-config parser.""" fdp = atheris.FuzzedDataProvider(data) text = fdp.ConsumeUnicodeNoSurrogates(fdp.remaining_bytes()) try: @@ -32,6 +33,7 @@ def one_input(data: bytes) -> None: def main() -> None: + """Run the Atheris coverage-guided fuzz loop against ``one_input``.""" atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/fuzz/fuzz_model_judge.py b/fuzz/fuzz_model_judge.py index 9058f8de1..08c1f3254 100755 --- a/fuzz/fuzz_model_judge.py +++ b/fuzz/fuzz_model_judge.py @@ -13,11 +13,13 @@ def one_input(data: bytes) -> None: + """Feed one Atheris-fuzzed byte string through model-judge reply parsing.""" fdp = atheris.FuzzedDataProvider(data) exercise_model_judge_reply(fdp.ConsumeUnicodeNoSurrogates(fdp.remaining_bytes())) def main() -> None: + """Run the Atheris coverage-guided fuzz loop against ``one_input``.""" atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/fuzz/fuzz_orchestration.py b/fuzz/fuzz_orchestration.py index 72087a3d1..25143fe17 100644 --- a/fuzz/fuzz_orchestration.py +++ b/fuzz/fuzz_orchestration.py @@ -24,6 +24,7 @@ def one_input(data: bytes) -> None: + """Feed one Atheris-fuzzed prompt/mode pair through end-to-end orchestration.""" fdp = atheris.FuzzedDataProvider(data) mode = _MODES[fdp.ConsumeIntInRange(0, len(_MODES) - 1)] prompt = fdp.ConsumeUnicodeNoSurrogates(fdp.remaining_bytes()) @@ -31,6 +32,7 @@ def one_input(data: bytes) -> None: def main() -> None: + """Run the Atheris coverage-guided fuzz loop against ``one_input``.""" atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/fuzz/fuzz_reasoning_effort_profile.py b/fuzz/fuzz_reasoning_effort_profile.py index 0e45a60b0..68178d3d8 100644 --- a/fuzz/fuzz_reasoning_effort_profile.py +++ b/fuzz/fuzz_reasoning_effort_profile.py @@ -22,6 +22,7 @@ def one_input(data: bytes) -> None: + """Feed one Atheris-fuzzed byte string through the profile parser.""" fdp = atheris.FuzzedDataProvider(data) text = fdp.ConsumeUnicodeNoSurrogates(fdp.remaining_bytes()) try: @@ -32,6 +33,7 @@ def one_input(data: bytes) -> None: def main() -> None: + """Run the Atheris coverage-guided fuzz loop against ``one_input``.""" atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/fuzz/fuzz_redaction.py b/fuzz/fuzz_redaction.py index 4e06a5d41..439e3b037 100644 --- a/fuzz/fuzz_redaction.py +++ b/fuzz/fuzz_redaction.py @@ -22,12 +22,14 @@ def one_input(data: bytes) -> None: + """Feed one Atheris-fuzzed byte string through secret/PII redaction.""" fdp = atheris.FuzzedDataProvider(data) text = fdp.ConsumeUnicodeNoSurrogates(fdp.remaining_bytes()) exercise_redaction(text) def main() -> None: + """Run the Atheris coverage-guided fuzz loop against ``one_input``.""" atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/fuzz/fuzz_request_body.py b/fuzz/fuzz_request_body.py index 291525ac3..ea01396d0 100644 --- a/fuzz/fuzz_request_body.py +++ b/fuzz/fuzz_request_body.py @@ -21,10 +21,12 @@ def one_input(data: bytes) -> None: + """Feed one Atheris-fuzzed byte string through the request-body parser.""" exercise_request_body(data) def main() -> None: + """Run the Atheris coverage-guided fuzz loop against ``one_input``.""" atheris.Setup(sys.argv, one_input) atheris.Fuzz() diff --git a/tests/test_auto_discovery_server.py b/tests/test_auto_discovery_server.py index 51baf1713..c80fd3b09 100644 --- a/tests/test_auto_discovery_server.py +++ b/tests/test_auto_discovery_server.py @@ -47,6 +47,61 @@ def test_auto_discovery_activates_only_chat_capable_agents(monkeypatch) -> None: assert "bootstrap_agent" in result["updated"] +def test_auto_discovery_activates_a_free_vision_model_but_free_pool_excludes_it( + monkeypatch, +) -> None: + """A free vision-input model becomes a normal agent, not a blind-free one. + + Reproduces the second locus of ContextualWisdomLab/.github#1198's incident + (Devin review on PR #933): ``_auto_discover_runtime_agents`` activates + every routable discovered model directly through + ``model_discovery.agent_from_discovered`` and never consulted + ``model_discovery.free_discovered_models``'s exclusion at all, so NVIDIA + NIM's ``meta/llama-3.2-90b-vision-instruct`` kept its ``cost:free`` tag and + stayed blindly selectable by ``orchestrator/free`` through this second + path even after that first function was fixed. Pre-fix, every assertion + from ``_is_general_free_agent`` onward here fails: the agent is (wrongly) + treated as blind-free-pool eligible and ``orchestrator/free`` is (wrongly) + advertised. + """ + vision = DiscoveredModel( + provider_name="nvidia_nim", + model_id="meta/llama-3.2-90b-vision-instruct", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("text", "image"), + output_modalities=("text",), + is_free=True, + ) + monkeypatch.setattr( + "contextual_orchestrator.__main__.discover_all_models", + lambda *_args, **_kwargs: ([vision], []), + ) + orchestrator = TaskOrchestrator( + [ModelAgent("bootstrap_agent", "bootstrap-model", tags=("bootstrap_seed",))] + ) + + result = _auto_discover_runtime_agents(orchestrator) + + assert len(result["added"]) == 1 + agent = next( + candidate for candidate in orchestrator.agents if candidate.id == result["added"][0] + ) + # The model is a legitimate chat-capable agent (e.g. for a caller that + # explicitly requests it with an image) and its price evidence is honest. + assert agent.disabled is False + assert "cost:free" in agent.tags + # Its own capability route can still serve it for free (price-only). + assert orchestrator._is_free_agent(agent) is True + # It must never be selectable by the capability-blind general chat pool. + assert orchestrator._is_general_free_agent(agent) is False + assert orchestrator.FREE_MODEL not in { + row["id"] for row in orchestrator.list_openai_models()["data"] + } + + def test_auto_discovery_activates_bare_chat_but_not_embedding_ids(monkeypatch) -> None: """A metadata-free gateway listing still activates chat deployments. diff --git a/tests/test_discover_models_cli.py b/tests/test_discover_models_cli.py index 896cdc176..f9a333d31 100644 --- a/tests/test_discover_models_cli.py +++ b/tests/test_discover_models_cli.py @@ -88,6 +88,7 @@ def test_discover_models_with_no_credentials_reports_zero_and_succeeds() -> None "privacy_policy_analysis": [], "enabled_agent_ids": [], "free_tier_count": 0, + "general_free_serving_count": 0, "free_data_privacy": {"supported": 0, "unsupported": 0, "unknown": 0}, "models": [], } diff --git a/tests/test_model_discovery.py b/tests/test_model_discovery.py index 9b72a0487..965fce61e 100644 --- a/tests/test_model_discovery.py +++ b/tests/test_model_discovery.py @@ -40,6 +40,7 @@ discover_all_models, discover_provider_models, free_discovered_models, + general_free_serving_candidates, openrouter_paid_inference_available, refresh_price_book, select_cheapest_discovered_agent, @@ -716,6 +717,291 @@ def test_discovery_retains_full_catalog_and_marks_free_models() -> None: assert agent_from_discovered(replace(discovered[0], evidence_only=False)).group_name == "" +def _nim_vision_model() -> DiscoveredModel: + """NVIDIA NIM's incident model: free, chat-capable, declares text + image.""" + return DiscoveredModel( + provider_name="nvidia_nim", + model_id="meta/llama-3.2-90b-vision-instruct", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("text", "image"), + output_modalities=("text",), + is_free=True, + ) + + +def test_general_free_serving_candidates_excludes_a_free_vision_only_input_model() -> None: + """The general-purpose free pool must exclude a zero-priced vision-input model. + + Relocated from ``free_discovered_models`` (ContextualWisdomLab/.github PR + #1198's original fix) onto the dedicated serving-eligibility selector once + Devin's review on PR #933 found that ``free_discovered_models`` itself must + stay a pure price-based inventory (see + ``test_free_discovered_models_still_counts_a_free_vision_only_input_model`` + below) -- the intent of the original regression test is unchanged. + + Reproduces ``ContextualWisdomLab/.github`` PR #1198's required Strix Security + Scan failure (run 33325907333, job 99295892400): NVIDIA NIM's free + ``meta/llama-3.2-90b-vision-instruct`` passes every existing chat-capability + check (Models.dev reports its cost as 0/0, its output modality is "text", + and its model id carries no disqualifying token), yet NIM's live deployment + rejected Strix's tool-calling request against it with a definitive HTTP 400 + (``invalid_request_error``) three independent times in a row -- because the + orchestrator/free pool has no other candidate to fail over to, this one + vision-input model alone exhausts the whole "free" tool-calling pool. + Models.dev's own ``tool_call`` field claims ``true`` for this exact model + (verified live), so that field cannot be the fix; its declared *input* + modality (``image``, alongside ``text``) is the only honest catalog + evidence distinguishing it from an ordinary text-only free worker. A + model requiring non-text input is a specialized multimodal deployment, not + a general-purpose worker a caller can route arbitrary (including + tool-calling) requests to without knowing in advance that it needs an + image -- so it must not enter the general free pool, while a text-only + free model of identical price remains fully eligible. + """ + vision_model = _nim_vision_model() + text_only_model = DiscoveredModel( + provider_name="nvidia_nim", + model_id="meta/llama-3.1-8b-instruct", + 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, + ) + no_modality_evidence_model = DiscoveredModel( + provider_name="nvidia_nim", + model_id="mistralai/mistral-small", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + is_free=True, + ) + + serving_candidates = general_free_serving_candidates( + [vision_model, text_only_model, no_modality_evidence_model] + ) + + assert [model.model_id for model in serving_candidates] == [ + "meta/llama-3.1-8b-instruct", + "mistralai/mistral-small", + ] + + +def test_free_discovered_models_still_counts_a_free_vision_only_input_model() -> None: + """Price-based free inventory must not lose a model excluded from serving. + + Finding 3 of Devin's review on PR #933: by filtering inside + ``free_discovered_models()`` itself, the original PR #1198 fix silently + undercounted a genuinely free model in every consumer of that function + that wants raw price inventory rather than serving-pool eligibility -- + ``--free-only`` CLI output, ``free_tier_count``, and the free-tier + data-privacy totals. This model is priced at zero and must be counted + here even though :func:`general_free_serving_candidates` correctly + excludes it from blind serving. + """ + vision_model = _nim_vision_model() + + assert free_discovered_models([vision_model]) == [vision_model] + assert general_free_serving_candidates([vision_model]) == [] + + +def test_general_free_serving_candidates_excludes_unroutable_free_models() -> None: + """Non-text price/modality evidence alone does not certify servability. + + Devin's review pass on PR #933 after ``efd44f6`` found that + ``general_free_serving_candidates`` admits any zero-priced, text-input + row regardless of whether it could ever actually become a serving agent: + an ``evidence_only`` catalog row (``agent_from_discovered`` refuses to + build an agent from one at all) and a free non-chat-capable model (e.g. + an embedding-only deployment) both pass the price and modality checks + while being fundamentally unroutable. ``general_free_serving_count`` + therefore overcounted models the general chat pool could never actually + serve. ``is_routable_discovered_model`` -- the same predicate + ``_auto_discover_runtime_agents`` and ``provider_bootstrap`` already use + to decide whether a discovered row may become an ordinary chat agent at + all -- is the missing check. + """ + evidence_only_free_text_model = replace( + DiscoveredModel( + provider_name="nvidia_nim", + model_id="evidence-only-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, + ), + evidence_only=True, + ) + embedding_only_free_model = DiscoveredModel( + provider_name="nvidia_nim", + model_id="embedding-only-free-model", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("embedding",), + input_modalities=("text",), + output_modalities=("text",), + is_free=True, + ) + routable_free_text_model = DiscoveredModel( + provider_name="nvidia_nim", + model_id="routable-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, + ) + + serving_candidates = general_free_serving_candidates([ + evidence_only_free_text_model, + embedding_only_free_model, + routable_free_text_model, + ]) + + assert [model.model_id for model in serving_candidates] == ["routable-free-model"] + # Both unroutable rows remain fully counted in the price-based inventory. + assert {model.model_id for model in free_discovered_models([ + evidence_only_free_text_model, + embedding_only_free_model, + routable_free_text_model, + ])} == { + "evidence-only-free-model", + "embedding-only-free-model", + "routable-free-model", + } + + +def test_general_free_serving_candidates_modality_shapes() -> None: + """Explicit three-way modality contract: text-only, image-only, text+image. + + Finding 2 of Devin's review on PR #933 argued the exclusion should spare a + model that "also supports text as a standalone input", so that only a + strictly vision-*only* model (no declared text input at all) is excluded. + Verified against this repository's own incident evidence and rejected: + NVIDIA NIM's real incident model (see ``_nim_vision_model``) declares + *both* ``text`` and ``image`` as supported inputs per Models.dev -- i.e. + it already satisfies "text is a supported standalone input" by Devin's own + proposed test -- yet NIM's live deployment rejected a plain tool-calling + request against it three times in a row. Models.dev's ``input_modalities`` + documents supported inputs, not which ones a given request must supply, so + it cannot certify that this exact model would have served a tool-calling + request that carried text alone. Narrowing the exclusion to spare + "text is also listed" models would therefore silently re-admit the very + model this incident is about, so this repository instead keeps excluding + any declared non-text input modality from blind serving (see + ``general_free_serving_candidates``'s and ``_requires_non_text_input``'s + docstrings for the full reasoning) -- while a model with *no* modality + evidence at all is not penalized for an absent catalog field. + """ + text_only = DiscoveredModel( + provider_name="nvidia_nim", + model_id="text-only-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, + ) + vision_only = DiscoveredModel( + provider_name="nvidia_nim", + model_id="vision-only-model", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("image",), + output_modalities=("text",), + is_free=True, + ) + text_and_image = _nim_vision_model() + + serving_candidates = general_free_serving_candidates( + [text_only, vision_only, text_and_image] + ) + + assert [model.model_id for model in serving_candidates] == ["text-only-model"] + # All three remain fully counted in the price-based inventory regardless. + assert {model.model_id for model in free_discovered_models( + [text_only, vision_only, text_and_image] + )} == {"text-only-model", "vision-only-model", "meta/llama-3.2-90b-vision-instruct"} + + +def test_discovery_and_orchestrator_modality_eligibility_cannot_drift() -> None: + """``general_free_serving_candidates`` and ``_is_general_free_agent`` agree. + + Devin's review on PR #933 (design-consistency note): the discovery-time + selector (over ``DiscoveredModel.input_modalities``) and the + selection-time predicate (over an agent's persisted ``input:`` + tags) must never independently reimplement "what counts as non-text + input" -- both now delegate to ``chat_capability.requires_non_text_input`` + for that classification. This locks the three fixture shapes already + established by ``test_general_free_serving_candidates_modality_shapes`` + (text-only, vision-only-input, text+image) so the two call sites cannot + silently diverge again. + """ + text_only = DiscoveredModel( + provider_name="nvidia_nim", + model_id="text-only-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, + ) + vision_only = DiscoveredModel( + provider_name="nvidia_nim", + model_id="vision-only-model", + credential_name="NVIDIA_NIM_API_KEY", + chat_base_url="https://integrate.api.nvidia.com/v1", + auth_scheme="Bearer", + capabilities=("chat",), + input_modalities=("image",), + output_modalities=("text",), + is_free=True, + ) + text_and_image = _nim_vision_model() + discovered = [text_only, vision_only, text_and_image] + serving_model_ids = {model.model_id for model in general_free_serving_candidates(discovered)} + + # ModelAgent.id must be two-or-more-word snake_case; provider model ids + # (e.g. "meta/llama-3.2-90b-vision-instruct") are not, so derive a + # compliant id distinct from the ``model`` field under test. + agent_id_translation = str.maketrans("/.-", "___") + agents = { + model.model_id: ModelAgent( + model.model_id.casefold().translate(agent_id_translation), + model.model_id, + tags=("cost:free", *(f"input:{value}" for value in model.input_modalities)), + ) + for model in discovered + } + orchestrator = TaskOrchestrator(list(agents.values())) + + for model in discovered: + agent = agents[model.model_id] + assert orchestrator._is_general_free_agent(agent) == ( + model.model_id in serving_model_ids + ), model.model_id + # Every one of these stays reachable through its own capability route. + assert orchestrator._is_free_agent(agent) is True + + def test_discovery_does_not_mark_multimodal_input_rows_free_without_unit_prices() -> None: """Non-text inputs require explicit zero non-token price evidence.""" source = ProviderModelSource( diff --git a/tests/test_multimodal_model_group_http.py b/tests/test_multimodal_model_group_http.py index 86e349c32..ac7ffb970 100644 --- a/tests/test_multimodal_model_group_http.py +++ b/tests/test_multimodal_model_group_http.py @@ -545,6 +545,63 @@ def test_free_virtual_model_uses_only_zero_cost_media_models() -> None: assert result["model"] == "provider/free" +def test_free_virtual_model_selects_a_free_agent_whose_own_capability_needs_non_text_input() -> None: + """The blind-chat modality exclusion must not reach capability-scoped routes. + + Devin's review on PR #933, fourth finding: ``TaskOrchestrator._is_free_agent`` + had grown a non-text-input exclusion meant only for the capability-blind + general chat pool, but it also backed ``_capability_agents`` (every + ``/v1/audio/transcriptions``, ``/v1/videos``, image, speech, and rerank + route). A free transcription agent naturally carries an ``input:audio`` + tag and a free image/video agent an ``input:image`` tag -- exactly the + modality its own capability-scoped free route is asking for, not a + surprise -- so the shared predicate made those genuinely free agents + unreachable through their own free route. Fails pre-fix (agent excluded, + ``RuntimeError``), passes post-fix (``_is_free_agent`` stays modality-blind; + only ``_is_general_free_agent`` -- the general-chat-only variant -- adds + the exclusion). + """ + orchestrator = TaskOrchestrator([ + ModelAgent( + "free_transcription", + "provider/free-transcription", + tags=("transcription", "cost:free", "input:audio"), + ), + ]) + + result = orchestrator.proxy_capability( + {"model": orchestrator.FREE_MODEL, "prompt": "demo"}, + capability="transcription", + endpoint="audio/transcriptions", + ) + + assert result["model"] == "provider/free-transcription" + + +def test_free_virtual_model_selects_a_free_video_agent_whose_input_is_image() -> None: + """Same regression as above for a free video/image-edit agent's own route. + + A video (or image-edit) agent naturally and correctly declares + ``input:image`` -- the request's own required extra input, not a + surprise -- so its own capability-scoped free route must still find it. + """ + orchestrator = TaskOrchestrator([ + ModelAgent( + "free_video_edit", + "provider/free-video-edit", + tags=("video", "cost:free", "input:image"), + ), + ]) + + result = orchestrator.proxy_capability( + {"model": orchestrator.FREE_MODEL, "prompt": "demo"}, + capability="video", + endpoint="videos", + ) + + assert result["model"] == "provider/free-video-edit" + + def test_free_virtual_model_fails_closed_without_zero_cost_media_models() -> None: orchestrator = TaskOrchestrator([ ModelAgent("paid_video", "provider/paid", tags=("video",)), diff --git a/tests/test_orchestrated_responses_stream.py b/tests/test_orchestrated_responses_stream.py index 9d23a0530..b958f20c5 100644 --- a/tests/test_orchestrated_responses_stream.py +++ b/tests/test_orchestrated_responses_stream.py @@ -294,6 +294,32 @@ def test_virtual_capability_models_resolve_to_eligible_upstreams() -> None: ) +def test_require_pool_model_serves_capability_free_route_despite_non_text_input() -> None: + """A free transcription agent's own capability route is not modality-gated. + + Devin's review on PR #933, fourth finding: ``_require_pool_model`` applies + ``TaskOrchestrator._is_free_agent`` a second time (independent of + ``_capability_agents``) when validating a capability-scoped + ``orchestrator/free`` request. A free agent naturally declaring + ``input:audio`` for a transcription capability must still resolve here; + only the capability-blind general-chat branch (``required_capability is + None``) applies the stricter general-chat exclusion. + """ + orchestrator = TaskOrchestrator([ + ModelAgent( + "free_transcription", + "free-transcription", + tags=("transcription", "cost:free", "input:audio"), + ), + ]) + + assert _require_pool_model( + orchestrator, "orchestrator/free", required_capability="transcription" + ) == "free-transcription" + with pytest.raises(RequestError, match="no enabled zero-cost model"): + _require_pool_model(orchestrator, "orchestrator/free") + + def test_virtual_capability_auto_rejects_non_zdr_pool() -> None: orchestrator = TaskOrchestrator( [ModelAgent("paid_embedding", "paid-embedding", tags=("embedding",))] diff --git a/tests/test_provider_bootstrap.py b/tests/test_provider_bootstrap.py index 2a11aef0e..2ea7a5b2e 100644 --- a/tests/test_provider_bootstrap.py +++ b/tests/test_provider_bootstrap.py @@ -283,6 +283,48 @@ def test_serving_tags_preserve_only_explicit_free_and_modality_evidence(): } <= set(tags) +def test_active_agent_from_discovered_free_vision_model_is_not_free_pool_eligible(): + """A bootstrap-activated free vision agent stays out of orchestrator/free. + + Third locus of ContextualWisdomLab/.github#1198's incident (Devin review + on PR #933): ``provider_bootstrap._active_agent_from_discovered`` (used by + ``bootstrap_provider_runtime`` and, through it, + ``provider_catalog_bootstrap.bootstrap_provider_catalog_runtime``) tags an + agent ``cost:free`` from raw price evidence alone, with no serving-pool + modality check -- exactly like the ``_auto_discover_runtime_agents`` path + this incident already fixed once. The fix lives in + ``TaskOrchestrator._is_general_free_agent`` (a single choke point every + *general-chat* ``FREE_MODEL`` selection path shares, including this one + and any future pool-construction path); ``_is_free_agent`` itself stays + price-only so the same honestly free agent remains reachable through its + own capability-scoped free route (Devin's PR #933 fourth-round finding). + Not in this tagging function: ``cost:free`` must keep meaning "honest + zero price" here, because + ``provider_catalog_store.py``'s durable catalog round trip reconstructs + ``DiscoveredModel.is_free`` from exactly this tag (see + ``test_serving_tags_preserve_only_explicit_free_and_modality_evidence`` and + ``test_last_known_good_restores_free_and_modality_evidence``). + """ + vision_model = replace( + _model("nvidia_nim", "NVIDIA_NIM_API_KEY", "meta/llama-3.2-90b-vision-instruct", 0.0), + capabilities=("chat",), + input_modalities=("text", "image"), + output_modalities=("text",), + is_free=True, + ) + + agent = provider_bootstrap._active_agent_from_discovered(vision_model) + orchestrator = TaskOrchestrator([agent]) + + assert agent.disabled is False + assert "cost:free" in agent.tags + # Price-only: honestly free, and (Devin's PR #933 fourth-round finding) + # still reachable through its own capability-scoped free route. + assert orchestrator._is_free_agent(agent) is True + # The blind general-chat orchestrator/free pool must still exclude it. + assert orchestrator._is_general_free_agent(agent) is False + + def test_serving_tags_preserve_explicit_no_zdr_evidence(): """Explicit unsupported zero-data retention survives tag normalization.""" model = replace(