CONTRIBUTORS.md

Contributors

Soup is built by its community. Thank you to everyone who has contributed code, tests, docs, and ideas. ❤️

This list is maintained by hand alongside the GitHub contributors graph. Merged a PR and don't see yourself here? Open a PR adding your line — that counts too.

Maintainer

• Alpamys (@MakazhanAlpamys) — creator & lead maintainer

Contributors

Listed by first contribution. PR numbers link the work.

• Salil Mhatre (@Deadpool2000)

  • soup version --json for machine-readable CI output (#6)
  • RAM + disk-space checks in soup doctor (#7)
  • soup runs clean for smart checkpoint space management (#9)
  • Official Docker support for easier onboarding (#20)
  • soup bench — model speed + VRAM measurement (#25)
  • --prompts-file option for soup bench (#30)
  • Happy-path + CPU-warning tests for soup bench (#31)
  • soup cost — cloud GPU training cost estimation (#42)
  • --nccl flag for soup doctor multi-GPU bandwidth checks (#178)
  • Ready-made qwen2.5-coder-7b-sft recipe (#285)
  • soup data split --stratify-semantic — a random split can leave a whole topic out of the validation set, so a regression in it is invisible; rows are now clustered by meaning and each cluster split proportionally (#388)

• Chinmaya Sahu (@csking101)

  • DPO example config, sample data, and tests (#48)
  • FP8 rowwise + rowwise_with_gw_hp scaling recipes (#62)

• Yixuan Xu (@mzl2233)

  • Guard diagnose-gate on distributed worker ranks (#169)

• dreamer0129 (@dreamer0129)

  • Rich-markup escape fix in legacy soup adapters commands (#175, adopted in-tree as #174)

• Vivaan Dhawan (@VIVAAN-DHAWAN)

  • Reject pickle/zip streams renamed to .safetensors via magic-byte check (#198)

• Shivam (@shivam2931120)

  • Tokenizer-aware repetition scoring for the echo-trap detector (#242)

• gittihub-jpg (@gittihub-jpg)

  • Manifest-level dotted-path custom transforms for soup build (#255)
  • --energy flag for soup bom emit — thread energy/CO₂ into the ML-BOM (#256)

• shatakshi-1404 (@shatakshi-1404)

  • Unit tests for the warmup.py auto-warmup-steps helper (#274)

• Kondamwar Akshaya Shrikant (@Akshaya-reddy18)

  • Friendlier error messages — richer CUDA-OOM hint + Hugging Face gated-repo and trust_remote_code mappings + tests (#282)

• Darsh (@CODING-DARSH)

  • Harden judge-URL validation against hostname prefix bypass (startswith → urlparse) in eval/gate.py (#288)
  • Apply configured vocabulary expansion (data.add_new_tokens / new_special_tokens) during SFT trainer init (#287)
  • Reuse the shared vocab-expansion helper in the vision + audio SFT paths (#291)
  • Honor configured vocab expansion in the DPO / IPO / KTO / BCO trainers (#293)
  • Honor configured vocab expansion in the ORPO / SimPO / GRPO trainers (#295)
  • soup mcp serve --allow-execute — the execution gate, kept a separate and stronger opt-in than --allow-mutating, with the tools still plan-only in this slice (#391)
  • Gated train_execute / export_execute behind a single-use server confirmation token, with the config snapshotted at plan time and protected directories digested by content rather than by mtime (#393)
  • Corrected a contributor's handle carried in the v0.73.2 CHANGELOG — a one-line fix to somebody else's credit, which is the kind of thing that normally goes unmade (#400)
  • Cross-tokenizer speculative drafts: a small model that would make a fine draft was unusable unless it happened to share a vocabulary. Four rounds, and what each one actually produced was a control that did not yet exist — a _FakeTarget that genuinely implements the repetition penalty (without it the fix was indistinguishable from a no-op), a _MAX_ALIGN_CHARS test that is not itself a 300-second wall-clock guard, and for the skip path both the (0,0) fixture AND the control that an extendable prompt still counts, since (0,0) alone is satisfied by a function that always skips. Then a clean textual merge of main silently restored the original symptom — #409's contract landed underneath and the assisted arm ran twice, the second call discarding the first's result — and they fixed it plus pinned the wiring, which had passed 163 tests while being deletable, because every CLI-level patch was lambda *a, **k (#417)

• Ekaanksh Patil (@Ekaanksh-dev)

  • Batch the PRM reward forward pass in PRMScorer.__call__ (single [B, T] forward) (#301)

• Sanjay Santhanam (@Sanjays2402)

  • Run built-in benchmark gate tasks through ForgettingDetector — every type: benchmark eval-gate task had always failed (#315)

• Nicolás Ramos (@nicolasramos)

  • backend: mlx was never dispatched — every MLX run trained through the transformers wrapper instead, and the saved MLX "adapter" was a full fine-tune because the model was never frozen before LoRA (#362)

• William Yang (@wilyan09007)

  • training.seed reached the SFT wrapper and nothing else — seventeen other task wrappers trained at HF's default 42 with no error, so replicates that differed only in the seed were the same run; the seed is now applied before the adapter is drawn, not only inside Trainer (#381)
  • Under use_fsdp2_compile, every checkpoint-* kept torch.compile's key prefix and resumed silently from a re-zeroed adapter — normalisation now runs as each checkpoint is written, ahead of anything that publishes it (#380)

• Amir Fathi (@AmirF194)

  • Repaired live flat-mode multipacking where the DataLoader ignored training.batch_size and capped each FFD bin at one sequence instead of the documented batch-sized token budget (#562).
  • Fixed transient non-finite metric handling by considering the newest logged value for each finite-state metric before artifact publication, with a regression mutation check (#560).
  • Honored training.quantize_reward_model in both PPO and reward-model training loaders, with explicit flag-off regression coverage (#586).
  • A streamed model's named_parameters() carried the wrapper's .inner. segment, so a name-keyed comparison against a resident model shared no names at all and a correctness gate reported 0/0 as a pass (#384)
  • training.stream_vram_override — the layer-streaming pre-flight measured free VRAM with a device-level driver query, so it could not see a per-process cap and there was no way to make it simulate one (#386)
  • The VRAM pre-flight never called its own calibration hook, so the guard against a stack whose loss path under-budgets by 12.5% sat inert with no caller (#390)
  • kl_control re-wrote the same β on every hold step, so a non-acting run was not the no-op log_only claims to be; the mitigation log now carries held / acted / released as a field rather than as free text (#414)
  • training.use_cut_ce: true was accepted and did nothing on every locally-produced checkpoint, because the architecture was matched against the model PATH rather than its config — the same defect v0.73.0 fixed for Liger. They found a second one on top of the reported bug: phi-2 dispatched to the Phi-3 patcher, which patches Phi3ForCausalLM and therefore patched nothing, while apply_cut_ce returned True and the run printed "Cut Cross-Entropy enabled" — success reported for work not done. Told the accurate version was stronger than their own framing, they took it rather than defending the original (#446)
  • live_eval.load_model_and_tokenizer had no quantization parameter, so every live-eval path loaded the base at full precision and an NF4-trained adapter was judged on a bf16 base it never saw. The reason this is worth reading is what happened when it was blocked: the parameter was threaded but no caller could reach it, and the CHANGELOG described five fixed commands — the exact "validated, documented, read by nothing" shape v0.73.3 is named for. Offered the choice of wiring it or splitting honestly, they split, and the entry now LEADS with "no live evaluation path sets it yet" and states the surviving user-visible defect in plain terms. They then closed two mutations that had survived, one of them the wire to get_compute_dtype() — the function v0.73.1 shipped for the T4 including_emulation trap, connected untested (#461)
  • build_optimizer_param_groups skips the catch-all base group when every parameter matched an lr_group pattern, and that branch had no test. Their one test is the sole guard on all three mutations I ran, including the realistic if base_bucket is not None: regression — an empty list is not None, so an empty base group gets emitted. It also pins group ORDER, which nothing did before and which matters more than it looks: optimizer.state_dict()["param_groups"] is positional, so a silent reorder breaks optimizer-state resume. And it deliberately does NOT assert dict equality, so adding a key to the emitted groups will not redden it — the restraint that keeps a new test from becoming a guard that fires on correct code (#469)
  • downsample()'s endpoint-pin branch was uncovered, and the gap was not the branch — it was that test_long_capped bounds the result with <=, so a regression in the stride arithmetic sails through it. Their test is the only thing that catches a + 1 there: 14 of 15 pass under that mutation. It asserts the exact row list plus an identity check on the endpoint, which is the right instrument for a branch guarded by is not. They also accepted that the assertion incidentally pins today's stride formula — which contradicts downsample's own docstring — and let it be filed as #473 rather than widening the PR (#470)
  • Two GPTQ export bugs that each made the artifact unusable: a quantize(tokenizer) fallback that could never work, and a shard filename the standard transformers loader cannot find. They got a real auto-gptq install running to prove the second — same weights, old name gives the issue's exact OSError, new name loads clean — which nobody here had done. Scope was honest without being asked: Refs #338, not Closes, leaving the packaging half (no wheel on 3.12, transformers pinned back to 4.52.4) explicitly open. On review they closed three gaps including the one that mattered most — two of their tests passed only where auto-gptq was absent, i.e. failed on exactly the machine the feature exists for (#475)
  • Spotted that #339/#471 fixed the frozen-base load dtype in the SFT wrapper only, filed #491 as an explicit follow-on crediting that work, and covered the other twelve trainers — checking the premise rather than assuming it, by confirming all twelve apply PEFT unconditionally with no full-FT branch. They used torch_dtype=, not the dtype= spelling that had already shipped broken elsewhere, and put the resolver in utils/gpu.py so there would be one implementation rather than two. Review found a real numerics regression their own fix introduced: peft upcasts LoRA adapters to fp32 but never ModulesToSaveWrapper, so the SEQ_CLS reward head — the one parameter whose output is the reward signal — would have trained in pure bf16. Their fix is scoped to requires_grad, so the frozen base stays bf16 and the PR does not undo its own VRAM saving; and the test pins both halves, carrying an explicit assert base_dtype == torch.bfloat16 control commented as proving the fixture reproduces the precondition rather than passing because everything is fp32 already (#492)
  • Closed two never-worked-once export paths in one day. TensorRT shelled out to a tensorrt_llm.commands.convert_checkpoint entry point upstream no longer ships, dying mid-subprocess instead of at the flag; their guard checks the entry point right after the lazy availability probe — no eager import added — and the error names what is missing, where conversion moved, and how to install it. The review mutation proved the test enforces the ordering that matters: neutering the raise dies on run was called ZERO times, i.e. abort before any subprocess. The MCP half blocks restart capacity off a persisted launching row committed before Popen(), extending #402's predicate and #424's shared liveness primitive rather than adding a third copy — with a test pinning that reconcile-on-read deliberately leaves launching rows alone (#508, #506)
  • Wired live-evaluation quantization and fallback dtype through every generator path, recognized indexed CUDA devices, and documented the remaining evidence/staleness follow-up honestly (#570).
  • Traced a dependency floor to the exact upstream commit rather than guessing at it: accelerate 0.27.0 is the first release whose save_fsdp_model / load_fsdp_model accept adapter_only, which transformers 5.16.1 feature-detects to keep a PEFT run's FSDP resume checkpoint to the adapter — so the old >=0.25.0 floor let pip resolve a version where the detection returns {} and the full frozen base is written into every checkpoint again. The part worth copying is what they declined to claim. Told the fix did not cover the issue's third acceptance criterion, they did not add a mocked round trip to close the box: they explained that Soup imports transformers.Trainer unmodified and the save/load happens inside accelerate, so a Trainer.train() test would mostly exercise upstream, and offered to build a real single-process gloo FSDP group if I wanted it at GPU-CI cost. I told them not to, and #352 stays open on that one item by agreement rather than by omission. They also corrected the framing on request, so the changelog says the bump prevents a resolvable regression rather than implying it fixed the measured 37 GB write — which the transformers>=5.16.1 floor had already closed (#352, #591)
  • Closed the half of #623 I had left open after #647, and picked it up from the issue thread rather than waiting for me to file it. The RAM tier's unpinned construction was unprotected on every CPU-only CI cell: before #647, mutating RamSource's pin_memory=self.pinned to True left 169 pre-existing streaming tests green, and after #647 it was caught only behind a CUDA gate, so nine of nine cells were blind to it. Both hops now die with no GPU — I re-ran them to confirm. What makes it reach the deeper hop is that it asserts the ALLOCATION (source.get(0, "weight").is_pinned() is False) rather than the recorded source.pinned attribute; an attribute assertion passes that mutation unchanged, which is precisely how the gap survived. Drives the real, unmocked _build_source against a genuine safetensors shard on disk, and corrects the file docstring its own change makes false. They also named the structural limit instead of implying full coverage: the mirror "never pin" direction cannot be tested without a device, so it stays CUDA-gated (#623, #653).

• Ben Younes (@ousamabenyounes)

  • MitigationLogWriter dropped every record in silence once its parent directory vanished mid-run — the controller kept acting while its evidence stopped growing (#398)
  • soup draft distill --steps N delivered only ~N/4.44 optimiser steps — val_split and gradient_accumulation_steps both divide the budget, and the epoch arithmetic ignored them (#399)
  • The soup ship MCQ scorer read �oxed {A} as no-answer — LaTeX permits a space before the brace and models emit it, and the cue tier cannot rescue it (#396)
  • --noise-floor shipped without a config surface, so it was the one soup ship gate-policy flag that could not be committed to soup.yaml; the bounds import from ship_verdict so the schema and the CLI validator cannot disagree (#410)
  • A dead MCP watcher left its run at running in the tracker forever — reconciled on read, with a Windows liveness branch because os.kill(pid, 0) there sends a console Ctrl+C rather than checking existence (#407)
  • The one-active-execution cap lived in process memory, so a restarted MCP server could double-book it (#408)
  • The soup ship leg-1 noise floor was measured in --task-mode metric only, so in the judge modes a win smaller than the instrument's resolution still counted; it is now measured everywhere and labelled, so a decode-only floor is distinguishable from one carrying judge variance (#419)
  • detect_disk_kind could not see through virtio, so a 1.5 GB/s cloud disk was classified HDD and refused the streaming tier — and when review found that the fix cited a rate from module state the cache never reset, producing 'hdd' (measured 2.00 GB/s, under the 1.0 GB/s NVMe floor), they removed the global rather than clearing it on the cache branch: the rate now travels in a frozen classification that is stripped on override, so the message cannot cite a verdict it did not produce (#411)
  • training.bnb_4bit_use_double_quant was validated and then read by nothing — every 4-bit path hardcoded True, so setting it changed the config fingerprint and nothing else. Made Optional[bool] = None rather than True, because a plain default emits the key into model_dump() and breaks round-tripping for 21 of 173 shipped configs; and when review showed the first round's tests fired on spelling rather than behaviour — and its companion passed against main's untouched file — both were deleted rather than patched (#418)
  • soup env check now audits the live environment against the bounds Soup declares about itself, so pip install vllm quietly downgrading transformers past the <5.0.0 cap is caught. The bound is read from package metadata rather than restated — and when review found the false-positive fix had, in closing it, made #368's own case unreachable, they narrowed enforcement to the ABI-relevant packages so both properties hold at once (#421)
  • soup bom emit / soup attest emit --attach-to-registry — a published soup card now carries its ML-BOM and in-toto attestation. Told the signed path registered the statement but not its detached .sig, so the card linked an attestation the registry alone could not verify, they fixed it the way they had already solved the same multi-file problem for bom --format both (#420)
  • soup draft measure and soup draft distill disagreed on what "shares a tokenizer" means, so a pair distill refuses could still be measured — and a completed acceptance measurement was then discarded when the assisted-throughput arm failed, because the report was only written after it. The persist-early half is the one worth naming: before this it was indistinguishable from writing at the end, and every pre-existing test passed against a version that only wrote from the except handlers. Four rounds, and each round they came back with the mutation matrix rather than an assertion that it was fixed (#409)
  • Layer streaming had no config key to disable pinning, so the only known workaround for a box that cannot page-lock enough memory was to not use the feature. training.stream_pin is Optional[bool] so unset reproduces today's behaviour exactly, and true announces and proceeds on the disk tier and on CPU rather than refusing an otherwise-fine config. The round that mattered was the third: their assertion on the require_pin wiring looked discriminating and was not — on CPU the expression is False whatever the config holds, so a constant satisfied it identically. They did not paper over that; they built the CUDA-gated version that distinguishes them and skips honestly on CI (#416)
  • Extended LISA to task: pretrain, and did the dtype half the way the tree now demands: pretrain's load goes through the shared full-FT-aware resolver (resolve_base_load_dtype) rather than a local copy, so pretrain+LISA loads fp32 master weights exactly like SFT+LISA — the semantic conflict with #492 resolved by having one implementation, not two — and both trainers share one apply_lisa_setup() so their LISA wiring cannot drift (#476)
  • Bounded the layer-streaming RAM tier by a physical-RAM ceiling, closing the #622 exit-137 OOM where a store that fit MemAvailable still saturated unevictable host memory — the reporter's second run had 27 GB of 30 free and died identically to the first, which no free-RAM story explains. What makes it right is what they refused to add: an os.statvfs("/dev/shm") check was proposed and declined with a reason, because the store is torch.empty(..., pin_memory=True) and that mount is not the allocation channel — so a check against it is wrong in both directions. The bound they shipped is mechanism-agnostic and therefore correct under either accounting, and a competing implementation converged on the same one independently. They also documented the 55% constant as a chosen safety margin, not a measured bound, with the comment naming the hole it stands in for — a container's --shm-size can still be smaller. Seven mutations, seven killed (#622, #644).

• Faisal Fayaz (@Faisal01011)

  • Added the qwen3.5-4b-pretrain recipe — and shipped it with a test pinning the literal repo id, which is the only thing that catches a consistently wrong id (wrong in both RecipeMeta.model and the inline base:, so the two still agree). Every catalog-wide invariant passes that mutation; this is the defect class that shipped glm-5 pointing at THUDM instead of zai-org (#422)
  • Added the deepseek-v4-flash-grpo recipe, carrying the same literal-repo-id guard a second time — the consistently-wrong-id mutation stays green through every cross-field invariant and is caught only by that test (#432)
  • materialize_meta_adapters returns a count, and on newer peft it returns 0 as a matter of course — so 0 stopped distinguishing "nothing to do" from the silent no-training case its own docstring warns about. Demoted the count to a diagnostic and moved the decision into a separate postcondition, so the caller can no longer choose to ignore it; the trigger needs a peft this repo does not pin, so the test stubs the capability and asserts the decision (#435)
  • Then took the non-blocking follow-up from that review unprompted: the guard's lora_ restriction was called deliberate in its docstring but nothing pinned it, so a later broadening would have started refusing healthy streamed builds (#437)
  • soup data best-of-n could only sample from a locally-loaded model, so anyone already serving a base through Ollama or vLLM had to load a second copy. They added --provider without disturbing the local default, reused make_magpie_generate_fn rather than hand-rolling a second HTTP client (so --base-url inherits the existing SSRF validators — localhost.evil.com is refused, which a prefix check would miss), and rejected anthropic BY NAME with a message naming the flag rather than the feature, one better than the precedent. Asked for coverage of four argument guards that a six-condition mutation had left untouched at 88 passing, they wrote tests asserting the message, not just exit 2 — so a test cannot pass because the wrong guard fired (#466)
  • Made FSDP + BNB 4-bit QLoRA dtype-compatible by resolving quantization storage and aligning trainable PEFT parameters, with recipe, documentation, and regression coverage (#588)

• Shutaru (@Shutaru)

  • Kept the Transformers SFT import off the MLX dispatch route, so backend: mlx cannot reach the PyTorch/TRL stack even if sft.py stops being import-light later — and, told the PR did not fix the defect its title claimed, retitled it to match reality rather than defending the framing, leaving #394 open for the unexplained hang. The mlx-smoke job it adds asserts mlx is present, not merely that torch is absent: the earlier shape went green having executed nothing (#431)
  • Found and fixed a silent data-corruption bug in assistant-only loss masking: BatchEncoding is not a dict, so the guard missed and the mask was built from the mapping's key strings — no exception, normal loss curve. Split it out of #426 on request, then decided the case the issue asked to be decided rather than inherited: an all-zero mask with assistant messages present is rejected rather than honoured (measured: 0 trained tokens before, 2 after) (#439)
  • Layer streaming built its buffer-pool spec from layer 0 and refused any checkpoint whose layers disagreed, which excludes an MoE decoder outright. They relaxed the invariant properly — every layer's header read, specs unioned, a shared key that disagrees on shape or dtype still refused — and then met the harder half of the ask. This project's rule is that an arch enters the streaming allowlist only after a bit-exact gate against a resident run, and they could not load the real 35B resident; rather than argue for an exception they built a CPU parity gate on a toy heterogeneous MoE decoder, torch.equal on streamed-vs-resident logits, no CUDA marker, so it runs on every CI cell like the nine dense families. I confirmed it discriminates by silently streaming expert weights from layer 0 — same shapes, no crash — and it fails on the numbers. They also named their own alias table _UNVALIDATED_STREAM_ARCH_ALIASES and wrote the missing-resident-control caveat into the docs before anyone asked, which is why this was a review about closing a gap rather than about finding a concealed one (#426)

• Achuth Reddy Bangaru (@AchuthReddy-16)

  • soup train --no-reexec printed a launch command with the user's own flags dropped, so following it trained without --fsdp while still succeeding. Rather than patch the printed copy, they deleted it and derived the hint from the argv that actually launches the run — then, asked for a guard, wrote one whose exclusion set forces a decision for every new soup train flag instead of letting silence make it (#415)
  • A tokenizer output that is dict-LIKE but not a registered Mapping was read as a sequence, so its KEYS became tokens and the error named input_ids[0]='input_ids' — pointing the reader at their data when the cause was the type check. They went past the reported instance to the second, quieter one: the same gate at _apply_template_with_mask does not raise, it silently drops assistant_masks and trains unmasked, so widening only the loud gate would have left the worse half in place. Also renamed the helper data_doctor had been importing across module boundaries under a private name (#447)
  • Eleven callback modules imported transformers at module scope because a lazy-LOOKING factory was called EAGERLY — class X(_try_import_callback_base()) resolves the base in the class header — costing ~4.4 s per import and a 5x soup --help regression the moment a light command touched one. Measured after: 4.30s -> 0.03s, torch absent from sys.modules. The instructive half is the defect their first attempt introduced: deferring the classes while the builders still referenced bare globals, which PEP 562 __getattr__ does not serve, so four of five builders raised NameError on a cold import — and CI reported ONE failure, because eleven of twelve vanish in a full-suite run when an earlier file caches the attribute into globals(). Told that, they fixed all five, removed the ten # noqa: F821 that were silencing the diagnostic, and wrote the subprocess test that is the only thing which catches it: under a re-break, 37 in-process tests pass and only that one fails (#455)
  • Gave --baseline artifacts a scorer_revision + soup_version stamp, so a snapshot taken on an older Soup is no longer diffed against a freshly-scored model on a different scale in silence — and deleted the v0.73.2 name-list mitigation rather than letting it grow a sibling constant per release, which is what #404 asked for. Two blockers, and the first is a lesson about locks: their revision lock did not lock anything, because the fingerprint generator was a stub under which six of eight bundled suites scored exactly 0.0 or 1.0 — pinned, so unable to move. Reverting the #346 arguments requirement, the very scorer change #404 cites as motivation, left the hash byte-identical and the lock green. They rebuilt the generator so every suite lands strictly between 0 and 1 (0 of 8 pinned now), and turned my mutation into a permanent test — test_removing_arguments_requirement_moves_fingerprint, plus one asserting the generator's own non-degeneracy so it cannot quietly regress. Second, nothing in src/ actually wrote a stamped file — both helpers were test-only while the docs already claimed otherwise — so they added soup eval gate --write-baseline, and a stamped baseline now resolves silently while an unstamped one warns exactly once, which was the entire point (#485)
  • Took an issue filed as CI hygiene and it turned out to be a shipped, user-facing breakage: twelve production from_pretrained sites passed dtype=, the transformers >=4.56 rename, so soup chat, soup serve, soup merge, soup infer, soup diff and soup export all failed at model load on the entire lower half of our own declared pin — and on the older-image Colab/Kaggle population #385/#387 is about. Two reviews disagreed on whether the wrong spelling raises or is silently ignored, so I installed transformers 4.46.1 against a locally-built bf16 checkpoint to settle it: TypeError at __init__, not a quiet no-op. Their static AST guard runs in all thirteen cells, not only the new floor job, so it does not depend on the floor cell being reached — reintroducing dtype= at one site turns it red by name. They also refused to relabel 4.55.4 as "the floor" and documented why 4.36.0 genuinely cannot resolve (4.46.0 is yanked in full; trl 0.14.0 requires >=4.46.0), both of which I verified against PyPI rather than accepted (#489)

• Emmanuel Ziggah (@blackcoderx)

  • On Windows a process that genuinely exits with code 259 was indistinguishable from STILL_ACTIVE, so it read as alive forever — defeating reconcile-on-read and able to wedge the MCP execution cap shut with no error an operator could act on. Disambiguated with WaitForSingleObject, and folded in the deduplication rather than fixing only the headline: two ~50-line copies of the liveness check became one shared module, with a test asserting identity so a third copy fails (#436)
  • soup data mix --optimize — the one command whose entire output is a config file — wrote one that would not parse, because data.train came out as a YAML list against a str field. Collapsed it to the highest-weighted dataset and kept the full ranked breakdown as a comment, so nothing the search learned is discarded; then found the same defect a second time in soup data mix --live's overlay and flagged it instead of widening the diff, along with the reason the suite was green on it — every --live test mocks subprocess.run, so the artifact is built and never loaded (#440)
  • Then found the same defect a second time, in soup data mix --live's overlay renderer, and fixed that too: every candidate proxy run was handed a config it could not load. Asked whether the PR should exist at all once they took on #443 rather than pushing or stalling silently — the right question, and the answer turned on something they had not seen (#443 has two resolutions and only one reverts it) (#445)
  • data.interleave was a documented, schema-validated key that nothing read at training time — the same "validated, documented, read by nothing" class v0.73.3 was named for, which they found while fixing #330 rather than being told about. Given a tightly-pinned scope they followed it literally, including the parts easy to fake: the back-compat guarantee holds by construction (a parallel loader beside the untouched single-path branch, not a refactor of both onto a shared path), and a renderer test loads its own YAML through the real schema and then through load_dataset(). Asked afterwards to close a hole in MY spec — widening train to a list blinded three consumers filtering on isinstance(path, str), one of them #297's plan->execute security gate, which contributed zero protected files — they fixed all three and wrote the ONE enumerating test that makes the next consumer a new row instead of the next defect (#460)
  • Every non-quantized model loaded in float32 because from_pretrained got no dtype, so a bf16 checkpoint cost 2x memory. Frozen bases now load the checkpoint's own dtype by delegating to the shared resolver and full-FT paths load explicit fp32 master weights — and they went past the ask twice: collapsed the VRAM pre-flight's drifted full-FT classification into one shared is_full_finetune() predicate (killing drift in both directions — it missed lisa_enabled/r==0, over-counted bare freeze_layers) and rebased onto #492 so there is exactly one dtype-resolution path in the tree, not two (#471)
  • Took data.interleave the rest of the way to streaming + hub entries (#459), and closed the review's MEDIUM SSRF finding with a correct diagnosis rather than a patch where it hurt: _looks_like_remote_uri had been conflating "is this meant to be remote" with "is the scheme allowlisted" by delegating to is_remote_uri, so https/http — not covered by _REMOTE_SCHEMES — fell through to local classification and reached hf_load with zero validation on the streaming path; the sniff is now a plain "://" check while the allowlist stays enforced exactly where it already lived in validate_remote_uri, and is_remote_uri itself is untouched (#468)

• Harshit Sharma (@harshitthek)

  • detect_device() did not know MLX, so an Apple Silicon run reported "CPU (no GPU detected)" and silently rewrote quantization: 4bit to none. The label was never the harm; asked for an explicit decision rather than a disappeared warning, they extracted resolve_quantization() with the mechanism named in its docstring — and extraction is also what made it testable, since the surrounding function is 0% covered (#428)
  • Added the qwen3.5-9b-grpo recipe with the literal-repo-id guard the house pattern calls for — the mutation that matters is an id wrong CONSISTENTLY in both RecipeMeta.model and the inline base:, which a test comparing the two to each other passes and which once shipped a broken recipe here. Also diagnosed a licence-header guard of mine that had turned nine CI cells red, inside half an hour and including the half I had missed — that an uncommitted file is absent from git ls-files, so the scanner never read itself until it landed (#448, diagnosis in #449)
  • A GPU-ceiling test compared two probes taken at different moments with a 10% tolerance standing in for machine drift, so it failed on correct code under bursty contention. The issue named widening that tolerance as the WRONG fix; they deleted it instead, moving the comparison inside a single measurement so there is no second moment to drift against — and then went past the ask by writing the control without @requires_cuda, so the guard the issue complained CI could never see now runs on all nine cells (#451)
  • Issue #456 reported that cut_ce split the model name on "/" only, so on Windows a parent directory naming an architecture selected a kernel for a model that is not that architecture, and it warned that liger might be differently wrong. They measured it instead of assuming: liger matched the WHOLE string, so it over-matched on every platform including POSIX, and it picks the wrong kernel rather than an unnecessary one — a Phi model under a llama_experiments/ parent got the Llama kernel. The finding was bigger than the report. Asked afterwards for the POSIX control that half of the fix had no test for, they added it, and it is load-bearing: a fix that normalizes separators but keeps the whole-string match still fails it (#458)
  • Two recipe PRs landed in sequence and the five documentation sites auto-merged silently while only the test files conflicted, so the stated recipe count drifted with nothing to catch it. Their ratchet derives the count from the catalog and fails on patterns that stopped matching, not only on wrong numbers — the half that keeps it working after someone rewords a doc line, and the half almost every guard of this kind gets wrong. It took two rounds on one principle: v1 asserted an exact site count and so reddened the build on a correct edit (a second, correctly-numbered mention; any sixth site), and relaxing that then silently gave up detecting a site dropped from DOC_SITES — a property the test's own docstring claimed. They closed both at once by pinning the roster's path AND pattern, and went past the ask with a test for the roster logic itself. They also reverted their own test-count edits back out on being shown those drift on every merge touching tests/, and filed #465 rather than widening the PR (#457)
  • Noticed that CONTRIBUTING.md's hand-maintained test-suite counts had gone stale and proposed a ratchet. Shown a measurement that a ratchet there would redden the build on more than half of all merges — 11 of the last 20 commits touch tests/ — they took the subtractive fix instead: delete the numbers so drift becomes impossible rather than merely detectable. Then, told their first shape test asserted "no digits" and would fire on this file's own house style ((v0.X.Y) tags sit on every sibling line), they narrowed it to a count shape. Verified both directions: a version tag with a bare digit keeps the suite green, a real count fails by name. Two rounds, both of which they resolved by making the guard smaller (#467)

• Osheun (@Osheun)

  • Added the glm-5.1-dpo recipe — and on a first PR, carried the literal-repo-id guard the house pattern calls for, in all four places, unprompted. It earned its keep immediately: the pre-existing family guard test_no_recipe_references_thudm_glm5 does NOT fire on a consistently-wrong THUDM/GLM-5.1, because it matches the lowercase glm-5 spelling — so the guard against that exact historical defect was theirs, not ours. They also updated CONTRIBUTING.md's recipe count, the site a sibling recipe PR missed and that I missed on a release (#452)

• Nurkhan Esenbek (@kok-o)

  • Re-enabled /v1/tools/bash under real OS-level isolation (unshare on Linux, sandbox-exec on macOS), retiring the 501 stub that had stood since v0.53.7. Four rounds, and every one of them was about a test that could not fail — which is the right number of rounds for an endpoint that executes shell commands. Round three fixed a Windows CI break by patching the resource module OBJECT rather than the "resource.setrlimit" string, since the string form makes pytest import a module that does not exist on Windows at all; they kept the test running everywhere instead of wrapping it in a skip. The last round restored the positive control, and it is the one that mattered: a suite testing only the refusal direction is green on a build where the sandbox never executes anything. Verified on Linux at merge, since the feature does not run on Windows and no earlier round could settle it — neutering the argv to run true fails the positive control alone, removing the launch_failed refusal fails the negative control alone, and reverting gives both green. Each mutation kills exactly its own control (#151, #527)
  • Picked up a six-hour-old issue about the cross-tokenizer acceptance bias and delivered both halves — the docstring caveat and the regression test. Two rounds, and both were about what a test PROVES rather than whether it passes. The first fixture fed a merged token INTO the kernel, where both sides decode to the same string and count_accepted_spans correctly returns full acceptance via its early return; the real bias is a SHORTER PROPOSAL LIST. And it used a 2-token target, where "dropped exactly one token" and "dropped half" are both 0.50 and indistinguishable — at 4 tokens they are 0.75 and 0.50, so the assertion encodes 1/n_gen as claimed. They rebuilt it to both, and the result kills the unsafe mutation: a kernel that OVER-reports acceptance, where a draft looks better than it is, now fails by name (#464)
  • Built the Lambda Cloud training backend behind soup train --cloud lambda, and the value of this one is in what the third revision looks like compared to the first. The original shipped the API key into cloud-init user_data while the same script ran set -x, so bash xtrace would have echoed the expanded credential into a boot log readable by anyone who can view instance logs — and the key was there for a reason, because the instance owned its own shutdown. Rather than patching around that they moved termination to a LOCAL controller that holds the key and never ships it, so user_data now carries no credential at all, and the finally both terminates and polls to confirm, forcing a non-zero exit if termination itself fails — more than #264 asked for, which said only terminate in a finally. They also took the structural fix over the convenient one twice: cloud/_common.py extracts what Modal and Lambda share instead of leaving a third copy, and when told the RunPod renderer was dead surface keeping the original defect one line away, they deleted the module and its PyPI extra rather than adding a guard around code nobody could reach. Verified independently: the runtime-forwarding mutation — a secret that never appears in the rendered text but reaches the launch payload at stub runtime — survived the entire suite at the previous head and is killed at this one, because the guard now exec()s the stub and inspects the payload instead of grepping text. Zero deleted test lines against main. Refs #264 rather than Closes, because all five live-validation criteria remain unmet and they said so in the docs themselves (#264, #528)

• AmixDigital (@Amix29)

  • Gave soup monitor a real Apple Silicon path, where it had reported nothing but an NVIDIA-shaped blank since v0.44.1. The security work is the part worth naming: fixed absolute executable paths so PATH cannot influence what runs, sudo -n that never prompts and never reads a password, stderr pattern-matched for permission markers but never printed, and a parser that returns [] against a billion-laughs entity bomb, an XXE file:/// reference, 5,000-deep nesting and embedded NUL bytes. Three guards had no test holding them up and they closed all three in twenty minutes — most importantly the call site that decides which implementation every Linux and Windows user gets, where if True or detect_apple_silicon() had left 126 tests passing. Their oversize-payload test had also been asserting on b"x" * (CAP + 1), which is not a valid plist and returns [] whether the cap exists or not; they rebuilt it around a valid oversized document. The test file shrank by 18 lines and every deletion was an upgrade — membership assertions became adjacency assertions. Asked to confirm the live plist schema, they said plainly that the Mac is not theirs and they cannot authenticate sudo, rather than guessing — while confirming the two things they could check against the real binary (#481)
  • Wrapped Aider's Polyglot benchmark as soup eval aider, and verified the upstream contract before writing a line — the aider-chat wheel carries no benchmark at all (235 entries, zero matching benchmark, an args.py with 135 add_argument calls and no --polyglot), so they wrapped the real source/image harness instead of a flag that does not exist, and said so in the issue first. The container contract held up to everything I threw at it: --rm, a readonly corpus mount, no docker.sock, no --privileged, shell=False, and credentials inherited by name so no secret value reaches argv. The blocker was a test that could not fail — pytest.raises(match="symlink") searches the whole message, the message embeds the path, and pytest's tmp_path is named after the test function, so removing both symlink guards still left 16 tests passing while a different guard tripped the assertion. They fixed it with the isolating case I could not have asked for more precisely: a symlink whose target stays inside the result root, which is the one case is_under containment cannot catch. Also closed three hardening gaps in the same push — non-finite counts escaping as bare ValueError, --image=--privileged landing in the image argv slot, and an exercises directory resolving outside cwd — and validated the new image check in both directions, since a validator that rejects ghcr.io/org/img:tag is worse than the bug (#482)
  • Caught that layer streaming's core promise — the frozen base "lives in CPU RAM" — was false on Apple Silicon: from PyTorch 2.7 on, torch.empty(device="cpu", pin_memory=True) hands back an mps:0 tensor whose is_pinned() is false, so a 256 MiB store raised torch.mps.current_allocated_memory() by exactly 256 MiB. Their CHANGELOG entry says out loud that the shipped soup train path already gated pinning on CUDA, so the fix is the lower-level boundary rather than a user-visible bug — declining a bigger claim than the evidence supports. All three guards I flagged had nothing holding them up, including the MPS gate itself, where the only test was skipif-gated and therefore skipped everywhere but a Mac: disabling the gate entirely left 15 tests passing. Their replacement is platform-independent and fails on a Windows box, which is the difference between coverage and green-because-skipped. They also rebuilt the test that could not discriminate the real bug — it simulated the misplaced allocation with device="meta", so a guard narrowed to == "meta" would have let the actual mps tensor through — and folded in the require_pin announcement so pinning is never dropped in silence (#480)
  • Made vision SFT actually work end to end by moving chat templating from dataset-prep to collation time — the rendered text had been carrying zero image tokens while an image was still handed to the processor, because Idefics3 does not preserve a legacy <image> string. Verified against three real processors. Review then found two defects the fix itself introduced, both of the kind that produce a plausible loss curve rather than an error: 1088 of 1141 label positions per row — 95.4% — were <image> placeholders being trained on, and add_special_tokens=False silently dropped BOS for LLaVA-1.5, one of the two architectures #302 names as must-not-regress. One push closed both, and the test for the first pins the trained-label count, not just positions — a positions-only assertion would have passed on the broken version. Their BOS fix derives the flag per architecture by comparing both tokenizer paths, rather than hardcoding the value that happened to suit SmolVLM (#488)
  • Ended the CHANGELOG conflict tax — every merge had been conflicting every open PR, because entries all went to the top of the same section, so the conflict rate tracked how many PRs were open rather than whether they overlapped. They reproduced all three candidate options from one base commit before writing any implementation, and the reproduction disproved the maintainer's own recommendation: appending at the end of the section still conflicts, and the merge=union driver silently fuses two different corrections to one entry into a duplicated bullet with contradictory text — a loud conflict traded for a quiet corruption. They also caught that my acceptance criteria asked them to edit .claude/CLAUDE.md, which is gitignored and unreachable from any public PR. Then they asked which direction to take rather than shipping the wrong one. The result is 338 lines of stdlib we own, no towncrier, verbatim assembly verified byte-for-byte against the real changelog, and — the part that makes it safe rather than merely tidier — a lost fragment is loud, guarded in every matrix cell plus a second gate before PyPI (#490)
  • Made the transformers floor derived instead of restated: #489 had written 4.46.1 into three places and derived it from none, so raising the pin in pyproject.toml left the suite green while CI went on testing the old version and calling it "the floor". Review found their guard firing on a correct future state — raising the declared floor to equal the tested pin, the exact migration the exception exists to allow, produced two message-less reds. They fixed it by deleting the over-specifying assertions rather than patching around them, and added a control proving the new test is not vacuous. They also repaired a weakness they had inherited rather than introduced: the workflow assertions checked code spelling, so gutting the real comparison while keeping the strings left everything passing. Now the test pulls the heredoc out of the workflow with yaml.safe_load and execs it under patched metadata, so a gutted comparison and a re-hardcoded expectation both fail by name (#496)
  • Answered #427 rather than tidying it. That issue was careful about its own claim -- nine Qwen3.5 recipes inherited modality: text from the schema default while every checkpoint reports Qwen3_5ForConditionalGeneration, and nobody had checked which of the two was wrong. They recorded the answer: text-only is the intent, written out instead of inherited, and covered Qwen3.6 as well, so twelve recipes rather than the nine the issue counted. The test is the sole guard possible here and they saw why: since the default is ALREADY text, no generic "every recipe loads" test can notice a missing declaration, so removing one line fails exactly one parametrisation and leaves the other eleven green. The part worth copying is the scope section: they state in the PR body that this changes no runtime behaviour today and that transformers 4.x cannot load qwen3_5 at all -- a fix(recipes): commit volunteering that it fixes nothing executable yet. It merged as-is because of that disclosure, not in spite of it, and it is deferential to the migration they proposed separately in #502 (#501)
  • Moved the whole training stack onto Transformers 5 in one reviewed chain: declared floor >=5.12.1,<6.0.0 (peft >=0.20, trl >=0.29), dense Qwen3.5 LoRA target resolution, a dual plotext 5.x/6.x compatibility layer, and — the part review tested hardest — the expired dtype-guard rationale removed and replaced with a stated policy rather than left stale under new pins or silently stripped hours after two other PRs extended it. When my review found their plotext-6 tests faked the module, the response was the structural fix: a constraints cell pinning plotext==6.0.0 so CI exercises the real major every push, closing the exact hole #522 fell through; and the doctor floors became pinned by a test whose mutation I verified bites. Both mutations on the core fix (target resolution, DPO prompt-cap block) died on named tests before I ran anything (#507)
  • Same-day delivery twice on the MCP execution-cap thread: took #524 the hour it was filed and shipped the recovery surface with the acceptance criteria met exactly as written — the live-pid refusal inside one transaction so a refused reconcile deletes nothing on the way out, every removed run_id printed, an explicit flag required, and a >=300 s age default that keeps the ms-wide crash window out of reach; review's mutation dropped the refusal and the named test caught it. Also pinned the quantize-binary contract test nobody had written — with review blame honestly establishing which half of #518 was already in main (#525, #520)
  • Closed the layer-streaming disk-surprise and the Qwen3.8 catalog gap in two clean rebases: the pre-flight projects the full materialize+shard footprint per volume and refuses BEFORE the download runs (the gate-order test is what review mutated — a pre-flight that fires after minutes of I/O is a post-mortem), reusing regular-file HF cache entries in place as the actual #374 fix; and the recipe whose literal-repo-id guard proved its worth exactly as designed — a consistently-wrong id sailed through 1644 catalog invariants and was caught only by their three lines (#510, #513)
  • Made MPS streaming ask instead of assume: the bf16 decision now probes the runtime with a live one-element allocation — capable Apple Silicon keeps bf16 end to end where it silently upcast to fp32, CUDA stays byte-identical, and the decision logic is fully capability-stubbed so it tests on every cell while only the real-hardware integration test skips off-Mac. Review's inverted-availability mutation failed both direction tests by name (#519)
  • Closed their own streaming finding same-day with a reproducer-driven fix: streamed adapters saved with a blank base_model_name_or_path (from_config leaves both identity fields empty and PEFT copies that into adapter_config.json, so soup chat / merge / serve could not auto-detect an otherwise-healthy artifact) — the configured reference is now stamped on the meta skeleton before PEFT wraps it, and the negative control keeps blank provenance failing loudly while the exact-comparison test kills a wrong-but-present stamp too (#531, #537); and closed #532 the same day it was filed — responses-only SFT rows whose targets truncation erased entirely trained to an all-NaN adapter at exit 0, now refused by name before Trainer construction on both the text and pre-tokenized paths, with the finite-state gate ordered ahead of every artifact write and three mutations each dying on named tests including the save-ordering one (#532, #535)
  • Brought dense Qwen3.5 text decoders into the streaming allowlist through honest aliases behind a native bit-exact parity gate — and when review flagged the gate's missing below-floor guard from an actually-stale environment, the fix was one commit: reuse _REQUIRES_TRAINING_FLOOR so sub-floor boxes read an honest skip instead of a misleading red. Verified by review on transformers 4.57.6: skip with upgrade hint, not ImportError (#515)
  • Treated an explicitly untied vocabulary embedding/head pair as one streamed large-layer slot while preserving the tied-model kernel path. The final gate compares non-zero untied LoRA gradients against the resident control bit-for-bit; the CUDA preference battery returned zero deltas (#526).
  • Made a complete cached Hugging Face snapshot materialize offline without re-contacting the Hub, with commit/blob binding, no-follow source handling, containment checks, and atomic publication (#538).
  • Fixed Apple-internal NVMe detection behind APFS Apple Fabric by resolving the physical store and matching its BSD identifier against the NVMe inventory; malformed and unmatched metadata remains fail-closed (#539).
  • Made Qwen3.8 response-only masking recover system-first prefixes from the first valid user-containing render, preserving explicitly trainable prefix content while preventing label drift (#543).
  • Kept the final real loss, learning rate, and gradient norm in the live training panel instead of replacing them with zeroes from a summary-only Trainer event (#544).
  • Recorded privacy-safe MPS accelerator, Apple chip, and unified-memory details in reproducibility receipts, pinned by an end-to-end serialized-receipt test (#545).
  • Enabled BF16 for resident MPS trainers only after a runtime capability probe and retained the necessary FP32 master-weight exception in PRM (#564).
  • Made Best-of-N fail closed for non-finite judge scores and malformed external prompt rows before either can select or publish a training artifact (#551, #552).
  • Preserved GRPO reward metadata across data-loading paths, derived assistant-reference answers safely, and made built-in reward validation fail closed on missing or empty gold values (#566).
  • Added Qwen4-Exp text LoRA scaffolding with validated all-linear target resolution and an instance-local scatter compatibility path for int64-only lower-bound runtimes (#572).
  • Made Best-of-N durable and resumable with exactly-once checkpoints and all-or-old SFT/DPO/manifest publication (#553).
  • Added offline Best-of-N materialization with atomic SFT/DPO publication, manifest validation, and sampler-boundary regression coverage (#554).
  • Enabled runtime-probed MPS BF16 for local Transformers GRPO with FP32 fallback and precision-policy regression coverage (#568).
  • Added routed-expert LoRA targeting for Qwen4, with focused regression coverage over PEFT target selection (#575).
  • Made the offline Best-of-N manifest a real commit marker rather than a file written beside the data: it is published last inside the same transaction, and a stale DPO left by an earlier run is now removed inside that transaction — backed up alongside the outputs, restored by the rollback loop, unlinked only after a full commit. The last review round blocked on exactly that ordering and the fix is genuine, not claimed: deleting the sibling before the transaction instead of within it fails test_failed_sft_only_replacement_restores_manifest_bound_dpo by name, and dropping the manifest from the publication list fails six tests. They also made offline mode refuse --resume / --checkpoint outright instead of accepting recovery flags it cannot honour, and — asked for it on this PR rather than their own — put --revision into the online run_digest so a changed local revision is rejected before the model loads, asserted as load_calls == [] (#557).
  • Closed the other half of the Best-of-N durability story: two-phase export buffered whole artifacts and could not resume, so an interrupted run replayed every prompt. Export now checkpoints each completed group durably and a resume replays nothing — and the resume is bound to what produced it, including the exact file content of a local model, so weights swapped in place at the same path are refused before sampling rather than silently mixed into one artifact. Offline validation streams through a mkdtemp-unique SQLite index instead of materialising both files as Python lists; no live path calls a whole-file materialiser any more. The part that made this safe to merge is that it did not quietly undo the previous one: neutering the rollback loop fails four tests, three of them belonging to [#557]'s own suite already on main, which is what proves a 267-line rewrite of best_of_n_artifact.py plus a new 647-line transaction kept the manifest-last and stale-DPO-inside-the-transaction guarantees. Asked for one disclosure and delivered it precisely: the export phase now seeds per prompt rather than once globally — required for resume determinism, consistent with what the online phase already did, and a change to the output of any existing --export-candidates --seed N run (#559)
  • Defined the AutoDistill Milestone A artifact contract, and the part worth copying is the boundary they drew themselves: the PR body states that this adds no task or SoupConfig field, no CLI, no teacher or student loading, no capture or training loop, no adaptive allocator, no cross-tokenizer ULD and no benchmark integration, so #580 stays open on the rest rather than being closed by a slice of it. The blocker was a failure mode they removed instead of working around. The first revision committed exact-byte .jsonl fixtures and asserted a 423-byte length and a SHA-256 over them; windows-latest 3.10 and 3.12 went red while macOS stayed green on the same commits, which is a checkout-layer signature rather than a logic error — core.autocrlf rewriting a committed newline underneath a byte-level commitment. Offered the choice, they deleted the fixtures and now build the payload in the test as canonical JSON plus one explicit LF, so there is nothing left for a newline conversion to touch. Re-ran five mutations on the rebased head rather than the reviewed one, since a rebase is a new tree: disabling the student_rollout branch, the top-k-union-forced equality, mass conservation, .. traversal and the student-rollout replay ban each fail a distinctly named test, where the first had zero execution before this round and is the teacher-only/student-only boundary the whole design turns on. Zero deleted lines in the entire PR (#580, #613)
  • Added Qwen4-Exp layer streaming with SSD-backed PLE n-gram embeddings, over three rounds, two of which were about the same finding: the gates existed and nothing proved they were reached. Deleting both gate call sites gave 40 passed, byte-identical to baseline — a suite fully green with the feature's fail-closed policies removed. The design they landed on is the right one and worth copying: gate bodies pinned by behaviour tests, gate call sites pinned by a single test that monkeypatches all four with recorders, drives the real _setup_streaming_transformers(), and asserts the ORDER they are reached in. Deleting each call site alone, two together, and all four together each fails it by name. The HIGH was worse than a missing test: the RAM refusal keyed on stream_source rather than stream_ngram_source, so on the DEFAULT stream_source: auto a 102 GB PLE table loaded into RAM with no refusal at all; verified fixed by execution at 100 GB free rather than by reading. They also drew boundaries themselves — loop_stages-style, the oQ RAM policy extracted into its own resolver, config.json folded into the cache fingerprint so a re-quantised base rebuilds instead of silently streaming stale weights, and a gate record that labels its own stopped production step not validated rather than omitting it. One gap I closed myself after merge: the planner half of that fingerprint was unreachable from their harness, because the stub for resolve_model_weights ignored the before_materialize callback the fingerprint is taken in (#602, #603)
  • Delivered AutoDistill Milestone B1 — same-tokenizer teacher-only capture — and the second round is the one worth recording. The first head shipped seven load-bearing guards that no test could distinguish from their absence: eight mutations survived the suite, including the teacher-only boundary named in the PR's own title. All seven now die to one distinctly named test each, calling the real function with real input. Two things make that coverage load-bearing rather than decorative. The title guard is probed by widening it — accepting student_rollout alongside teacher_expert, which is the regression someone would actually write, not a deletion nobody would — and it fails exactly one test. And every guard was also mutated to reject everything, which takes down genuine happy-path tests (9 failures on the trajectory_kind guard alone), so none of the seven is the one-sided assertion that passes a reject-all implementation. The hash-chain test is the strongest shape available here: it publishes for real, tampers manifest.complete.json on disk, and re-publishes. They also closed the MEDIUM finding from the earlier round — teacher weights now hash through bounded 8 MiB streaming reads, pinned by a test that makes Path.read_bytes raise. Zero deleted test lines against main and zero between heads; no CLI surface, no schema field, no new dependency, and Refs #580 rather than Fixes, because #580 is a proposal and not an approved design (#580, #629).

• Srinivasan R (@Srinivasan8888)

  • Corrected the MCP execution-tool refusal so it gives an actionable --allow-execute message instead of claiming shipped tools are unimplemented (#574).

  • Added the training.lisa_train_embeddings freeze control with conservative preflight validation, parameter-count regression coverage, and user documentation (#576).

  • Added the catalogued SmolLM3-3B SFT recipe, exact model-ID regression coverage, and user-facing documentation (#582).

  • Ported v0.73.0's two vLLM serve fixes to the SGLang backend, which had the identical pair standing: a third hand-rolled User:/Assistant: prompt and a hardcoded finish_reason. The PR took three rounds and every one of them was about a test that could not fail. The first revision's mutation set left two survivors — hardcoding the streaming chunk to "length" (a server that reports it on every response, which makes a continue-on-length client loop forever) and deleting the tokenizer wiring in serve.py entirely, which left 112 tests green while every real soup serve --backend sglang fell back to the legacy prompt. The second round found the defect that mattered most: _serve_sglang loaded the tokenizer with trust_remote_code defaulting to False while create_sglang_runtime hardcodes True and the startup panel promises exactly that — so on custom-code models, the ones where a chat template matters most, the load failed, tokenizer stayed None, and the fix silently did nothing. Offered both repairs, they took the narrower one and said the wider change to the SGLang trust contract belongs in its own PR. They also extended test_no_second_hand_rolled_prompt_remains_in_the_serve_backends to scan sglang.py, which had covered serve.py and vllm.py only — leaving the one file the whole PR exists because of unprotected against re-introduction. Scope honesty throughout: Refs #360, not Fixes, because the issue's live-runtime item needs SGLang on Linux (#360, #581)

  • Added the qwen3.5-35b-a3b-dpo recipe, and deviated from the issue text on purpose, saying so before being asked. #276 says to copy qwen2.5-7b-dpo, which carries no modality: line; tests/test_issue427_qwen35_text_modality.py holds every Qwen3.5/3.6/3.8 recipe to an explicit modality: text and its family audit fails on an unregistered sibling, so a literal copy would have shipped a recipe escaping a contract written for that family. They found it as a red test rather than by reading. The guard that decided the merge is that the model id is pinned on two independent surfaces — mutating RecipeMeta.model alone and mutating the YAML base: alone each fail a named test — which is exactly what a competing PR for the same issue lacked, and they had it without being asked. They also reported a surviving mutation instead of manufacturing a kill for it: removing dpo_beta: 0.1 yields a byte-identical config because schema.py:1237 defaults it to 0.1, so it is an equivalent mutation and the correct response is to say so. Their claim comment carried git diff -- tests/ | grep -c '^-[^-]' unprompted, and every one of its assertions held when re-checked independently (#276, #615)

  • Found that the trust_remote_code gate did not apply to the SGLang backend at all — the deliberately-deferred half of #581, which they kept out of that PR so it could be argued on its own terms and which turned out to be a security hole rather than the tidy-up they expected. serve.py resolved the flag once for every backend and the sglang branch then called _serve_sglang without it, while create_sglang_runtime hardcoded trust_remote_code=True at both call sites — so the startup panel announced the model was trusted and gave the user no way to decline. A notice, not a gate. The fix is deliberately breaking: a custom-code model on that backend now fails to load without --trust-remote-code. What makes it trustworthy is per-surface coverage — the adapter path and the plain path each fail on their own, so an edit that repairs one and forgets the other cannot pass (#360, #619)

  • Took up a coverage note raised on #581 and closed it properly. sglang 0.5.16 returns json.dumps(response.json()) — a string — where older versions return a dict, and the app-level doubles only ever produced the dict, so the tests could not see the shape that actually ships. The value is in what the mutation shows, not the parametrization: reducing create_sglang_app to the dict-only path — reintroducing #76 exactly — now fails four tests, every one of them a [json_string] variant while every [dict] variant stays green. Before this, that same mutation passed the entire app-level suite in silence (#360, #620)

  • Repaired and published the STEP 20 FSDP per-rank sharding probe, and the repair is larger than the issue asked for because they disproved the issue's own attribution. #373 said the probe hit a bare FSDP module; FSDP defines no __len__ at all, and the recorded text is torch/_dynamo/eval_frame.py's OptimizedModule.__len__, which reports the name of the wrapped class — so the failing object was a torch.compile wrapper one indirection deeper than the 0.5B arm. That matters because dropping the len() call alone stops the crash and leaves a probe that counts OptimizedModule where the record counts FullyShardedDataParallel and concludes FSDP was never engaged: a silent wrong number in place of a loud failure. unwrap_compiled() is applied before every class-identity and accounting question and pinned by tests. Two of the fourteen tests assert the doubles themselves reproduce the recorded OptimizedModule message, since a double that does not is pinning nothing, and world_size == 1 is treated as a control expecting the FULL count so a probe that always divides fails instead of passing everything. They reported a surviving mutation and then wrote the test that kills it: loosening is_sharded to local <= expected_local passed the first pass, and an under-count — a rank whose parameters were never materialised — would have read as sharded. Scope stated as Refs, not Fixes: three of the four acceptance criteria need eight H100s and stay open, and the PR makes no claim about whether the 70B base is sharded (#373, #618)

  • Documented the SGLang trust_remote_code change from their own #619 on both pages a reader would look at, and wrote it as a change rather than as a feature: a custom-code model on --backend sglang now fails to load without the flag where it previously loaded and ran silently. That framing is the half a user needs, since the breakage is the visible part. docs-only, zero deletions (#360, #626)

  • Added the qwen3.5-9b-dpo recipe, completing the sft/grpo/dpo trio for that base and holding the same two-surface bar they set on #615: mutating RecipeMeta.model alone fails four tests and mutating the YAML base: alone fails three, so an edit that repairs one id and forgets the other cannot pass. The mutation worth naming is the one they did not have to run: keeping the recipe while UNREGISTERING it from EXPECTED_QWEN35_TEXT_RECIPES fails the family audit by name. Deleting the recipe only proves the easy direction; a sibling that exists but escapes the family contract is the failure that actually happens. Seven mutations, seven killed, no survivors, and all five documentation count sites synchronised — a repo-wide search for the old count returns nothing (#275, #632)

  • Closed the fourth appearance of the ANSI-in-CLI-output failure class, and did the half that stops a fifth. Three tests fail on a clean checkout under FORCE_COLOR because soup recipes show and soup data mix --apply emit syntax-highlighted output that the assertions read raw — one of them feeding \x1b to a YAML parser. The evidence that decided the merge is a mutation result in a colour-free environment: reverting the #477 repair fails the new static guard under NO_COLOR=1, i.e. the scanner catches the regression on runners that cannot see the symptom, which is the entire point of a guard for a hazard that only manifests on some terminals. It also correctly flags the v0.73.2 INSUFFICIENT fix — a whitespace-collapse mistaken for an ANSI strip — which is the exact pattern that reddened CI before. One gap I closed myself afterwards rather than sending back: the regex widening that makes show_result.output reachable had no test, because every scanner fixture named its variable result (#633, #635)

  • Implemented the warning phase for unknown config keys: every undeclared key is gathered into one actionable report with a v0.75 rejection deadline, while a typo in soup sweep --param fails before any arm can turn a no-op grid into a plausible result. The crucial repair was above the helper: the first guard was caught by the per-arm exception loop and exited 0, so the final version validates the grid before that loop. The mutation is the proof: disabling the detector produces 21 failures in its 37-test suite; correct configs, all 160 shipped recipes, templates, examples, and generated starter configs remain clean (#627, #628).

  • Repaired both defects that reached main when I merged #628 prematurely — one of them a live regression their own patch had introduced, which they reported rather than let me discover. The IndexError half restores an empty --max-runs grid to an empty table and exit 0. The half worth copying is the other one, because it answers a question tests usually cannot: the unknown-key wiring in config/loader.py was never broken, only unwitnessed — deleting both call sites left 43 tests green while a typo'd config loaded silently. The seven new tests load real configs through the real entry points, and each call site dies INDEPENDENTLY: mutating only load_config fails exactly two named tests, mutating only load_config_from_string fails exactly its own two. I verified that separation by hand, and it is the property that matters — a test covering one branch leaves the other exactly as exposed as before. Three controls pin that a clean config stays silent, so a warn-on-everything wiring cannot pass, and two more pin the UNKNOWN_KEY_SEVERITY = "error" flip that v0.75 will force on somebody. The #628 file-text scanner is kept with a docstring saying it is not a substitute — the correction, not a deletion. Zero deleted test lines; 13/13 CI green (#627, #643).

  • Added the glm-5.1-grpo reasoning recipe, and the process is what I would point others at: they searched for overlapping work (gh pr list --search glm --state all), confirmed all six named children of #275 were closed, and claimed the item before writing code — exactly what would have prevented the two claim collisions I mishandled that week. The recipe validates through the real schema rather than merely being a well-formed dict (GRPO cross-validators pass with reward_fn and num_generations present), and the base id is byte-identical to its two siblings — checked deliberately, since a glm-5 recipe once shipped with THUDM instead of zai-org. It landed hours after #640 changed the snapshot fixture to delta encoding, and regeneration is byte-exact with the new delta restating nothing from the baseline. Their own mutation table self-reported a survivor and then closed it — a RecipeMeta.size gap that had no coverage anywhere in the suite (#275, #656).

  • Covered the two branches the #654 merge comment recorded as untested, and one of the three mutations they ran is beyond what that comment asked for: nothing previously pinned the narrowness of _is_cuda_oom, which could have been widened from RuntimeError to Exception with no test objecting — swallowing the illegal-access and device-assert errors its own docstring says must propagate. They also corrected a premise of mine: I had written that no CI cell could check the real torch.AcceleratorError assumption because the author's box had torch 2.5.1, and torch>=2.5.0 is a floor rather than a pin, so CI resolves a recent wheel and the guard executes. Where the symbol is genuinely absent the test skips with a stated reason instead of passing vacuously. They declined to rewrite _is_cuda_oom to match the class by name, correctly treating a behaviour change as mine to call, and stated the consequence instead. Verified independently: both mutations survive the pre-existing suite (10 passed) and are killed by the new file (#654, #657)

  • Returned to cover the one uncovered statement in the unknown-key guard they had shipped themselves in #628, found by a --cov-branch run over the full suite rather than by eye. The reusable lesson is in which tests carry the weight: a "does not raise" test alone would have passed an early-return rewrite that silently stops reporting every key after the first non-string one, so the two tests that matter assert the walk keeps going — a typo'd key placed after a non-string key is still reported with its suggestion, at the top level and one section down. That is the same silent-no-op failure class #627 exists to fight. Verified independently: guard removed and continue -> return both survive the pre-existing 45 tests and are killed by the new class (#627, #658)

• Sohaib Kamash (@Nick-800)

  • Added seven validated Qwen2.5-Coder, Qwen2.5-Math, and DeepSeek-R1-Distill-Qwen SFT recipes; synchronized all catalog-count sites and repaired the Mistral Small model identifier. Literal repository IDs and per-recipe learning rates are now load-bearing assertions (#536).
  • Added four validated DeepSeek-R1-Distill recipe entries with synchronized catalog counts, documentation, and Hub-backed model metadata tests (#569).

• Tal (@taljeon)

  • First contribution, and it closed a gap in a guard that was one day old: the dtype= scanner from #489 matched only class names starting with AutoModel, so the same load-breaking kwarg on a CONCRETE class — WhisperForConditionalGeneration.from_pretrained in the ASR training path — was invisible to it. They took the suffix rule rather than a name allowlist, which is the harder and correct choice: an allowlist silently stops covering a class added later. Verified with a negative control rather than by inspection — dtype="auto" on the real Whisper site fails by name on their branch and survives on main, so the gap was real. Two things I check on every widening they got right unprompted: the existing AutoModel* coverage was not traded away for the new coverage, and the scanner still ignores FastLanguageModel, PeftModel, torch.zeros and wrapper kwargs — a guard that fires on correct code is one people delete. All six deleted lines are renames or a reworded message; the file goes 11 tests to 12 (#497)

• k4its1t (@k4its1t)

  • First contribution, picking up where #497 stopped one day earlier. The dtype scanner's predicate already matched anything starting with AutoModel, so every AutoModelFor* load was covered and the surviving hole was concrete, non-Auto classes: Qwen2VLForImageTextToText.from_pretrained(..., dtype=) or BertForMaskedLM.from_config(..., dtype=) were invisible and would have shipped a kwarg that TypeErrors below the declared transformers floor. Four suffixes, a focused regression test each, both load methods. Their own mutation claim was the coarse one -- removing all four suffixes makes four tests fail -- and removing them one at a time is stronger than they claimed: each single removal produces 1 failed, 15 passed, so every named test is the sole guard for its own suffix rather than the four reddening together. The two things I check on any widening they got right: the four existing false-positive controls survive untouched (a guard that fires on correct code is one people delete), and all six deleted lines are a rename plus reworded docstrings, not a test. The rename _is_transformers_model_load -> _is_dtype_guarded_model_load describes the contract being enforced instead of implying every matched class comes straight from transformers. Scope was not oversold, which is why it is worth writing down: none of the four suffixes appears in src/ today and the scanner reports zero live hits, so this is prospective -- Whisper loads as WhisperForConditionalGeneration and #497 already covered it. Timely prospective work, though: concrete vision classes are exactly what #488, #501 and #502 are moving through right now (#500)

• lesterppo (@lesterppo)

  • Reported #425 from a real Colab crash and fixed it themselves: on pre-Ampere cards (T4/P100/V100/GTX 16xx — the entire free Colab/Kaggle tier) fp16 training crashed at step 0 because peft creates LoRA adapters in the base checkpoint's bf16 dtype while the fp16 GradScaler needs fp32 gradients. The design decisions are the ones worth keeping: cast only trainable *lora_* params so full-FT paths (lora.r: 0, Spectrum, LISA) do not double trainable memory after the VRAM pre-flight has passed; call the shared helper from every trainer train() site rather than patching sft alone (#328's shape); and write the source-scanning test that makes a nineteenth call site a new row instead of a silent gap. They also opened #433 out of the review — the meta/non-meta peft finding someone else merged. Two weeks of silence after an accepted review, so the three mechanical items landed per the dated plan under a clearly-marked maintainer commit while everything that matters stayed theirs (#429)

• Pernav Jain (@here-2007)

  • Replaced hard-coded eval-gate judge normalization with active-rubric min-max scaling, validation, and regression coverage (#578)
  • Routed GRPO objective variants through TRL per-token log-probability computation with runtime-contract regression coverage (#585)
  • Closed the abbreviated-IPv4 SSRF bypass one layer out from the localhost hole #598 had just fixed — ipaddress.ip_address() raises on 127.1, 2130706433, 0x7f000001 and 0177.0.0.1, so the guard returned False and treated them as public hostnames. They answered the question the issue could not: I had filed it saying I could not tell from a Windows box whether glibc actually resolves those forms, and declined to claim a live bypass. They measured it — glibc 2.39 resolves every one, 2852039166 reaching 169.254.169.254, while Windows returns gaierror for all of them. Using socket.inet_aton rather than a hand-rolled parser is the load-bearing choice: across 26 exotic forms the guard and the resolver diverge only together, because both track the platform's own parser. They also found and fixed a second bypass nobody had reported (--slack-url https://2852039166/ reaching cloud metadata through webhooks.py) and closed the trailing-dot FQDN form raised as non-blocking. Review found three changelog defects — a direct CHANGELOG.md edit that would have published #577 twice and added a duplicate ### Fixed heading, and a fragment filed under another contributor's PR number — and all three were gone in one push (#600, #604)
  • Closed the hostname-indirection bypass in the telemetry SSRF guard: a host that was not a literal IP passed regardless of what it resolved to, so 10.0.0.1.nip.io (real DNS -> 10.0.0.1) and localtest.me (-> 127.0.0.1) were both accepted while the literal 10.0.0.1 was refused. They pushed back on the design before building it, and were right to. Handed a sketch that resolved every custom endpoint, they pointed out that socket.getaddrinfo takes no timeout in Python and that send_telemetry calls httpx.post synchronously, so an unbounded resolver call would block air-gapped runs — and proposed a tiered design where a trusted allowlist short-circuits before any lookup. Verified by measurement rather than by reading: with getaddrinfo patched to raise, the default endpoint still validates with zero DNS calls. They also took the one correction handed back — no lru_cache, because caching a permissive verdict for the life of a process means a rebinding attacker need only win once — and they documented the DNS layer as defence in depth rather than as SSRF prevention, which it cannot be across two separate lookups. Each of the three tiers is killed by a named test, and the one place that margin is a single test wide is stated in the merge commit rather than glossed. The self-hosted-PostHog test was adapted, not deleted — mocked to a public IP, still asserting True, so the self-hoster path stays covered (#599, #624)

• Samarth Mukhija (@Samearth17)

  • First contribution, and it answers the awkward half of #379: the repo could not reproduce its own benchmark records, because most of those harnesses only ever existed in a scratchpad on a machine that is gone. They published the upstream bitsandbytes #2034 NF4 reproducer as a standalone, download-free script and left the methodology and the existing table row untouched — a reproducibility slice that reproduces, rather than a rewrite. Review blocked on the one thing that matters for a harness: it exited 0 whether or not the defect was present, so it could not distinguish the mechanism it claims to demonstrate from a machine where nothing is wrong. They fixed it in both directions — an explicit --bypass-pool negative control, and an ordinary invocation that now requires the NF4 mismatch, so a removed pool.acquire() fails instead of passing quietly. Verified on real CUDA at merge: normal run reproduces on 6/8 layers with the bf16 control clean on 8/8; cutting pool.acquire() exits 1 on the merged head and exited 0 on the pre-fix commit. They also said plainly that they could not run it themselves — no GPU — instead of implying they had (#589)
  • Published the layer-streaming bit-exactness harness, which #379 had wanted since it was filed: the ~20 harnesses behind the v0.72.x gate records lived only in a scratchpad on a machine that is gone, so the published numbers could be read but not re-measured. The control they added unprompted is what makes the tool worth trusting — --skip-lora-sync forces the comparison to fail, and on real CUDA it reports max_abs_diff=3.687500e+00 and exits 1 against a clean run's 0.0 and exit 0. For an instrument whose entire output is these two agree to 0.0, being able to say no is the whole question, and make_non_vacuous_lora closes the other axis so "gradients exact" cannot mean "both sides are zero". Two rounds, both on the same defect class: as submitted the harness could not complete a single run, because copy_lora keyed on raw named_parameters(), which is not canonical on a streamed model — every wrapped layer carries an extra .inner. segment — and the first repair reached for a suffix fallback that also cannot work, since .inner. sits in the MIDDLE of the name. A maintainer suggestion was withdrawn in their favour: the per-parameter raise they wrote catches a partial mismatch and names the tensor, where the assert_canonical_parameters_intersect they were asked for only catches an empty intersection (#379, #597)
  • Ended the five-release blind spot from #596: test_sft_smoke and test_dpo_smoke carry a module-wide pytest.mark.smoke against an addopts of -m 'not smoke', so they ran in neither the default suite nor CI — which is how one of them stayed red for five releases. The new pytorch-smoke job runs them for real (the CI log names both in its warnings, so it is a genuine model load and real training steps, not a collection that happened to succeed), and both halves of -o addopts= -m smoke are load-bearing: without the first, the job would fail on --cov-fail-under=77 rather than on the tests. The revision worth recording is the second one. The first named tests/test_smoke_train.py, which left TestOnlineDpoTrainLogsReward's two smoke-marked tests running in none of the three places — deselected by default, out of scope for mlx-smoke, outside the job's path — measured as -m smoke tests/ collecting 5 against the job's 3. That is #596's own sentence surviving inside the job written to end it. They dropped the path argument, so the job now covers the marker and the next contributor who marks something smoke gets it run without knowing this job exists. 2m01s, of which 13s is the tests, in parallel with 32-minute Windows cells (#596, #601).

• chadetov (@chadetov)

  • First contribution, and it is the kind that is easy to file as a one-line fixture tweak and much harder to diagnose: test_sft_smoke had been red since v0.36.0, and they traced why nobody knew. pyproject's addopts carry -m 'not smoke', and the only smoke job in CI is mlx-smoke, which selects -k mlx_sft_smoke on an install that asserts the PyTorch stack is absent — so test_sft_smoke and test_dpo_smoke run in neither the default suite nor CI, which is how a fixture drifted out of sync with a hard error added five releases earlier. The scope discipline is what got it merged unchanged: the MLX fixture carries the same format: chatml line and they left it alone, because it reaches the MLX trainer rather than sft_format.py and is the one smoke test CI actually runs and passes — verified, not assumed. They also declined to wire the PyTorch smoke tests into CI in the same PR and said why (model downloads, runner minutes), leaving the call to the maintainer instead of taking it (#594). Separately, and on the same day, they measured utils/log_level.py's branch coverage on #273 and filed #595 — four code sites quoting a probe-denominated VRAM ratio under a "real peak" label, plus a schema field description still carrying a justification the gate record had explicitly withdrawn

• Harshit Sharma (@harshitthek)

  • Closed a guard that existed, was cited in its own docstring, and could never fire: _telemetry_endpoint_is_safe promised that https://10.0.0.1/ could not reach an internal network, and accepted it — the HTTPS precondition sat upstream of a scheme-conditional private-IP check, so the rejection was structurally unreachable. They improved on the fix I proposed rather than implementing it. My sketch would have replaced the delegation outright; they pointed out it would lose CRLF, null-byte, type and 0.0.0.0 sanitisation, and — the part that mattered — that _is_private_or_link_local("localhost") returns False, because ipaddress.ip_address raises on domain names. A guard that blocks 127.0.0.1 and waves through its own hostname is the half-fix I would have merged. They asked which direction I wanted before writing any of it. The result layers a strict predicate on top of the existing sanitisation, leaves hubs.py alone because a private HTTPS endpoint is a legitimate self-hosted hub, and covers bracketed IPv6, uppercase, trailing-dot and IPv4-mapped forms — the last unasked for. 25 tests, every one asserting against the real function with no mocks, and they discriminate in both directions: deleting the strict layer fails 19 named tests, dropping only the loopback half fails exactly the five localhost tests, and making the validator reject everything fails the six acceptance tests, so a validator that is too strict would not have merged either (#593, #598)

• UmranPros (@umran666)

  • Caught that four code sites quoted a probe-denominated VRAM ratio under the label "the real peak" — 0.830x is the formula against the probe, while against the real training run the same shape is 0.934x. The corroboration is what makes it a finding rather than a guess: the quoted series turns over between seq 3072 and 4096, which the gate record identifies as the probe's crossover, not the training path's. They also found the stream_vram_probe field description still carrying a justification the gate record had explicitly withdrawn — that the probe under-measures preference losses "badly enough to make the gate unsafe", when it measures +13.5% HIGH — contradicting the validator 1,900 lines below it in the same file. Asked to keep both series with their denominators named rather than swap one number for another, they did, and explained the gap (the probe runs 12.5-14.3% above the real step, which is what makes it safe as a gate) — dropping the probe series would have left no record in the code that it over-reads. When review found their revision had kept an unsourced determinism claim while vaguening it, they went and found the source (#395) rather than deleting the sentence or leaving it (#595, #605)
  • Pinned both reward-hack ladder thresholds with n-minus-1 controls, which is the difference between a test that discriminates and one that merely goes green — an assertion that the ladder "fires eventually" passes for an off-by-one, and both of mine died on their new controls. They also reconciled the two ladders' documentation with what the code does (_run_bang_bang has no rollback rung at all; rollback lives only in _run_pid, and the schema enforces that by rejecting reward_hack_rollback for every other mitigation), each claim checkable against a named line rather than asserted. The scope discipline is the part worth copying: Part of #371, never Fixes, with item 1 explicitly deferred to GPU hardware they do not have and item 3 credited to another contributor's PR — on an issue where a test-only change could easily have been dressed up as closing it. Source diff is +22/-2 and comments only, which I verified by comparing ASTs with docstrings stripped rather than by reading (#371, #611)
  • Added the kimi-k2.6-grpo recipe, and pinned the model id on two independent surfaces — mutating RecipeMeta.model alone and mutating the YAML base: alone each fail a named test, so an edit to one that forgets the other cannot pass. That is precisely the guard a competing PR for the same issue lacked, and it was here before anyone asked. When the red Windows cell turned out to be #382 rather than their code, they diagnosed it correctly from the faulthandler signature instead of retrying the job. On learning the issue was already covered by an older PR they had not been able to see — the issue carried no claim comment — they offered to defer immediately and said they would search before claiming in future, which is the habit the whole collision cost a day to establish. The rebase this needed was caused by merge ordering rather than by anything in the work: two recipe PRs bump the same catalog counter and append a test class at the same point, so whichever landed second was always going to conflict (#281, #614)
  • Took utils/log_level.py to 100% branch coverage, and the PR is more correct than the issue it closes. #273 described the uncovered branch 74->73 as "an existing handler whose _soup_log_tier MATCHES the requested tier", with the repro "call setup_logging twice with the same tier" — that is a different edge, 75->73, and following it would not have closed the branch. Line 74 is if getattr(existing, "_soup_log_tier", None) is not None: and the missing case is a FOREIGN handler, one Soup did not install. They read the source rather than the ticket. Non-vacuity demonstrated rather than asserted: under the load-bearing mutation the 15 pre-existing tests ALL pass, so the two new tests are the only guard on that branch. The four deleted lines are an in-place strengthening of test_idempotent and it earns its place — restoring the old version on top of a mutation that destroys and rebuilds the handler every call gives a fully green suite, because the old assertion compared handler COUNTS, which that mutation preserves (#273, #631)
  • Raised the [train] torch floor to the one that actually binds, and took the review's duller design over their own clever one. Their first version read the floor from installed metadata so only one copy could exist; the shipped version keeps a literal in doctor.py pinned by a test against pyproject.toml. The reason is the reusable part: dist-info records what was installed, not what the repo declares, and those diverge exactly when it matters — measured on a box whose install predated the raise, soup doctor printed >=2.0.0 beside a declared 2.5.0, marked the row OK, and emitted pip install -U 'torch>=2.0.0', advising a user to install below the real floor. Two guards also did not survive review as written and they fixed both: the drift test compared declared-against-installed transformers, so reverting pyproject to 2.3.0 passed on any box below 5.16.1 while the file's own comment claimed that mutation failed by name; and the guard's stated home, the transformers-floor job, never collected it — it ran only in the ordinary matrix, and only by coincidence of 5.16.1 being the newest release on PyPI, so 5.16.2 shipping would have made it dormant with no repo change and no signal. That one is verifiable in a log rather than argued: the job goes from 36 passed to 39. Reverting the pyproject floor, changing the DEPS literal, and reintroducing a second declaration through a module constant are each killed by two named tests — the last of those survived two earlier rounds (#636, #641).
  • Made soup sweep --dry-run validate instead of returning before the config loads, and then reproduced a claim I had publicly said did not reproduce — which is the part worth recording. Their first version asserted that every real sweep on Python 3.10 died with issubclass() arg 1 must be a class; I could not make that happen and blocked on it. They came back with the discriminating variable, and it was never the Python patch level: pydantic 2.10.6's ModelMetaclass does not define __subclasscheck__ and inherits ABCMeta's, which raises, while 2.13.4 defines a tolerant one, so the PEP-585 alias never reaches abc.__subclasscheck__ on a current stack. Confirmed on my own box — the quirk isinstance(dict[str,float], type) is True is live there, and issubclass returns False rather than raising. Both observations were the same fact. They also characterised the trigger rather than leaving it abstract: training.preference_loss_weights: Optional[dict[str, float]], reached through a full model_dump(), i.e. every config. The guard that replaced the environment-dependent test is the model answer — an ABCMeta stand-in monkeypatched over pydantic.BaseModel recreates the pre-override metaclass, so removing the filter dies on any 3.10 box whatever pydantic is installed, verified on the masked stack where every earlier test had passed with the filter gone. Twelve tests became fourteen, all through the real CLI with a clean-config control; zero deleted test lines (#642, #645).
  • Settled a disputed bug by measuring it, and the answer was that my own status note was wrong — which is the harder thing to deliver than a fix. #623 said stream_pin: false was accepted and ignored; they showed the field did not exist at the v0.73.3 tag the reporter was running (grep -c stream_pin -> 0, against 18 for stream_layers as a control), so pydantic's extra="ignore" dropped the key, the default pinned path ran, and the reported OOM is what a correctly pinned run looks like. The wiring is honoured end to end on main. The PR is still worth merging, and for a reason that is the point of it: nothing guarded the chain below the layer every existing test stops at. Mutating the deepest hop so the flag is recorded and then ignored at allocation (pin_memory=self.pinned -> True) leaves 169 pre-existing streaming tests green; this file is the only thing that catches it — because it asserts is_pinned() on the real allocation rather than the recorded attribute, which is what an attribute assertion would have missed. Tests only, no source change, correctly no changelog fragment, and a non-empty-store guard so the CUDA test cannot pass while measuring nothing (#623, #647).

• Aravind S (@ARAVIND281)

  • Measured the layer-streaming VRAM fit on a second GPU and software stack — an A10G 23 GB on Ubuntu with torch 2.13 / transformers 5.16.1 / trl 0.29.1, against the RTX 3050 laptop on Windows and torch 2.5.1 that every previous figure came from. All three findings are constraints on an open question rather than extra data points beside it: the v0.73.1 long-sequence under-prediction does not reproduce (flat 1.1621-1.1694 across seq 2048-6144 where the RTX series reads 0.787x at 6144), the logits loss term measures 12.000000 exactly and invariant across vocabulary and torch version — so the shipped 14's +2 retained copy, which #327 has never identified, demonstrably does not happen here — and the SDPA hypothesis #395 names as leading is not supported, with the math backend included as a positive control that IS expensive enough (3.27 GB at seq 6144) to prove the measurement could have found it. They retracted their own headline figure mid-review — a back-solved 11.83 B/element — and then enforced the retraction in CI rather than in prose: writing 11.83 back into the constant fails a named test. The discarded first sweep is kept in the record because its failure is silent: data.max_length truncates rather than pads, so short rows run at their own length while the prediction keeps climbing, producing a clean-looking over-prediction that is entirely an artifact and would mislead the next person identically (#395, #610)
  • Closed the coverage gap in #370, whose point is that the gap did not merely miss a bug — it produced a wrong localisation. During the #328 diagnosis "only [dpo] and [kto] failed" read as a sharp finding about the reference forward; it was wrong, because the class was parametrized over those two only. ORPO and SimPO are genuinely reference-free and fail identically. The acceptance criterion they satisfied is the one most PRs skip: the test had to turn red when the fix is reverted, verified by reverting rather than assumed — and they published both revert forms with results. Reproduced at merge on a different stack than theirs (RTX 3050 / torch 2.5.1 / trl 0.19.1 against their A10G / torch 2.13 / trl 0.29.1): baseline 8 passed, dropping and not stream_layers gives 8 failed with the issue's exact Tensor on device cuda:0 is not on the expected device meta!, on all four losses — which makes the #328 failure not torch-2.13-specific, stronger than the PR claimed. They also split it into its own class rather than adding arms to the VRAM test, reasoning that hanging regression cover off a memory assertion means a later refactor deletes it silently (#370, #609)
  • Wired the MII serve backend into the shared chat-template builder and the real finish_reason, the last of three backends still hand-rolling its own prompt. Landed against a concurrent independent fix (#607) and won on a measured difference, not a timestamp: the same four mutations run against both trees showed that hardcoding finish_reason to "length" — the mirror image of the bug, which makes a continue-on-length client loop forever — passes the other suite and fails three tests here. Asked for the one gap both PRs shared — neither tested the serve.py call site, so deleting the tokenizer wiring left every test green while production fell back to the legacy prompt — they added TestServeMiiCallSite, which drives the real CLI with the pipeline factory and uvicorn patched and asserts on the kwargs build_mii_app receives, and verified it against the mutation rather than assuming. Two named tests now fail when that line is removed (#606, #608)

• Siddhardha Nanda (@SID-6921)

  • First contribution, and it found a live SSRF bypass while doing the tidy-up it was filed as. utils/tracing.py's _is_private_ip predated the #604 fix and was never folded into it, so validate_otlp_endpoint accepted https://2852039166:4317 and https://0xa9fea9fe:4317 — both 169.254.169.254, the cloud-metadata address — while refusing the dotted-quad form. The blocking finding from round one is fixed and I verified it by mutation into real files rather than by reading: six duplicates reintroduced, six caught, including the leading-underscore spelling that defeated the previous guard and two placed outside utils/, which the whole-tree walk now reaches; on the unmodified tree it reports zero offenders, because a guard that fires on correct code is one people delete. Zero deleted test lines. Two claims in the PR did not survive measurement and I corrected them myself rather than opening a third round: the consolidation is not "strictly more restrictive" — 468 differential probes show the reserved/multicast clauses change no caller's outcome, since hf.py and hubs.py raise on both branches of their if — and loop_stages is marginally more permissive, though every newly-accepted spelling canonicalises to an address it already allowed. Being wrong in a PR body is ordinary; the fix belongs in the changelog, which ships verbatim to users, not in another round-trip (#616, #625)
  • Implemented the recipe-config snapshot guard, and the number that justifies it is the one they measured rather than argued: changing ONE schema default (dpo_beta 0.1 -> 0.2) turns 149 named recipe tests red on the branch and nothing red on main. That is #621's exposure 2 — a one-line schema edit silently retuning recipes nobody touched — caught exactly. What makes it trustworthy is what it deliberately does NOT catch: deleting a recipe line that duplicates a default still kills nothing, because the issue excluded that direction, so the guard did not quietly answer a question it was told to leave alone. Five review follow-ups landed in one push and none was done to the letter while missing the point — the one worth copying is the non-vacuity controls, now exercising the diff on synthetic dicts so they stay GREEN while the real tests go red, which is what makes "did the guard break, or did the data move?" answerable from a CI log. They also pre-empted this repo's core.autocrlf fixture trap in the docstring before review raised it, and closed the one item review left open — regeneration writing CRLF on Windows — by adding newline="\n" on top of the .gitattributes belt. Verified byte-exact: git status --porcelain empty, 0 CRLF. Zero deleted test lines. Analysis, measurement and the option list are @Srinivasan8888's in #621 — they offered to implement whichever option I picked and waited on an answer I did not give for two days, which is how the work ended up split (#621, #637)
  • Delta-encoded the snapshot fixture they had built one release earlier — 1,571,967 bytes down to 66,560, a 23.6x reduction — and the encoding is provably lossless rather than merely adequate: reconstructing every recipe as baseline + delta reproduces the old fixture with zero mismatches across all 162, and the key union is 296 either way. So the drop from 149 identical failures to 1 is a change in reporting shape, not in what is detected; the two failing sets are perfectly disjoint and their union is the whole catalog. The half worth copying is the honesty: the fragment states the worst case rather than the favourable one — for a near-universally pinned field (epochs 3 -> 1) it reddens 158 named recipes, a wall of red on a change that alters no recipe's behaviour — which they volunteered without being asked. When review caught that an earlier revision explained 158 as "158 of 162 declare it explicitly" (all 162 declare it; the real rule is 126 pinning the old default plus 32 pinning the new), they re-measured the distribution independently and it matched mine exactly. Regeneration byte-exact, the Windows CRLF trap pre-empted, no assertion removed, and the property that must NOT fire still does not — deleting a recipe line that merely duplicates a default kills nothing, because #621 excluded that direction (#638, #640).
  • Made --resume work on the MLX backend, which had been silently restarting from scratch: the resolver only knew the transformers checkpoint-* shape, so mlx-lm's flat NNNNNNN_adapters.safetensors snapshots were invisible. The instructive part is the second round. Their first version copied the transformers base / experiment_name nesting into the MLX path — but mlx_sft.py writes flat — so any config with an experiment_name still produced the reporter's exact symptom. The fix is structural rather than conditional (_resolve_mlx_checkpoint takes no experiment_name at all), and the writer/resolver coupling is now pinned by a test that runs train() for real and resolves what it just wrote — coupled in both directions, so mutating either side fails it. Three inspect.getsource assertions became tests that call the code, and the MLX suite executes rather than skips by faking mlx/mlx_lm through sys.modules. They also closed an unbounded read found alongside it — --resume <a yaml file> demanded 8.6 quintillion bytes and raised MemoryError; all four hostile shapes now give one ValueError naming the path, bounded by the file size rather than a magic constant — and found a sibling themselves (exists() vs is_dir()). Seven mutations, seven killed; zero deleted test lines. Reported and diagnosed by @imahsanali, whose measurement retracted one of the two findings this was blocked on (#634, #639).

• MKnaomi2 (@MKnaomi2)

  • Made soup doctor recommend a CUDA wheel the driver can actually run, instead of telling everyone to install cu121. The fallback directions are argued, not defaulted: an unreadable nvidia-smi header falls back to cu121 rather than the newest tag, because a parse failure most likely means an old driver and a too-new wheel is the failure mode that looks like success — pip installs happily and CUDA initialisation dies later. They also verified every tag the mapping can emit against the real PyTorch index with a negative control to defeat soft-404s. Review found one blocking defect and it is the interesting one: their first tests asserted "cu121" not in advisory, which matches the installed torch version string rather than the recommendation — so they passed in CI, where torch is CPU-only, and failed on every machine with a CUDA build, the same green-on-the-runner shape that has reddened this repo four times. They fixed it by asserting on whl/cu121 and adding a test that pins the trap, so the naive form cannot return. All four non-blocking items landed in the same push, including the CWE-427 absolute-path resolution of nvidia-smi and the stale cu121 advice in README.md (#612)

• Samran Asif (@webdevsamran)

  • First contribution, and it fixed the half of #650 that mattered rather than the obvious one. The live training panel built its GPU cell from memory_allocated() in on_log, which fires between steps once activations and gradients are freed, so it reported the trough: 5.8/15.9 GB against nvidia-smi's 15,893 MiB on the same run — and 5.8 is not an underestimate, it is 1.54 B params x 4 bytes, the fp32 weights at rest and nothing else. Swapping the counter is the easy half; the reporter's actual complaint was "an unlabeled single number is the problem", so the cell is now GPU peak: and says what it is. They also wrote down why the lifetime peak is used instead of a per-window one: reset_peak_memory_stats() is process-global and would clobber the grad-accum advisor's own reading at callback.py:566, which is #650's third criterion — no decision path changes. The mocked-torch test returns 2 GB from memory_allocated and 8 GB from max_memory_allocated and asserts the panel shows 8.0, so it fails for the reason it exists; both the counter and the label die under mutation (#650, #652).

• Yuri P. Baumgartner (@YuriPerro)

  • Two precise reports in one night, then fixed the harder of them. The batch probe decided "fits" purely by the absence of OutOfMemoryError, but under WDDM the allocator does not raise when VRAM is exhausted — it spills to host RAM and the step completes — so it approved batches that do not physically fit, cached the answer, and did so on the default path (batch_size: auto). Their diagnosis is the part I could not have written: the second probe did not inherit the first one's defence — layer streaming already reads max_memory_* after its step for exactly this reason. I reproduced their bug on unrelated hardware before merging: a batch peaking at 4.87 GB on a 4.29 GB card was approved on main and is refused by their fix, while a genuinely-fitting batch still passes. Both load-bearing choices are argued from measurements already in this repo rather than picked for looking reasonable — max_memory_allocated over reserved (which runs 1.08-1.41x and is not a constant), and no safety fraction at all, because the measured probe is already +12.5-14.3% conservative. They also bumped the cache key so entries poisoned by the old probe cannot survive, and added the AcceleratorError-at-synchronize case I had not thought of. 10 of 11 mutations killed; zero deleted test lines. Separately, they held first refusal on #650 and declined it when another contributor's PR already delivered it (#649, #650, #654).

---

Want to join this list? See CONTRIBUTING.md — good first issues are labelled in the issue tracker.