Snapshot test the argv of all model scripts - #1908
Merged
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
July 29, 2026 01:02
58a7ede to
23a86aa
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
2 times, most recently
from
July 29, 2026 01:45
1d8d111 to
e0ec16e
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
July 29, 2026 01:45
23a86aa to
9b6824a
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
from
July 29, 2026 02:06
e0ec16e to
7735661
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
July 29, 2026 02:06
9b6824a to
797680e
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
from
July 29, 2026 02:26
7735661 to
c749c4b
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
July 29, 2026 02:26
797680e to
1b8df7a
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
from
August 4, 2026 04:13
c749c4b to
c8a0daf
Compare
fzyzcjy
requested review from
Shi-Dong,
Zhichenzzz,
guapisolo,
jybsuper,
maocheng23 and
yueming-yuan
as code owners
August 4, 2026 04:13
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
August 4, 2026 04:13
1b8df7a to
265d781
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
from
August 8, 2026 02:38
c8a0daf to
7863e78
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
August 8, 2026 02:38
265d781 to
c7ef431
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
from
August 8, 2026 06:36
7863e78 to
0e0a105
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
August 8, 2026 06:36
c7ef431 to
78c5dd4
Compare
Squashed from: - Fix PYTHONBUFFERED typo in launch scripts and command utils - Fix the same typo in the NPU docker patch - Unbuffer the ray workers, not only the submitting client - Unbuffer the launchers that submit ray jobs of their own
Squashed from: - Add a shell launch script test harness - Make the shell harness report stderr and emit shim stdout correctly - Drop the deprecated huggingface-cli shim - Exercise the shim behaviours the single real script never reaches - Poll the ray cluster the way the real scripts do in the synthetic script - Group the harness tests by what they exercise - Make the harness record commands in fork order and refuse to be unfrozen
…r paths Squashed from: - Fix launch scripts whose model config path could never resolve - Fix two launcher entrypoints that raised before issuing any command - Cover the two regressions this op fixes
… scripts Squashed from: - Derive the miles checkout location instead of hardcoding it - Quote the derived train.py path and pin the invariant - Find the shell scripts without shelling out to git
Squashed from: - Snapshot the external commands of every shell launch script - Apply isort and black to the shell launch script test - Intercept ps so the recordings do not read the host process list - Harden the harness against host state the rollout exposed - Share the snapshot compare-or-update step and stop running each script twice - Keep the generated snapshots under one obvious tests/snapshots tree - Group the launch script tests by subject - Name the shell launcher test after what it covers - Assert the recorded order for every script, including the concurrent ones - Regenerate the concurrent launcher's snapshot in its true command order
ExecuteTrainConfig.num_nodes read SLURM_JOB_NUM_NODES into a class-level
default, so the value was fixed when command_utils was imported. A test that
wants a deterministic launch command cannot undo that with monkeypatch, and a
process that sets the variable after import does not see it either.
A default_factory reads it at construction instead, but dataclass_cli copied
the parameter's declared default straight into the click signature, and for a
factory field that default is dataclasses' _HAS_DEFAULT_FACTORY sentinel,
which click then type-casts:
TypeError: int() argument must be ... not '_HAS_DEFAULT_FACTORY_CLASS'
Every scripts/run_*.py exposes this config through that bridge. Resolve the
factory when the signature is built, the way the argparse bridge already does.
…ript Squashed from: - Snapshot the commands built by every python launch script - Apply isort and black to the python launch script test - Reuse the shell harness sanitizer and snapshot helper - Move the python launcher snapshots into the shared tree too - Share the command recorder with the command_utils tests - Freeze the launcher environment that the snapshots actually depend on - Regenerate the launcher snapshots for the ray runtime unbuffering - Snapshot the config files a launcher generates, not just its commands - Record the generated precision config in the deepseek-v4 snapshots
Squashed from: - Cover the public surface of command_utils with unit tests - Group the command_utils tests by the function under test - Close the gaps that let the command_utils tests pass on broken behaviour - Keep the command_utils tests in one file
Squashed from: - Rename exec_command by the resource its command needs - Point the nvlink and single-node conversion tests at the gpu helper - Re-record the multi-node label the rename changed - Rename the last two exec_command call sites the split missed - Re-record the multi-node label in the rsync_simple test too
Squashed from: - Move the shell exec helpers next to their only consumers - Carry NodeAffinitySchedulingStrategy along with the moved exec helpers - Stop patching command helpers on a module that no longer has them
…yloads Squashed from: - Accept inline base64 payloads for the config file arguments - Pass config documents inline instead of through a temp file - Make the inline config payload reach every consumer and fail loudly - Apply pre-commit import ordering - Regenerate the deepseek-v4 snapshots for the inline config payload
Squashed from: - Snapshot the launchers that build their own command line - Record what the self-executing launchers submit today - Freeze the pid these launchers embed in their cleanup command
…ures Squashed from: - Let a p2p profile's rotary_base reach the model script it configures - Test the model args command run.py actually builds, not a copy of its logic - Flip the p2p snapshots to the rotary base each profile declares
The next ops rewrite all 62 scripts/models/*.sh into python. Once the shell versions are gone there is no source of truth left to prove the rewrite was faithful, so record the argv each of them expands to now. What these golden files pin is agreement with the shell era, not merely agreement with today's behaviour; the rewrite may only change the producer, never these files. They also give the 18 models that no launcher snapshot reaches their first coverage of any kind.
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-16
branch
from
August 9, 2026 10:00
0e0a105 to
4e3fcfd
Compare
fzyzcjy
force-pushed
the
tom/refactor-miles/op8-17
branch
from
August 9, 2026 10:00
78c5dd4 to
f9ce73b
Compare
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
1 participant
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.
Part of #1837