feat: add OpenRouter provider support#71
Conversation
mossishahi
force-pushed
the
feat/openrouter-provider-support
branch
2 times, most recently
from
221ca47 to
dcc6bf6
Compare
Marius1311
left a comment
Marius1311
left a comment
There was a problem hiding this comment.
Looks great, thanks for the contribution! Just a few changes.
| assert provider_with_key._api_key == "test-key" | ||
|
|
||
|
|
||
| class TestOpenRouterProvider: |
There was a problem hiding this comment.
For this to work, I guess we would have to store an OpenRouter API key as a secret in the repo? I think that's how I do it for the other providers.
There was a problem hiding this comment.
@Marius1311 Yes, exactly — the real-API tests are guarded by @pytest.mark.skipif(not os.getenv("OPENROUTER_API_KEY"), ...) and will be skipped unless the secret is configured. The unit tests (initialization, repr, factory pattern, error handling) work without any key. If you'd like to enable the real-API tests in CI, an OPENROUTER_API_KEY secret would need to be added to the repo (same pattern as the existing OPENAI_API_KEY, GEMINI_API_KEY, and ANTHROPIC_API_KEY secrets). Happy to leave that to your discretion.
There was a problem hiding this comment.
I think we should set that up - I have it for the other providers, making sure that CI only queries there very cheap models, so that one scheduled CI run costs about 0.01 USD or so.
There was a problem hiding this comment.
@Marius1311 Sounds good! The default OpenRouter model is already set to openai/gpt-4o-mini (same as the OpenAI default), which is one of the cheapest options available. The real-API tests use that default with max_completion_tokens=50, so cost per CI run should be well under $0.01. Happy to help set up the secret once this PR is merged, or you can add OPENROUTER_API_KEY to the repo secrets at your convenience.
Add OpenRouter as a first-class LLM provider alongside OpenAI, Gemini, and Anthropic. OpenRouter aggregates models from many upstream providers behind a single API key, enabling access to a wide range of models (e.g. openai/gpt-4o-mini, anthropic/claude-3.5-sonnet, deepseek/deepseek-v3.2). Changes: - Register 'openrouter' in supported_providers and default_models. - Add OpenRouterProvider (extends OpenAIProvider with custom base_url). - Add OPENROUTER_API_KEY to APIKeyManager config. - Auto-detect OpenRouter from slash-style model IDs (e.g. provider/model). - Accept any listed OpenRouter model in test_query (catalog-based check). - Add structured-output JSON fallback for models that don't support OpenAI's .parse() endpoint (with text-repair recovery path). - Fix BaseAnnotator.query_llm signature to accept agent_description kwarg. - Update README with OpenRouter setup and usage documentation. - Add tutorial notebook: 110_openrouter_sample_annotation.ipynb. - Extend tests for providers, API keys, model detection, and integration. Made-with: Cursor
mossishahi
force-pushed
the
feat/openrouter-provider-support
branch
from
02b5095 to
a6a219f
Compare
for more information, see https://pre-commit.ci
|
@mossishahi , are tests passing for you locally? I think it's because the PR is from a fork, where the API keys aren't set up. |
@Marius1311 Yes, confirmed locally — the same 13 tests fail with ValueError: No API keys found both with and without my changes. All 13 are test_detect_provider_from_model parametrizations + test_provider_auto_detection, which require at least one API key to be set. The 2 new OpenRouter parametrizations fail identically to the 10 pre-existing ones. The remaining 38 tests pass, 7 are skipped (guarded by skipif). This is purely a missing-secrets issue in the fork PR context — once merged and run from the upstream CI with secrets, all tests should pass. |
Marius1311
left a comment
Marius1311
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @mossishahi, and welcome to the project! The OpenRouter integration is well-scoped and modeling it as a subclass of OpenAIProvider is the right call since OpenRouter exposes an OpenAI-compatible API. Tests and README are updated appropriately, and I ran the full test suite locally on this branch — all 193 tests pass (2 OpenRouter real-LLM tests skip without OPENROUTER_API_KEY). Docs also build cleanly.
One blocking concern on the code side, one blocking concern on the tutorial, and a few smaller items — all detailed inline. The main code ask is around the new JSON-fallback and text-repair path in OpenAIProvider, which touches a critical invariant we track in AGENTS.md ("Never parse free-form LLM text in production paths").
A quick note on CI: the 4 failing hatch-test jobs are all failing at llm_interface.py:50 with No API keys found, because repository secrets are not exposed to PRs from forks — not your fault. A maintainer will need to re-run after merge-readiness. (Separately, this surfaces a pre-existing latent bug where LLMInterface(_skip_validation=True) still raises when both provider and model are None; out of scope here.)
Asks, in priority order
- [blocking] Make the JSON-fallback + text-repair path opt-in — default off for
OpenAIProvider, default on forOpenRouterProvider. See inline on_providers.py. - [blocking] Rework the new tutorial to match the style of the existing tutorials (env-var keys, real downloadable data). See inline on the notebook.
- Use
response_format.model_validate_json(...)in the JSON-candidate parse step instead ofresponse_format(**json.loads(...)). - Narrow the bare
except Exceptioninqueryso we don't silently routeKeyboardInterrupt-adjacent or import errors into the fallback. - Strengthen the data-privacy note in the README for OpenRouter.
- Document the weaker
test_queryguarantee for OpenRouter.
Happy to discuss structure before you dig in, especially on #1. Thanks again!
| provider = "gemini" | ||
| elif any(claude_name in model_lower for claude_name in ["claude", "anthropic"]): | ||
| provider = "anthropic" | ||
| elif "/" in model and not model_lower.startswith("models/"): |
There was a problem hiding this comment.
Nit / follow-up: this "/" in model and not model_lower.startswith("models/") rule is duplicated in llm_interface.py:126. Worth pulling into a small shared helper in a follow-up PR — not blocking here.
There was a problem hiding this comment.
Folded into this PR as bf52712 — extracted a shared detect_provider_from_model free function in _api_keys.py backed by PROVIDER_CONFIG[*].model_keywords. Both validate_model_access and LLMInterface._detect_provider_from_model now delegate to it; the unified keyword set is the union of the two prior lists.
When both OPENROUTER_API_KEY and OPENAI_API_KEY are set in the environment, ``OpenRouterProvider()`` previously inherited the OpenAI key via the shared SDK client, producing 401 'Missing Authentication header' errors against ``https://openrouter.ai/api/v1``. Resolve the OpenRouter key from env in ``__init__`` so the OpenAI SDK's default lookup never gets a chance to pick up the wrong key. Manual ``api_key=`` overrides still win. The two ``_api_key is None`` assertions in the init tests are dropped -- they encoded the previous (broken) behaviour.
``LLMInterface(_skip_validation=True)`` previously still raised
``ValueError("No API keys found")`` when both ``provider`` and ``model``
were ``None`` and the environment had no provider keys. This blocked the
fork-PR CI runs noted in the PR review and any caller relying on the
validation skip.
Skip the env-keys probe when ``_skip_validation`` is set or a manual
``api_key`` is supplied; default to ``"openai"`` and its default model.
The slash-detection rule (``"/" in model and not model_lower.startswith("models/")``)
was duplicated in ``APIKeyManager.validate_model_access`` and
``LLMInterface._detect_provider_from_model``, with subtly diverging keyword
lists. Pull both into a single ``detect_provider_from_model`` free function
in ``_api_keys.py`` backed by ``PROVIDER_CONFIG[*].model_keywords``. The
unified keyword set is the union of the two prior lists -- strictly more
permissive than either, so no callers regress.
Addresses follow-up nit on _api_keys.py:194.
``LLMInterface.test_query`` had a special branch for OpenRouter that bypassed ``query_llm`` and only checked catalog membership, on the assumption that structured outputs were too unreliable on OpenRouter to test live. With the upcoming tiered fallback chain in OpenAIProvider (``extra_body`` json_schema → plain ``json_object`` → optional text-repair), the same structured-output round-trip succeeds for OpenRouter slugs too. Drop the special branch -- ``test_query`` now exercises the real path end-to-end for every provider. This also removes the asymmetry that was forcing skip-guards in the parametrized ``test_query_*`` unit tests. Addresses review thread on llm_interface.py:200 (weaker test_query guarantee for OpenRouter): the right fix is to make the guarantee uniform, not to document the gap.
The PR's ``OpenAIProvider`` JSON fallback applied a free-form-text recovery
pipeline (``_query_with_json_fallback`` + ``_repair_text_to_json``) to *all*
callers via a bare ``except Exception``. This conflicts with the project
invariant in AGENTS.md ("Never parse free-form LLM text in production paths"):
text-repair hides schema drift, doubles token spend on silent failure, and
the bare ``except`` rerouted unrelated errors (KeyboardInterrupt-adjacent,
import errors, etc.) into the recovery path.
Three changes, all driven by the review:
1. Gate text-repair behind ``enable_text_repair: bool = False`` on
``OpenAIProvider``; ``OpenRouterProvider`` opts in (``True``) where
upstream-model variability legitimately needs a last-resort recovery.
2. Reshape ``_query_with_json_fallback`` into a tiered chain:
Tier 1: ``extra_body={"response_format": {"type": "json_schema", ...}}``
-- the canonical OpenRouter structured-output path (per
OpenRouter docs and LiteLLM #11652). Strongest signal short
of ``.parse()``; many upstream models honour it even when
``.parse()`` doesn't work end-to-end.
Tier 2: plain ``json_object`` mode with the schema interpolated into
the prompt (existing behaviour, retained as a backstop).
``model_validate_json`` is now used consistently across both
the strict and ``_extract_json_candidate`` paths.
Tier 3: ``_repair_text_to_json`` -- gated on the new flag.
Adding tier 1 also means OpenRouter rarely reaches text-repair in
practice -- weak slugs go through the schema-aware path first.
3. Narrow the bare ``except Exception`` in ``query()`` to
``except (ValueError, TypeError)``; re-raise unconditionally when
``enable_text_repair`` is off. The ``openai.OpenAIError`` branch
stays as the legitimate fallback for native OpenAI users.
Bumps two recovery-path log statements from ``debug`` to ``warning`` so the
fallback is visible when it engages, and adds a TODO next to
``_extract_json_candidate`` flagging its ``find('{')/rfind('}')`` brittleness.
Adds:
- 5 unit tests in ``TestJSONFallback`` covering flag wiring, chain
ordering, the gate, and the local-error re-raise.
- 1 real-LLM test (``anthropic/claude-haiku-4-5`` via OpenRouter) that
exercises the new ``extra_body`` tier end-to-end.
Addresses review threads on _providers.py:202 (gate text-repair),
_providers.py:276 (model_validate_json consistency + _extract_json_candidate
TODO), and the bare-except concern.
OpenRouter forwards requests to a chosen upstream provider under OpenRouter's own ToS, so the effective attack surface is wider than for the three native providers. Spell out the forwarding behaviour, point at both privacy policies, and flag that some tiers may log prompts by default unless the user configures otherwise via account settings. Addresses review thread on README.md:125.
The tutorial added by the PR required users to paste an API key inline and supply their own ``ADATA_PATH``, so it could not run end-to-end out of the box. Reworking it to match the heart-atlas style (env-var key + figshare-backed dataset) is better folded into an existing tutorial demonstrating ``provider="openrouter"``, which is a follow-up. The ``:glob:`` directive in ``docs/notebooks/tutorials/index.rst`` removes the listing automatically -- no other doc references. Addresses review thread on the tutorial notebook.
|
Pushed 7 commits addressing the review (replies inline above). Two of them are out-of-band fixes worth flagging:
Verified locally:
|
…in env ``test_provider_auto_detection`` constructs ``BaseAnnotator()`` without ``_skip_validation``, so it really does need at least one provider key in env to exercise the auto-select branch. On fork-PR CI runs (where repo secrets are not exposed) it crashes with "No API keys found". Mark it ``skipif`` when none of the four provider keys are available, matching the pattern used for ``real_llm_query`` tests. Locally with keys configured, the test still runs and asserts auto-detection picked a provider.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
===========================================
- Coverage 79.95% 46.61% -33.34%
===========================================
Files 15 15
Lines 1237 1375 +138
===========================================
- Hits 989 641 -348
- Misses 248 734 +486
🚀 New features to boost your workflow:
|
2 participants
Add OpenRouter as a first-class LLM provider alongside OpenAI, Gemini, and Anthropic. OpenRouter aggregates models from many upstream providers behind a single API key, enabling access to a wide range of models (e.g. openai/gpt-4o-mini, anthropic/claude-3.5-sonnet, deepseek/deepseek-v3.2).
Changes: