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).
  • Closed a validation leak that was total, not partial at realistic ratios. Under data.interleave with over or probs, oversampling duplicated rows before val_split cut the validation set, so the same row landed in both halves: measured through the real load_dataset, 1000 of 1000 distinct val rows also appeared in train on [10000, 1000] over 0.1, and again on probs. The fix carves validation out per source, before padding, on the local and hub paths alike, and the two are pinned independently — breaking only the local gate fails 4 tests while the hub test stays green. The part worth copying is what the second review round produced. The test named after the issue was VACUOUS: not (train & val) is trivially true against an empty val set, and its companion assertion was a total, which is invariant under never-splitting — so it passed on a build where validation did not happen at all. One added assert val_texts and it now fails alone under that mutation. They also folded in three findings that were not defects but consequences: a genuinely empty source no longer gets blamed on val_split, the runtime warning branches per strategy, and the fact that train and val now come from different mixtures once padding runs (50.0% source A in train against 90.9% in val) is documented rather than left to be discovered by someone oversampling to rebalance. Six mutations, six killed; zero test lines removed. Scope stated rather than implied: the streaming path still leaks and is filed as #702 (#680, #701)
  • Two pull requests that together close the cross-tokenizer ULD path, and they have to be read as a pair. #704: wasserstein and topk_align forwarded the student's own input_ids straight to the teacher, clamped into its vocab range — and clamping does not translate tokens between vocabularies, so a mismatched pair trained on logits the teacher computed for different text while reporting a finite, plausible loss. Silently wrong, not a crash, and the clamp did nothing at all when the teacher vocabulary was larger. #711: the ULD loss ignored the response-only label mask and the causal shift the CE term already applied, so under the default train_on_responses_only it optimised prompt tokens too — measured, per-position gradient L1 at the prompt position goes from 0.343096 to exactly 0.0. The fixture design in #711 is the part to copy. A causal-shift test proves nothing unless shifted and unshifted give different numbers, and most do not; labels=[[-100, 5, 2]] is discriminating by construction, so the off-by-one mutation (labels[:, 1:] -> labels[:, :-1]) dies where it would normally survive. They also verified the loss numerically against an independently recomputed W1 rather than asserting it merely changed, and got right an asymmetry that would have been invisible: in uld_aligned_loss the teacher logits are deliberately not shifted, because they live in teacher token space and are aggregated onto student positions by align. Nine mutations across the two, none surviving; zero test lines removed. On review they took a stale docs/commands.md line that still called both strategies cross-tokenizer and would have walked a reader into the new error (#681, #704, #682, #711)
  • Wired training.gradient_checkpointing into MLX's TrainingArgs, which had been dropping it for mlx-lm's False default while the written adapter_config.json recorded the user's intent — the artifact agreeing with a setting the run never used. The review round is the story. The first submission passed 7 tests and the mutation that restores the bug also passed 7, because the kwarg and the metadata write read the same variable and one assertion covered both. They rebuilt it so the two fail disjoint sets, and then added a mutation I had not asked for that closes a gap I had missed: misspelling the kwarg as grad_ckpt= now fails 7 tests, where _FakeTrainingArgs doing __dict__.update(kwargs) would have swallowed the typo silently. They also checked the field name against the real mlx-lm 0.31.3 sdist rather than against their own fake, which is the only way a stub-based test can be trusted about an upstream API (#685, #698)
  • Fixed the gspo GRPO variant centring its importance log-ratio over padding positions: a plain log_ratio.mean(dim=0) ran across the whole tensor, so the control variate — and therefore every real token's loss and gradient — moved with values belonging to no completion. Measured, not argued: varying only a masked position's log-ratio across -2.0 / -6.0 / +50.0 moved the loss -0.33205155 / -0.53440636 / -0.21428600 before and left it fixed at -0.28571433 after, with the gradient at that position going from 0.0800301731 to exactly 0.0. The property that made it safe to ship is the one they checked without being asked: with an all-ones mask the loss is BITWISE identical to before and to mask=None, so no user whose batches never pad sees a change, and the other six variants are unchanged to the last digit. Three mutations, three killed, including dropping the .clamp(min=1.0) that stops an entirely-masked column dividing by zero. What I would point others at is the scope honesty: #723 reports two defects, they fixed one, said so in the body before being asked, titled the PR to the half they did, and marked it Refs rather than Fixes — so the issue stayed open for the sequence-level objective instead of being silently closed under a partial fix (#723, #735).
  • Closed a blind spot nobody had noticed by pinning soup doctor's accelerate floor to the one pyproject.toml declares. The two were independent, and the drift direction that matters is doctor reporting the LOWER number: it would then certify an environment where save_fsdp_model(adapter_only=...) does not exist, which is #352's defect — a LoRA run under FSDP writing the full frozen base into every checkpoint. The gap was real, not theoretical: with doctor.py reverted and this test absent the existing suite passes, because the neighbouring floor test never reads doctor.py and _RUNTIME_FLOORS covers transformers/trl/peft but not accelerate. The part worth copying is that it pins AGREEMENT rather than a constant, so it fails when either side moves alone and stays green when a maintainer legitimately raises both — which is exactly the failure this repo shipped in #496, where a guard fired on the correct future state and went red on the very migration its exception existed to allow (#753)

• 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)
  • Took the follow-up left open on #23 after #665 landed the MLX display bridge: the benchmark harness — the one artifact that exists to demonstrate that bridge — still ran the wrapper with no display and no tracker, so nothing in the repo exercised it. The part worth copying is what review turned up and how they answered it. Two defects surfaced that were not in the original scope, and they fixed both here rather than filing them: the stdout tee dropped terminal detection, so Rich could not refresh the panel during an interactive run; and the report parser read a wrapped progress counter as a stale value, because Rich's Live/FileProxy inserts ANSI controls and folds lines. Their parser fix strips ANSI first and collapses whitespace second, and the two steps are pinned independently — removing only the strip fails 3 tests, removing only the collapse fails 24, reverting both fails 25, which a single blunt assertion would not have distinguished. The width sweep is range(20, 201) rather than hand-picked values, and 21 of those 181 widths genuinely discriminate the fix. 345 tests pass across the wider surface and 345 again under FORCE_COLOR=1 — the Rich per-character ANSI trap that has reddened this repo's CI four times. Nine mutations, no survivors, zero mocks: the display, tracker, console, tee and the harness module itself are all real objects driven through the real main(). They also disclosed, unprompted, that the Apple Silicon measurements are unchanged and a hardware run with the bridge enabled is still outstanding (#23, #703)

• 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)
  • 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)

• 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)
  • Added the kimi-k2.6-dpo recipe, completing the task trio for that base, and handled the part git would have got wrong silently. Two recipe PRs were open against #275 at once, both making the identical 163 -> 164 edit at five documentation sites and four assert len(RECIPES) sites — so git auto-merges them with no conflict and leaves the tree claiming 164 while len(RECIPES) is 165, which I materialised into six red tests before saying so. They rebased onto the other PR, took the count to 165 at all nine locations, and kept both lines of the one real textual conflict rather than dropping the sibling's. The recipe itself pins the model id on RecipeMeta.model and the YAML base: independently plus size and the license string; seven mutations, seven killed, re-run after the rebase to confirm the pins survived it (#275, #664)
  • Repaired the two shipped MLX recipes that named Hugging Face repos which do not exist — soup recipes use followed by soup train could not work for anyone — and the correction I care most about is to their own changelog. The fragment first cited the old id as "returns 401", which is the one piece of evidence #661 opens by disproving: an invented repo returns the same 401, and the authoritative signal is huggingface_hub's RepositoryNotFoundError. Publishing the 401 would have taught the wrong diagnostic to the next person; they replaced it. Likewise the new test's docstring now states plainly that it pins declared ids and does not check that they resolve, because a docstring claiming network validation is how someone concludes the ids are checked automatically and stops looking. Seven mutations, seven killed, including a third also-nonexistent id and the untouched Llama recipe; the RecipeMeta.model surface is caught by their test and by nothing else, since the snapshot guard only ever sees the YAML-derived config. They also flagged the recipe rename as a user-visible break and softened an unmeasured 8 GB memory claim to 16 GB rather than defend it (#661, #666)
  • Wired the opt-in telemetry flywheel, and shipped an implementation safer than their own pull-request description claimed. The body said httpx was moved into core dependencies and that first-run consent was captured via a rich prompt and stored on disk; neither is in the diff — pyproject.toml is untouched because delivery uses stdlib urllib.request, which solves #318's "a plain pip install silently no-ops the sender" with zero new core dependencies, and there is no prompt because default-OFF makes one hostile in CI. Both are better than the issue specified; the body was corrected rather than the code. The two review findings they closed are the ones that mattered: the sender now REFUSES to transmit while the PostHog key is still the placeholder (zero network calls, a notice on stderr) so the feature is honest in every state instead of posting into a void — my preferred option of the two I offered, and the one that unblocks merging without waiting on my account admin; and build_telemetry_payload no longer writes ~/.soup/telemetry_id unconditionally, which had been creating a persistent identifier for users who never opted in and in every developer's home directory that ran the suite. Payload pinned hardware-only by a test that fails when a field is added; all four historical IPv4 bypass spellings still rejected; measured no startup cost on the default path (1.355s -> 1.114s on soup --help, noise in their favour); six mutations, six killed (#318, #529)
  • Two hardening PRs on the same evening, and both got better under review rather than merely passing it. #705: lora.r: 0 is how the rest of Soup spells "no adapter, train the base", and on task: embedding it reached peft.LoraConfig and surfaced a raw ValueError. The interesting part is what the first version got wrong and how completely they fixed it. It implemented delegation twice — explicit properties and a catch-all __getattr__ — so three separate mutations survived, and the __getattr__ made attach_empty_param_group_guard (#336/#359) return True and stamp its marker on the OUTER wrapper while Trainer.train() calls create_optimizer on the inner one: an honest decline turned into a false success. They deleted it, and then wrote the inverse mutation test — re-adding __getattr__ now fails — which defends against the pattern returning rather than describing today's state. They also closed a subtler one I raised: lora.r: 0 made is_full_finetune true while the loader still used the frozen-base dtype helper, which returns float16 on pre-Ampere CUDA — exactly the combination v0.74.0's #425/#429 reasoning had excluded by assuming full-FT bases load at fp32. #707: fifteen Web UI read endpoints returned run configurations and live training logs with no token, on a product that ships soup ui --public and a scannable QR code. Their SSE design was right first time and better than what I would have asked for — a single-use 30-second ticket over an authenticated POST, never a ?token= in a URL that lands in proxy logs and browser history. Zero test functions deleted across 58 removed test lines: the four tests named *_no_auth_required were inverted in place rather than removed, so the file keeps a guard where a future refactor would otherwise find a hole — and that choice is measurable, since the drop-auth mutation now fails 6 tests where it failed 4. They also removed, on request, a --no-auth flag their first version had invented: there was no way to disable auth on main, so it would have made a hardening PR the first supported way to run without any. Nine mutations across the two, none surviving (#700, #705, #687, #707)
  • Third merge of the same evening, and the one whose first round taught the most. The Web UI started training with stdout=PIPE and only the SSE endpoint ever read it, so with no browser subscribed the pipe filled at ~8 KiB and the training subprocess blocked on its next write — against a minimal run emitting 14.7 KiB, i.e. the ordinary case. Their daemon-thread ring buffer is the right mechanism and shipped correctly first time. What did not was the evidence. Three tests, all green, all reasonable — and the mutation restoring the defect exactly (_train_drain_thread.start() to a no-op) left all three passing, with the suite's wall clock dropping 4.5x because nothing was waiting on a thread that no longer ran. Every test had started the drain in its own body or assigned the buffer by hand, so the helper was covered and the wiring was not. They extracted _resolve_train_argv so the real POST /api/train/start path could be driven, and kept the three original tests rather than replacing them — they were good tests, just not the one the PR needed (#688, #706)
  • Replaced the grpo_variant: gspo kernel with the published Group Sequence Policy Optimization objective (arXiv:2507.18071), and the finding worth keeping is that the issue and the PR describe different defects. #723 named a masked-token leak through batch centering — but that half had already been closed by #735, and checking main directly shows masked gradients already at zero. What #735 left behind is the one this fixes: column centering zeroes the policy gradient on any column with a single unmasked member, so 2 of 9 unmasked positions in an ordinary fixture were learning nothing. Nine of the sixteen new tests fail on unmodified main, which is what separates a rewrite from a restatement. They also caught a dead path of their own making between their two commits — the first added delta handling to the gspo kernel while the schema still rejected grpo_delta for every variant but two_sided, making it unreachable from any real soup.yaml — and fixed the schema rather than leaving code nobody could call. Verified at merge by mutation on the exact head: dropping the length-normalization divisor and reducing over all tokens instead of unmasked ones each fail four tests by name, against an 88-passing baseline. The three lines removed from the pre-existing test_v0500_part_a.py are schema-forced, not bent — the delta-rejection test is retargeted to a variant that must still reject, with a positive case added beside it. The changelog fragment states plainly that existing gspo runs will not reproduce prior numerics (#723, #744)

• 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).
  • Made the Web UI discoverable from the front door, and the blocking item is the one worth recording: the section would have promoted a command that exits 1 on a default install, because commands/ui.py imports fastapi and uvicorn from the [ui] extra and the README mentioned that extra nowhere — grep 'ui]' against the pre-merge README returned zero hits, against an issue body that said it was mentioned in the installation text. They added the install-and-launch block and the address held up to checking rather than to reading: 7860 and 127.0.0.1 are both real defaults, and the docs anchor lands on ## Web UI and not on ## Web UI Plugin Registry + Env Knobs 84 lines below it. The screenshot is the part that is not a given — soup ui prints a bearer token at startup and --public embeds it in a scannable URL, so a casually-taken screenshot of this app is a plausible way to publish a credential; theirs carries no token, no bearer URL and no local path. They also held a compact scope through three rounds and waited out a hold on #706 that was not their doing (#728)

• 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)
  • Diagnosed and fixed the release-day breakage of main five hours before I did, and the sequence is the part worth recording: my v0.74.0 release commit assembled the per-PR changelog fragments — which is exactly what a release is supposed to do — and a test that opened changelog.d/0.73.3/603.added.md by hard-coded path went red on all nine cells. They read the mechanism correctly on first contact (the credit MOVES at release time), opened directly rather than claiming first because the tree was broken for everyone, and said so explicitly. I then pushed my own repair without checking the open PR list — the overlap rule I ask every contributor to follow, skipped by me, on a PR sitting at the top of the queue. Their version was also better in one respect and that part is what shipped: the search glob 603.*.md rather than my 603.added.md, which hard-codes the CATEGORY as well as the number, so a fragment re-filed as 603.fixed.md would silently fall out and leave the test depending on CHANGELOG.md alone (#669, adopted in 450442f)
  • 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)
  • Added the deepseek-v4-flash-dpo recipe, and the hyperparameters are measured rather than plausible: re-derived against all twelve shipped DPO recipes before merge, lr=5e-6 holds 12/12, dpo_beta=0.1 holds 12/12, epochs=3 holds 11/12 — and the one deliberate deviation, batch_size: auto with grad-accum 8 taken from the SFT sibling rather than GRPO's 1/16, matches qwen3.5-35b-a3b-dpo byte for byte. What lifts it above a catalog row is where the pin goes: the model id is asserted on RecipeMeta.model and on the YAML base: independently, because the pre-existing snapshot test only ever sees the YAML-derived config, so a wrong RecipeMeta.model slips past everything — the exact shape of #661, where a shipped recipe named a Hugging Face repo that does not exist. Seven mutations, seven killed, including each model surface alone. They also flagged, unprompted, that they had cherry-picked another PR's fix onto this branch to get their own checks green, rather than letting me find it in the diff, and force-pushed it back out once that PR merged (#275, #662)
  • Turned the MLX smoke test from "the adapter file exists" into "loading it attaches LoRA parameters", and the distinction is the entire value rather than a wording preference. Per resolve_mlx_target_keys' own docstring (#392), a run can ship {"keys": ["auto"]}, linear_to_lora_layers matches nothing, load_weights(strict=False) drops every LoRA tensor in silence, and the result is an adapter that exists, satisfies both pre-existing assertions, and generates bit-identically to the base model — so a "loading did not raise" check would have been vacuous against the exact defect it was written for. Verified against four stubbed load outcomes: the healthy case passes, the #392 shape fails naming both parameter counts, and a missing or corrupt adapter fails for the right reason; neutering the assertion while injecting the #392 shape passes, which is the proof that the existence checks alone ship the inert adapter undetected. And it is not skipped where it matters — mlx-smoke runs on macos-14 on every pull request and reported 1 passed on this head, so the assertion executed against real mlx-lm on real Apple Silicon; the local skip off-Apple is correct behaviour, not dead coverage. They also scoped it honestly, stating in the PR that this proves attachment and not that the weights are the trained ones (#23, #668)
  • Published the first Apple Silicon measurements in benchmarks/ — four 4-bit models, 0.5B to 8B, on an M1 with 8 GB unified memory, all of which fit — and then corrected their own headline number downward by 2.2x when review showed the column did not close. The finding was arithmetic rather than opinion: two rows sharing a tokenizer over identical data must share a trained-token total, and at the published figures the 0.5B row implied 98.5 tokens per iteration against the 7B row's 46.5. Ground truth came from the captured mlx-lm fixtures in their own sibling PR — ~45 — so 240 tok/s was a late instantaneous reading mixed into an otherwise whole-run table, and the honest figure is 108.1. What lifts it above a correction is that they carried the consequences into the parts that made the record look worse: the unexplained 7B-slower-than-8B result was left standing at 31.5 against 32.6, because a metric fix that also dissolved an unexplained oddity would be suspicious rather than reassuring; and the warm-cache caveat, which had compared instantaneous against instantaneous and called the spread ~2x, was restated on one metric as 3.5x — larger than they had claimed. The harness now computes throughput from mlx-lm's own cumulative counter rather than recomputing tokenisation, which would drift silently the first time the chat template changed upstream, and prints nothing when the counter is absent instead of inventing a figure. The record is the working one: a failed rung with the mechanism traced, a wrong prediction left in, and three self-reported corrections — one of which, that a bare 401 from the Hub proves nothing because an invented repo returns the same, is directly load-bearing for #661 (#23, #663)
  • Gave the MLX backend the live panel, the SQLite tracker and the SSE stream it had never had — and it is genuinely reached by the command, which is the thing worth checking here rather than assuming: this repo has four recorded cases of MLX code that exists, is tested by calling the helper directly, and is never invoked by soup train (#363, #423, #659, #672). Driving the real CLI with a recording display returns start(48), per-report updates and one stop. Two decisions carry the PR. speed maps iterations_per_second, not tokens_per_second — feeding the latter into a field labelled it/s is a ~44x overstatement — and the docstring that listed the wrong key, inside the PR whose whole reason for existing is that mistake, now names the one actually read. And grad_norm is absent rather than 0.0, because a plausible zero on one backend beside a real value on another is worse than an obviously missing field. Review caught that their own test for that permitted in (None, 0.0), i.e. accepted exactly what its docstring called worse; an injected 0.0 survived, and the strengthened assertion kills it. Best of all, they wrote a test, it failed, and they corrected the test rather than the code: it had asserted the Web UI should be fed with no display attached, which the transformers path does not do either — two backends disagreeing about when /api/train/stream goes quiet is worse than both being quiet together. Thirteen tests, zero skipped on non-Apple hardware, because they drive the real train() through a fake mlx instead of gating on machines CI does not have; three mutations, three killed (#23, #665)
  • Found and closed the unauthenticated API-surface leak on soup ui --public: the token gate from #687 covered /api/* but never reached /docs, /redoc or /openapi.json, because FastAPI mounts those itself before any dependency of ours applies — so a public bind published all 24 route names, parameter shapes and schemas to the network. They chose the stronger of the two available designs: the routes are removed on a non-loopback bind rather than auth-gated, so there is no route left to get the wiring wrong on, and the test that decides it asserts 404 with a valid Bearer token. Five mutations, five killed, including both over-broad shapes — docs always on, and docs always off; the second matters as much as the first, since a test proving only the public case would accept a patch that also breaks local development. What I would point others at is the weakness they volunteered. They reported, unprompted, that three of their own mutations were killed only by an attribute assertion, because FastAPI gates every docs route behind openapi_url first and a behavioural probe alone cannot distinguish them — then converted that into a real test with a positive control pinning the upstream contract the whole design leans on, instead of leaving it as a footnote that made the suite look stronger than it was. They also enumerated all 25 routes on a public bind so the remaining surface is stated rather than assumed, and corrected a security paragraph that had been stale since #707 (#731, #732).
  • Gave validation loss the three sinks it had never reached, and the design choice that makes it trustworthy is the one that refuses to be convenient. SoupTrainerCallback read logs["loss"] and never logs["eval_loss"], so a run with val_split set computed a number that reached no panel, no database column and no API consumer. The naive fix -- hold the last value and write it every step -- fabricates data: nine database rows for two measurements, indistinguishable from real ones afterwards. They split it deliberately, so the panel is sticky (it must not blink out between evaluations) while the tracker and the SSE wire receive a value only when one was measured, and metrics.val_loss is nullable so a train-only row stores NULL rather than 0.0. The finding they most wanted on the record was against their own work: they had verified the sink at the dataclass boundary, one layer above where the value actually died in _ALLOWED_KEYS, so the first round's "all three sinks" claim was false as shipped -- they said so plainly rather than quietly repairing it. The last round closed exactly that class: a field can no longer be added to the dataclass and to _ALLOWED_KEYS while being absent from the to_payload tuple, and their new test is the single failure under that mutation. Ten mutations at merge, ten killed; zero deleted test lines; the Rich-rendered assertions verified under FORCE_COLOR=1 so they cannot break the coloured CI cells (#23, #713)
  • Built the weekly guard that resolves every recipe model id against the Hub, and the constraints are better than the feature. 165 recipes carry two independently editable surfaces each, and nothing checked that either names a repository that still exists. The guard runs on schedule and workflow_dispatch only, asks for contents: read and nothing else, references no secrets, and the script refuses to start if a Hub token is in scope -- an authenticated run would resolve gated repos the ordinary user cannot reach, which is the opposite of the question being asked. A rate limit prints "could not check" and returns 0; only a real not-found fails the job, so an unrelated docs PR can never go red because an upstream repo moved. The subtle half is pinned twice: GatedRepoError subclasses RepositoryNotFoundError, so catching the parent first silently reclassifies every gated repo as missing -- they inject both classes as parameters and assert the subclass relationship against the installed library, so if upstream un-nests them the suite says so instead of the ordering becoming quietly pointless. The budget is measured, not assumed: 118 unique ids from 330 surfaces, 64% of calls saved by caching, 29.8 s, zero limits hit. Nine of ten mutations killed by name; the tenth is currently equivalent and was recorded rather than hidden. No network in the default suite, verified by making socket.connect raise (#677, #715)
  • Gave MLX SFT the response-only mask it had been documenting and not doing — data.train_on_responses_only defaults to true and reached nothing on that path, so every MLX run trained on system and user turns while the adapter metadata recorded mask_prompt: false regardless. The measurement is why they did not simply set upstream's flag: ChatDataset masks one prefix ending before the last message, so on multi-turn chat it supervises only the final assistant turn — 772 supervised tokens before, 71 with upstream's flag, 146 with a correct mask, over 16 two-turn conversations on their own M1. Taking the flag would have looked like a fix and trained on a tenth of the intended tokens. Three review rounds, and the correction that mattered was not code: they had been mutating mlx_masking.py, the part they designed, and never mlx_sft.py, where the feature is connected — so the mutation restoring the original defect verbatim left the suite identical at 21 passed, 7 skipped. They stated that diagnosis back in their own words, killed all four survivors on a runner with no MLX, and generalised it to their other open PR without being told twice. They also volunteered that one of their own claims was false once I gave line numbers, and named the single shipped recipe whose behaviour changes rather than describing it in the abstract (#683, #733)
  • Wired the four MLX optimizer settings that the schema validated, documented and nothing read — warmup_ratio, scheduler, weight_decay, optimizer were all dropped by a build_optimizer that constructed AdamW(learning_rate=lr), so a config asking for cosine decay with warmup trained flat and said nothing. Four review rounds, and the last one is the one worth recording: the test that would have caught the final survivor already existed and asserted exactly the right property, but evaluated the schedule through a helper opening with pytest.importorskip("mlx.core"), so it was skipped on every runner this project has — while the builder-level control meant to cover for it passed warmup_ratio=0.0, which returns before the MLX import, making its assertion true for every possible implementation of the branch it was named after. They closed it by parametrising both spellings that reach the branch and asserting both halves (no cosine curve AND exactly one linear_schedule, the second being what catches a fall-through specifically), then renamed the misleading control rather than deleting it, with a docstring recording why it cannot discriminate. Nine mutations, nine killed, all on an interpreter with no mlx. They also pinned the benchmark harness against their own change before being asked (#686, #734)
  • Built the MLX validation-loss producer, and corrected the review that was blocking it. mlx-lm calls on_val_loss_report on every evaluation and the base-class body is pass, so an MLX run with a validation split computed a number and discarded it. Two blockers, both closed by measurement rather than by argument: a test named for the SSE wire passed no display, and on_train_loss_report returns at its display is None guard before both sinks, so training steps emitted nothing and its assertion held for every implementation — the same shape they had already caught and documented on the tracker sibling, which had not been carried across; and the val hook wrote a row with loss/lr/speed at their 0.0 defaults, fabricating over half the loss series. My proposed remedy for the second was wrong — I said the transformers path folds the value into one call and creates no extra row; it does create a row, and what it actually does is carry the last measured training values. They measured the merged backend, found the review's premise false, and shipped the faithful mirror instead of what they were asked for. Eight mutations, eight killed (#23, #739)
  • Found and closed the third MLX setting in one week that the schema validated, the docs described, and no code read: training.max_grad_norm. Fifteen transformers trainers forward it, one site each; MLX forwarded it nowhere and did not warn, since _check_unsupported did not list it either — so an MLX run silently trained with no gradient clipping while every other backend clipped. Their wrapper hands the clipped gradients to the inner optimizer and delegates attribute access so it stays transparent to mlx-lm. The two mutations worth recording both leave a correct-looking clip_grad_norm call in place and change only what reaches the optimizer — applying the raw gradients and discarding the clip result, and clipping after the update rather than before — so a test asserting merely that the clipper was called would pass on both; theirs asserts the clipper's return value is what the optimizer receives, and dies on each. Six mutations, six killed on a runner with no MLX. They were also honest about the boundary rather than letting a green tick imply more: no CI test asserts a clipped gradient magnitude, the arithmetic is their own M1 measurement, and what covers the real path is the macos mlx-smoke job driving the actual CLI through mx.compile (#749, #750)
  • Reported and fixed, twenty-five minutes apart, the crash that ended every clean soup eval auto run: a typer command called as a plain Python function keeps OptionInfo objects as its unpassed defaults, and OptionInfo defines neither __bool__ nor __len__, so it is always truthy — which is why the --output block fired on every direct call and died in os.fspath, past the results table and past the tracker write, in a TypeError the surrounding handler does not catch. The two-line fix is not why this PR is here: it ships a repo-wide AST guard that resolves each callee to its declaration, including through from X import name, and compares the call's keywords against whatever that command declares at the time the suite runs — neither of today's two parameters appears in it, and adding a brand-new option to the command a release from now fails five tests naming the caller, the callee and the new parameter. A second PR arrived against the same issue with a byte-identical production change, and what separated them is that theirs could be silently disabled — a leak finder returning nothing left its suite green, while this one loses two tests. They also ran the vacuity sweep on themselves unprompted and reported the number that invited the question, and declined to characterise the competing PR's results on the grounds that they were not theirs to report (#752, #754)
  • Built the guard that fails the suite when a config field is declared in schema.py and read by nothing outside it — the "validated, documented, accepted, and read by nothing" defect two consecutive releases have been named for. What makes it more than a grep is that reads are collected structurally: Load-context attribute access, Load-context string subscripts, and the name argument of getattr/get/pop/setdefault/hasattr, with writes and bare constants never collected, so a field appearing only in an error string or a docstring cannot launder itself into looking wired — and @property reads fold in only when the property is itself called. I verified it against the real schema rather than its own fixtures: an unwired field added to TrainingConfig turns it red naming that field, and a module that reads it turns it green again. The 39-entry allowlist is pinned exactly so it fails when it grows and when it goes stale, and no entry may hand-wave its reason. They also measured the hole rather than claiming there was none — an unwired training.top_k slips through where training.verbose does not, because that attribute name occurs elsewhere in the tree — and filed #761 for the four settings the guard turned out to have no consumer at all (#748, #751)
  • Turned a review request for a docstring note into an executable pin, and gave the reason that justifies it: a leak table in a docstring goes stale silently, which is the exact failure mode the guard exists to prevent. The guard's name space is global, so an unwired field whose attribute name occurs elsewhere in the tree reads as consumed — I reproduced their table by injecting real unwired fields into the actual TrainingConfig and running the real guard, with a deliberately unique control name to prove the guard fires at all: verbose caught, top_k/top_p/temperature/dtype leaked, attribution exact down to probability_policy.top_k in the CLI-unwired autodistill/ subsystem. They pinned the harder direction, that verbose must stay catchable, which guards against silent WEAKENING — a module that starts reading a .verbose attribute would otherwise leave the guard green while quietly seeing less. My first mutation survived because their assertion is hard to falsify rather than weak: renaming two of top_k's ~14 consumers is not enough, and rewriting all of them across 19 files fires with a message telling the reader to update the table rather than edit the assertion (#748, #770)
  • Gave soup doctor a --config leg that answers, before a run, which of the settings your config actually writes are read by nothing on the backend you chose — the question the "validated, documented, read by nothing" pattern had been answering one field at a time in #683, #686, #745 and #749. The part worth copying is the argument for declaring rather than inferring, which they made with measurements instead of assertion: reachability over the import graph detected none of five independently-known MLX gaps, reading the trainer module alone invented gaps for fields that live in helpers, and --dry-run exits before a trainer exists. The declared table is kept honest by a guard in both directions, and that guard shrank their own table from twelve entries to seven once #734 and #750 wired five of them — reported openly rather than quietly. Only fields the user wrote are listed (model_fields_set, not ~275 defaults), and every other task/backend pair reports nothing rather than guessing. Chasing a review note about COLUMNS=35 turned up a real product defect — at that width the report named no setting at all — fixed with folding rather than a test tweak. Verified before merge: 111 passed across the affected suites, ANSI-safe under FORCE_COLOR=1 and at COLUMNS=35, and four mutations killed, including check_config returning nothing (6 failed) and dropping the model_fields_set filter (#755, #756)

• 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).
  • measure_gemm_tflops hard-coded torch.bfloat16, so the streaming pre-flight benchmarked a dtype pre-Ampere cards never use and under-forecast throughput on exactly the hardware the free tier runs on. The trap is why this looked correct for so long: torch.cuda.is_bf16_supported() returns True on a T4 through emulation, so the obvious check agrees with the wrong answer — the probe now resolves through resolve_stream_dtype, the same function the streaming path itself uses. Review took two rounds and the second is the interesting one: the first version's panel assertion did not exist, so a mutation deleting the dtype from the user-visible string survived; their new parametrized panel test kills it, and kills it on a CPU-only run, so all nine CI cells can catch it rather than only a GPU box. They drove the pre-Ampere direction end to end without a pre-Ampere card by faking exactly one thing — is_bf16_supported(including_emulation=False) returning False, which is a T4's truth — and showed the real soup train printing measured on this card now using float16. The docs line now matches the emitted string byte for byte. Four removed test lines, all audited in-place edits, one of them a strengthening (#617, #648)
  • Closed the dependency floor that v0.74.0 shipped knowing it was wrong. That release declared torch>=2.5.0 alongside trl>=0.29, and the two cannot both be satisfied: at torch 2.5.1 torch.distributed.fsdp.FSDPModule does not exist, trl 0.29 fails to import, and DPO, KTO, GRPO and BCO are all dead. pip cannot see it because trl declares no torch dependency at all, and CI never saw it because >=2.5.0 always resolved to the newest torch. The release deliberately shipped evidence instead of a bump, because 2.5.1 was measured to fail and 2.6 was not measured to work — and answering that objection is what this PR is. Their first version raised the numbers and made the pinned-stack CI job install torch 2.6.0; review found that green proved installation and not compatibility, since not one of the five test files that job ran imported trl. They added a step that asserts FSDPModule exists and then actually does import trl / from trl import DPOTrainer, KTOTrainer, which is the failure this issue is about. The guard was then shown to discriminate rather than assumed to: run against this box, still on the old floor, the assert fires and the import fails — so it rejects exactly the version it exists to reject, while the job is green on the version being adopted. #651's hardest criterion, a test catching the "two numbers in two files nobody compares" drift, is met in both directions: reverting either the pyproject floor or the CI pin alone fails a test. Eight removed test lines, all audited, none weakened (#651, #717)

• 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

• 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).
  • Closed a gap that a merge commit admitted rather than one an issue advertised: #738 wired training.loraplus_lr_ratio to a real PEFT LoRA+ optimizer and declared resume unguarded. This proves optimizer momentum and scheduler state survive save/resume, by running a real four-step Trainer.train() twice over the same seeded data — once straight through, once stopped at the on-disk checkpoint and resumed to the same max_steps — and asserting the full per-group get_last_lr() list, since four groups at four distinct rates is the LoRA+ split and a scheduler that rewound while keeping the shape would otherwise pass. The mutations are what earn it its place: patching Trainer._load_optimizer_and_scheduler to a no-op, and separately rewinding only the scheduler, each leave seven tests green and kill this one alone — nothing else in 20,000+ tests catches either. They also stated its limit unprompted, that a run-versus-run comparison cannot see a defect damaging both runs equally, and gated it below torch 2.6 with a tuple comparison rather than the lexicographic trap that would have skipped it forever (#724, #747)

• 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).

• Abdulwaarith Zakariyya (@abdulwaarith0)

  • First contribution, and it closed a crash nobody had reported: _convert_multimodal called msg.get("content") without checking msg is a dict, and AttributeError is not in the tuple format_to_messages catches to route malformed rows to its drop path — so one bad line in a large JSONL file killed the entire dataset load instead of being skipped like every other malformed row. Confirmed at the loader rather than only in the converter: _format_rows has no try/except, so on main a 5-row file with one bad row raises straight out of load_dataset, and returns 4 rows with the fix. The test is the strong form, and that is the part worth copying: replacing the raise with continue — which keeps the row and lets the malformed message through verbatim — fails all four parameters, because the assertion is is None rather than "no exception". That is exactly the silent-corruption case _require_str_content's docstring warns about, and the easy version of this test would have passed it. Scope verified complete for the class it names across four bad shapes and ten format paths, and the qualification is theirs to be credited for too — they did not overclaim the blast radius, and review found detect_format never returns multimodal, so the path needs an explicit format: multimodal. Zero test lines removed. Review of this PR turned up the silent half of the same contract in chatml/audio/video, filed as #676 rather than widened into their diff (#670)

  • Second merge, and it closed the half their own first PR's review had split off. soup data validate answered a different question from the loader: it checked that the keys in FORMAT_SIGNATURES[fmt] were present, while load_dataset runs the converter and drops whatever raises. The gap ran both ways — a row could carry every required key and still be dropped, and six formats absent from that table were never validated at all — so on multimodal the validator reported 5/5 rows valid on a file the loader cuts to 4. The part worth copying is what they did to make the refactor reviewable. Extracting the converter dispatch touched +87/-46 in a file every training run goes through, which is exactly the diff a reviewer has to take on trust; instead they made it checkable, and I verified their claim with a differential harness of 20 formats x 51 row shapes = 1020 cases comparing output and raised exception type and message between the two trees — byte-identical, zero diffs. They also ran every shipped examples/data/*.jsonl through both builds to show no good data starts failing, and confirmed validate still exits non-zero only on a missing file or an undetectable format, so the soup ci init PR gate cannot start blocking merges. Zero mocks — the tests call the real converters. Four mutations, four killed, including neutering the per-row reason so no row is ever invalid, which fails 16 of 34 tests. Two removed test lines, both audited: a docstring, and an assertion on a message string that no longer exists, replaced by the equivalent check plus a new row-index assertion (#695, #712)

  • Third merge, and it made a documented option true for the first time. training.loraplus_lr_ratio was accepted by the schema, printed in the docs as lr_B = lr x 16, and had never run: the SFT, pretrain and embedding wrappers inserted it into training_kwargs and forwarded that to TrainingArguments, which has no such field, so enabling LoRA+ raised TypeError before the first step. The choice worth copying is refusing the mock. Their tests build a real PEFT model and a real transformers.Trainer, because a mock auto-creates the LoRA parameter groups this path depends on and would have hidden the defect -- the easy version of this test passes on the broken code. They also found the spelling that actually works rather than the one that looks right: PEFT ignores a plain weight_decay here and honours loraplus_weight_decay, and swapping the two fails exactly one named test. The second round is what made it hard to delete: a wiring-coverage scan mirroring test_issue359_deepspeed_guard_coverage, requiring every wrapper that applies a LoRA adapter to either call the helper or be listed with a reason in _LORAPLUS_NOT_IMPLEMENTED. I verified the premise for it rather than asserting it -- with the attach call replaced by pass in all three wrappers, the five pre-existing files still report 303 passed, 1 skipped, and only the new scan notices. The exemption list is checked in both directions, so an invented module fails it and an exempt module that gains the call fails it too. Seven mutations, seven killed; zero deleted test lines. They also scoped honestly without being asked: save/resume of optimizer state is untested and the thirteen preference/RL trainers are exempt rather than wired, both recorded in the exemption set instead of quietly left out (#724, #738)

• GitFromRatatouille (@swalla02)

  • First contribution, and it is the third instance of this repo's most persistent bug class found by someone else (#363, #423, #659): soup migrate refused any input named *.jsonl, a valid LLaMA-Factory YAML config included, because the guard branched on the filename while the content sniff written to gate it had zero call sites in src/. The diagnosis is worth more than the one-line fix — the test was named after the command and its body called the helper, so it passed on a codebase where migrate never imported it. Then review found a regression their own fix introduced, and the way they closed it is the part to copy: the sniff read utf-8, so a UTF-8 BOM survived as U+FEFF, which str.strip() does not remove, and a real BOM'd JSONL silently lost the friendly error it used to get — on a project with three Windows CI cells where PowerShell's Out-File writes that BOM by default. Their BOM test guards its own fixture (asserting the file really contains a BOM before asserting behaviour) and their bounded-read test measures the property with tracemalloc rather than pinning the spelling of the loop. Six mutations, six killed; zero assertions removed. They also declined to widen the fix to files whose name gives no hint, correctly citing the .ipynb case the original comment warns about, and left that as an open question rather than guessing (#672, #675)
  • Fixed the README's very first command. pip install soup-cli fails on Debian 12 and Ubuntu 23.04 or later with error: externally-managed-environment — PEP 668, the default on current Debian and Ubuntu, so the ordinary first-run experience on Linux rather than an edge case — and nothing in Soup can rescue it, because the package is not installed yet when that line runs. The install now leads with pipx and uv tool, which is right on the merits: Soup declares a soup console script, so it is an application, and both tools sit outside PEP 668's scope. Every pip line survives, under a sentence saying when it is the right choice. The best judgement in the PR is the place they refused to recommend pipx: for the contributor setup in CONTRIBUTING.md they wrote out venv instead, because an editable checkout has to be importable by the test suite and pipx isolation is precisely what stops that — recommending it there would have manufactured, for every new contributor, the resolve-to-the-wrong-copy failure this repo already fights. They reached that unprompted, and likewise left four other docs pages alone on the reasoning that an extras hint mid-prose describes a package name rather than an install procedure (#671, #673)

• Yuzhong Zhang (@BetterAndBetterII)

  • First contribution, and the fastest response to an issue this project has had: #676 was filed at 19:17 and their pull request opened at 19:48. format_to_messages documents a drop contract — a malformed row returns None so one bad JSONL line is skipped rather than corrupting the dataset — and _convert_chatml, _convert_audio and _convert_video passed a non-dict messages element through verbatim, so {"messages": ["hello"]}, or 42, or a nested list where a message object belongs, survived into training. chatml is what detect_format returns for a bare {"messages": [...]} row, so this was the default path for the most common dataset shape. The part worth copying is how they handled losing a comparison. A second pull request (#679, @v01dst) landed on the same issue ninety minutes later; an identical battery picked theirs on validator/loader agreement and mutations killed, and review then found the one thing the other PR had that theirs lacked — a isinstance(messages, list) guard, since rejecting a non-list by iterating it works by accident for "hello" and 42 and silently keeps any empty non-list iterable such as "" or {}. They folded that guard in with credit, plus the fixtures for both shapes, within two minutes of it being handed over. Verified by mutation on the merged tree: removing @v01dst's list guard fails 6 tests and removing their own element guard fails 14, so the two halves are pinned independently rather than one masking the other. Zero test lines removed (#676, #678)

• Mahmoud Ahmed (@v01dst)

  • Found the defect that neither the issue nor the competing fix had seen, and then gave it away. Their #679 arrived on #676 alongside #678; an identical battery run against both picked #678, and the single most valuable idea in either PR was theirs — an explicit isinstance(messages, list) check before the element loop, because iterating the value directly catches "hello" and 42 only by accident and silently keeps any empty non-list iterable. Both {"messages": ""} and {"messages": {}} are JSON-reachable and both detect as chatml. The finding that came out of reviewing their own PR is the one to remember: deleting that guard passed all 199 of their tests, because only an empty non-list iterable can discriminate it and that was the one case they had not written a fixture for — an implementation that was right and a suite that could not tell. Told the comparison had gone against them, they closed their own PR citing the measurement, then posted the guard into the other thread ready to apply so its author would not have to reverse-engineer it from their branch; it was merged two minutes later. Standing down on someone else's number, and then doing the work to make their win cheaper, is worth more to this project than the PR would have been (#676, #679, merged in #678)

• Jagadeep Mamidi (@jagadeepmamidi)

  • Arrived with three pull requests in one evening against a batch of freshly filed bugs, and two of them repaired v0.74.0 release regressions. #708: EmbeddingTrainerWrapper.train() reads self.trainer.model and self.trainer.args, and _EmbeddingTrainer delegated neither, so the whole transformers embedding path died after setup — a 100% crash reproduced here through the real CLI at exit 1. Eight lines. The tests are what make it, and they are the direct answer to the trap #690 was filed about: a real LlamaModel on disk, the real load_config_from_string, the real setup(), and an explicit isinstance(wrapper.trainer, _EmbeddingTrainer) — where tests/test_embedding.py holds 79 green tests over that same crashing path because a MagicMock auto-created the two missing attributes. Five mutations killed, including model returning None and args returning a bare object(), which is coverage of the delegation target rather than a presence check. #709: packing: true was a hard TypeError on the only trl range Soup supports, and the diff also retires packing_cross_doc_attn_mask — a call I asked them to justify and they did, with evidence I could check: attention_free appears in zero of nine trl wheels from 0.7.11 to 0.29.1, so the path was never live and nobody had a working cross-document run to lose. Told on review that trainer/pretrain.py carried the identical crash one file away, they took the extend-this-PR option rather than leaving a half-fixed defect, and it is pinned by its own mutation. Zero test lines removed across both (#690, #708, #691, #709)

• Dipak Chaudhari (@dchaudhari7177)

  • Took multipack FFD placement from O(N^2) to O(N log N) by descending a segment tree of per-bin remaining capacity instead of scanning every open bin — 5.5x faster at 1,000 rows, 37x at 10,000, 96x at 30,000, with per-10x-N scaling dropping from 104-130x to 8.9-13.9x. The property that decides a change like this is not that the packing is valid but that it is IDENTICAL, since moving one item to a different bin silently changes every multipack training run with no test failure to announce it; verified against the pre-change function over 6,000 randomized cases, 5,384 of them containing repeated lengths, for zero mismatches. They also wrote the one test that a behaviour-preserving revert cannot escape — no equivalence assertion can detect a pure performance regression, so a single timing test is the only guard against silently returning to quadratic, and they wrote it as a ratio measured inside one process rather than an absolute threshold, which is the shape that survives a slow CI runner. @Srinivasan8888 stood down on #716 in favour of their design on a neighbouring PR, saying it made the pin unnecessary to trust rather than merely pinning it (#694, #726)

• Thanniru Sai Teja (@iam-saiteja)

  • Made soup data validate single-pass with type-tagged row signatures — and the reason this is worth reading is the recovery, not the speedup. The first revision was written against a pre-#712 copy of validator.py, so it silently reverted that fix and soup data validate went back to reporting 3/3 rows valid on a file the loader drops two thirds of; twenty existing tests said so. They then did more than was asked on the second finding: told that a test pinning both copies of the join-separator arithmetic would be sufficient, they removed the duplication instead, so one helper now serves both functions and mutating it fails two tests where the second copy had survived +0 failures across 1432. The _to_hashable collision class ({"messages": []} and {"messages": {}} both canonicalising to () and reporting as duplicates on real chat data) is closed and verified over a 184-case differential harness across all eighteen formats. And they corrected their own number rather than defending it — the benchmark now interleaves the two implementations instead of timing them sequentially, reporting 1.34x-1.57x, where the version first shipped printed a slowdown for a change that was faster. Unrequested scope, said so at the time (#771)

• Ercan Er (@Ercaner1988)

  • Replaced the path-containment guard that could not catch the next occurrence. The old one asserted ".relative_to(cwd)" not in src over seven named files, so it leaked in both directions at once — a new module was never looked at, and inside the seven it matched exactly one literal spelling, letting .relative_to(base), a formatter-split call and .is_relative_to(...) sail past. Theirs walks the AST of all 501 modules under src/soup_cli/ and keys on the call rather than the text around it, which is what makes the spelling irrelevant. The judgement call worth copying is the one they declined to make: separating a ValueError handler that denies from one that cosmetically falls back needs taint analysis, and they reported that the cheap heuristic misclassified commands/adapters.py:38 in both directions while they were writing it, so both surviving sites went into an allowlist keyed by (file, line) with a mandatory re-derivable reason — plus test_the_allowlist_has_no_dead_entries, so a moved site is re-verified instead of rotting into a silent hole. The scan finds nothing outside that allowlist today, which is exactly why TestTheScannerCanActuallyFail is load-bearing, and they had written it before being asked. Verified here against a real offender dropped into src/soup_cli/ rather than synthetic strings: caught by file and line, as were an intermediate-assignment spelling and an aliased Path as P / PurePath receiver, and a fresh offender appended to the allowlisted adapters.py was still reported. One assertion removed, replaced in place by a strictly stronger one (#777, #778)
  • Translated the README into Turkish and built the gate that keeps a translation honest, after I made that gate the condition for accepting any translation. A sha256 stamp alone proves when a file was synced, not what it contains, so they added a structural check against README.md — heading sequence, code-fence languages and inline-code spans — which, run against their own earlier draft, found 29 dropped and 3 invented inline-code spans that heading counts could not see. Files are discovered by glob so a new language cannot land unstamped, the hash is LF-normalised so it survives core.autocrlf, and the controls call the same predicates the real assertions use. They took the narrow version when it was offered — one language instead of six — and put the cost of the ratchet into numbers themselves: 54 of the last 60 README.md commits were direct pushes that will now turn main red until the Turkish file is re-synced. Verified: 18/18 and 3/3 headings in order, 13/13 fences, commands byte-identical, all 37 relative links and localised anchors resolving (#769, #852)

• KJO92 (@Konuktor)

  • Stopped every in-process inference path from prompting a tuned model with a sequence it never trained on. soup chat, soup serve, soup infer / soup bench, soup diff, the --mole decoder, soup data generate and the live_eval generators behind soup ship / diagnose / advise all rendered the chat template to text and then let the tokenizer add its own special tokens, so a Llama-3 / Gemma / Mistral template produced a doubled BOS while Soup's default SFT path trains with one, and Soup's own data.chat_template presets (which render none) were prompted with one. The design choice worth copying is the rule they chose over the obvious one: not "drop a duplicate BOS" but "text a template rendered gets no tokenizer special tokens" — a startswith-BOS check would have fixed the vendor case and silently missed the presets, which need 1 -> 0, not 2 -> 1. fallback_on_error is keyword-only with no default, so none of the seven call sites could quietly inherit a different error contract, and four hand-rolled copies of the legacy fallback prompt were deleted rather than fixed a fifth time. 62 offline tests over a real fast tokenizer with a genuine BOS/EOS post-processor (30 of them red on main), a train/infer prefix property, and an AST ratchet with controls proving it can fail. They reported running 24 mutations; I re-ran three myself and all three were killed — the encoder reverted to pre-fix behaviour failed 23 tests. A first contribution (#781, #782)

• Suprovo Mallick (DYNO) (@DYNOSuprovo)

  • Made the DeepSpeed empty-param-group guard forward transformers' model argument. The wrapper it installed took no arguments while the method it shadows is create_optimizer(self, model=None), so the positional self.create_optimizer(model) call on the delayed-optimizer path raised TypeError — verified against a real transformers 5.17.0 Trainer, where the keyword form failed too. They departed from the issue's suggested fix for the right reason: forwarding unconditionally breaks an existing guard test whose stub takes no arguments, which the issue required to keep passing, so they split on whether a model was passed. A sentinel is received_model identity check catches the worse failure the issue named — a wrapper that accepts model and silently drops it. Mutations: the old zero-arg wrapper fails 4 of the 5 new tests; accepting model but calling original() fails 3. A first contribution (#784, #786)

• tristangrech (@tristangrech)

  • Measured where the layer-streaming pinned store is actually accounted, on an L4 — the hardware nobody in the #622 thread could supply — and published it in benchmarks/ as a working record. 4 GiB pinned lands in RssShmem in both pinned runs while a pageable control of the same size lands in RssAnon, which is what makes the attribution to pinning conclusive rather than suggestive. Host-wide Shmem drift is kept rather than rounded away, per-process and cgroup counters carry the weight, the probe refuses to run without 2x headroom and is embedded with its SHA-256, and the conclusion stays exactly as narrow as the data: statvfs on /dev/shm is not a capacity bound on that stack, and nothing is claimed about others. They claimed the issue before measuring. A first contribution (#655, #787)

• Aaron Probha (@AaronProbha18)

  • Closed the next spelling hole in the repo-wide path-containment ratchet the week it shipped: a relative_to guarded by with contextlib.suppress(ValueError): — an idiom already in src/soup_cli/ — passed a scanner that only understood try/except. They reused the rule instead of writing a second one: the ValueError predicate now serves both spellings, recursing into tuples so suppress((OSError, ValueError)) matches too. Asked to either catch the aliased imports or list them as non-goals, they caught both (suppress as quiet, cl.suppress). Only the with body counts, not the with items or code after the block — the same If.test vs If.body distinction the scanner already drew — and every control they wrote discriminates: deleting the new branch fails six red cases, and dropping the body-only gate, alias handling, tuple recursion, the attribute form or the function-boundary stop each fails at least one. Verified against real modules under src/soup_cli/, not only synthetic strings. Earlier they turned down a large hardware-bound issue because they could not verify it, and picked one they could. A first contribution (#783, #789)

---

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