chore: resolve Copilot review comments from PR #579#598
Merged
Conversation
Addresses three Copilot reviewer comments on PR #579: - validate-bicep-params.yml: replace hard-coded GitHub.com run URL with GitHub Actions context (server_url/repository/run_id) so the email notification links work correctly on GitHub Enterprise Server. - validate_bicep_params.py: replace the bespoke _html_escape with a thin wrapper around stdlib html.escape(text, quote=True) so single quotes are also escaped and the escaping rules stay correct as the template evolves. - content_understanding.get_image_from_analyze_operation: tighten the docstring/return type to reflect that the helper is intentionally JPEG/image-specific (matches the image_id parameter and the image/jpeg Content-Type assertion), return Optional[bytes], document the ValueError/AssertionError it can raise, and switch the failure branch from print() to the class logger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shreyas-Microsoft
requested review from
Avijit-Microsoft,
Prajwal-Microsoft,
Roopan-Microsoft,
Vinay-Microsoft,
aniaroramsft,
dgp10801,
nchandhi and
toherman-msft
as code owners
Shreyas-Microsoft
temporarily deployed
to
production
GitHub Actions
Inactive
Coverage Report •
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses prior Copilot review feedback from PR #579 by improving workflow portability (GitHub.com vs GHES), hardening HTML escaping in CI-generated email content, and clarifying/cleaning up a Content Understanding helper’s contract and logging.
Changes:
- Update the Bicep param validation workflow to derive the run URL from GitHub Actions context (server/repo/run ID) rather than hard-coding
github.com. - Replace custom HTML escaping with
html.escape(..., quote=True)in the Bicep params validation report generator. - Improve documentation and logging behavior for
get_image_from_analyze_operationin the Content Understanding helper.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/validate-bicep-params.yml |
Build RUN_URL from Actions context for GHES-compatible email links. |
infra/scripts/validate_bicep_params.py |
Delegate HTML escaping to stdlib html.escape with quote escaping enabled. |
src/ContentProcessor/src/libs/azure_helper/content_understanding.py |
Clarify JPEG-only helper contract and switch failure reporting from print() to structured logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prajwal-Microsoft
approved these changes
May 20, 2026
|
🎉 This PR is included in version 2.0.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Purpose
Resolves the three Copilot reviewer comments left on PR #579 (which is itself the
dev→mainmerge PR). Lands the fixes on a fresh branch offdevso they flow naturally into the next merge upstream.Changes
.github/workflows/validate-bicep-params.yml– Replace the hard-codedhttps://github.com/...RUN_URLwith the GitHub Actions context (server_url/repository/run_id). Email notifications will now link to the correct run on GitHub Enterprise Server too.infra/scripts/validate_bicep_params.py– Replace the bespoke_html_escapewith a thin wrapper around the stdlibhtml.escape(text, quote=True), so single quotes are also escaped and the escaping rules stay correct (and easy to maintain) as the email template evolves.src/ContentProcessor/src/libs/azure_helper/content_understanding.py–get_image_from_analyze_operationis now clearly documented as JPEG/image-specific (matching itsimage_idparameter and theimage/jpegContent-Typeassertion). Return type is annotated asOptional[bytes], theValueError/AssertionErrorit can raise are documented, and the failure branch now logs through the class logger instead of usingprint().Related
Resolves review comments on PR #579:
Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
validate_bicep_params.pysmoke-tested locally (--dir infraproducedHTML report written to ..., exit code 0).