feat(registry): resolve registry-only gateways without an enum member - #1217
Conversation
|
Warning Review limit reached
Next review available in: 4 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
WalkthroughChangesRegistry-only OpenAI-compatible gateways now resolve through Registry provider resolution
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
fe08c12 to
c52e2bd
Compare
c52e2bd to
cd029fa
Compare
cd029fa to
7deb56b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/providers/test_databricks_provider.py`:
- Line 40: Strengthen the provider enum assertions to require identity with the
expected LLMProvider member: update
tests/unit/providers/test_databricks_provider.py:40 to use
LLMProvider.DATABRICKS, tests/unit/providers/test_lmstudio_provider.py:121 to
use LLMProvider.LMSTUDIO, tests/unit/providers/test_moonshot_provider.py:38 to
use LLMProvider.MOONSHOT, and
tests/unit/providers/test_perplexity_provider.py:38 to use
LLMProvider.PERPLEXITY. Ensure each test uses an identity assertion rather than
string equality.
In `@tests/unit/test_registry.py`:
- Line 122: Resolve the Ruff and typing violations in the affected tests: add
pytest.mark.usefixtures("community_row") to tests that only use community_row
for setup, then remove those unused parameters; update api_function annotations
to Callable[..., object] and replace call_kwargs: dict[str, Any] with an
appropriate concrete value type. Run pre-commit across all files and verify Ruff
lint and strict mypy pass.
- Around line 254-296: Expand the API routing test coverage beyond the current
completion, embedding, moderation, and image_generation cases to include
responses, messages, transcription, speech, rerank, and their async
counterparts. Add registry-only happy-path assertions and unsupported-provider
error cases for every changed wrapper, using async tests and awaiting async
entry points where applicable; update
test_api_entry_points_resolve_registry_only_rows and related parametrizations
without altering the existing enum-provider coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0bd9e0cd-174f-49c8-bc09-40b38cb73331
📒 Files selected for processing (8)
scripts/generate_api_docs.pysrc/any_llm/any_llm.pysrc/any_llm/api.pytests/unit/providers/test_databricks_provider.pytests/unit/providers/test_lmstudio_provider.pytests/unit/providers/test_moonshot_provider.pytests/unit/providers/test_perplexity_provider.pytests/unit/test_registry.py
7deb56b to
7cb7888
Compare
#1197 promised that adding a community gateway means one registry row. It did not, because the row was only wired into the two loader entry points. Four other paths still went through LLMProvider and rejected a name with no enum member: - get_supported_providers() omitted it - get_all_provider_metadata() omitted it, so it never reached the docs table - split_model_provider() raised on "name:model" - api.py's explicit provider= argument raised, via LLMProvider.from_string Adds AnyLLM.resolve_provider_key, which returns an LLMProvider member when one exists and the bare name for registry-only rows. Both forms are already accepted by create() and get_provider_class(). split_model_provider and all 17 provider= call sites in api.py now use it, and its return type widens to str | LLMProvider accordingly. Normalization matches LLMProvider.from_string, so both paths accept the same spellings. get_registry_provider_names() lists rows that have no enum member, which get_supported_providers appends. get_provider_enum's unsupported-provider error now reports the full resolvable set too; it previously listed the enum alone, which diverged from get_supported_providers() as soon as a registry-only row existed and broke test_unsupported_provider_error_attributes. Found by adding a bare row and running the suite, so there is now a regression test for it. Verified end to end: with a single row added and nothing else (no enum member, no package directory, no pyproject extra) the name is listed as supported, resolves through create(), "name:model", and provider=, and tests/unit plus tests/docs stay green. A row with an enum member is unaffected: split_model_provider still returns the enum for those, and the 11 migrated providers keep their members and shims. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7cb7888 to
c7948e0
Compare
## Description Layer 3 of the #1197 stack, and the payoff for the other two. Docs only. #1215 documented the registry-row path honestly: four steps, because a row needed an `LLMProvider` member, a `pyproject.toml` extra plus `all` group entry, and a package `__init__.py` on top of the row itself. That was accurate but embarrassing, since #1197's whole pitch was "one row instead of touching six files". #1217 removed the reasons for the other three steps, so section 2a collapses to the row and the live verification evidence. Verified rather than assumed: I added a single row to `PROVIDER_REGISTRY` and nothing else, then confirmed the name appears in `get_supported_providers()`, resolves through `create()`, `"name:model"`, and `provider=`, reports the community tier, and leaves `tests/unit` plus `tests/docs` green. That check is also what surfaced the `get_provider_enum` bug fixed in #1217. Two notes kept in the text, because both would otherwise make the tree look like it contradicts the instructions: - The registry providers already in the repo carry a package directory and an `LLMProvider` member. Those are compatibility shims from before they were migrated to rows, kept so their deep-import paths and `LLMProvider.<NAME>` references keep working. When to retire them is still an open question on #1197. - A row is not added to the `tests/conftest.py` model maps. Promotion to the verified tier is a maintainer change and takes a repository secret, an `EXPECTED_PROVIDERS` entry, an `LLMProvider` member so the enum-driven test matrix picks the provider up, and conftest entries for the capabilities it supports. ## PR Type - 📚 Documentation ## Relevant issues Part of #1197. Together with #1215 this completes the CONTRIBUTING acceptance-rule item; the issue stays open for shim retirement and the registry-knobs question. ## Checklist - [x] I understand the code I am submitting. - [ ] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [x] Documentation was updated where necessary - [x] I have read and followed the [contribution guidelines](https://github.com/mozilla-ai/any-llm/blob/main/CONTRIBUTING.md) - [x] **AI Usage:** - [ ] No AI was used. - [x] AI was used for drafting/refactoring. - [ ] This is fully AI-generated. No unit tests, since this is documentation only; the behavior it documents is tested in #1217. `tests/docs` only globs `docs/**/*.md`, so nothing in CI executes `CONTRIBUTING.md`, which is why the one-row claim was checked by hand against the real tree. Anchors, punctuation, and pre-commit verified. ## AI Usage Information - AI Model used: Claude Opus 5 - AI Developer Tool used: Claude Code - Any other info you'd like to share: Drafted by Claude through back and forth with @njbrake. The decisions are his; the prose is Claude's. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…1430) ## Description `AnyLLM.get_supported_providers()` lists registry rows that have no declared `LLMProvider` member (`ovhcloud` today), but `LLMProvider(name)` rejects them. Downstream code that builds a provider picker from `get_supported_providers()` and validates the choice with `LLMProvider(...)` offers those providers and then refuses them. That is what happens in Otari today: adding OVHcloud as a hosted provider fails with `'ovhcloud' is not a known provider implementation.` This PR makes `LLMProvider` accept every name `get_supported_providers()` returns, rather than adding an `OVHCLOUD` member: - `LLMProvider._missing_` resolves any `PROVIDER_REGISTRY` row to a member (`LLMProvider("ovhcloud")` gives `<LLMProvider.OVHCLOUD: 'ovhcloud'>`). Members are cached, so identity, hashing, equality with the plain string, pickling and `deepcopy` behave like declared members. A row that is removed from the registry stops resolving. - Iterating `LLMProvider` still lists declared members only. The enum drives the test matrix, and a registry-only row has no package or extra to test, so `get_registry_provider_names()`, `get_supported_providers()` and the matrix are unchanged. - `resolve_provider_key`, `get_provider_enum` and `split_model_provider` now return a member for registry-only rows too, instead of a bare string or an error. `resolve_provider_key` keeps its `str | LLMProvider` signature. - `LLMProvider.from_string`'s error lists every supported provider, not just the enum, matching `get_provider_enum` and `create`. - CONTRIBUTING §2a documents that `LLMProvider("examplegw")` resolves a new row. A new row still needs no declared member, so §2a's policy is unchanged. Rows that have a declared member are unaffected. ## PR Type - 🐛 Bug Fix ## Relevant issues Follow-up to #1217 and #1322. ## Checklist - [x] I understand the code I am submitting. - [x] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [x] Documentation was updated where necessary - [x] I have read and followed the [contribution guidelines](https://github.com/mozilla-ai/any-llm/blob/main/CONTRIBUTING.md) - [ ] **AI Usage:** - [ ] No AI was used. - [x] AI was used for drafting/refactoring. - [ ] This is fully AI-generated. ## AI Usage Information - AI Model used: Claude Opus 5.5 - AI Developer Tool used: Claude Code - Any other info you'd like to share: `uv run pytest tests/unit` passes (3518 passed, 110 skipped), and the new code in `constants.py` and `any_llm.py` is fully covered. `pre-commit run --all-files` is clean apart from the known voyage and watsonx missing-SDK mypy errors that CI resolves by installing those extras. - [ ] I am an AI Agent filling out this form (check box if true) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Registry-only provider names can now be used wherever a provider name is accepted, including through `LLMProvider`. Supported-provider listings and error messages include these providers. * **Documentation** * Clarified how registry-only providers are resolved and how supported-provider listings differ from enum iteration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Description
Layer 1 of the #1197 stack. Makes a registry row resolvable everywhere a provider name is accepted, so adding a community gateway really is one row.
#1201 added the config registry and wired it into the two loader entry points (
_create_providerandget_provider_class). Four other paths still went throughLLMProviderand rejected a name with no enum member:get_supported_providers()get_all_provider_metadata()split_model_provider("name:model")UnsupportedProviderErrorprovider="name"inapi.pyUnsupportedProviderError, viaLLMProvider.from_stringI confirmed all four against an injected row before changing anything, and the last one is the interesting find: the explicit
provider=keyword is the form the docs recommend most, and it failed the same way the string form did.Adds
AnyLLM.resolve_provider_key, which returns anLLMProvidermember where one exists and the bare name for a registry-only row.create()andget_provider_class()already accept either.split_model_providerand all 17provider=call sites inapi.pynow use it, andsplit_model_provider's return type widens totuple[str | LLMProvider, str]. Normalization matchesLLMProvider.from_string(strip plus lowercase) so both paths accept the same spellings.get_registry_provider_names()lists rows without an enum member, whichget_supported_providers()appends.One bug this surfaced
get_provider_enum()raisedUnsupportedProviderErrorwith the enum alone as its supported list. That is fine today, because all 11 registry rows also have enum members, but it diverges fromget_supported_providers()the moment a row without one exists, which broketest_unsupported_provider_error_attributes. I only found it by adding a bare row and running the full suite rather than trusting the design. It now reports the full resolvable set, and there is a regression test.Compatibility
Rows that have an enum member are unaffected:
split_model_providerstill returns the enum for those, and the 11 migrated providers keep their members and import shims. Retiring those shims is a separate open question on #1197.get_supported_providers()gaining registry names is a visible behavior change, and it is the intended one: previously a registry-only gateway was resolvable but invisible.Typing-visible consequence, worth a reviewer's attention. Widening
split_model_providermeans any caller doingsplit_model_provider(...)[0].valuenow fails type checking, because astrhas no.value. It keeps working at runtime for enum-backed providers. Four of our own tests hit this, and the fix is to drop.value:LLMProvideris aStrEnum, so a member compares equal to its own string value. If we would rather not expose that to downstream type checkers, the alternative is to leavesplit_model_providerreturningLLMProviderand add a separate resolver for the string form, at the cost of two functions that differ only in whether they accept community gateways.I found this only after CI caught it: I had run
pre-commit run --files <changed files>, and the four failures were in files I had not touched, so a scoped run could not see them.--all-filesis clean now apart from the pre-existingvoyageandwatsonxmissing-SDK errors that CI resolves by installing those extras.PR Type
Relevant issues
Part of #1197. Closes the
provider:modelrouting gap I filed against it during the #1201 rollout. Does not close the issue.Checklist
Testing: 14 new tests in
tests/unit/test_registry.pycovering resolution of both forms, normalization parity, enum passthrough, metadata and supported-provider listing, the error-list regression, and the api-levelprovider=and"name:model"entry points.tests/unitplustests/docsgreen. The decisive check was adding a single row with no enum member, no package directory, and nopyproject.tomlextra, then confirming the suite stays green and the name routes.CONTRIBUTING.mdis updated in #1219 rather than here, so the doc change lands with the rest of the stack.AI Usage Information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation