Conversation
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>
There was a problem hiding this comment.
💡 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 |
There was a problem hiding this comment.
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 👍 / 👎.
| permissions: | ||
| actions: read |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 👍 / 👎.
| echo "failure_class=infra" >> "$GITHUB_OUTPUT" | ||
| echo "GPU capacity failure; no candidate information" | ||
| ;; | ||
| timeout | none) |
There was a problem hiding this comment.
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 👍 / 👎.
| # Requires GH_TOKEN and GITHUB_REPOSITORY in the environment. | ||
|
|
||
| set -u | ||
|
|
There was a problem hiding this comment.
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 | |||
There was a problem hiding this comment.
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 | |||
There was a problem hiding this comment.
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 👍 / 👎.
Motivation
The master full suite on
modal-torch-latestgrew 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
pushtomasternow diffs against the tip it replaced (github.event.before) instead of forcingall; the fetcher's fail-safe still widens to the full suite whenever the merge-base is unavailable.schedule(cron, daily) run always executes the full suite.concurrencygroup now includesgithub.event_nameso 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.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)nightly-last-greengit tag to the tested SHA (persistent good endpoint for bisect; operational failure days simply leave it in place and widen the next range).git bisect runbetweennightly-last-greenand the failing SHA, dispatchingmodal-torch-latestat 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
FakeImagechain, and the removedreinstall Torch packagesruntime command).ci/test_torch_latest.pyis 24/24 green; pre-commit passes on all changed files.Known limitations (why this is a draft)
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.