parallel-make-check: warn when a job runtime drifts >50% from "minutes"#10684
Merged
dgarske merged 3 commits intoJun 15, 2026
Merged
Conversation
The "minutes" field is only a scheduling estimate; when it goes stale it just packs the schedule a little worse, and there was no signal that a value needed updating. Emit a non-fatal warning when a config that explicitly sets "minutes" finishes more than 50% above or below it (a GitHub ::warning:: annotation in CI, a plain line locally) and flag the row in the step-summary table with the value to copy over. Configs that omit "minutes" keep riding the 1.0 default and are left alone. The warning never touches the exit status, so it cannot fail the job.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a non-fatal signal to CI/local runs when a config’s explicitly provided minutes estimate has drifted significantly from the measured runtime, making stale scheduling weights easier to spot and update.
Changes:
- Track whether
minuteswas explicitly provided in the JSON config (vs default placeholder). - Emit a per-config warning when runtime is >±50% away from the estimate (GitHub Actions annotation in CI; plain warning locally).
- Add a
:warning:note in the step-summary table row when drift is detected.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A config name comes from JSON and is only checked for emptiness and a '/', so it can carry %, CR or LF. Passed straight into the ::warning:: workflow command those would truncate the annotation or be parsed as a second command, so escape them in the GitHub branch of warn() per GitHub's documented command-data encoding (% first). Local output is unchanged.
…w command Addresses review feedback: - The "minutes" header comment described the check backwards (the estimate drifting from the measured time). Reword it to match the code, which warns when the measured time lands more than +/-50% away from the estimate. - Centralize the GitHub workflow-command escaping in gh_escape() and apply it to the ::group:: title in dump() and the ::error:: summary in main(), not just warn(), so a config name or step carrying %, CR or LF cannot corrupt those commands either.
|
retest this please |
dgarske
approved these changes
Jun 15, 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.
The
minutesfield inparallel-make-check's config JSON is only ascheduling estimate: configs run longest-first and
--shardbalances shardsby it. When a value goes stale it just packs the schedule a little worse, and
until now there was no signal that it needed updating.
This adds a non-fatal warning when a config that explicitly sets
minutesfinishes more than ±50% away from it:
::warning::annotation in CI (a plainWARNING:line locally),printed next to the per-config pass line, and
:warning:note in the step-summary table row, carrying the measuredvalue to copy over.
Configs that omit
minuteskeep riding the1.0default placeholder and areleft alone. The warning never touches the exit status, so it cannot fail the
job — it's just an easy-to-spot nudge to refresh a drifted estimate.
Touches only
.github/scripts/parallel-make-check.py(CI tooling).