fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry#9012
fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry#9012Elaina-Lee wants to merge 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
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. |
| const config = AGENT_MODEL_CONFIG[selectedModelId]; | ||
| const modelName = selectedModelId; |
There was a problem hiding this comment.
In the Foundry path, selectedModelId here is coming from the deploymentId combobox option value (currently value: deployment.name in getEditorAndOptions), so it’s a deployment name, not the model id (deployment.properties.model.name). Using it as the key for AGENT_MODEL_CONFIG will usually miss and also sets deploymentModelProperties.name to the deployment id instead of the model name. Consider restoring a deployment lookup (deploymentsForCognitiveServiceAccount.find(d => d.name === selectedModelId)) and then using deploymentInfo.properties.model.name for modelName / AGENT_MODEL_CONFIG lookup (or alternatively change the combobox option values for MicrosoftFoundry to emit the model id).
| const config = AGENT_MODEL_CONFIG[selectedModelId]; | |
| const modelName = selectedModelId; | |
| const deploymentInfo = deploymentsForCognitiveServiceAccount.find((deployment) => deployment.name === selectedModelId); | |
| const modelName = deploymentInfo?.properties?.model?.name ?? selectedModelId; | |
| const config = AGENT_MODEL_CONFIG[modelName]; |
| if (currentModelType === 'MicrosoftFoundry' && selectedModelId) { | ||
| const config = AGENT_MODEL_CONFIG[selectedModelId]; | ||
| const modelName = selectedModelId; | ||
| const modelFormat = config?.format ?? 'OpenAI'; | ||
| const modelVersion = config?.version; |
There was a problem hiding this comment.
selectedModelId originates from the deploymentId combobox options (currently value: deployment.name), so for Foundry it is a deployment name rather than the model id used as keys in AGENT_MODEL_CONFIG. This will cause deploymentModelProperties.name to be set incorrectly and the config lookup to fail. Suggest mapping deployment name -> deployment.properties.model.name before looking up AGENT_MODEL_CONFIG (or change the combobox option values for MicrosoftFoundry to return the model id).
📊 Coverage CheckNo source files changed in this PR. |
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