Repository navigation
Fix visual failure reporting and unstable captures - #1296
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
On reruns, the screenshot summary can select an artifact from an earlier attempt and post stale failures instead of the current job’s fallback.
1 open issue
What changed in this PR
Fixes artifact selection in ToolHive’s screenshot-summary workflow so visual regression failures can produce PR comments, with fallback information preserved when artifact discovery fails.
Changes:
- Corrects the jq assignment used to prefer compact failure artifacts.
- Publishes the visual-job conclusion and URL before artifact discovery.
- Adds five workflow-step regression tests and includes them in CI.
| File | Description |
|---|---|
scripts/pr-screenshot-workflow.test.mjs |
Tests artifact selection, expiration, missing artifacts, and discovery failures. |
.github/workflows/pr-screenshot-summary.yaml |
Fixes artifact selection and preserves outputs needed for fallback comments. |
.github/workflows/_static-checks.yaml |
Runs the new workflow tests alongside screenshot-summary helper tests. |
Review effort: Balanced
danbarr
enabled auto-merge (squash)
October 30, 2026 17:19
danbarr
changed the title
reyortiz3
approved these changes
Oct 30, 2026
This branch was successfully deployed
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.
Description
The artifact-selection query added in #1289 uses jq's update assignment (
|=), which evaluates its right-hand side against the artifacts array. The nested.artifacts[]access then fails, so the failure-comment step receives no visual-job conclusion and crashes. This prevented a comment on #1295 even though its compact failure artifact was available.Use ordinary assignment (
=) to select the compact artifact from the response object. Publish the visual-job conclusion and URL before artifact discovery so a discovery or parsing failure still allows the existing fallback comment. Add CI tests that execute the actual workflow shell step with representative GitHub API responses, covering compact selection, expired artifacts, missing artifacts, API failures, malformed JSON, and reruns. Only artifacts created during the current visual job are eligible, so earlier attempts cannot supply stale failure details.Capture the theme-preview page in separate light and dark page loads, with the color scheme selected before navigation and both Mermaid diagrams rendered before the screenshot. This avoids measuring page height during an asynchronous theme-change render. Restore the complete dark baseline from before #1288; it is byte-for-byte identical to #1296's latest CI retry, whereas the intervening baseline was truncated. Update the snapshot metadata to name the separate test cases.
Validation: the updated CI run passed all 12 visual tests without retries. The new workflow-step suite reproduces the original query failure and all ten tests pass with the fix; the three description-updater tests and 15 failure-comment tests pass. Replayed #1295's real 21 KB artifact and recovered all three failure rows. TypeScript checking, production build, focused ESLint and Prettier,
git diff --check, and offline zizmor pass. Local actionlint passes after normalizing the existing$/syntax for its older parser. Live comment delivery requires merging this fix and rerunning #1295's On PR workflow to trigger a fresh summary run. Rerunning the old summary run would reuse its original workflow revision.Type of change
Related issues/PRs
Submitter checklist
Content and formatting
Reviewer checklist
Content