-
Notifications
You must be signed in to change notification settings - Fork 310
fix: move ai best practice into best practice namespace #1323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2c196c2 to
a498fc4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR consolidates AI application best practices guidance by moving the AIAppBestPracticesCommand from a standalone Azure.Mcp.Tools.AzureAIBestPractices tool into the existing Azure.Mcp.Tools.AzureBestPractices namespace as a subcommand. This refactoring improves organization by grouping all Azure best practices under a single tool.
Key changes:
- Moved AI app best practices command from separate tool to
AzureBestPracticesasai_appsubcommand - Removed entire
Azure.Mcp.Tools.AzureAIBestPracticesproject and references - Updated command name from
azureaibestpractices_gettoget_bestpractices_ai_app
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.AzureBestPractices/src/Commands/AIAppBestPracticesCommand.cs | Added AI app best practices command to AzureBestPractices namespace with updated naming (ai_app) |
| tools/Azure.Mcp.Tools.AzureBestPractices/src/AzureBestPracticesSetup.cs | Registered AIAppBestPracticesCommand as a subcommand under the bestpractices command group |
| tools/Azure.Mcp.Tools.AzureBestPractices/src/Resources/*.txt | Moved AI-related resource files (ai-best-practices-core.txt, ai-background-knowledge.txt, ai-error-patterns.txt) with minor content updates |
| tools/Azure.Mcp.Tools.AzureBestPractices/tests/Azure.Mcp.Tools.AzureBestPractices.UnitTests/AIAppBestPracticesCommandTests.cs | Added comprehensive unit tests for the new AI app best practices command |
| tools/Azure.Mcp.Tools.AzureAIBestPractices/src/*.cs | Removed old standalone tool source files |
| tools/Azure.Mcp.Tools.AzureAIBestPractices/tests/**/*.cs | Removed old test files from standalone tool |
| servers/Azure.Mcp.Server/src/Program.cs | Removed AzureAIBestPracticesSetup registration from server startup |
| servers/Azure.Mcp.Server/docs/azmcp-commands.md | Updated command documentation to reflect new command location under bestpractices |
| servers/Azure.Mcp.Server/docs/e2eTestPrompts.md | Moved test prompts from Azure AI Best Practices section to Azure Best Practices section |
| servers/Azure.Mcp.Server/README.md | Removed Azure AI Best Practices from service areas list |
| core/Azure.Mcp.Core/src/Areas/Server/Resources/azure-rules.txt | Updated rule to reference new command name get_bestpractices_ai_app |
| AzureMcp.sln | Removed AzureAIBestPractices project references |
| .github/CODEOWNERS | Updated code ownership to add @XiaofuHuang to AzureBestPractices tool |
core/Azure.Mcp.Core/src/Areas/Server/Resources/consolidated-tools.json- Line 1013 still referencesazureaibestpractices_getand should be updated toget_bestpractices_ai_appeng/tools/ToolDescriptionEvaluator/prompts/prompts.json- The keyazureaibestpractices_getshould be renamed toget_bestpractices_ai_app
These files are required to be updated according to the PR checklist item: "For tools with new names, including new tools or renamed tools, update consolidated-tools.json".
Comments suppressed due to low confidence (1)
servers/Azure.Mcp.Server/src/Program.cs:222
- Incorrect indentation: This line should be indented with 8 spaces to align with the statement above (line 221), but it currently has only 4 spaces of indentation.
await telemetryService.InitializeAsync();
d51ec38 to
08a5d30
Compare
08a5d30 to
6c0bb32
Compare
|
/azp run mcp - pullrequest - live |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| This command set also includes the command to get AI application best practices, which provides specialized guidance | ||
| for building AI applications, offering recommendations for agents, chatbots, workflows, and other | ||
| AI-powered features leveraging Microsoft Foundry. | ||
| When the request involves AI in any capacity, including systems where AI is used as a component, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when the request involves AI in any capacity
Do we want to narrow this down to code generation instead? For example -
when the request involves code generation of AI components/applications in any capacity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the suggestion. I’ve updated it to focus specifically on code generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am interested to see the results of some prompt which tries to create AI app with non-Microsoft technology, such the corresponding technology from aws to see if this tool gets triggered. Ideally, it shouldn't.
| - Use Azure AI App Code Generation Best Practices: When generating code for AI applications, AI agents, workflows, | ||
| or chatbots, invoke your `get_bestpractices_ai_app` command (under `get_bestpractices` tool) if tool available | ||
| Call this tool first before creating any plans, todos or code. | ||
| Only call this tool when you are sure the user is discussing Microsoft Foundry or AI feature code generation; do not call it for Azure resource operations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only call this tool when you are sure the user is discussing Microsoft Foundry or AI feature code generation; do not call it for Azure resource operations.
Wouldnt this negate calling the best practice tool at all for purposes other than AI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change "Only call this tool" to "Only call this command" to avoid misunderstandings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove or AI feature code generation. I am afraid that when the user wants to generate AI apps using other technology, this tool will be invoked as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want Copilot to trigger this command not only when users mention Microsoft Foundry, but also when they request AI app or code generation in Azure without specifying a technology stack (for example, “Create an AI app to manage travel queries”). In these cases, we’ll recommend Foundry.
If the user specifies a different technology, such as Azure Functions, Copilot will use that technology’s best-practice command and also call this command to optionally suggest Foundry, without overriding the user’s choice.

| the user is discussing Azure and code generation; do not call it for Azure resource operations. | ||
| - Use Azure AI App Code Generation Best Practices: When generating code for AI applications, AI agents, workflows, | ||
| or chatbots, invoke your `get_bestpractices_ai_app` command (under `get_bestpractices` tool) if tool available | ||
| Call this tool first before creating any plans, todos or code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a more generic rule for all Azure best practices scoped to Azure related intents - @fanyang-mono
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right @kvenkatrajan. @XiaofuHuang Can you rephrase it to something like this?
When generating code for AI applications, AI agents, workflows,
or chatbots using Microsoft Foundry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the same reason mentioned above, we want Copilot to trigger this command not only when users mention Microsoft Foundry, but also when they request AI app or code generation in Azure without specifying a technology stack. Therefore, we should not limit this rule to Microsoft Foundry only.
| when working with Azure services. It should be called for any code generation, deployment or | ||
| when working with Azure services. | ||
| It should be called for any code generation, deployment or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the newline help LLM understand the description better? From what's recommended by the data scientists, newlines don't make a difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We added these new lines for human to understand. Removed these new lines to save tokens.
What does this PR do?
Move Azure AI best practice tool into best practice namespace
GitHub issue number?
#1110
Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline