Add actionable suggestions and multi-line formatting to preflight warnings#8059
Open
Add actionable suggestions and multi-line formatting to preflight warnings#8059
Conversation
…nings Extend preflight warnings with optional Suggestion and Links fields so that each warning can provide context-specific, actionable next steps. This is the first step of the validation improvement roadmap (actionable suggestions → targeted fixes → agentic fixes). Changes: - Add Suggestion/Links fields to PreflightCheckResult and PreflightReportItem - Render suggestions and links below each warning in ToString() - Include structured suggestion data in MarshalJSON() for machine consumption - Generate dynamic suggestions for AI quota warnings: - ai_model_not_found: verify model/SKU/version + docs link - ai_model_quota_exceeded: reduce capacity or change location + portal link - Add suggestions to role_assignment and reserved_resource_name warnings - Reformat all warnings to two-line layout (title + detail) with proper indentation on continuation lines - Update confirm prompt to be more concise - Add unit tests for suggestion rendering, multi-line indentation, and JSON Fixes #8058 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request enhances the preflight validation UX by adding optional actionable guidance (suggestions + reference links) to findings, improving multi-line rendering in text output, and introducing structured JSON serialization for preflight reports.
Changes:
- Extend the preflight finding data model with
SuggestionandLinksfields (and link types) from infra checks through to UX rendering. - Update
PreflightReport.ToString()rendering to support multi-line messages plus inline “Suggestion:” and bulleted links. - Change
PreflightReport.MarshalJSON()to emit a structured{ type, summary, items[] }payload (and update/expand unit + functional tests accordingly).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/test/functional/preflight_quota_test.go | Updates functional assertions to match new capitalization/phrasing in quota/model warnings. |
| cli/azd/pkg/output/ux/preflight_report.go | Adds suggestion/link support, multi-line rendering helper, and new structured JSON output for preflight reports. |
| cli/azd/pkg/output/ux/preflight_report_test.go | Adds/updates unit tests for suggestion rendering, links, multi-line indentation, and JSON serialization shape. |
| cli/azd/pkg/infra/provisioning/bicep/local_preflight.go | Extends PreflightCheckResult with Suggestion and Links fields (and introduces a link type). |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go | Populates suggestions/links for select diagnostics and maps check results into the UX report; tweaks the confirm prompt text. |
- Fix writeItem doc comment to match actual indentation behavior - Strip ANSI escape sequences from Message/Suggestion in MarshalJSON - Use partition ordering (warnings-first) in MarshalJSON to match ToString - Add test for ANSI stripping and ordering verification Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 6 snapshot tests covering each warning diagnostic type individually plus one combined test showing all warnings together: - role_assignment_missing (with suggestion) - role_assignment_conditional (no suggestion) - reserved_resource_name (with links, no suggestion) - ai_model_not_found (with suggestion and links) - ai_model_quota_exceeded (with suggestion and links) - all warnings combined Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix import ordering: stdlib → external → internal - Revert MarshalJSON to use EventEnvelope via EventForMessage for consistency with the --output json JSONL event stream - Preallocate links slice in validatePreflight Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix import ordering in test file: stdlib → external → internal - Update PR description to match actual MarshalJSON behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
cli/azd/pkg/output/ux/preflight_report.go:60
- The
PreflightReporttype comment says “Each entry is separated by a blank line”, butToString()only inserts a single\nbetween items (so there is no blank line between entries, especially noticeable now that each entry can be multi-line). Either update the comment to match the current formatting, or insert an extra newline between items to preserve the documented layout.
for i, w := range warnings {
if i > 0 {
sb.WriteString("\n")
}
writeItem(&sb, currentIndentation, warningPrefix, w)
}
if len(warnings) > 0 && len(errors) > 0 {
sb.WriteString("\n")
}
- Update PreflightReportLink.Title doc comment to note terminal-only behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Guard writeItem against empty message to prevent rendering empty prefix - Align PreflightCheckLink.Title doc comment with UX counterpart Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
cli/azd/pkg/output/ux/preflight_report.go:41
- The
PreflightReportdoc comment says “Each entry is separated by a blank line,” butToString()only inserts a single\nbetween items (no empty line). Either update the comment to reflect the actual formatting, or adjustToString()to emit an extra newline between items if a blank line is intended.
// PreflightReport displays the results of local preflight validation.
// Warnings are shown first, followed by errors. Each entry is separated by a blank line.
type PreflightReport struct {
Items []PreflightReportItem
- Handle zero remaining quota in suggestion text: show 'No quota is available' instead of misleading 'Reduce capacity to 0' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vhvb1989
requested review from
JeffreyCA,
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
wbreza and
weikanglim
as code owners
jongio
reviewed
May 6, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Clean approach - separating "what's wrong" (Message) from "what to do" (Suggestion) from "learn more" (Links) makes each warning more actionable without bloating the message text. The multi-line rendering with writeItem is well-factored, and the
emaining == 0 branch in the quota suggestion avoids the "reduce to 0" pitfall.
A few observations (non-blocking):
- The MarshalJSON path only emits a summary count. If consumers later need structured preflight data in JSON mode (per-item diagnostics, suggestions, links), that'll be a follow-up. Current approach is consistent with how other UxItems in this package serialize.
- The writeItem early-return on empty Message means a hypothetical item with only a Suggestion (no Message) would silently drop. Fine in practice since all callers always set Message, but worth noting for future maintainers.
CI is green on lint/build/spell. Looks ready once the Azure DevOps pipeline checks finish.
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
wbreza
reviewed
May 6, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #8059
Reviewer focus: Correctness, API compatibility, error handling, test coverage, style, security, documentation alignment, performance.
🟠 High Priority
1. Functional tests don't verify the headline feature (preflight_quota_test.go)
The new Suggestion: rendering is the core deliverable of this PR, but functional tests only assert old message substrings. None verify that Suggestion: actually renders in the output. A rendering regression could ship undetected.
Suggested fix: Add
equire.Contains(t, output, "Suggestion:") to at least one functional test case.
2. Duplicate link types across packages (local_preflight.go + preflight_report.go)
PreflightCheckLink and PreflightReportLink have identical structures (URL string, Title string) in different packages, requiring manual field-by-field conversion in bicep_provider.go. If either type changes, the other must be manually updated.
Suggested fix: Consider consolidating into a shared type, or document the dependency. A follow-up is acceptable if out of scope.
🟡 Medium Priority
3. Trailing newline edge case in writeItem() (preflight_report.go)
strings.Split(message, "\n") on messages ending with \n produces an empty trailing element, rendering a spurious blank continuation line in terminal output. Several messages in bicep_provider.go use \n in their format strings.
Suggested fix: strings.TrimRight(item.Message, "\n") before splitting, or skip empty lines in the loop.
4. Portal link URLs not URL-encoded (bicep_provider.go)
resourceGroupName is interpolated directly into Azure portal URLs without url.PathEscape(). While Azure naming rules make special characters rare, defense-in-depth says to encode. One-line fix.
Suggested fix: Use url.PathEscape(resourceGroupName) in the fmt.Sprintf call.
5. Missing edge case test coverage (preflight_report_test.go)
No tests for: trailing newlines in messages, nil vs empty Links slices, empty suggestion strings, or multi-line messages with leading/consecutive newlines. These are the edge cases where writeItem() logic could regress.
Suggested fix: Add table-driven tests covering these inputs.
🔵 Low Priority
6. No validation for empty link.URL (preflight_report.go)
If a PreflightReportLink has an empty URL, it renders an orphaned bullet point. Currently all links are hardcoded with valid URLs, so this requires a future code change to trigger.
Suggested fix: Add if link.URL == "" { continue } guard in the link rendering loop.
ℹ️ Nit
7. PR description "Before" example is blank — The "Warning format (before → after)" section has an empty "Before" block.
✅ What Looks Good
- Clean additive data model — new fields are zero-value safe
- MarshalJSON() correctly uses EventForMessage — consistent with all other UxItems
- All 3 earlier Copilot bot comments properly addressed
- 13 new unit tests + 6 snapshot tests — solid base coverage
- Well-structured writeItem() refactor with proper indentation logic
- Context-specific suggestions for all 5 diagnostic types with relevant doc links
- CI fully green (29/29 checks)
- Add Suggestion: assertion to functional tests (wbreza #1) - Consolidate PreflightCheckLink into ux.PreflightReportLink, eliminating duplicate type and manual conversion (wbreza #2) - Add 7 table-driven edge case tests for writeItem (wbreza #5) - Fix PR description Before block (wbreza #7) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Thanks for the thorough review @wbreza! Here's how I addressed each item:
All changes in commit 4d31935. |
Member
Author
|
Thanks @jongio! Good observations noted. The MarshalJSON summary-only approach is intentionally consistent with other UxItems for now — structured JSON output can be a follow-up when consumers need it. And agreed on the empty-Message edge case — added a guard with early return plus edge case tests to document the behavior. |
hemarina
approved these changes
May 7, 2026
Contributor
hemarina
left a comment
hemarina
left a comment
There was a problem hiding this comment.
Thanks for tackling the actionable-suggestions roadmap — the new Suggestion / Links data model is clean and the multi-line writeItem factoring reads well. The existing review threads (Copilot bot + @wbreza + @jongio) cover most of what I''d flag; just two small supplemental observations, both non-blocking.
🟡 Low — role_assignment_conditional lacks Suggestion/Links
File: cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go (~L2783, the role_assignment_conditional result)
Four of the five warning diagnostics in this PR ship with a Suggestion and/or Links, but role_assignment_conditional only reformats its Message. Users hitting this warning are told the deployment "may fail if the condition does not permit the specific role assignments" but given no concrete next step.
Suggested wording:
Suggestion: Review the ABAC condition on your role assignment to confirm it permits the role assignments declared in your template, or use an account with unconditional
Microsoft.Authorization/roleAssignments/writepermission.
A docs link to ABAC condition syntax (e.g. https://learn.microsoft.com/azure/role-based-access-control/conditions-overview) would round it out.
🔵 Nit — writeItem doesn''t handle multi-line Suggestion text
File: cli/azd/pkg/output/ux/preflight_report.go (the if item.Suggestion != "" branch in writeItem)
Message is split on \n and each continuation line is indented:
for _, line := range lines[1:] {
sb.WriteString(fmt.Sprintf("\n%s%s", indent, line))
}The Suggestion branch is a single fmt.Sprintf with no split, so a future Suggestion containing \n would render its second line at column 0 instead of under the indent. All current callers produce single-line Suggestions, so this is latent — but worth either applying the same split/indent loop, or documenting "Suggestion must be single-line" on the field.
Otherwise looks good — solid test coverage and the diagnostic-specific suggestions/links are well-targeted.
jongio
approved these changes
May 8, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Addresses my earlier feedback. The type unification (dropping PreflightCheckLink in favor of ux.PreflightReportLink directly) is a nice simplification - eliminates the parallel type hierarchy and the manual mapping loop without introducing new coupling, since bicep already imports ux. The new edge case tests are thorough.
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.
Summary
Extends preflight warnings with optional Suggestion and Links fields, and reformats warning messages to a two-line layout with proper indentation.
This is the first step of the validation improvement roadmap from #7817: actionable suggestions → targeted fixes → agentic fixes.
Changes
Data model:
Suggestion(string) andLinks(slice) fields toPreflightCheckResultandPreflightReportItemUX rendering:
ToString()now renders suggestions below each warning withSuggestion:label + bulleted linksMarshalJSON()usesEventForMessageto emit a standardEventEnvelopewith a summary string (consistent with all other UxItems in the codebase)Dynamic suggestions:
ai_model_not_found: "Verify the model name, SKU, and version are correct" + docs linkai_model_quota_exceeded: "Reduce capacity to N or change location via `azd env set AZURE_LOCATION`" + portal linkrole_assignment_missing: suggests required rolesreserved_resource_name: links to docsWarning format (before → after):
Before:
After:
Testing
Fixes #8058