Skip to content

Add service group support to Advisor recommendation summaries - #3604

Closed
Ravi Gupta (ravigupta99) wants to merge 38 commits into
microsoft:mainfrom
ravigupta99:guptaravi/advisor-recommendation-summary
Closed

Ravi Gupta (ravigupta99) wants to merge 38 commits into
microsoft:mainfrom
ravigupta99:guptaravi/advisor-recommendation-summary

Conversation

@ravigupta99

@ravigupta99 Ravi Gupta (ravigupta99) commented Sep 8, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Extends the existing advisor recommendation summary tool from #3520 so the same MCP tool can summarize either subscription-scoped or Azure service-group-scoped Advisor recommendations.

The unified summary tool:

  • Accepts --subscription or --service-group and rejects requests that explicitly provide both.
  • Preserves configured-default subscription behavior when neither scope is supplied.
  • Rejects --resource-group with service-group scope instead of silently ignoring an ambiguous filter.
  • Accepts the repository-standard globally unique service-group ID segment and queries the exact ARG pivot:
    properties.serviceGroupId =~ "/providers/Microsoft.Management/serviceGroups/{serviceGroup}".
  • Keeps subscription summaries and recommendation list on the existing isempty(properties.serviceGroupId) population.
  • Runs service-group queries at tenant scope without adding request-level subscriptions, so the pivot can span subscriptions.
  • Reuses all existing grouping, metadata enrichment, filtering, ordering, truncation, top, and response logic.
  • Preserves the existing summary command ID and { key, label, count } response contract.
  • Adds nullable serviceGroupId to the shared recommendation properties model for update and future list reuse.

The change also extracts the subscription-or-service-group binding and validation from the reviewed update implementation into RecommendationScopeCommand<TOptions, TResult>. Both update and summary now share explicit option-presence handling, default-subscription resolution, mutual exclusion, and service-group ID validation. The update tool's behavior remains unchanged.

Dependency and merge order

This PR is intentionally stacked on #3517, which has already completed team review.

Current service-group data availability

Service-group-pivoted Advisor recommendations are not yet broadly available in ARG. Tenant discovery currently found one active 64-character recommendation under service group commerce.

That record was used for the available exact-binary validation. Broader multi-row, multi-subscription, filter-intersection, and lifecycle coverage will be rerun after the service-group recommendation rollout following September 15, 2026. A zero-row or single-row result is not considered the final live acceptance gate.

Validation completed

  • 413 Azure Advisor tests passed.
  • Shared update command scope behavior: 67/67 focused tests passed after refactoring.
  • Exact local azmcp.exe service-group matrix: 14/14 passed.
    • All seven grouping dimensions were compared with independent tenant-scoped ARG queries.
    • Category, impact, recommendation type, resource type, status, and subcategory returned the same one-record totals and buckets as ARG.
    • Retirement-date grouping correctly returned an empty result for the available non-retirement recommendation.
    • Explicit tenant, top, subscription regression, dual-scope rejection, resource-group/SG rejection, blank SG, and invalid SG path cases passed.
  • Subscription regression remained healthy with 1,242 active recommendations and five category buckets at test time.
  • ToolDescriptionEvaluator:
    • 29/29 summary prompts ranked in the top three.
    • Score range: 0.424563–0.600615.
    • All eight service-group summary prompts ranked first.
    • The temporary Entra bearer-token evaluator patch was reversed and evaluator source files were verified clean.
  • Focused Vally service-group routing: 7/7 prompts passed.
    • Trace inspection confirmed every SG prompt called advisor_recommendation_summary and supplied the service-group argument.
  • Advisor formatting verification passed.
  • Azure MCP Server and Advisor builds passed with zero warnings and errors.
  • Command metadata generation and README validation passed.
  • Repository spelling validation passed.
  • Build-Local.ps1 -VerifyNpx passed; inner builds, packaging, cache cleanup, and Azure/Fabric/Template npm tool-list invocations completed without command failures.
  • NativeAOT was attempted with Build-Local.ps1 -IncludeNative. The inner Azure MCP native publish remains blocked by the pre-existing Cosmos Microsoft.Azure.Documents.JsonSerializable IL2026/IL3050/MSB3077 failure; no Advisor-specific native diagnostic was reported.
  • Full changelog compilation remains blocked by the unrelated existing copilot-fix-ai-search-consolidated-description.yml entry using invalid section Bug Fixes; the new entry uses valid Features Added and Other Changes sections.

Post-rollout live acceptance required before merge

After broader SG recommendations are indexed:

  1. Re-run every grouping against multi-row service-group populations.
  2. Compare totals and buckets with immediately executed independent tenant-scoped ARG queries.
  3. Exercise category, impact, recommendation type, resource type, resource, search, subcategory, retirement-date, and top filters.
  4. Validate lifecycle status populations beyond New.
  5. Validate a service group spanning multiple subscriptions.
  6. Re-run exact local-binary, ToolDescriptionEvaluator, Vally, build, packaging, spelling, and native-analysis gates after reconciling with post-Extend Advisor recommendation update to service groups #3517 main.

Public references

GitHub issue number?

Follow-up to #3520 and dependent on #3517.

Azure DevOps PBI: https://dev.azure.com/msazure/One/_workitems/edit/39592401

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history separates prerequisite merge, shared scope refactor, implementation/tests, and documentation
    • Added comprehensive tests for new/modified functionality
    • Created a changelog entry
  • For MCP tool changes:
    • Primary behavior change is limited to advisor_recommendation_summary; update changes are a behavior-preserving shared refactor
    • Updated servers/Azure.Mcp.Server/README.md
    • Validated README changes using Process-PackageReadMe.ps1
    • ToolDescriptionEvaluator score is at least 0.4 with top-three ranking for every summary prompt
    • Updated consolidated-tools.json
    • Added public Azure Service Groups documentation
  • Extra steps for Azure MCP Server tool changes:
    • Updated servers/Azure.Mcp.Server/docs/azmcp-commands.md
    • Ran Update-AzCommandsMetadata.ps1
    • Updated servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
  • Merge Extend Advisor recommendation update to service groups #3517 first and reconcile this branch with current upstream/main
  • Complete broad post-rollout SG live acceptance after September 15, 2026

Invoking Livetests

Copilot submitted PRs are not trustworthy by default. Users with write access to the repo need to validate the contents of this PR before leaving a comment with the text /azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.

…ecommendation-patch

# Conflicts:
#	servers/Azure.Mcp.Server/docs/azmcp-commands.md
#	servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
#	tools/Azure.Mcp.Tools.Advisor/src/Commands/AdvisorJsonContext.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Commands/Recommendation/RecommendationListCommand.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/AdvisorService.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/Models/RecommendationDescription.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/Models/RecommendationProperties.cs
#	tools/Azure.Mcp.Tools.Advisor/tests/Azure.Mcp.Tools.Advisor.Tests/Recommendation/RecommendationListCommandTests.cs
#	tools/Azure.Mcp.Tools.Advisor/tests/Azure.Mcp.Tools.Advisor.Tests/Services/AdvisorServiceConversionTests.cs
…-patch' into guptaravi/advisor-sg-recommendation-patch

# Conflicts:
#	tools/Azure.Mcp.Tools.Advisor/src/Services/AdvisorService.cs
…r-recommendation-summary

# Conflicts:
#	servers/Azure.Mcp.Server/docs/azmcp-commands.md
#	tools/Azure.Mcp.Tools.Advisor/src/Commands/Recommendation/RecommendationListCommand.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Options/Recommendation/RecommendationListOptions.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Options/Recommendation/RecommendationSummaryOptions.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/AdvisorService.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Validation/RecommendationFilterValidator.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Validation/ServiceRetirementFilterValidator.cs
#	tools/Azure.Mcp.Tools.Advisor/tests/Azure.Mcp.Tools.Advisor.Tests/Services/AdvisorServiceFilterBuilderTests.cs
…ecommendation-summary

# Conflicts:
#	servers/Azure.Mcp.Server/README.md
#	servers/Azure.Mcp.Server/docs/azmcp-commands.md
#	servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
#	servers/Azure.Mcp.Server/src/Resources/consolidated-tools.json
#	tools/Azure.Mcp.Tools.Advisor/src/AdvisorSetup.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Commands/Recommendation/RecommendationListCommand.cs
…g-recommendation-patch

# Conflicts:
#	servers/Azure.Mcp.Server/README.md
#	servers/Azure.Mcp.Server/docs/azmcp-commands.md
#	servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
#	servers/Azure.Mcp.Server/src/Resources/consolidated-tools.json
#	tools/Azure.Mcp.Tools.Advisor/src/AdvisorSetup.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Commands/Recommendation/RecommendationUpdateCommand.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Options/Recommendation/RecommendationUpdateOptions.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/AdvisorService.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/IAdvisorService.cs
#	tools/Azure.Mcp.Tools.Advisor/tests/Azure.Mcp.Tools.Advisor.Tests/Recommendation/RecommendationUpdateCommandTests.cs
#	tools/Azure.Mcp.Tools.Advisor/tests/Azure.Mcp.Tools.Advisor.Tests/Services/AdvisorRecommendationUpdateServiceTests.cs
…ecommendation-summary

# Conflicts:
#	servers/Azure.Mcp.Server/src/Resources/consolidated-tools.json
…g-recommendation-patch

# Conflicts:
#	servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
#	tools/Azure.Mcp.Tools.Advisor/src/AdvisorSetup.cs
…ion-patch' into guptaravi/advisor-recommendation-summary

