feat(lfx): let extension bundles register model providers#13916
Merged
Conversation
Model providers were hardcoded across lfx core (MODEL_PROVIDER_METADATA, the class-import registries, LIVE_MODEL_PROVIDERS, and the credential / instantiation / live-discovery dispatch), so a third party could not add a provider without editing core -- adding vLLM (#13910) touched 12 files. Introduce a supported extension point: an extension manifest may declare a `providers[]` block, which the loader merges into the core provider tables in place via a new `provider_registry`. Every existing accessor (the lru_cached variable maps, the shared `model_provider_metadata` reference, the /api/v1/models surface) then sees a registered provider with no further edits. - new lfx/base/models/provider_registry.py: ProviderSpec + register_provider with core-wins precedence, failure isolation, and a clear() test seam; live-discovery and validator callables resolved lazily by dotted path. - manifest: declarative providers[]; allow a provider-only extension (empty bundles); require at least one bundle or provider. - loader: register manifest providers during load_extension (the startup chokepoint), failure-isolated as warnings. - dispatch seams made registry-aware: get_model_providers union, live-model discovery fallback, credential-validation fallback, api-key-optional check. - zero added cost and byte-identical behavior when no provider bundle is installed. Groundwork for shipping providers as standalone bundles (e.g. lfx-vllm, superseding #13910).
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a new ChangesBundle Provider Registry Support
Sequence Diagram(s)sequenceDiagram
participant Extension as Extension Manifest
participant Loader as load_extension
participant Registry as provider_registry
participant ModelUtils as model_utils / credentials
Extension->>Loader: providers[] declared in manifest
Loader->>Registry: register_provider(spec) per provider
Registry->>Registry: validate, merge metadata/class/mapping tables
Registry-->>Loader: success or provider-invalid warning
Loader-->>Loader: continue or return early (provider-only)
ModelUtils->>Registry: live_discovery_for(provider) / validator_for(provider)
Registry-->>ModelUtils: callable or None
ModelUtils->>ModelUtils: invoke callable, return result or []
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 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 |
Contributor
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
enhancement
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.11.0 #13916 +/- ##
==================================================
- Coverage 59.58% 58.78% -0.80%
==================================================
Files 2349 2351 +2
Lines 224820 225218 +398
Branches 33523 33581 +58
==================================================
- Hits 133954 132403 -1551
- Misses 89347 91268 +1921
- Partials 1519 1547 +28
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@src/lfx/src/lfx/base/models/model_utils.py`:
- Around line 718-724: The live discovery path in model_utils.py returns
extension output from discovery(user_id, model_type) directly, so invalid values
like None or non-list results can leak past the list[dict] contract. Update the
live-discovery handling in the function that calls live_discovery_for to
validate the returned value and only accept a list; if the callable returns
anything else, treat it like a failed discovery by logging the debug message and
returning an empty list, just as the existing exception path does.
In `@src/lfx/src/lfx/base/models/provider_registry.py`:
- Around line 254-261: _resolve_callable() should fail softly for malformed or
non-callable extension references instead of letting import-time exceptions
break discovery. Update the provider loading/validation path around
_resolve_callable and the caller in the provider registry to catch broader
import/lookup failures such as SyntaxError and other runtime exceptions from
importlib.import_module, then verify the resolved attribute is actually callable
before accepting it. If resolution fails, log/record it and return None so bad
extensions stay isolated and do not stop registry validation.
- Around line 154-156: Validate provider metadata keys in ProviderRegistry
before any registration writes happen: in the spec validation path that checks
spec.model_class_name and in the registration block that mutates
_MODEL_CLASS_IMPORTS and EMBEDDING_PARAM_MAPPINGS, reject specs whose
lazy-import class key is missing, already claimed, or not backed by either an
existing import entry or spec.model_class. Also guard embedding_param_key
against overwriting an existing mapping, including core mappings, so clear()
cannot remove a preexisting entry; perform these checks under the registry lock
before updating the global tables.
In `@src/lfx/src/lfx/base/models/unified_models/credentials.py`:
- Around line 421-425: The provider validation path in
is_registered/validator_for currently lets bundle validator exceptions escape as
non-ValueError, breaking the function’s expected user-facing contract. Wrap the
bundle_validator(provider, variables, validation_model) call in the
registered-provider branch so any transport/client or validator failure is
caught and normalized into a ValueError, while preserving the existing early
return behavior for successful validation.
In `@src/lfx/src/lfx/base/models/unified_models/instantiation.py`:
- Around line 101-107: `get_llm` is still forwarding an `api_key` kwarg even for
providers marked optional via `is_api_key_optional`, unlike `get_embeddings`.
Update the LLM instantiation path in `instantiation.py` so optional providers
omit the API-key argument entirely when no key is resolved, and keep the
existing guard for required providers. Add a regression test around `get_llm`
that instantiates an `api_key_required=False` provider and verifies no `api_key`
is passed.
In `@src/lfx/src/lfx/extension/loader/_orchestrator.py`:
- Around line 279-300: The provider registration loop in the orchestrator is
ignoring the False result from register_provider(), so manifest providers that
collide with built-ins or already-registered extensions are silently dropped.
Update the loader logic around manifest.providers and register_provider() to
detect a failed registration, record that provider as skipped in the returned
LoadResult (using the existing result structure in the loader/orchestrator
flow), and surface enough context to identify the conflicting provider name.
In `@src/lfx/tests/unit/extension/test_provider_extension.py`:
- Around line 112-129: The current test name implies it verifies provider
registration failures are downgraded to warnings, but removing
metadata.mapping.model_class causes ExtensionManifest.model_validate() to fail
first, so it only checks manifest-invalid. Update
test_invalid_provider_is_warning_not_fatal in test_provider_extension.py to
either rename it to reflect the manifest validation failure or, preferably, add
a schema-valid manifest that reaches _register_manifest_providers() and triggers
the provider-invalid path in _orchestrator, then assert result.warnings and that
provider_registry remains unregistered.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11a136e7-09fe-4b81-aed7-c4c67735f0e7
📒 Files selected for processing (11)
src/lfx/src/lfx/base/models/model_utils.pysrc/lfx/src/lfx/base/models/provider_registry.pysrc/lfx/src/lfx/base/models/unified_models/credentials.pysrc/lfx/src/lfx/base/models/unified_models/instantiation.pysrc/lfx/src/lfx/base/models/unified_models/provider_queries.pysrc/lfx/src/lfx/extension/loader/_orchestrator.pysrc/lfx/src/lfx/extension/manifest.pysrc/lfx/tests/unit/base/models/test_provider_registry.pysrc/lfx/tests/unit/extension/test_manifest.pysrc/lfx/tests/unit/extension/test_provider_extension.pysrc/lfx/tests/unit/extension/test_schema.py
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
get_llm/get_embeddings resolved base_url (and attribution headers) only via hardcoded per-provider branches, so a bundle-contributed OpenAI-compatible provider could not point its client at a custom endpoint -- it would silently hit the default API. Add a generic, is_registered-gated step that applies each non-secret metadata variable to its declared langchain_param (base_url localhost-rewritten) or HTTP header, mirroring the core branches. Core providers keep their explicit branches, so behavior is unchanged. Completes the provider-bundle seam so an OpenAI-compatible provider (e.g. vLLM) works fully from a bundle with zero core edits.
github-actions
Bot
added
enhancement
…viders langchain_openai's ChatOpenAI / OpenAIEmbeddings raise when constructed with api_key=None, so an OpenAI-compatible provider that declared api_key_required=False (e.g. a local vLLM server without auth) would fail to instantiate even though the model system correctly skips the "API key required" error. Pass a non-empty "EMPTY" placeholder for such providers so the client constructs. Only applies to registered (bundle) providers that opted out of API keys; core providers are unaffected.
github-actions
Bot
added
enhancement
…oviders get_embeddings only passed an API key when the provider's param_mapping declared an explicit "api_key" slot. Bundle providers can omit that slot, so apply the resolved key (or the api-key-optional placeholder) under the OpenAI-compatible "api_key" kwarg for registered providers -- mirroring how get_llm already resolves the key. Lets an OpenAI-compatible embedding provider (e.g. vLLM) work from a bundle without that slot.
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
…13910 (#13919) feat(bundles): add vLLM model-provider bundle (lfx-vllm) Ships vLLM as a standalone Langflow Extension Bundle built on the provider-registry seam: its extension.json declares a providers[] entry that registers a vLLM model provider into the unified model system, with zero core edits. vLLM is OpenAI-compatible, so the provider reuses ChatOpenAI / OpenAIEmbeddings and discovers served models live from /v1/models (SSRF-guarded); the same model is offered for both Language Model and Embedding Model use, and no API key is required for local servers. Inherits the original vLLM provider from #13910 by Yash Pareek (@pareek-ml), reconstructed as a bundle so it no longer edits the 12 core files that PR did. Co-authored-by: Yash Pareek <yash.pareek@usi.ch>
github-actions
Bot
added
enhancement
- model_utils: normalize non-list live-discovery returns to [] (list[dict] contract) - provider_registry: validate lazy-import/embedding keys before mutating any global table (reject unknown or conflicting model/embedding classes and embedding_param_key collisions, so clear() can't drop a core mapping); _resolve_callable verifies the target is callable; callable resolution now catches broad import-time failures (SyntaxError, side effects) and degrades to None - credentials: normalize non-ValueError bundle-validator failures to ValueError so they can't escape as an unhandled 500 - loader: surface skipped (name-collision) provider registrations as a typed provider-skipped warning instead of returning silent success - errors: register provider-invalid / provider-skipped codes + format templates - tests: cover the register-time warning-isolation and collision paths, unknown/conflicting keys, non-callable and non-list discovery, and validator normalization
Manifest providers[] (model-provider registration) and the optional 0-or-1 bundles[] (provider-only extensions) touch in-scope BUNDLE_API files (manifest.py, _orchestrator.py, errors.py); record the additive contract change + the new provider-invalid / provider-skipped codes so the changelog gate passes. No BUNDLE_API_VERSION bump (all additive).
github-actions
Bot
added
enhancement
Addresses two review findings on the provider seam: - [P1] get_model_provider_variable_mapping() fell back to the FIRST provider variable when no *required* secret existed, so a provider whose API key is an optional secret (e.g. vLLM's VLLM_API_KEY) mapped to its required non-secret base URL. get_api_key_for_provider then resolved and forwarded the endpoint as the bearer token, skipping the "EMPTY" placeholder. Now prefer a required secret, then any secret, then the first variable. Covered by a test that exercises the real resolver (no patched stub). - [P2] discover_installed_extensions / discover_seed_extensions indexed manifest.bundles[0], crashing with IndexError on a valid provider-only manifest (bundles=[]) and breaking `lfx extension list` / registry discovery. Guard the empty-bundle case; DiscoveredExtension.bundle_name and registry.Extension.bundle_name are now str | None, and the list CLI renders a dash for provider-only extensions. BUNDLE_API.md changelog updated for the discovery/registry surface nullability.
github-actions
Bot
added
enhancement
This was referenced Jun 30, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Model providers are hardcoded across lfx core (
MODEL_PROVIDER_METADATA, the class-import registries,LIVE_MODEL_PROVIDERS, and the credential / instantiation / live-discovery dispatch), so a third party cannot add a provider without editing core — adding vLLM (#13910) touched 12 files. This PR introduces a supported extension point so an extension bundle can register a model provider declaratively, with zero core edits and byte-identical behavior when no provider bundle is installed.This is PR 1 of 2:
lfx-vllm, a standalone provider bundle that rebuilds feat: add vLLM as a first-class Model Provider with dynamic model discovery #13910 on this seam, attributed to the original author — superseding feat: add vLLM as a first-class Model Provider with dynamic model discovery #13910.How
lfx/base/models/provider_registry.py(new):ProviderSpec+register_provider, which merges a provider into the core tables in place. Every existing accessor — the@lru_cached variable maps, the sharedmodel_provider_metadatareference, the/api/v1/modelssurface — then sees the provider with no edits. Core providers win on name collision; live-discovery and validator callables resolve lazily by dotted path;clear()is a test seam.extension.json): new declarativeproviders[]block; a provider-only extension may ship no component bundle (emptybundles); an extension must declare at least one bundle or provider.load_extensionregisters manifest providers at the startup chokepoint, failure-isolated (a bad provider is a warning, never a load abort).get_model_providersunion, live-model discovery fallback, credential-validation fallback, api-key-optional check.Author contract
A bundle declares, per provider:
name+metadata(mirrors aMODEL_PROVIDER_METADATAvalue), an optional lazymodel_class/embedding,api_key_required,live/conditional_live, and dotted-pathlive_discovery/validatorcallables.Back-compat & safety
_COREtables keep today's exact values; the merge is identity when nothing is registered → no behavior change, no new imports.Tests
test_provider_registry.py— register / precedence /clear(), api-key-optional, live-discovery + validator dispatch, embedding wiring, baseline restoration, invalid specs.test_provider_extension.py— end-to-end through the real loader (a provider-only extension appears inget_model_providers, live discovery dispatches) plus manifest-schema rules.tests/unit/extension+tests/unit/base/modelssuites pass (721) under the new contract.Refs #13910
Summary by CodeRabbit
New Features
Bug Fixes