Notify Slack when the tests of the latest release fail - #6900
Merged
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
qgallouedec
approved these changes
Aug 24, 2026
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.
This PR makes the
Tests latest TRL release with dev dependenciesworkflow notify Slack when it fails.Alternative to #6899, which removes the workflow instead. The two are mutually exclusive, pick one.
Motivation
The
Post to Slackstep of that workflow has noif:, so it inherits the defaultif: success()and only runs when every preceding step succeeded. It can only ever post🟢 Tests are passing!, andstatus: ${{ job.status }}is alwayssuccesswhen it is evaluated.The consequence is that the workflow has never reported a failure. 29 of its last 100 runs failed, including six consecutive days from 2026-07-23 to 2026-07-28, without a single notification. It is the only call site missing the condition:
slow-tests.yml,tests_python_versions.yml,tests.yml,tests_transformers_branch.ymlanddocker-build.ymlall gate their Slack step onalways()or on an explicit condition including it.Which one to pick
The case for removing it (#6899) is that it spends a GPU runner nightly on a signal nobody reads, and that the failures it does produce are not the ones it exists to catch. The 2026-07-24 one is representative:
That is the T4 of
aws-g4dn-2xlarge, compute capability 7.5. The workflow is on that runner group rather than the L40S used elsewhere because it checks out an old release branch, so its results diverge from the rest of our CI.The case for keeping it is that nothing else covers what it covers:
tests.ymlruns the same three dependencies from git in itsTests with dev dependenciesjob, but it has noschedule: it only runs on pushes. Breakages coming from upstream are driven by the calendar, not by our pushes.tests_python_versions.ymldoes run daily, but installs".[dev]"only, so it gives no signal on development versions.So this is currently the only daily signal against the development versions of
accelerate,datasetsandtransformers, and it also covers the released branch rather thanmain, which matters because we ship patch releases.There is evidence of what it catches, which @qgallouedec documented in #6899 (comment) and which is worth recording here, since #6899 will be closed and this is the PR that stays in the history of the file:
start_timeargument toTrainer.log(), merged 2024-11-19, and broke the released TRL when it shipped in transformers 4.47.0 on 2024-12-05. A user reported it in TypeError: DPOTrainer.log() takes 2 positional arguments but 3 were given #2445 before we noticed, an emergency pin went out the same day as Pin transformers version <4.47 #2447, released as v0.12.2, and the workflow was added four days later in ⚖️ Addtests_latest.ymlworkflow file #2457 so that users are not our canary. It could have been caught 18 days earlier.mm_token_type_ids, merged 2026-02-24) broke us again.mainwas fixed two days later in Handle mm_token_type_ids in SFT/GRPO/RLOO to fix IndexError #5178, so the dev-dependencies job onmainwent back to green, while users on v0.29.0 hit theIndexErrorfor real once transformers 5.3.0 shipped on 2026-03-04. For the 16 days until v0.29.1 delivered the fix on 2026-03-20, this nightly was the only CI reporting the breakage.That second incident is the point the analysis above misses: the dev-dependencies job on
maincannot catch this class of breakage by design, becausemaingets fixed within days and its CI returns to green while the released version stays broken. This workflow reports what users will hit when the next upstream release ships, which is also what tells us what the next patch release needs.Changes
if: always()to thePost to Slackstep oftests_latest.yml, so failures are reported like in every other workflowNote
Low Risk
One-line CI condition change; no application, auth, or data-handling code is modified. Only Slack notification behavior on job failure changes.
Overview
Makes the daily
tests_latest.ymlSlack step run even when tests fail by addingif: always().Previously the step inherited GitHub’s default
if: success(), so it never posted failures. This matches how other CI workflows already gate Slack notifications.Reviewed by Cursor Bugbot for commit af2f656. Bugbot is set up for automated code reviews on this repo. Configure here.