Skip to content

Latest commit

 

History

History
185 lines (167 loc) · 61.8 KB

File metadata and controls

185 lines (167 loc) · 61.8 KB

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

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 (startswithurlparse) 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).
    • 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).
  • 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)
  • 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)
  • Shutaru (@Shutaru)

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

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

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

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

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

    • 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)
  • 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 failpytest.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 routed-expert LoRA targeting for Qwen4, with focused regression coverage over PEFT target selection (#575).
  • 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).
  • 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)

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