fix(discovery): persist shared metadata refresh evidence

[seonghobae]

Summary

• persist secret-free shared metadata refresh evidence for the Models.dev join used by multi-provider discovery
• store one durable external_metadata_refresh_run record alongside provider refresh evidence and expose it through the bootstrap report
• update the product gap baseline with current evidence explaining the customer-visible freshness gap and the targeted regression coverage

Why

orchestrator/free and related free/privacy decisions consume shared Models.dev metadata, but the durable catalog could not distinguish "not free", "no metadata matched", and "metadata fetch failed so the gateway failed closed". This change adds bounded freshness evidence without inventing price or free-tier facts.

Testing

• uv run pytest tests/test_provider_catalog_store.py tests/test_provider_catalog_store_boundaries.py tests/test_provider_catalog_bootstrap.py tests/test_model_discovery.py
  • 158 passed in 11.89s

---

[coderabbitai]

Warning

Review limit reached

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 6a3a4c84-8393-4506-8d08-cbeb22222223

📥 Commits

Reviewing files that changed from the base of the PR and between c594b68 and 6c3d5c7.

📒 Files selected for processing (10)

• contextual_orchestrator/model_discovery.py
• contextual_orchestrator/provider_catalog_bootstrap.py
• contextual_orchestrator/provider_catalog_store.py
• docs/database_design.sql
• docs/product-technical-gap-baseline.md
• docs/provider_catalog_database.sql
• tests/test_provider_catalog_bootstrap.py
• tests/test_provider_catalog_bootstrap_boundaries.py
• tests/test_provider_catalog_store.py
• tests/test_provider_catalog_store_boundaries.py

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

Fresh post-custom-store audit found and fixed two exact-head validation regressions in 169f2df1:

1. The existing CLI boundary tests still patched the removed provider_catalog_bootstrap.discover_all_models symbol. They now patch discover_all_models_with_metadata_evidence and return the required three-value contract with empty metadata evidence, preserving the custom-discovery compatibility path.
2. model_discovery.py referenced Sequence in the existing OpenRouter admission annotation without importing it. Restoring the typing import makes the touched module Ruff-clean (the omission also exists on the current base).

Locked proof:

• RED: provider catalog suite had exactly 2 failures, both AttributeError on the stale fake target.
• GREEN: the two focused CLI boundary tests: 2 passed.
• Expanded model-discovery/provider-catalog suite: 176 passed.
• Ruff: pass.
• Interrogate: pass.
• git diff --check: pass.

The concurrency evidence boundary remains intact: offset capture, external-evidence writes, provider refresh writes, and tail slicing are within the shared _CATALOG_REFRESH_EVIDENCE_LOCK. Custom discovery emits an empty metadata evidence tuple and never calls the optional external-evidence store methods. Persistence failures still propagate through the existing credential rollback path; no failure was suppressed.

[seonghobae]

Cross-PR integration contract: routing identity is provider-neutral model_group only; do not add or preserve a provider-family abstraction. OpenRouter discovery must retain concrete free model IDs, while the aggregate openrouter/free router is not a serving candidate. OpenCode, Noema, and Strix must call contextual-orchestrator. Do not impose fixed wall-clock deadlines on inference, initial ping, readiness/health, provider discovery, or OpenRouter ZDR-list retrieval; use explicit cancellation and evidence-backed transport failure instead. Reconcile this PR with #971 and central .github #1508 before merge.

[seonghobae]

Merge conflict resolved + a latent test breakage found and fixed

Merged current main (4d143601) into this branch, new head 30f64e12.

• contextual_orchestrator/model_discovery.py: real conflict in discover_all_models_with_metadata_evidence's return — this PR's own metadata_refreshes third return value vs. main's independently-added discovery_complete INFO log line. Kept both: log, then return the 3-tuple.
• docs/product-technical-gap-baseline.md: additive, kept both dated entries.
• tests/test_provider_catalog_bootstrap.py: main independently landed a redirect-header-leak security hardening for the discovery list-fetch path (_open_trusted_discovery_request, a custom OpenerDirector replacing the plain urllib.request.urlopen() call) plus a bounded-read cap on the response body (response.read(MAX_DISCOVERY_RESPONSE_BYTES + 1)). Neither change touches this PR's own file (no conflict), but this branch's two new tests (test_default_discovery_reports_models_dev_refresh_success, test_default_discovery_reports_models_dev_refresh_failure_without_fabricating_free) still mocked the old urllib.request.urlopen symbol directly and used a local _Response stub whose .read() didn't accept a size argument — both tests silently regressed to a ProviderBootstrapError/TypeError once merged with main's transport change. tests/test_model_discovery.py had already been updated for this elsewhere; applied the identical fix here (patch _open_trusted_discovery_request alongside urllib.request.urlopen, and give _Response.read() an optional amt parameter).

Verified: tests/test_provider_catalog_store.py, _boundaries, test_provider_catalog_bootstrap.py, _boundaries, test_model_discovery.py, _boundaries, test_discover_models_cli.py — 236 passed. interrogate 100%.

---

Generated by Claude Code

---

Generated by Claude Code

[devin-ai-integration]

Devin Review found 2 new potential issues.

[devin-ai-integration]

🟡 Default discovery breaks custom catalog stores

With a custom store and any Models.dev-backed source, bootstrap_provider_catalog_runtime calls evidence methods absent from previously valid stores. The catalog refresh aborts.

Prompt for agents

Preserve bootstrap_provider_catalog_runtime compatibility with existing custom ProviderCatalogStore implementations when default discovery emits Models.dev evidence. The public catalog_store seam previously required only provider catalog methods, but lines 571-584 now unconditionally require the two new external-metadata methods whenever a Models.dev-backed credential is registered. Add a compatibility strategy that still lets legacy stores complete provider refreshes, while built-in stores continue to persist and report external metadata evidence. Cover a legacy structural store used with default discovery and a Models.dev-backed source.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[seonghobae]

Re-verified as of the current head: still real, but not currently exercised by any built-in store or test double, and not fixed here — deliberately left for @seonghobae's call rather than decided unilaterally (see the fuller reasoning in #984 (comment)). ProviderCatalogStore is a Protocol where every method is required, and this repo's own test doubles already follow "the Protocol is the whole contract, implementors update together" rather than duck-typed partial degradation for earlier protocol extensions. Adding a hasattr-guarded compatibility shim here would be a real, repo-wide policy change (silent degradation vs. fail-loud for legacy ProviderCatalogStore implementors), not a narrow bug fix, so it isn't something to make unilaterally on this pass. Leaving this thread open and unresolved for that decision.

---

Generated by Claude Code

[seonghobae]

Devin's two findings

"Canonical database design omits refresh evidence" — fixed (head a4ca117e). Verified: every existing table in docs/provider_catalog_database.sql (provider_account, provider_model, model_serving_tag, catalog_refresh_run) is also duplicated in docs/database_design.sql, but the new external_metadata_refresh_run table (and its index) was only added to the former. Added both to docs/database_design.sql matching its existing lowercase-DDL style and consolidated index-block convention. tests/test_conventions.py still passes.

"Default discovery breaks custom catalog stores" — verified real but not currently exercised; leaving for your call rather than pushing a fix. Confirmed: bootstrap_provider_catalog_runtime only calls the new store.record_external_metadata_refresh(...)/store.external_metadata_refresh_evidence() when metadata_refreshes is non-empty (default discovery + at least one Models.dev-backed source). A store written against the pre-#984 ProviderCatalogStore Protocol shape would indeed raise AttributeError there. However: ProviderCatalogStore is a Protocol where every method is required, not optional, and this repo's own test doubles (e.g. _StubCatalogStore in tests/test_provider_catalog_bootstrap_boundaries.py) already omit other required methods like record_privacy_assessment_success/privacy_assessments from an earlier protocol extension — i.e. the established convention here has been "the Protocol is the whole contract, implementors update together," not gradual duck-typed degradation. No current test or built-in store hits this gap. Adding a hasattr-guarded compatibility shim is mechanically small, but it's a real change to that convention (silent degradation vs. fail-loud), so I'd rather flag it than decide it unilaterally. Happy to add the shim if you want the Protocol to support partial/legacy implementations going forward.

---

Generated by Claude Code

---

Generated by Claude Code

