Skip to content

Nightly full-suite run with classified failure triage and automated git-bisect - #8606

Open
delock wants to merge 6 commits into
masterfrom
ci/nightly-full-suite-triage
Open

delock wants to merge 6 commits into
masterfrom
ci/nightly-full-suite-triage

Conversation

@delock

@delock delock commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Motivation

The master full suite on modal-torch-latest grew from ~52 to ~70 minutes within a month and keeps climbing ~7-8 min/week, repeatedly exhausting its Sandbox/job time budgets (the 3600 -> 4200 -> 5400s raises are chasing the growth). This PR stops paying the full-suite cost on every push and adds automated, noise-free triage for the nightly regression signal.

Changes

1. Master pushes select their own tests; nightly runs the full suite

  • push to master now diffs against the tip it replaced (github.event.before) instead of forcing all; the fetcher's fail-safe still widens to the full suite whenever the merge-base is unavailable.
  • A schedule (cron, daily) run always executes the full suite.
  • concurrency group now includes github.event_name so a nightly run and pushes don't cancel each other.

2. Controller classifies its own failures (ci/torch_latest.py)

  • infra (exit 75): the Sandbox never got a GPU instance — no test ran, no candidate information.
  • timeout (exit 124): the Sandbox lifetime budget was exhausted mid-run.
  • test (exit 1): everything else, raised with a full traceback as before.
  • Each class prints a DS_CI_FAILURE_CLASS=<class> sentinel line that survives into job logs even when the job is killed before exiting.

3. Nightly triage (nightly-bisect.yml, ci/nightly_classify.sh, ci/bisect_nightly.sh)

  • green → move the nightly-last-green git tag to the tested SHA (persistent good endpoint for bisect; operational failure days simply leave it in place and widen the next range).
  • infra → no-op (per design, no retry).
  • timeout → open an issue (a timeout is not bisectable; intermediate commits would time out too). A failed run with no sentinel is treated as a timeout (job was killed).
  • test failuresgit bisect run between nightly-last-green and the failing SHA, dispatching modal-torch-latest at each step (each step is a full run, up to ~105 min; the 6-hour hosted-job cap bounds the range). The culprit is reported in an issue. Inconclusive intermediate steps abort the bisect rather than skip, so the searched range is never silently narrowed.

4. Housekeeping: refreshed stale self-tests that no longer matched master (sandbox timeout 4200 -> 5400 left behind by #8571, merge-group trusted-ref expression, the baked-image FakeImage chain, and the removed reinstall Torch packages runtime command). ci/test_torch_latest.py is 24/24 green; pre-commit passes on all changed files.

Known limitations (why this is a draft)

  • Bisect steps dispatch the full suite (workflow_dispatch has no base ref); a nightly failure spanning many days of merges may exceed the 6-hour job budget and abort inconclusively. A follow-up could pass the failing test list as a subset to shrink steps to ~15 min.
  • Revisions that predate this PR's sentinel have ambiguous classification in bisect; the first bisect over a range crossing this merge may abort and need one manual rerun.
  • The nightly run doubles as an upstream-drift sentinel (it installs transformers from git main); red nightlies caused by upstream drift will bisect to a "culprit" only if the drift genuinely breaks our tests — the issue text keeps the run link for human judgment.

The master full suite has grown ~33% in a month (52 -> 70 min) and is
exhausting its time budgets. Instead of every push paying that cost:

- push to master now selects tests against the tip it replaced
  (github.event.before); the nightly schedule always runs the full suite
- the controller classifies its own failures as infra (no GPU instance),
  timeout (Sandbox lifetime exhausted), or test, via a
  DS_CI_FAILURE_CLASS log sentinel plus exit codes 75/124/1
- nightly-bisect.yml routes on that class: green moves the
  nightly-last-green tag, infra is a no-op, timeout opens an issue,
  and real failures git-bisect between nightly-last-green and the bad
  SHA by dispatching modal-torch-latest at each step, then report the
  culprit in an issue; inconclusive steps abort rather than skip so the
  searched range is never silently narrowed

Also refresh stale self-tests that no longer matched master (sandbox
timeout 4200->5400, merge-group trusted ref, baked-image fake, removed
'reinstall Torch packages' runtime command).

Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
A manual dispatch may now pin an explicit target list (test_targets
input, written through the same validate-selection allowlist as a
graph-selected list). The bisect job extracts the nightly's FAILED test
files and passes them to each step, cutting a step from a ~70-minute
full run to install overhead plus the failing tests. Targets missing at
a step commit are dropped from that step's dispatch (a test cannot fail
where it does not exist, but a different failing target may still
condemn the commit); a step where none survive is good without
dispatching.

Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
Two review findings, both confirmed:

- git bisect run treats any exit in 1..127 (except 125) as 'bad', so the
  previous exit 33 for inconclusive steps marked them bad instead of
  aborting; an infra/timeout on a middle commit could have been reported
  as the first bad commit without ever running its tests. Inconclusive
  now exits 129, which is outside the good/bad window and aborts.

- The dispatch API only accepts branch/tag refs: a real POST against a
  bare commit SHA returns 422 'No ref found'. Each step now publishes a
  temporary bisect/<run>/<sha> tag, dispatches by tag, and deletes it
  on exit; the bisect job grants contents: write for the tag push.

Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
Master receives no direct pushes; every landing is a merge-queue entry
whose merge_group run already tested the merged tree, so a post-push
Sandbox run duplicated that work on every merge. Remove the push
trigger and the github.event.before selection branch; cross-entry
interaction regressions remain the nightly full suite's job.

Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
Review follow-ups:

- A persistent regression failed the same way every night and opened a
  fresh issue each time (timeout / culprit / inconclusive). Reports now
  go through ci/nightly_report.sh: label nightly-triage, and a recurring
  outcome comments on its still-open issue instead of filing a new one.
  Titles are stable across recurrences (per-night SHAs live in the body);
  culprit titles keep the culprit SHA because a different culprit is a
  different regression.
- Triage/tag/bisect checkouts are pinned to the SHA the nightly actually
  ran (workflow_run.head_sha), making explicit that the classifier and
  the sentinel protocol it parses come from the same revision even when
  master has moved on overnight; the bisect bad endpoint is that same
  SHA, so newer master commits are outside the searched range anyway.

Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
The merge queue already gates every entry on its own impacted tests, so a
nightly full-suite failure is either a cross-entry interaction (worth human
judgment, which an issue provides) or an operational flake -- too rare to
justify the bisect machinery's cross-workflow dispatches, actions:write
token, and dispatch-by-tag lifecycle. Keep the pieces with standalone value:
the nightly itself, the controller's failure classification (infra/timeout/
test) that makes reports actionable, the nightly-last-green tag (the good
endpoint any future bisect would start from), and deduplicated issue
reporting for timeouts and real test failures.

Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
@delock
delock marked this pull request as ready for review September 22, 2026 04:03
@delock
delock requested a review from loadams as a code owner September 22, 2026 04:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7890eb2936

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

if: needs.triage.outputs.action == 'timeout'
permissions:
issues: write
labels: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Replace the unsupported labels permission

GitHub Actions does not define a labels token permission; label operations are covered by issues: write. Because this unknown key is present here and again in report-failures, GitHub rejects the workflow instead of registering the workflow_run trigger, so none of the nightly triage runs. Remove both labels entries; the supported keys are listed in GitHub's workflow permissions reference.

Useful? React with 👍 / 👎.

Comment on lines +35 to +36
permissions:
actions: read

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Grant contents access to checkout jobs

Specifying job-level permissions replaces the workflow-level defaults and sets every omitted permission to none, so this override removes the top-level contents: read. The immediately following actions/checkout therefore lacks its documented required permission; the two reporting jobs have the same problem after their own overrides. Add contents: read to every job that checks out the repository, as required by the checkout action's recommended permissions.

Useful? React with 👍 / 👎.

uses: actions/checkout@v7
with:
ref: master
fetch-depth: 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fetch the nightly commit before tagging it

When master advances while the roughly 70-minute nightly run is executing, this depth-one checkout contains only the new master tip, not workflow_run.head_sha. The subsequent git tag ... "$NIGHTLY_SHA" then fails with a nonexistent-object error, so successful nightlies do not update nightly-last-green. Checkout the nightly SHA directly or fetch it before creating the tag.

Useful? React with 👍 / 👎.

Comment thread ci/nightly_classify.sh
echo "failure_class=infra" >> "$GITHUB_OUTPUT"
echo "GPU capacity failure; no candidate information"
;;
timeout | none)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Distinguish missing sentinels from actual timeouts

A failed scheduled workflow can lack a sentinel for reasons other than a timeout: checkout, selector self-tests, artifact handling, dependency installation, or the controller's combined primary-and-cleanup-error path can all fail before printing one. Routing every such run through timeout | none therefore files a misleading timeout report and hides the actual failure; inspect the run/job result or report an explicit unknown class instead of assuming timeout.

Useful? React with 👍 / 👎.

Comment thread ci/nightly_report.sh
# Requires GH_TOKEN and GITHUB_REPOSITORY in the environment.

set -u

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate reporter command failures

With only set -u, failures from gh label create, gh issue list, gh issue comment, or gh issue create do not stop this script. In particular, a failed issue creation leaves number empty, after which the final echo succeeds and the workflow reports success even though no incident was filed. Enable error and pipeline propagation or explicitly check each gh invocation.

Useful? React with 👍 / 👎.

@@ -0,0 +1,172 @@
name: nightly-triage

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required license header

This newly added workflow starts directly with name: and omits the repository-required SPDX and DeepSpeed Team header. Add the prescribed two comment lines at the beginning of the file.

AGENTS.md reference: AGENTS.md:L13-L17

Useful? React with 👍 / 👎.

@@ -0,0 +1,172 @@
name: nightly-triage

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required sign-off trailer

The reviewed commit is non-merge, and git log -1 --format='%b' 560f43cdedaf9f5834e059cb015c7735a3ab74cd confirms that its message has no Signed-off-by trailer. Recreate the commit with the configured author's sign-off so it satisfies the repository's commit requirement.

AGENTS.md reference: AGENTS.md:L8-L9

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant