Repository navigation
Let a re-run of failed host shards complete the same Percy build - #6875
Conversation
The Percy nonce is the run id, not the run id and attempt. Finalize seals the build only when every shard passed, so a failed run leaves the build open, and "Re-run failed jobs" adds the re-run shards' snapshots to it. The tally gate reads each shard's latest tally across attempts. The reject on a host-test failure and the partial re-run check are removed, because a build with a failed shard is no longer sealed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jtn9kCHY8C4FfiQkzHPAgr
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42857dee86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Host Test Results 1 files ±0 1 suites ±0 1h 28m 17s ⏱️ - 2m 28s Results for commit 01e4e1c. ± Comparison against earlier commit 270db6c. Realm Server Test Results 1 files ±0 359 suites ±0 2h 6m 5s ⏱️ + 5m 12s Results for commit 7bfa986. ± Comparison against earlier commit 7dba379. |
A sealed build cannot take more snapshots, so "Re-run all jobs" (or one job re-run after the run passed) needs a new build. The finalize job now uploads a percy-sealed-<attempt> artifact after it seals a build, and the percy-nonce action derives the nonce from the latest sealed attempt. The tally gate ignores tallies from that attempt and earlier ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jtn9kCHY8C4FfiQkzHPAgr
richardhjtan
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] Reviewed the Percy nonce, seal-marker and finalize/reject logic for how a build spanning several attempts reaches main's baseline.
One blocking issue: on main, a build sealed after "Re-run failed jobs" now becomes the baseline carrying the failed attempt's snapshots, with no reject left to stop it. Detail inline on the reject step's condition.
# Conflicts: # .github/workflows/ci-host.yaml
Percy keeps the first copy of a snapshot name, so a build that a re-run completed can hold a snapshot the re-run shard took in the attempt where it failed. The tally step flags an attempt later than the one after the last seal, and on main the reject step and the unrejected note now also act on that flag. Pull requests keep the one complete build to review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jtn9kCHY8C4FfiQkzHPAgr
This reverts commit 7dba379. "Re-run all jobs" on a run whose Percy build was already sealed reused the nonce on a test branch, and Percy started a new build with every snapshot, so the marker guarded a case that already works. Without the marker, the tally step flags any attempt after the first, and main rejects that build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jtn9kCHY8C4FfiQkzHPAgr
The finalize job now asks the jobs API which host shards started in this attempt and whether an earlier attempt's finalize succeeded. - completed_by_rerun is set only when a shard that ran in this attempt also ran earlier and took snapshots there, or left no earlier tally. - A shard that ran in this attempt counts only this attempt's tally, so a failed upload shows as missing. - Finalize is skipped when no shard ran and the build is already sealed, so a re-run of the finalize job alone makes no empty build. - Two comments now give the reasons that hold today. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jtn9kCHY8C4FfiQkzHPAgr
Background and Goal
The Percy nonce had the run attempt in it, so "Re-run failed jobs" made a new Percy build that held only the snapshots of the shards that ran again. That build showed "N missing snapshots found", or "Visual review requested but no snapshots were uploaded" when the re-run shard takes no snapshots. On
main, such a build became the baseline, and later PR builds then showed 50 to 118 snapshots with no baseline. RunsOn starts these re-runs automatically after a spot interruption, so this happened often. The attempt was added to the nonce in #1333, because attempt 1 always sealed its build and a re-run could not add to a sealed build.This PR keeps one Percy build for each run:
${{ github.run_id }}.main, a build that a re-run of failed shards completed is rejected when a re-run shard took snapshots in an earlier attempt, or left no tally there. A shard that failed before any snapshot does not cause a reject.Key decisions and non-obvious mechanics
started_at, and itsrun_attemptfield reports the new attempt. So the finalize job comparesstarted_atwith the attempt'srun_started_atto find the shards that ran. If the jobs API cannot answer, it falls back to counting each shard's latest tally and treating every later attempt as a re-run.mainthat build is rejected (item 5). Pull requests keep it for review. The reject on a tally shortfall onmainstays, because a shard can pass and still lose an upload.🤖 Generated with Claude Code
https://claude.ai/code/session_01Jtn9kCHY8C4FfiQkzHPAgr