feat(bundles): add lfx-azure-ai-foundry provider bundle#13925
Open
pareek-ml wants to merge 9 commits into
Open
feat(bundles): add lfx-azure-ai-foundry provider bundle#13925pareek-ml wants to merge 9 commits into
pareek-ml wants to merge 9 commits into
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds a new ChangesAzure AI Foundry bundle and static model seed support
Sequence Diagram(s)sequenceDiagram
participant Loader as Extension Loader
participant Orchestrator as _orchestrator
participant Registry as provider_registry
participant Validator as validate_azure_ai_foundry_credentials
participant ChatModel as AzureAIOpenAIApiChatModel
Loader->>Orchestrator: load lfx-azure-ai-foundry manifest
Orchestrator->>Registry: register_provider(spec with models seed)
Registry->>Registry: append seeds to _STATIC_MODELS_DETAILED
Loader->>Validator: validate_azure_ai_foundry_credentials(provider, variables, model_name)
Validator->>Validator: check API key, endpoint, model_name
Validator->>ChatModel: construct and invoke("test")
ChatModel-->>Validator: success or exception
Validator-->>Loader: pass or raise ValueError
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 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 |
github-actions
Bot
added
enhancement
![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: 2
🧹 Nitpick comments (1)
src/lfx/src/lfx/extension/manifest.py (1)
330-337: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd structural validation for
models[]entries, mirroring_metadata_has_model_class.
modelsis accepted as an arbitrarylist[dict[str, Any]]with no check that each entry has aname, or that an optionalproviderkey (if present) matchesself.name. A malformed bundle manifest (e.g., a model entry missingname) would fail much later/less clearly downstream when consumed byget_models_detailed(), rather than at manifest validation time.♻️ Proposed validator
validator: StrictStr | None = Field( default=None, min_length=1, description="Dotted-path callable 'module:attr' validating credentials: (provider, variables, model) -> None.", ) models: list[dict[str, Any]] = Field( default_factory=list, description=( "Optional static seed model catalog injected into _STATIC_MODELS_DETAILED. " "Each entry is a ModelMetadata dict (same shape as create_model_metadata produces). " "Use for providers whose model list is fixed (e.g. deployment-specific clouds)." ), ) `@field_validator`("metadata") `@classmethod` def _metadata_has_model_class(cls, value: dict[str, Any]) -> dict[str, Any]: mapping = value.get("mapping") if isinstance(value, dict) else None if not isinstance(mapping, dict) or not mapping.get("model_class"): msg = "provider.metadata must include a 'mapping' object with a non-empty 'model_class'" raise ValueError(msg) return value + `@model_validator`(mode="after") + def _models_are_well_formed(self) -> ProviderManifestEntry: + for entry in self.models: + if not entry.get("name"): + msg = f"provider {self.name!r} has a models[] entry without a non-empty 'name'" + raise ValueError(msg) + provider = entry.get("provider") + if provider is not None and provider != self.name: + msg = f"provider {self.name!r} has a models[] entry with mismatched provider {provider!r}" + raise ValueError(msg) + return self + `@model_validator`(mode="after") def _live_mutually_exclusive(self) -> ProviderManifestEntry:🤖 Prompt for 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. In `@src/lfx/src/lfx/extension/manifest.py` around lines 330 - 337, Add structural validation for the manifest’s models list so malformed entries are rejected during validation, not later in get_models_detailed(). Update the manifest model class in manifest.py to add a validator for models that mirrors _metadata_has_model_class: each item must be a dict containing a name field, and if provider is present it must match self.name. Use the existing manifest/model validation patterns and keep the check close to the models field definition or the relevant Pydantic validator method.
🤖 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/bundles/azure-ai-foundry/src/lfx_azure_ai_foundry/validator.py`:
- Around line 32-45: The Azure AI Foundry validator only wraps llm.invoke() in a
try/except, so failures during AzureAIOpenAIApiChatModel construction can escape
as raw exceptions. Move the model instantiation into the same guarded block in
validate_azure_ai_foundry_credentials so any setup, pydantic, or
dependency-related error is logged and re-raised as the documented ValueError,
using the existing msg/raise pattern.
- Around line 19-25: Update the docstring in validator.py for the Azure AI
Foundry validator to match the actual behavior of the validation function: the
current text on the validator callable overstates that it raises ValueError for
missing required fields, but the implementation in the validator function
returns silently in that case. Adjust the wording around the validator function
name and its contract so it only claims ValueError is raised for Azure endpoint
rejection or other actual validation failures, not for missing fields.
---
Nitpick comments:
In `@src/lfx/src/lfx/extension/manifest.py`:
- Around line 330-337: Add structural validation for the manifest’s models list
so malformed entries are rejected during validation, not later in
get_models_detailed(). Update the manifest model class in manifest.py to add a
validator for models that mirrors _metadata_has_model_class: each item must be a
dict containing a name field, and if provider is present it must match
self.name. Use the existing manifest/model validation patterns and keep the
check close to the models field definition or the relevant Pydantic validator
method.
🪄 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: f132c878-85e0-4e69-a56f-1bd74d4772f4
📒 Files selected for processing (9)
src/bundles/azure-ai-foundry/pyproject.tomlsrc/bundles/azure-ai-foundry/src/lfx_azure_ai_foundry/__init__.pysrc/bundles/azure-ai-foundry/src/lfx_azure_ai_foundry/extension.jsonsrc/bundles/azure-ai-foundry/src/lfx_azure_ai_foundry/validator.pysrc/bundles/azure-ai-foundry/tests/conftest.pysrc/bundles/azure-ai-foundry/tests/test_azure_ai_foundry_bundle.pysrc/lfx/src/lfx/base/models/provider_registry.pysrc/lfx/src/lfx/extension/loader/_orchestrator.pysrc/lfx/src/lfx/extension/manifest.py
Comment on lines
+19
to
+25
| """Validate Azure AI Foundry credentials by constructing and invoking the chat model. | ||
|
|
||
| Raises ``ValueError`` with an actionable message when required fields are | ||
| missing or when the Azure endpoint rejects the credentials. Returns silently | ||
| on success. ``provider`` is part of the registry validator contract but is | ||
| not used here since this callable is only registered for Azure AI Foundry. | ||
| """ |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Docstring overstates the error-raising contract.
The docstring states the function raises ValueError "when required fields are missing," but the implementation (Line 29-30) silently returns on missing fields instead of raising. This mismatch could mislead callers/maintainers about the validator's guarantees — a misconfigured provider (e.g. missing endpoint) will silently pass validation rather than surfacing an actionable error.
📝 Proposed docstring fix
"""Validate Azure AI Foundry credentials by constructing and invoking the chat model.
- Raises ``ValueError`` with an actionable message when required fields are
- missing or when the Azure endpoint rejects the credentials. Returns silently
- on success. ``provider`` is part of the registry validator contract but is
- not used here since this callable is only registered for Azure AI Foundry.
+ Returns silently when required fields (API key, endpoint, model name) are
+ not yet configured, deferring validation until all are present. Raises
+ ``ValueError`` with an actionable message when the Azure endpoint rejects
+ the credentials. ``provider`` is part of the registry validator contract
+ but is not used here since this callable is only registered for Azure AI
+ Foundry.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| """Validate Azure AI Foundry credentials by constructing and invoking the chat model. | |
| Raises ``ValueError`` with an actionable message when required fields are | |
| missing or when the Azure endpoint rejects the credentials. Returns silently | |
| on success. ``provider`` is part of the registry validator contract but is | |
| not used here since this callable is only registered for Azure AI Foundry. | |
| """ | |
| """Validate Azure AI Foundry credentials by constructing and invoking the chat model. | |
| Returns silently when required fields (API key, endpoint, model name) are | |
| not yet configured, deferring validation until all are present. Raises | |
| ``ValueError`` with an actionable message when the Azure endpoint rejects | |
| the credentials. ``provider`` is part of the registry validator contract | |
| but is not used here since this callable is only registered for Azure AI | |
| Foundry. | |
| """ |
🤖 Prompt for 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.
In `@src/bundles/azure-ai-foundry/src/lfx_azure_ai_foundry/validator.py` around
lines 19 - 25, Update the docstring in validator.py for the Azure AI Foundry
validator to match the actual behavior of the validation function: the current
text on the validator callable overstates that it raises ValueError for missing
required fields, but the implementation in the validator function returns
silently in that case. Adjust the wording around the validator function name and
its contract so it only claims ValueError is raised for Azure endpoint rejection
or other actual validation failures, not for missing fields.
Comment on lines
+32
to
+45
| from langchain_azure_ai.chat_models import AzureAIOpenAIApiChatModel | ||
|
|
||
| llm = AzureAIOpenAIApiChatModel( | ||
| credential=api_key, | ||
| endpoint=endpoint, | ||
| model=model_name, | ||
| max_tokens=1, | ||
| ) | ||
| try: | ||
| llm.invoke("test") | ||
| except Exception as exc: | ||
| msg = f"Azure AI Foundry credential validation failed: {exc}" | ||
| logger.error(msg) | ||
| raise ValueError(msg) from exc |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Model construction failures aren't wrapped in ValueError.
The try/except only wraps llm.invoke() (Line 40-45); construction of AzureAIOpenAIApiChatModel (Line 34-39) can raise (e.g. pydantic validation errors, missing dependency attrs) and would propagate as a raw, undocumented exception type instead of the actionable ValueError the docstring promises.
🛡️ Proposed fix to also guard construction
- llm = AzureAIOpenAIApiChatModel(
- credential=api_key,
- endpoint=endpoint,
- model=model_name,
- max_tokens=1,
- )
try:
+ llm = AzureAIOpenAIApiChatModel(
+ credential=api_key,
+ endpoint=endpoint,
+ model=model_name,
+ max_tokens=1,
+ )
llm.invoke("test")
except Exception as exc:
msg = f"Azure AI Foundry credential validation failed: {exc}"
logger.error(msg)
raise ValueError(msg) from exc📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from langchain_azure_ai.chat_models import AzureAIOpenAIApiChatModel | |
| llm = AzureAIOpenAIApiChatModel( | |
| credential=api_key, | |
| endpoint=endpoint, | |
| model=model_name, | |
| max_tokens=1, | |
| ) | |
| try: | |
| llm.invoke("test") | |
| except Exception as exc: | |
| msg = f"Azure AI Foundry credential validation failed: {exc}" | |
| logger.error(msg) | |
| raise ValueError(msg) from exc | |
| try: | |
| llm = AzureAIOpenAIApiChatModel( | |
| credential=api_key, | |
| endpoint=endpoint, | |
| model=model_name, | |
| max_tokens=1, | |
| ) | |
| llm.invoke("test") | |
| except Exception as exc: | |
| msg = f"Azure AI Foundry credential validation failed: {exc}" | |
| logger.error(msg) | |
| raise ValueError(msg) from exc |
🤖 Prompt for 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.
In `@src/bundles/azure-ai-foundry/src/lfx_azure_ai_foundry/validator.py` around
lines 32 - 45, The Azure AI Foundry validator only wraps llm.invoke() in a
try/except, so failures during AzureAIOpenAIApiChatModel construction can escape
as raw exceptions. Move the model instantiation into the same guarded block in
validate_azure_ai_foundry_credentials so any setup, pydantic, or
dependency-related error is logged and re-raised as the documented ValueError,
using the existing msg/raise pattern.
pareek-ml
changed the title
github-actions
Bot
added
enhancement
erichare
force-pushed
the
release-1.11.0
branch
2 times, most recently
from
3924fa1 to
951ea94
Compare
…ai-foundry bundle Part 1 — registry extension (provider_registry.py, manifest.py, loader): Adds a `models: list[dict]` field to ProviderSpec so a bundle can seed a static model catalog without editing core files. register_provider() appends the list to _STATIC_MODELS_DETAILED in-place; clear() reverses it. The extension.json schema gains a matching `models[]` array; the loader passes it through to ProviderSpec. Zero behavior change when no bundle is loaded. Part 2 — lfx-azure-ai-foundry bundle (src/bundles/azure-ai-foundry/): Ships Azure AI Foundry as a provider-only Extension Bundle with zero core edits, superseding the core-edit approach in langflow-ai#13912 and langflow-ai#13924. - extension.json: declares provider metadata (AZURE_AI_FOUNDRY_API_KEY + AZURE_AI_FOUNDRY_ENDPOINT), AzureAIOpenAIApiChatModel lazy import, dotted- path validator, and a seed catalog (gpt-4o, gpt-4o-mini, gpt-4.1, o3-mini, Mistral-Large-3) via the new models[] field. - validator.py: constructs AzureAIOpenAIApiChatModel and calls invoke(); early- returns when api_key, endpoint, or model_name is absent. Tests (10): - models[] injection into _STATIC_MODELS_DETAILED and clear() reversal. - End-to-end load through the real extension loader: provider appears in get_model_providers() and seed models in get_models_detailed(). - Validator: early-return paths (missing model, key, endpoint), success, and ValueError on invoke() failure. Refs langflow-ai#13912, closes langflow-ai#13924
…dels[] README.md was referenced by pyproject.toml but not committed — hatchling build was failing with "Readme file does not exist". BUNDLE_API.md changelog entry documents the new providers[].models[] field added to ProviderEntry and ProviderSpec to satisfy the changelog gate CI check.
CodeRabbit reported 78.95% docstring coverage on the PR (threshold 80%). Added one-line docstrings to: - 7 pre-existing private validators in manifest.py - 1 pre-existing _warn() helper in _orchestrator.py - 2 inner FakeModel stub classes + 4 of their methods in test file All 10 bundle tests pass; coverage is now 100% across changed files.
…on in try/except Two bugs flagged by CodeRabbit: 1. Docstring incorrectly stated ValueError is raised when fields are missing — the actual behavior is a silent early return. Updated docstring to match. 2. AzureAIOpenAIApiChatModel(...) construction was outside the try/except so pydantic validation errors or missing-dependency raises would propagate as raw exceptions instead of the documented ValueError. Moved construction inside the try block. Added test_validator_raises_on_construction_failure to cover the new path.
Adds _models_are_well_formed() model validator to ProviderManifestEntry: - Rejects any models[] entry that is missing a non-empty 'name' key - Rejects any entry whose optional 'provider' key doesn't match self.name Catches malformed bundle manifests at load time instead of letting them propagate to get_models_detailed() with a cryptic downstream error. Adds 4 tests covering the two rejection paths and the two acceptance paths.
pareek-ml
force-pushed
the
feat/azure-ai-foundry-bundle
branch
from
aa00cfd to
da493ed
Compare
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
Author
|
Hey @erichare @dkaushik94 , @ogabrielluiz , this adds azure foundry as first class model provider. All 143 CI checks pass and CodeRabbit has no open issues. Would appreciate a review when you get a chance! |
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
2 participants
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.
Summary
Follows the bundle pattern established in #13916 and #13919. Adds Azure AI Foundry as a first-class model provider with zero edits to core lfx files beyond the small registry extension below.
Closes the provider gap raised in #13912, fixes the root cause opened in #12771.
Part 1 — registry extension (3 core files, minimal)
src/lfx/src/lfx/base/models/provider_registry.pymodels: list[dict]toProviderSpec; inject into_STATIC_MODELS_DETAILEDonregister_provider(), undo onclear()src/lfx/src/lfx/extension/manifest.pymodels: list[dict]toProviderEntrysrc/lfx/src/lfx/extension/loader/_orchestrator.pymodels=list(entry.models)through toProviderSpecThis allows any future bundle to seed a static model catalog without touching
provider_queries.py.Part 2 —
src/bundles/azure-ai-foundry/(new, self-contained)extension.jsondeclares:AZURE_AI_FOUNDRY_API_KEY,AZURE_AI_FOUNDRY_ENDPOINT)AzureAIOpenAIApiChatModelfromlangchain-azure-ai(lazy import)gpt-4o(default),gpt-4o-mini,gpt-4.1,o3-mini,Mistral-Large-3Tests (10, all pass, ruff clean)
models[]injection into_STATIC_MODELS_DETAILEDandclear()reversalget_model_providers(), seeds inget_models_detailed()ValueErroron invoke failureTest plan
python3 -m pytest src/bundles/azure-ai-foundry/tests/ -q→ 10 passedruff checkon all changed files → all checks passedlfx-azure-ai-foundrybundle alongside Langflow — Azure AI Foundry appears in the model provider dropdown with seed models pre-populatedSummary by CodeRabbit
New Features
Tests