Skip to content

Filter out restarted jobs from CH queries - #6954

Merged
jeanschmidt merged 1 commit into
mainfrom
autorevert-filter-out-restarted-jobs-from-ch-queries
Jul 30, 2025
Merged

Filter out restarted jobs from CH queries#6954
jeanschmidt merged 1 commit into
mainfrom
autorevert-filter-out-restarted-jobs-from-ch-queries

Conversation

@izaitsevfb

Copy link
Copy Markdown
Contributor

As the prerequisite for running autorevert in shadow mode, we need to filter out restarted jobs from the existing queries where they might skew the results.

@vercel

vercel Bot commented Jul 28, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
torchci ✅ Ready (Inspect) Visit Preview Jul 28, 2025 4:23pm

@izaitsevfb
izaitsevfb requested a review from jeanschmidt July 28, 2025 16:23
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 28, 2025
@jeanschmidt
jeanschmidt merged commit a9b4e21 into main Jul 30, 2025
8 checks passed
@jeanschmidt
jeanschmidt deleted the autorevert-filter-out-restarted-jobs-from-ch-queries branch July 30, 2025 14:15
izaitsevfb added a commit that referenced this pull request Aug 20, 2026
…_query

Reverts the restart-exclusion clauses PR #6954 added to these two queries, so
autorevert's workflow_dispatch restart runs (head_branch = 'trunk/<sha>') are
present in the job data HUD reads.

#6954 added the filter as a measurement-isolation guard for autorevert shadow
mode -- keeping the queries reading the same signal autorevert itself acted on.
That goal no longer applies; test-infra#8300 now wants the restart signal
surfaced and credited.

Deliberately NOT reverted here:
  - commit_jobs_batch_query (viable/strict gate) -- measured regression, see PR
  - autorevert_checker.py, flaky_tests/across_jobs -- dead code

Refs: #8300
izaitsevfb added a commit that referenced this pull request Aug 21, 2026
…ial revert of #6954) (#8494)

✴️ iz2: Partial revert of #6954 — the UI half of #8300.

**Problem.** Autorevert's restart runs (`workflow_dispatch` on
`trunk/<sha>`) were filtered out of the HUD, so their results —
including real failures — never reached the grid. And a multi-run cell
was merged pairwise, newest `job.id` wins, with the flaky flag taken
from the *loser*: a pass followed by a restart failure rendered plain
red, the reverse order rendered flaky `F`.

**Proposed solution.** Invariant: **who issued a run does not change how
the HUD aggregates it.** A push, a re-run attempt and a restart are all
just runs; a cell's verdict follows from the set of conclusions, not
from the issuer and not from arrival order. Origin is reported on the
tooltip, never aggregated on. Rules, in the new
`torchci/lib/mergeCellRuns.ts`:

- one run → show that run
- success + a real failure, in any order → show the success, marked
flaky `F`
- `cancelled` loses to any non-cancelled run, and is deliberately not
failure evidence (unlike `isFailure()`); `skipped` loses to a real
result, as before
- otherwise rank success > failure > pending > neutral > skipped, newest
run breaking a tie within a class

**What changes?** Restart results reach the grid on the same terms as
any other run, and a mixed cell renders `F` in either order. This
governs every multi-run cell, not just restarts — a twice-scheduled
periodic job or a re-run now follows the ranking above instead of
newest-id-wins, so an older success outranks a newer pending run.

Measured over 14 days on the 266 `pytorch/pytorch` trunk shas that had
restart runs (cells without a restart were not measured): **14,405 cells
differ from `main`** — 13,687 restart-only cells now show their real
conclusion, 208 turn `F` (restart failed, natural run passed), 195 the
reverse, 223 stop being `F` (a cancelled co-run no longer counts as
failure), 92 in smaller classes.

**Changed, at a high level**

- `hud_query` — admit restart runs; project the run's origin and
dispatching identity
- `mergeCellRuns.ts` + `types.ts` — the rules above and which run
represents the cell; unit-tested on both permutation axes (order,
issuer)
- `fetchHud.ts` — collect every run per cell and delegate; the old
reducer is gone
- `JobTooltip.tsx` — one radio per run behind the cell, with the grid's
own status glyph and a `gh` link to that run; picking one rebinds the
log viewer, links and failure classification to it
- `commit_jobs_query` — admits restarts that *passed* (all were hidden
before); its comment records the divergence below

**Known gaps.** A restart that *failed* still doesn't show on the commit
page: `fetchCommit` dedups by newest id with no flaky marker, so
admitting one would replace the natural conclusion instead of combining
with it. Grid and commit page disagree there until that page can render
`F` — tracked as a follow-up. `commit_jobs_batch_query` and its
`viable_strict_sole_blocker` mirror are untouched: `is_green()` has no
per-job-name dedup and vetoes on the first non-success row, so restarts
only add vetoes (17 currently-green commits would go red, 0 red would go
green) — #8300's second action item.

Refs #8300

---------

Co-authored-by: Ivan Zaitsev <izaitsevfb@meta.com>
Co-authored-by: Ivan Zaitsev <izaitsevfb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants