refactor: repair and polish audit vrt tools PR

[arii]

This PR implements architectural improvements and fixes identified in the previous audit of PR #9592. It repairs the audit-vrt-changes.sh script by adding necessary dependency checks, simplifying the matching logic, removing verbose comments, and refactoring output aggregation to avoid generating hundreds of files for large repositories. It also resolves a documentation duplication issue in .github/copilot-instructions.md by referencing the existing .github/instructions/vrt-stability.instructions.md file directly. No manual VRT checklists were found in CONTRIBUTING.md. Tests run and pass cleanly.

---

PR created automatically by Jules for task 2189911283084012697 started by @arii

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[github-actions]

👋 Welcome to HRM!

Thanks for your contribution. This repository uses Gemini AI for automated triage, code review, and generation.

🤖 Gemini Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-enrich	Run PR Enrichment (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message
@pr-squash	Squash PR commits (PR only)
@conflict-resolve	Resolve merge conflicts (PR only)

For more details and GitHub CLI examples, see the Manual Trigger Guide.

[arii]

🤖 AI Technical Audit

PR Review: Repair and Polish Audit VRT Tools

Summary

This PR streamlines the VRT auditing process by reducing output fragmentation and consolidating documentation. The shift from file-based auditing to PR-based auditing is a significant improvement for reviewer cognitive load. However, there are regression risks in the script's regex and the CI failure in Build Check suggests a dependency or environment mismatch introduced by these changes.

Architectural Impact

• Consolidation: Moving VRT standards to a single source of truth (.github/instructions/vrt-stability.instructions.md) reduces documentation rot.
• Storage Strategy: Switching audit-vrt-changes.sh to generate one file per PR rather than one file per changed spec prevents directory pollution in large repos but changes the review workflow from 'asset-centric' to 'change-centric'.

Anti-AI-Slop

1. OVERLY VERBOSE COMMENTS: Removed ~10 lines of comments in audit-vrt-changes.sh that merely described standard Bash operations (e.g., "Process each PR"). Good deletion.
2. OVER-ENGINEERING: The previous logic created safe filenames using tr for every file path. This was overkill for internal audit logs; the new $pr_num.md approach is simpler and more robust.
3. CODE RATIO: Found >10 lines for removal. The redundant echo "" and repetitive header logic in the loop can be further optimized.
4. STALE FEATURES: Verified that the inline documentation in .github/copilot-instructions.md was correctly replaced by a reference to the external file.

File-by-File Analysis

.github/copilot-instructions.md

Problem: While reducing duplication is good, the reference is a plain string. If the referenced file is renamed, Copilot loses context without a build-time failure.
Implementation Sample:
Ensure the path is checked via a pre-commit hook or internal link checker.

scripts/audit-vrt-changes.sh

Problem 1: The regex VRT_PATTERN="tests/playwright/.*\.ts$" is too broad. It will now capture every TypeScript file in the playwright directory (including unit tests, utils, and config), whereas the original specifically targeted spec.ts and known helpers.
Implementation Sample:

# Revert to a more specific pattern to avoid noise
VRT_PATTERN="tests/playwright/.*(spec\.ts|visual\.ts|test-helpers\.ts)"

Problem 2: Performance. gh pr diff "$pr_num" --patch is called inside a nested loop for every file path. For a PR with 50 files, this makes 50 network calls for the same diff data.
Implementation Sample:

# Fetch the diff once per PR and store in a variable or temp file
pr_diff=$(gh pr diff "$pr_num" --patch)
echo "$pr_diff" | awk -v path="$file_path" ...

Review automatically published via RepoAuditor.

[google-labs-jules]

🤖 AI Technical Audit

PR Review: Repair and Polish Audit VRT Tools

Summary

This PR streamlines the VRT auditing process by reducing output fragmentation and consolidating documentation. The shift from file-based auditing to PR-based auditing is a significant improvement for reviewer cognitive load. However, there are regression risks in the script's regex and the CI failure in Build Check suggests a dependency or environment mismatch introduced by these changes.

Architectural Impact

• Consolidation: Moving VRT standards to a single source of truth (.github/instructions/vrt-stability.instructions.md) reduces documentation rot.
• Storage Strategy: Switching audit-vrt-changes.sh to generate one file per PR rather than one file per changed spec prevents directory pollution in large repos but changes the review workflow from 'asset-centric' to 'change-centric'.

Anti-AI-Slop

1. OVERLY VERBOSE COMMENTS: Removed ~10 lines of comments in audit-vrt-changes.sh that merely described standard Bash operations (e.g., "Process each PR"). Good deletion.
2. OVER-ENGINEERING: The previous logic created safe filenames using tr for every file path. This was overkill for internal audit logs; the new $pr_num.md approach is simpler and more robust.
3. CODE RATIO: Found >10 lines for removal. The redundant echo "" and repetitive header logic in the loop can be further optimized.
4. STALE FEATURES: Verified that the inline documentation in .github/copilot-instructions.md was correctly replaced by a reference to the external file.

File-by-File Analysis

.github/copilot-instructions.md

Problem: While reducing duplication is good, the reference is a plain string. If the referenced file is renamed, Copilot loses context without a build-time failure.
Implementation Sample:
Ensure the path is checked via a pre-commit hook or internal link checker.

scripts/audit-vrt-changes.sh

Problem 1: The regex VRT_PATTERN="tests/playwright/.*\.ts$" is too broad. It will now capture every TypeScript file in the playwright directory (including unit tests, utils, and config), whereas the original specifically targeted spec.ts and known helpers.
Implementation Sample:

# Revert to a more specific pattern to avoid noise
VRT_PATTERN="tests/playwright/.*(spec\.ts|visual\.ts|test-helpers\.ts)"

Problem 2: Performance. gh pr diff "$pr_num" --patch is called inside a nested loop for every file path. For a PR with 50 files, this makes 50 network calls for the same diff data.
Implementation Sample:

# Fetch the diff once per PR and store in a variable or temp file
pr_diff=$(gh pr diff "$pr_num" --patch)
echo "$pr_diff" | awk -v path="$file_path" ...

Review automatically published via RepoAuditor.

Thank you for the review! I have addressed the points:

1. VRT_PATTERN: Reverted the pattern to tests/playwright/.*(spec\.ts|visual\.ts|test-helpers\.ts) to be more specific and avoid capturing all TypeScript files.
2. Performance: Modified the script to fetch the diff patch only once per PR (pr_diff=$(gh pr diff "$pr_num" --patch)) and use that variable inside the inner file loop to reduce network calls significantly.

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

[arii]

🤖 AI Technical Audit

PR Review: Repair and Polish Audit VRT Tools

Summary

This PR streamlines the VRT auditing process by refactoring the audit-vrt-changes.sh script and consolidating documentation. The architectural shift from file-based tracking to PR-based tracking in the audit script is a significant improvement for reviewer ergonomics.

Anti-AI-Slop Directives

1. OVERLY VERBOSE COMMENTS: The removal of block comments in audit-vrt-changes.sh (e.g., "Clean and recreate the output directory") is a positive change. No new verbose/obvious comments were added.
2. OVER-ENGINEERING: The script refactoring actually reduces engineering complexity by moving away from a complex many-to-many mapping (files to PRs) to a simpler one-to-one mapping (PR to a list of its VRT changes).
3. DUPLICATE HOOKS/TYPES: N/A for this Bash/Markdown PR.
4. CODE RATIO: Found 12 lines of redundant logic in the original script regarding safe-name generation and conditional file header initialization that are now correctly removed.
5. STALE FEATURES: The PR correctly deletes the inline VRT standards in .github/copilot-instructions.md in favor of the specialized instruction file, preventing "source of truth" drift.

File-by-File Analysis

scripts/audit-vrt-changes.sh

Problem: The awk command is executed inside a nested loop, calling gh pr diff --patch repeatedly for the same PR for every file matched. In a large PR with 50+ VRT files, this will trigger 50+ CLI calls to the GitHub API/Git, causing significant performance degradation and potential rate limiting.

Implementation Sample:

# Move diff fetch outside the file loop
pr_diff=$(gh pr diff "$pr_num" --patch)
# This is already partially addressed in the diff, but the awk logic 
# can still be optimized to avoid piping the entire pr_diff string 
# through awk in a loop. Consider a single pass if possible.

.github/copilot-instructions.md

Problem: While referencing the external file is good, the reference is a plain text sentence. If the filename .github/instructions/vrt-stability.instructions.md changes, this reference breaks silently.

Implementation Sample:

### 8. Visual Regression Testing (VRT) Stabilization Standards

Strictly adhere to the standards defined in: 
[vrt-stability.instructions.md](.github/instructions/vrt-stability.instructions.md)

Architectural Impact

The change shifts the audit output from a "File-Centric View" (which file is affected by which PRs) to a "PR-Centric View" (which VRT changes does this PR introduce). This is better for short-term review but loses the ability to see cumulative drift on a specific asset across multiple concurrent PRs. However, given the current workflow, the PR-centric view is much more actionable.

Best Practices

• Security: The script uses $pr_num and $branch variables derived from gh. While usually safe, it's good practice to quote them to prevent word splitting: "$pr_num".
• Performance: Added dependency checks (gh, jq) at the start prevent runtime failures halfway through the audit.

Review automatically published via RepoAuditor.

[google-labs-jules]

🤖 AI Technical Audit

PR Review: Repair and Polish Audit VRT Tools

Summary

This PR streamlines the VRT auditing process by refactoring the audit-vrt-changes.sh script and consolidating documentation. The architectural shift from file-based tracking to PR-based tracking in the audit script is a significant improvement for reviewer ergonomics.

Anti-AI-Slop Directives

1. OVERLY VERBOSE COMMENTS: The removal of block comments in audit-vrt-changes.sh (e.g., "Clean and recreate the output directory") is a positive change. No new verbose/obvious comments were added.
2. OVER-ENGINEERING: The script refactoring actually reduces engineering complexity by moving away from a complex many-to-many mapping (files to PRs) to a simpler one-to-one mapping (PR to a list of its VRT changes).
3. DUPLICATE HOOKS/TYPES: N/A for this Bash/Markdown PR.
4. CODE RATIO: Found 12 lines of redundant logic in the original script regarding safe-name generation and conditional file header initialization that are now correctly removed.
5. STALE FEATURES: The PR correctly deletes the inline VRT standards in .github/copilot-instructions.md in favor of the specialized instruction file, preventing "source of truth" drift.

File-by-File Analysis

scripts/audit-vrt-changes.sh

Problem: The awk command is executed inside a nested loop, calling gh pr diff --patch repeatedly for the same PR for every file matched. In a large PR with 50+ VRT files, this will trigger 50+ CLI calls to the GitHub API/Git, causing significant performance degradation and potential rate limiting.

Implementation Sample:

# Move diff fetch outside the file loop
pr_diff=$(gh pr diff "$pr_num" --patch)
# This is already partially addressed in the diff, but the awk logic 
# can still be optimized to avoid piping the entire pr_diff string 
# through awk in a loop. Consider a single pass if possible.

.github/copilot-instructions.md

Problem: While referencing the external file is good, the reference is a plain text sentence. If the filename .github/instructions/vrt-stability.instructions.md changes, this reference breaks silently.

Implementation Sample:

### 8. Visual Regression Testing (VRT) Stabilization Standards

Strictly adhere to the standards defined in: 
[vrt-stability.instructions.md](.github/instructions/vrt-stability.instructions.md)

Architectural Impact

The change shifts the audit output from a "File-Centric View" (which file is affected by which PRs) to a "PR-Centric View" (which VRT changes does this PR introduce). This is better for short-term review but loses the ability to see cumulative drift on a specific asset across multiple concurrent PRs. However, given the current workflow, the PR-centric view is much more actionable.

Best Practices

• Security: The script uses $pr_num and $branch variables derived from gh. While usually safe, it's good practice to quote them to prevent word splitting: "$pr_num".
• Performance: Added dependency checks (gh, jq) at the start prevent runtime failures halfway through the audit.

Review automatically published via RepoAuditor.

Thank you for the continued review. I have addressed the points:

1. VRT standards link: Updated the reference in .github/copilot-instructions.md to be a clickable markdown link pointing to [.github/instructions/vrt-stability.instructions.md](.github/instructions/vrt-stability.instructions.md) so it doesn't break silently if the file changes and can be verified by link checkers.
2. Performance & Variables: The script was already updated in the previous commit to pull pr_diff only once per PR to avoid excessive gh pr diff network calls inside the inner loop. The pr_num and branch variables are correctly quoted ("$pr_num", "$branch") where used to prevent word splitting.

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

[arii]

🤖 AI Technical Audit

PR Review: Repair and Polish Audit VRT Tools

Summary

This PR successfully consolidates documentation and refactors the audit-vrt-changes.sh script to be more efficient and less noisy. The architectural shift from per-file audit logs to per-PR audit logs significantly improves the scalability of the auditing process.

Anti-AI-Slop Directives

1. OVERLY VERBOSE COMMENTS: The script had several comments like # Process each PR or # Clean and recreate the output directory which were removed. This is a positive change.
2. OVER-ENGINEERING: The previous iteration attempted to use tr to create complex safe-filenames for every file changed. The new approach of grouping by PR ID (pr-${pr_num}.md) is much simpler and more maintainable.
3. DUPLICATE HOOKS/TYPES: N/A for this Bash-heavy PR.
4. CODE RATIO: The PR is roughly net-zero lines (64 deletions, 58 additions). I have identified a redundant grep call in the script that can be merged into the primary regex.
5. STALE FEATURES: The PR correctly deletes the inline VRT standards in .github/copilot-instructions.md in favor of the external reference, preventing documentation drift.

File-by-File Analysis

scripts/audit-vrt-changes.sh

Problem: The script performs a grep -v "\.png$" after a grep -E. This can be handled within the primary regex to reduce process piping. Additionally, the script does not handle a potential failure of the gh pr list command, which would cause the jq command to fail.

Implementation Sample:

# Combine into a single grep with a negative lookahead or simpler exclusion
changed_files=$(gh pr diff "$pr_num" --name-only | grep -E "$VRT_PATTERN" | grep -v ".png")
# Suggestion: Use -v in grep more effectively or ensure VRT_PATTERN excludes images explicitly.

.github/copilot-instructions.md

Problem: Excellent cleanup. No issues found. The reduction of ~25 lines of duplicated markdown is a major win for maintainability.

Architectural Impact

The change to the audit script reduces I/O operations significantly. In a repository with 50 changed files across 10 PRs, the old script would create 50 files; the new script creates 10. This makes the vrt-file-audits directory actually navigable by humans.

Best Practices

• Security: The script uses jq -r and handles PR titles/branches as variables, which is good practice. However, echo "$pr_diff" | awk ... could fail on extremely large diffs if the string exceeds shell limits.
• Performance: Fetching the diff once per PR (pr_diff=$(gh pr diff "$pr_num" --patch)) is much faster than fetching it inside the inner file loop.

Review automatically published via RepoAuditor.

[google-labs-jules]

🤖 AI Technical Audit

PR Review: Repair and Polish Audit VRT Tools

Summary

This PR successfully consolidates documentation and refactors the audit-vrt-changes.sh script to be more efficient and less noisy. The architectural shift from per-file audit logs to per-PR audit logs significantly improves the scalability of the auditing process.

Anti-AI-Slop Directives

1. OVERLY VERBOSE COMMENTS: The script had several comments like # Process each PR or # Clean and recreate the output directory which were removed. This is a positive change.
2. OVER-ENGINEERING: The previous iteration attempted to use tr to create complex safe-filenames for every file changed. The new approach of grouping by PR ID (pr-${pr_num}.md) is much simpler and more maintainable.
3. DUPLICATE HOOKS/TYPES: N/A for this Bash-heavy PR.
4. CODE RATIO: The PR is roughly net-zero lines (64 deletions, 58 additions). I have identified a redundant grep call in the script that can be merged into the primary regex.
5. STALE FEATURES: The PR correctly deletes the inline VRT standards in .github/copilot-instructions.md in favor of the external reference, preventing documentation drift.

File-by-File Analysis

scripts/audit-vrt-changes.sh

Problem: The script performs a grep -v "\.png$" after a grep -E. This can be handled within the primary regex to reduce process piping. Additionally, the script does not handle a potential failure of the gh pr list command, which would cause the jq command to fail.

Implementation Sample:

# Combine into a single grep with a negative lookahead or simpler exclusion
changed_files=$(gh pr diff "$pr_num" --name-only | grep -E "$VRT_PATTERN" | grep -v ".png")
# Suggestion: Use -v in grep more effectively or ensure VRT_PATTERN excludes images explicitly.

.github/copilot-instructions.md

Problem: Excellent cleanup. No issues found. The reduction of ~25 lines of duplicated markdown is a major win for maintainability.

Architectural Impact

The change to the audit script reduces I/O operations significantly. In a repository with 50 changed files across 10 PRs, the old script would create 50 files; the new script creates 10. This makes the vrt-file-audits directory actually navigable by humans.

Best Practices

• Security: The script uses jq -r and handles PR titles/branches as variables, which is good practice. However, echo "$pr_diff" | awk ... could fail on extremely large diffs if the string exceeds shell limits.
• Performance: Fetching the diff once per PR (pr_diff=$(gh pr diff "$pr_num" --patch)) is much faster than fetching it inside the inner file loop.

Review automatically published via RepoAuditor.

Thank you for the review. I have removed the redundant grep -v "\.png$" since the updated VRT_PATTERN ending in \.ts) already guarantees that only TypeScript files (and thus no .png files) will be matched. I've also added a check for the gh pr list command to gracefully handle failure and exit early if it fails.

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide