fix(discovery): gate OpenRouter evidence_only/zdr_capable per model, not per provider - #950
fix(discovery): gate OpenRouter evidence_only/zdr_capable per model, not per provider#950seonghobae wants to merge 1 commit into
Conversation
…not per provider ProviderModelSource.evidence_only was hardcoded True for OpenRouter's provider source declaration only (952996e), applying to every discovered OpenRouter model unconditionally regardless of that model's own evidence. Every other one of the six provider sources uses the False default. This contradicts ADR 0032's binding design for this exact subsystem: "Privacy discovery is also model-specific rather than inferred from price ... discovery does not turn either ambiguous state into blanket non-support." Per-model ZDR evidence for OpenRouter already exists and is actively computed elsewhere in this file (_merge_openrouter_zdr_metadata / _merge_openrouter_provider_privacy fetch OpenRouter's authoritative /api/v1/endpoints/zdr feed and per-endpoint provider privacy policy) but was discarded for OpenRouter's own rows -- used exclusively to donate ZDR status to other providers' rows sharing the same canonical model id. _apply_discovered_model_evidence now gates evidence_only/zdr_capable for OpenRouter's own rows through that identical per-model feed match already used to donate evidence to other providers: a row becomes routable only when its own model id is present on the feed. Fail-closed is preserved -- a model absent from the feed, an empty feed, or any feed-fetch failure still leaves the row evidence_only=True/zdr_capable=False and unroutable, exactly as before. PROVIDER_MODEL_SOURCES's openrouter entry now leaves evidence_only at the False default like every other provider source. Fixes the three zdr_capable expectation tests that were flipped in lockstep with 952996e to assert the wrong blanket-exclusion behavior, plus the evidence_only source-declaration test. Adds two new negative cases: a genuinely non-attested OpenRouter model (stays evidence_only=True, unroutable) and a total ZDR-feed fetch failure (every OpenRouter row stays evidence_only=True, never falls back to the new False default). A companion, independent blanket filter in ContextualWisdomLab/.github's scripts/ci/contextual_orchestrator_review_launcher.py (_routable_discovered_models()) is tracked and fixed separately, not here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
|
Warning Review limit reachedNext included review available in 12 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing as superseded by #949, which merged into #949 independently fixes the same root bug this PR addresses (removes the blanket Since #949 is a strict superset of this PR's scope and is already merged, closing this one without merging. Branch Generated by Claude Code |
The bug (confirmed via a 5-agent investigation)
ProviderModelSource.evidence_only(defaultFalse) was hardcodedTruefor OpenRouter's provider source declaration only, at module load, before any HTTP call — applying to every discovered OpenRouter model unconditionally, regardless of that specific model's own evidence. OpenRouter was the only one of six provider sources with this override; every other provider (openai,opencode_zen,nvidia_nim,nvidia_nim_sub,bytez,configured_gateway) uses theFalsedefault.That flag gated three downstream chokepoints, all firing regardless of the individual model's real evidence:
zdr_capable=not model.evidence_only and matches(model.model_id)— forcedFalsefor every OpenRouter row even when it genuinely matched OpenRouter's own ZDR feed.result = [replace(model, evidence_only=source.evidence_only) for model in discovered]— stamped the provider-level flag onto every row.is_routable_discovered_modelshort-circuited before any capability check.agent_from_discoveredhard-refused withValueError("evidence-only model cannot become a serving agent").The critical part: per-model ZDR evidence for OpenRouter already existed and was actively computed in this same file —
_merge_openrouter_zdr_metadata/_merge_openrouter_provider_privacyfetch OpenRouter's authoritative/api/v1/endpoints/zdrfeed and per-endpoint provider privacy policy, building a real per-modelzdr_capablematch. But that computed match was thrown away for OpenRouter's own rows — used exclusively to donate ZDR status to other providers' rows that happen to share the same canonical model id (e.g. annvidia_nimrow). OpenRouter never got to use its own evidence for its own serving eligibility.This contradicts
docs/planning/adrs/0032-model-group-cost-aware-discovery.md(accepted, still the standing design for this exact subsystem):The blanket
evidence_only=Truewas added later (952996ec, a bare one-line commit message, no rationale, no accompanying ADR amendment) and never reconciled against ADR 0032. Notably,git show 952996ec~1shows OpenRouter was not blanket-excluded before that commit — the regression flippedevidence_onlyfrom theFalsedefault to a hardcodedTruewith no documented reason.What changed
contextual_orchestrator/model_discovery.py:evidence_only=Truefrom the OpenRouterProviderModelSourcedeclaration — it now leaves it at theFalsedefault like every other provider._apply_discovered_model_evidencenow computesevidence_only/zdr_capableper model for OpenRouter's own rows, reusing the exact same feed-match mechanism (zdr_model_ids, from OpenRouter's/api/v1/endpoints/zdr) already used to donate evidence to other providers:evidence_only=False/zdr_capable=Trueonly when its own model id is present in the fetched ZDR feed.evidence_only=True/zdr_capable=False— fail-closed, never a blanket default independent of that model's own feed coverage.PROVIDER_MODEL_SOURCESdeclaration anddiscover_all_models) that asserted OpenRouter "is never selected as an inference upstream here."tests/test_model_discovery.py:test_default_sources_request_openrouter_full_modality_catalog(source-levelevidence_onlyis nowFalse).zdr_capableexpectation tests that were flipped in lockstep with952996ecto assert the wrong blanket-exclusion behavior (test_discover_all_models_applies_model_zdr_evidence_to_other_sources,test_discover_all_models_does_not_match_a_shared_zdr_model_suffix,test_discover_all_models_rejects_an_ambiguous_zdr_model_suffix) — each now asserts OpenRouter's own exact-feed-match row correctly becomeszdr_capable=True.test_discover_all_models_openrouter_model_absent_from_zdr_feed_stays_evidence_only— a real negative case: an OpenRouter model with no attestation on the feed staysevidence_only=True/unroutable, confirming the fix doesn't turn every OpenRouter row into a serving agent.test_discover_all_models_openrouter_zdr_feed_failure_keeps_every_row_evidence_only— a total feed-fetch failure leaves every OpenRouter rowevidence_only=True, confirming it never silently falls back to the newFalsedefault.CHANGELOG.mdanddocs/product-technical-gap-baseline.md: new entries documenting the fix and reconciling the earlier (now-superseded) "OpenRouter is deliberatelyevidence_only=True... never serves inference" claims recorded in ADR 0041 and an earlier gap-baseline entry. ADR 0041's actual decision (nomodels_dev_provider_idjoin for OpenRouter, since it reports its own real pricing) is unaffected — only the incidental blanket-exclusion claim is superseded.Verification
tests/test_model_discovery.py: 81/81 passed.test_model_discovery_boundaries.py,test_chat_model_capability_isolation.py,test_auto_discovery_server.py,test_provider_bootstrap.py,test_review_gateway.py,test_batch_embeddings.py,test_discovery_bootstrap_selection.py): 129/129 passed.python -m pytest tests -q): 2825 passed, 2 failed. Both failures are pre-existing sandbox environment gaps unrelated to this change (test_privacy_policy_analysis.py::test_pinned_mcp_client_renders_and_closes_camoufox_tab— installedmcppackage lacksClientattribute;test_psychometric_routing.py::test_fast_mlsirm_fit_uses_judge_acceptance_item_for_context_score—fast_mlsirmRust extension not installed in this sandbox). Neither touchesmodel_discovery.pyor anything downstream ofevidence_only/zdr_capable.interrogateon the changed file: 100% (20/20).evidence_only/zdr_capablefor OpenRouter (is_routable_discovered_model,agent_from_discovered,general_free_serving_candidates,select_cheapest_discovered_agent/select_top_n_cheapest_discovered_agents,refresh_price_book,privacy_tags_for_discovered, the CLI report in__main__.py) to confirm fail-closed is preserved: a model with no real ZDR evidence can never become servable through this change, only models with genuine per-model evidence gain a changed outcome.Companion fix (not in this PR)
ContextualWisdomLab/.github'sscripts/ci/contextual_orchestrator_review_launcher.pyhas a separate, independent blanket filter at_routable_discovered_models()(~line 150) that also strips every OpenRouter row before.github's own already-correct per-route ZDR mechanism (zdr_policy.py'sis_zdr_model()/openrouter_endpoints_feed) ever gets to evaluate them. That is being handled separately in.github, not touched here.🤖 Generated with Claude Code
https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
Generated by Claude Code