Repository navigation
ci: keep the nightly gate inside its job and name each running phase - #7070
Conversation
kixelated
changed the title
|
Design change: replacing option A with option B, as the maintainer chose in a
(Written by Claude Opus 5.5) |
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title accurately summarizes the main changes: keeping the Nightly gate within its job and reporting each running phase. Description check The description directly explains the timeout problem, the dispatcher changes, the Nightly and release-PR split, live phase reporting, tests, and validation results. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 5 files. (7 skipped: 7 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
quest/m0/nightly/gate-hang
- Resolve merge conflict in branch
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5485bf097
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: c5485bf0979e42f798047f3aee1ca90f82923b2a.
One actionable blocker, already reported independently: P1: clear inherited CHECK_MODULES in dispatcher test fixtures. I confirmed the code path: both new split callers export CHECK_MODULES=-rs_shapes, gh test runs this suite, and QueryTests.setUp preserves that variable. The new unconditional three-call assertion then excludes its expected feature-shapes-changed call. Clear the variable in the fixture and cover running the suite under the actual split environment; no duplicate inline comment added.
Overall direction is sound: separating the expensive sweep preserves the same crate selection and queue coverage rather than weakening it or raising timeouts. Release gate depends on Features, and the live phase logging retains the original stderr correctly. No additional actionable bugs found in the independent static pass.
Verification limits: no tests or hosted jobs run. The split's ability to finish within 60 minutes still needs a hosted measurement; reported local timings and test results were not independently verified.
(Written by OpenAI)
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 6fa5bb98cc618410226f5b463a946c321f86a8dd, compared with my last review at c5485bf0979e42f798047f3aee1ca90f82923b2a.
The prior P1 inherited CHECK_MODULES finding is fixed: QueryTests.setUp now clears the caller's filter before fixture subprocesses run, while explicit filter tests still supply their own values. No new actionable bugs found in this delta.
Direction: this narrow fixture-isolation fix preserves the split-gate design and its filter coverage without changing production selection. Verification was static GitHub inspection with an independent check; I did not run tests or hosted jobs. Hosted completion within the timeout remains unverified here.
(Written by OpenAI)
|
quest land removed this pull request from the queue: it conflicts with the base or the PRs queued before it in .github/workflows/check.yml, .github/workflows/nightly.yml, doc/setup/dev.md, sh/gh/queue.test.ts; merge the base, push, and land it again (run). |
…7070) * chore(quest): claim quest/m0/nightly/gate-hang.md Quest-Claim: fce0ca6d05eeb753 * ci: keep the nightly gate inside its job and name each running phase Nightly's gate did not hang: its Rust lane was still compiling the test build when the 60-minute job timed out. Since the isolated feature shapes (cargo hack, one clippy pass per package at both extremes) began running on whole-workspace selections, the lane outgrew the 4-vCPU hosted job. A whole-workspace selection now leaves that sweep to a Features job (`just rs feature-sweep`), which Nightly and release PRs run. Buffered dispatcher lanes also announce each phase's start and end on the run's stderr, so a run killed mid-phase names the phase it stopped in. * ci: split Nightly's gate across hosted jobs with a dispatcher module filter The maintainer kept the isolated feature shapes in the queue's gate for every selection and split Nightly's gate instead. The shapes become their own dispatcher module, rs_shapes, in the rs lane, and CHECK_MODULES (`just gate [MODULE|-MODULE]...`) runs or skips named modules. Nightly's Tests job runs `gate -rs_shapes` and Features runs `gate rs_shapes`; release-PR Check skips rs_shapes too, since Features runs on release PRs. This reverts the earlier whole-workspace skip and the separate feature-sweep recipe. * test(gh): pin the gate's split across Check, Tests, and Features * test(ci): clear inherited CHECK_MODULES in dispatcher fixtures Nightly's `gate -rs_shapes` and release-PR Check export the filter, which the fixtures inherited, skipping the rs_shapes module their assertions expect. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
647f500 to
9619a9b
Compare
Problem
Nightly's
Tests (gate)job has timed out at 60 minutes every day since 10-27: run 40506938673, 10-28, 10-29.It was not a hung test. The gate's Rust lane outgrew the 4-vCPU hosted job.
rslane took 2874s of that (check rs2122s,test rs752s), and 1023s of it was the two unified feature-shape passes.cargo hack, which runs one clippy pass per package at each extreme. Since then therslane has not finished inside the job. On 10-29 every other lane finished 24 minutes in, andrswas still running at the 60-minute timeout.dart, which runs alone, never started.Visibility: concurrent lanes buffer their output until the lane ends, and GitHub's timeout signals only the step's top process. So neither the lane log nor the dispatcher's timing report was ever printed.
Decisions
How to bring the gate back inside its job. Settled by the maintainer in a
/quest-completesession, replacing option A, which this PR first implemented.gateacross several hosted jobs, adding the module filter the dispatcher needs.Reason: the queue keeps isolated shapes for every selection, including lockfile, toolchain, and
.cargo/changes, instead of leaving them to Nightly.Changes
rs_shapesdispatcher module: the isolated feature-shape sweep (rs feature-shapes) moves out ofcheck rsinto a module of its own. It has the same impact pattern, sits in the same lane, and runs aftertest rs. So the queue andjust checkstill run it for every Rust selection, in the same lane, just as before. Only thelintpath ofsh/rs/select.sh, which the dispatcher calls, drops it. Thers check-changedandrs check-test-changedrecipes still run it.CHECK_MODULESfilter: it runs only the modules it names, or skips each one named with a leading-. An unknown name fails with the valid list.just gate [MODULE|-MODULE]...passes it through.gate -rs_shapes, and Features runsgate rs_shapes. Features runs on Nightly and on every release PR.Check: Check now runs only on release PRs, over--all, on the same 60-minute hosted job, so it setsCHECK_MODULES=-rs_shapes. Their Features job runs the sweep. It passes today only becausereleasehas no feature shapes yet (moq-dev/moq#7202: 20 minutes). The next release cut would have brought the same timeout.ci: test rs started,ci: check rs passed in 2122s) on the run's stderr. A run killed mid-phase now names the phase it stopped in, and each phase's time is logged even when the final report never prints. These lines replace the per-laneci: started <modules>line.dispatch-query.test.pychecks thers_shapesorder in its lane, both halves of theCHECK_MODULESsplit, and the error for an unknown name. The cancellation test also checks that the running phase's start line reaches stderr while the phase is still blocked.select.test.pychecks thatlintleaves the shapes to the newfeature-shapesaction.sh/gh/queue.test.tspins the split: release-PRCheckskipsrs_shapes, Features runsgate rs_shapes, and Nightly's Tests runsgate -rs_shapesinstead ofgate.README.mdanddoc/setup/dev.mddescribe the filter and the split.Public API: none. Wire: none.
Choices made without asking
rs_shapesthere costs one env line and no new job. The alternative, splittingCheckitself, would rename the requiredCheckcontext or need an aggregator job.cargo hack --partition. The 10-26 numbers put the gate without shapes at about 31 minutes forrs. I have no hosted measurement of the shapes alone. The live phase lines will log it on the first Nightly, and--partition M/Nacross Features entries is the next step if it runs long.rs_shapesshares therslane. Both compile into onetarget/and would only queue on Cargo's build-directory lock in separate lanes. This keeps queue behavior unchanged.Checks
sh/dispatch-query.test.py(28 OK),sh/rs/select.test.py(17 OK),sh/gh/queue.test.ts, shellcheck, shfmt, actionlint,just --fmt --checkon both justfiles, andquest check --base origin/main.just ci origin/mainpassed every module exceptcheck flake. That one failed because this host forbidsunsharewhile building thegithub-runnerVM image, a derivation this PR does not touch. Locallycheck rs_shapestook 386s besidecheck rs339s andtest rs184s.CHECK_MODULES=-rs_shapes python3 sh/dispatch-query.test.py: 28 OK, after the fixture fix in 6fa5bb9.bun test sh/gh/(19 pass),bun sh/gh/queue.test.ts,actionlint,sh/dispatch-query.test.py, andsh/rs/select.test.pypass.just check origin/mainpassed every Rust/JS module includingcheck rs_shapes(516s);c,cpp,obs_compile,kt,go, anddartfailed because this host's mbx cargo shim rejects thecargo build-ffialias (unexpected argument 'build', reproduced standalone), andflakewas skipped for theunsharerestriction. None touch this PR's files.Review
quest review, reviewer Codex gpt-6.1-sol, effort medium.origin/mainbdc4785 with hand-resolved conflicts): 0 findings.merge_groupand runs Check only on release PRs withCI_BASE: --all, so Check now setsCHECK_MODULES: -rs_shapesunconditionally andsh/gh/queue.test.tspins it as a plain value. Nightly keeps main's newtest resume-modelrecipe besidegate -rs_shapes.doc/setup/dev.mdkeeps main's wording plus the split.Suggested follow-ups
select.shisolates seeds plus every dependent, so a moq-net change costs about 26 minutes of isolated clippy on the queue runner (run 40624404802). The 2026-10-24 decision says the gate checks shapes for "the crates a branch changes".##[group]for each package, and GitHub does not nest groups.--log-group noneinrs feature-shapeswould keep the dispatcher's grouping.target/and wait on Cargo's build-directory lock, soCHECK_JOBS=3on a 4-vCPU runner mostly queues them.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code