fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry#9012
Merged
Elaina-Lee merged 1 commit intomainfrom Apr 6, 2026
Merged
fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry#9012Elaina-Lee merged 1 commit intomainfrom
Elaina-Lee merged 1 commit intomainfrom
Conversation
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | None required. |
| Commit Type | ✅ | None required. |
| Risk Level | Consider updating to risk:medium or add explicit migration/compatibility justification in the PR body. |
|
| What & Why | ✅ | Add a short note about effect on existing persisted manifests. |
| Impact of Change | ✅ | Clarify whether any migration or compatibility behavior is expected for existing workflows. |
| Test Plan | Add unit tests for the dependency-building/clearing logic or justify why manual testing suffices; if manual testing was done, check the corresponding checkbox. | |
| Contributors | ✅ | None required. |
| Screenshots/Videos | ✅ | None required. |
Final Notes
- Pass: This PR passes the PR title/body template checks.
- Advised risk: I recommend
risk:medium(higher than yourrisk:low) because these changes modify manifest defaults and UI-driven parameter population/clearing behavior which can affect persisted manifests and agent behavior. Please either update the PR label/body to reflect medium risk or add a short justification for why low risk is accurate (e.g., server-side migration/backwards compatibility is guaranteed). - Tests: Please add unit tests covering the new/changed logic (clearing and population of deploymentModelProperties), or add a clear justification in the Test Plan for why only manual testing is appropriate and when automated tests will be added.
Please update the PR title/body with the recommendations above (risk label or justification, small note about existing manifest compatibility, and test additions/justification). Thank you for the clear description and code comments — this made review straightforward!
Last updated: Sun, 05 Apr 2026 02:37:17 GMT
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the agent-loop authoring experience to avoid populating deploymentModelProperties for AzureOpenAI (where the backend can derive model info from the deployment) and to remove stale hardcoded defaults from the agent loop manifest.
Changes:
- Removed hardcoded defaults for
deploymentModelProperties.{name,format,version}and restricted their visibility toagentModelType === MicrosoftFoundryin the agent loop manifest. - Added dependency updates to clear
deploymentModelPropertieswhen switchingagentModelTypeaway fromMicrosoftFoundry. - Updated the deploymentId change handler to only populate
deploymentModelPropertiesforMicrosoftFoundryin both Designer and Designer V2.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| libs/logic-apps-shared/src/designer-client-services/lib/standard/manifest/agentloop.ts | Removes stale defaults and limits visibility of deploymentModelProperties fields to MicrosoftFoundry. |
| libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx | Clears deploymentModelProperties when leaving Foundry and conditionally populates them only for Foundry. |
| libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx | Same conditional populate + clearing behavior as Designer, for the v2 panel. |
libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx
Show resolved
Hide resolved
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx
Show resolved
Hide resolved
📊 Coverage CheckNo source files changed in this PR. |
This was referenced Apr 5, 2026
takyyon
approved these changes
Apr 6, 2026
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
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.
Commit Type
Risk Level
What & Why
When an agent node is created,
deploymentModelProperties(name, format, version) were hardcoded with defaults (gpt-4.1,OpenAI,2025-06-10) in the manifest. These values were baked in at node creation and never updated when the user selected a different model, becausebuildDependentParamalways fell back to the schema defaults.Additionally,
deploymentModelPropertiesshould only be populated forMicrosoftFoundry— forAzureOpenAIthe backend derives model info from the deployment itself.This PR:
deploymentModelPropertiestoMicrosoftFoundryonlydeploymentModelPropertieswhen switching away fromMicrosoftFoundrydeploymentModelPropertieswhen a Foundry model is selected (using the model ID directly +AGENT_MODEL_CONFIGlookup)Impact of Change
gpt-4.1. AzureOpenAI agents no longer carry unnecessarydeploymentModelProperties.deploymentModelPropertiesfields in the manifest are now scoped toMicrosoftFoundryvisibility only.Test Plan
deploymentModelPropertiesare NOT populatedname/format/versionpopulate correctlyContributors
@hyehwalee
Screenshots/Videos