[seonghobae]

noema-review failure investigation + branch update

Actual noema-review failure: not pure staleness. The job log shows the review model itself failed:

Noema bounded repair transport was exhausted; initial failure: Noema LLM response was not
valid JSON (Expecting property name enclosed in double quotes: line 1 column 1230 (char 1229)).
... repair failure: NoemaRepairDeadlineExceeded: Noema repair exceeded 900-second absolute
wall-clock deadline

i.e. the reviewer model emitted malformed JSON and the bounded repair path timed out before producing a usable verdict — a transient review-model output failure, not a defect in this PR's code. All other checks on the prior head (e30297ea) were green (CodeQL, Semgrep, Trivy, osv-scanner, Scorecard, dependency-review, Full unit and contract suite, Atheris/Hypothesis fuzz, etc.); only noema-review failed. The PR was also mergeable_state: behind against main (stale since 8839081), so a rebase was due regardless.

What I did:

1. Merged current origin/main (212ff437) into this branch — no conflicts, including in docs/product-technical-gap-baseline.md (this PR's own dated section landed cleanly as the top-most section; no duplication, no markers).
2. Verified locally in a fresh venv (Python 3.11.15, hash-pinned requirements.lock):
   • PR's own stated scope: pytest tests/test_provider_catalog_store.py tests/test_provider_catalog_store_boundaries.py tests/test_provider_catalog_bootstrap.py tests/test_model_discovery.py → 184 passed (up from 158 due to tests merged in from main).
   • Tests touched by the merge diff (rater_observation, evaluation_criterion_binding, review_gateway, admin, fuzz property tests) → all passed except one pre-existing bug on main itself (see below).
   • interrogate docstring coverage on contextual_orchestrator/: 100%.
   • Full suite python -m pytest tests -q --ignore=tests/test_psychometric_routing.py (that file needs numpy, lock-pinned only for Python ≥3.12; this venv is 3.11): 3347 passed, 2 skipped, 2 failed in 13m50s. Both failures are pre-existing and not regressions from this merge:
     • tests/test_admin_contract.py::test_model_group_mutations_refresh_audit_events — NameError: name 'json' is not defined; confirmed via git show origin/main:tests/test_admin_contract.py that json is never imported in that file on main itself, unrelated to this PR.
     • tests/test_spend_analytics.py::test_exact_output_without_prompt_usage_is_explicitly_unavailable — a documented pre-existing sandbox-only failure.
3. Pushed the merge commit directly to this branch (no force-push): e30297e..81e707a.

New head: 81e707a8. This should let noema-review retry with a fresh verdict and an up-to-date base.

---

Generated by Claude Code

[seonghobae]

"Full unit and contract suite" failure — same already-fixed admin-contract bug, branch updated

What failed: tests/test_admin_contract.py::test_model_group_mutations_refresh_audit_events with NameError: name 'json' is not defined (run 33659617569).

Root cause: not a defect in this PR's diff (it never touches tests/test_admin_contract.py). This PR's base (212ff437) predates commit 39a4348e (PR #1035), which fixed the exact same missing-import json regression on main that had already been diagnosed on this PR's prior head in an earlier comment on this thread as "pre-existing... unrelated to this PR." Several other PRs based on the same stale main state hit the identical failure.

Fix applied: merged current origin/main (c594b682, includes 39a4348e) into this branch — clean merge, no conflicts. New head: 6c3d5c76.

Verification:

• tests/test_admin_contract.py + this PR's own affected surface (tests/test_provider_catalog_store.py, _boundaries, tests/test_provider_catalog_bootstrap.py, _boundaries, tests/test_model_discovery.py): 205 passed.
• Full suite (python -m pytest -q, matching the CI job's exact invocation): 3394 passed, 2 skipped, 0 failed in 927s (locally substituted the sandbox-blocked fast-mlsirm GitHub-archive dependency with a prebuilt wheel via a [tool.uv.sources] override that was never committed — reverted before push; pyproject.toml/uv.lock are unchanged from the prior head).

The pending Protocol-compatibility thread on ProviderCatalogStore (external metadata evidence methods) was left untouched, as previously agreed — still open for your call.

---

Generated by Claude Code