feat(run): record the resolved scheduler in run receipts - #543
Merged
Conversation
The DAG ready-queue scheduler became the roster default via limits.scheduler,
but run.json never recorded which scheduler actually ran. The DAG-to-waves
fallback printed only to stderr, so a receipt could not distinguish a real DAG
dispatch from a silent degrade to waves, and no DAG-vs-waves comparison was
possible from run history.
run.json now carries a scheduler block:
"scheduler": {"requested": "dag", "used": "dag", "fallback_reason": null}
- requested is written at record_run_start, so a run that dies before dispatch
reports what it was asked to do instead of claiming a mode it never reached.
- used and fallback_reason are filled by a new on_scheduler_resolved callback
out of run_transport.dispatch, following the existing on_stage_start and
on_interrupt callback pattern. used stays null until dispatch resolves it.
- record_run_termination merges into the existing payload rather than
overwriting, so the field survives startup failures.
- Scheduler precedence (--scheduler, then roster limits.scheduler, then waves)
now lives in one helper, cli/run.py::_resolved_scheduler, instead of being
spelled out separately at each call site.
Co-authored-by: Claude <noreply@anthropic.com>
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary
The DAG ready-queue scheduler (#447) became the roster default via
limits.scheduler(#472), butrun.jsonnever recorded which scheduler actually ran. The DAG-to-waves fallback printed only to stderr, so a receipt could not tell a real DAG dispatch from a silent degrade to waves.run.jsonnow carries:requestedis written atrecord_run_start, so a run that dies before dispatch says what it was asked to do rather than claiming a mode it never reached.record_run_terminationmerges into the existing payload, so the field survives startup failures.used/fallback_reasonare filled by a newon_scheduler_resolvedcallback out ofrun_transport.dispatch, following the existingon_stage_start/on_interruptcallback pattern.usedstaysnulluntil dispatch resolves it.--scheduler, then rosterlimits.scheduler, thenwaves) now lives in one helper,cli/run.py::_resolved_scheduler.Why this was blocking
An audit of the 25 most recent runs in the maintainer's checkout found that of the 20 with a parsed plan, only 2 were DAG-eligible, and only 1 of those ran after the roster flip. The other 18 were single-worker (
--worker) runs whose assignments carry nocovers, so_dag_placement_errorreturns"plan not fully covered"and they degrade to waves every time.That degrade is correct behavior, but it means any DAG-vs-waves comparison must use multi-seat runs only. Counting all runs understates DAG engagement to near zero. Without the scheduler field there was no way to establish that from receipts, which left the route-recomposition decision (
docs/proposals/route-recomposition.md) undecidable.Blast radius
Established with GraphTrail before the first edit:
_dag_placement_error<-run_transport.dispatch:625<-aboyeur.dispatch:1196. Two hops, contained._run_payload<-record_run_start:2128and theaboyeur.run._payloadclosure:2315, whichaboyeur.runcalls at 11 sites. Injecting the field in the closure covers all 11 at once rather than editing each.run.jsonconsumers (release_cmd/candidate.py,release_cmd/commands.py) read existing keys only; the new key is additive.Tests
Four new tests, written red-first (failing receipt
20260726-065836-work-verify-e65eb5, exit 1, before implementation):test_scheduler_resolution_reported_when_dag_engagestest_scheduler_resolution_reports_wave_fallbacktest_scheduler_resolution_reports_missing_route_dependenciestest_run_cli_records_requested_scheduler_before_dispatchVerification
brigade work verify run --target . --command "./scripts/verify" --capture brigade-workReceipt
20260726-070342-work-verify-edd4dd:completed, exit 0, 505.3s, 4218 passed / 3 skipped, coverage 82.67% (floor 78%). Tree fingerprint54dc40a635bcac6411bca7b88fa1f54a078c79c97cfa8de4bbf995860c5f8fd5.Smoke against the branch source wrote
"scheduler": {"requested": "dag", "used": null, "fallback_reason": null}to a dry-runrun.json(usednull is correct: a dry run never dispatches).Caveat: that green predates #542.
./scripts/verifycurrently fails on this machine with 6 failures intests/test_repos_sweep_health_cmd.py, which reproduce on a clean checkout ofmainand are unrelated to this diff. The gate should be re-run once #542 is resolved.Follow-ups
brigadeis pipx 0.25.1 and still writes scheduler-less receipts;pipx install --forceis needed before this affects real runs.