# Conflicts:
#	servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
#	tools/Azure.Mcp.Tools.Advisor/src/AdvisorSetup.cs
#	tools/Azure.Mcp.Tools.Advisor/src/Services/IAdvisorService.cs
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The unresolved AdvisorService API-version concern and pending reconciliation/live acceptance make safe automated approval inappropriate.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds Azure service-group scope support to Advisor recommendation summaries while preserving subscription behavior and sharing scope/query infrastructure with updates.

Changes:

  • Adds service-group validation, tenant-scoped ARG queries, filtering, and routing.
  • Extends models and shared command infrastructure.
  • Updates tests, documentation, metadata, and changelog entries.
File Description
tools/​Azure.Mcp.Tools.Advisor/​tests/​Azure.Mcp.Tools.Advisor.Tests/​Services/​RecommendationSummaryServiceTests.cs Tests service-group summaries and tenant scoping.
tools/​Azure.Mcp.Tools.Advisor/​tests/​Azure.Mcp.Tools.Advisor.Tests/​Services/​RecommendationQueryBuilderTests.cs Tests service-group ARG predicates.
tools/​Azure.Mcp.Tools.Advisor/​tests/​Azure.Mcp.Tools.Advisor.Tests/​Services/​AdvisorServiceConversionTests.cs Tests service-group property conversion.
tools/​Azure.Mcp.Tools.Advisor/​tests/​Azure.Mcp.Tools.Advisor.Tests/​Services/​AdvisorRecommendationUpdateServiceTests.cs Tests subscription and service-group update requests.
tools/​Azure.Mcp.Tools.Advisor/​tests/​Azure.Mcp.Tools.Advisor.Tests/​Recommendation/​RecommendationUpdateCommandTests.cs Tests update scope validation and routing.
tools/​Azure.Mcp.Tools.Advisor/​tests/​Azure.Mcp.Tools.Advisor.Tests/​Recommendation/​RecommendationSummaryCommandTests.cs Tests summary scope validation and routing.
tools/​Azure.Mcp.Tools.Advisor/​src/​Validation/​RecommendationScopeValidator.cs Validates recommendation scopes.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​RecommendationSummaryService.cs Implements service-group summaries.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​RecommendationQueryScope.cs Defines query scopes.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​RecommendationQueryBuilder.cs Builds service-group predicates.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​Models/​RecommendationProperties.cs Parses serviceGroupId.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​IRecommendationSummaryService.cs Extends the summary service contract.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​IAdvisorService.cs Extends the Advisor service contract.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​AdvisorService.cs Handles Advisor update and service-group requests.
tools/​Azure.Mcp.Tools.Advisor/​src/​Services/​AdvisorResourceGraphQueryExecutor.cs Executes tenant-scoped ARG queries.
tools/​Azure.Mcp.Tools.Advisor/​src/​Options/​Recommendation/​RecommendationUpdateOptions.cs Adds service-group update options.
tools/​Azure.Mcp.Tools.Advisor/​src/​Options/​Recommendation/​RecommendationSummaryOptions.cs Adds service-group summary options.
tools/​Azure.Mcp.Tools.Advisor/​src/​Options/​Recommendation/​IRecommendationScopeOptions.cs Defines shared scope options.
tools/​Azure.Mcp.Tools.Advisor/​src/​Models/​RecommendationProperties.cs Exposes nullable serviceGroupId.
tools/​Azure.Mcp.Tools.Advisor/​src/​Commands/​Recommendation/​RecommendationUpdateCommand.cs Routes updates by scope.
tools/​Azure.Mcp.Tools.Advisor/​src/​Commands/​Recommendation/​RecommendationSummaryCommand.cs Routes summaries by scope.
tools/​Azure.Mcp.Tools.Advisor/​src/​Commands/​Recommendation/​RecommendationScopeCommand.cs Shares scope processing and validation.
tools/​Azure.Mcp.Tools.Advisor/​src/​AdvisorSetup.cs Updates Advisor registrations and descriptions.
servers/​Azure.Mcp.Server/​src/​Resources/​consolidated-tools.json Updates consolidated tool descriptions.
servers/​Azure.Mcp.Server/​README.md Adds service-group usage examples.
servers/​Azure.Mcp.Server/​docs/​e2eTestPrompts.md Adds service-group prompts.
servers/​Azure.Mcp.Server/​docs/​azmcp-commands.md Documents new scope options.
servers/​Azure.Mcp.Server/​changelog-entries/​guptaravi-advisor-service-group-recommendation-update.yml Records update scope support.
servers/​Azure.Mcp.Server/​changelog-entries/​guptaravi-advisor-recommendation-summary-service-group.yml Records summary scope support.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/Azure.Mcp.Tools.Advisor/src/Services/AdvisorService.cs
@ravigupta99

Copy link
Copy Markdown
Member Author

Superseded by #3799, which is based on the latest dev branch after the recommendation-update prerequisite merged through #3798. The dev PR contains only the recommendation-summary SG changes and shared reuse refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants