Every printed and documented `pip install 'soup-cli[extra]'` was bash / zsh /
PowerShell syntax and failed on Windows cmd.exe:
ERROR: Invalid requirement: "'soup-cli[train]'": Expected package name at
the start of dependency specifier
cmd.exe has no single-quote quoting, so it passes the quotes to pip verbatim
and pip rejects the requirement. Nothing in Soup can fix that once the command
is typed -- pip and the shell own it, and Soup is not installed yet when the
README line runs -- so the fix is the spelling we print.
Migrated 147 sites across 67 files to `pip install "soup-cli[extra]"`:
- 64 in src/ (Rich console hints + plain ImportError text)
- 57 in README.md + docs/
- 22 in src/soup_cli/templates/*.yaml + examples/configs/*.yaml
- 3 in examples/README.md
Double quotes are the only spelling valid in every shell (cmd, PowerShell,
bash, zsh), which is why the repo already used `pip install -e ".[dev]"`.
Measured on Windows: single quotes fail ONLY on cmd; double quotes pass
everywhere; bare passes on Windows but zsh globs `[extra]` and fails.
Method note (the PR #247 class): the hints sit INSIDE double-quoted Python
string literals, so a blind ' -> " sed produces SyntaxError. A tokenize-based
rewriter escaped `\"` in DQUOTE tokens and left bare `"` in TRIPLE / COMMENT
tokens; every touched .py was compile-checked. The full suite (not ruff, not
compile-check) caught two rewriter blind spots: the real YAML templates under
src/soup_cli/templates/ (byte-identical drift test) and examples/README.md.
A regression test (tests/test_v07137.py) scans the package and every docs code
block for the single-quoted form; prose may still name it so a reader from an
older tutorial recognises the error.
Also bundles #315 (@Sanjays2402): eval-gate benchmark tasks now run via
ForgettingDetector instead of a helper that never existed. Closes#310.
Test count: 16283 -> 16288 (+4 in tests/test_v07137.py).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Live smoke: `soup data canary check --manifest nope.json` surfaced
"[WinError 2] Не удается найти указанный файл: 'nope.json'" -- a raw,
locale-dependent OS error leaking straight through from os.path.getsize,
where every other Soup command says "File not found: <path>".
The v0.71.36 replay smoke hit this and could not proceed: `soup merge`
writes a directory like ./denseA whose NAME carries no size marker, so
model_size_from_name fell through to its 7B default, the hardware-fit gate
predicted 16.3 GB peak VRAM (weights 14.0) and REFUSED to train a 135M
model on a 4 GB box.
That blocks `soup merge` -> train-from-merged, which is the ordinary
continual-learning flow that --replay exists to serve: to show replay
prevents forgetting you must start from a model that already knows the old
task. The gate made that impossible.
A local checkpoint knows its own size, so ask it: parse the safetensors
header (u64 length + JSON with each tensor's shape) and sum the elements.
Only the header is read -- a few KB -- never the weights. Measured
./denseA at 0.135B where the name guess said 7.0B.
Falls back to the existing name guess for hub ids and for anything
unreadable, so nothing else changes: Llama-3-8B still 8B, SmolLM2-135M
still 0.135B, whisper-tiny still 0.039B, a missing dir still 7.0.
Truncated and absurd-length headers fall back rather than crash.
This is the third instance of the same class -- v0.71.32 (whisper sizes)
and v0.71.33 (the M suffix) were both the name guess over-predicting a
small model and the gate then refusing to train it. All three were found
by a live smoke, never by a unit test.
8 new tests; 151 existing gpu/hardware-fit tests still green.
The Step-6 live smoke on SmolLM2-135M found a real defect that 268 green
tests did not: the "MAJOR iff ANY canary <= 1%" rule ignores that the
chance of SOME canary dipping low grows with the canary count.
Measured on a model that never saw the canaries: percentiles spanned
1.6%-93%, two of ten under 10% -- pure noise -- and the old rule returned
MINOR. Analytically it is worse: under the null each percentile is
~Uniform(0,1), so at the DEFAULT --count 16 the old rule fires
MAJOR ~15% of the time on a CLEAN model (1 - 0.99^16)
MINOR ~81% of the time (1 - 0.90^16)
MAJOR exits 2, so a leak detector wired into CI would have cried wolf
roughly one run in seven. That is not shippable as a gate.
The verdict now asks whether MORE canaries look memorized than chance
explains: the observed count is tested against a binomial tail and fires
only at p < 0.05. This costs no sensitivity, because a genuinely memorized
set is unmistakable -- every canary lands at percentile 0.0.
Verified live on the same two models after the fix:
clean model MINOR -> OK (exit 0)
memorized model MAJOR -> MAJOR (exit 2)
False-alarm rate on clean models, 4000 Monte-Carlo trials per K:
K=10: MAJOR 0.5% K=16: MAJOR 1.2% K=32: MAJOR 4.4% (was 15% at K=16)
A partial leak still fires: 2 of 10 memorized is P=0.4% by chance -> MAJOR.
Tests pin the exact clean-model distribution the smoke observed, the
one-of-sixteen non-fire, the partial-leak fire, and a Monte-Carlo property
test bounding the false-alarm rate under alpha.
This overrides the spec's locked "any memorized -> MAJOR" rule. The lock
existed to prevent design thrash, not to protect a rule that measurement
showed to be broken.
ECC tdd pass, run as an 18-mutant sweep across the areas the earlier
per-module harnesses had not covered (topics internals, embed validation,
semdedup guards). 16 mutants were caught; the sweep found one real vacuity,
which is the 7th of this release.
test_bad_model_id_rejected asserted only
`pytest.raises((ValueError, TypeError))` over ["", " ", None, 123, True].
That is too broad to mean anything here: with the empty-check DELETED, ""
falls straight through to the fallback refusal ("cannot verify pooling")
and still raises ValueError, so the test passed for a completely different
reason than its name claims. Proven by mutation: deleting the empty guard
left the suite green. The length cap had no test at all; the null-byte
check had none either.
Replaced with one test per guard, each asserting the message that guard
actually emits (non-empty / null byte / too long / must be str), plus a
monkeypatched _fetch_pooling_config that fails the test if the guard lets
execution reach a network fetch. Re-ran the sweep: all three guards now
CAUGHT where two previously survived.
Also added: model_id is stripped before the allowlist lookup, so
surrounding whitespace cannot defeat it.
Verified NOT vacuous (mutation-checked, no change needed): the ctfidf
top_n slice, resolve_k's cap/clamp/n<4 rules, kmeans' iteration loop,
build_topic_report's coverage denominator + gap warning, embed's row cap /
non-str / bare-string / truncation / batch bounds / L2 zero-guard /
allowlist lowercasing, semdedup's row cap / 2-D check / nearest-kept-row
provenance.
268 tests green (1 POSIX skip), ruff clean.
ECC security-review. Its core point stands: canary.py is BY ITS OWN
DOCSTRING a secret store, yet it was the one new file skipping the two
conventions this codebase applies to every other secret-bearing artifact.
Both claims verified before fixing, not assumed.
MEDIUM — manifest was world-readable. atomic_write_text does no chmod, so
under the usual 022 umask the file lands 0644 and any local user on a
shared box can read every canary without ever running `check`. 9+ existing
modules (registry/store.py, adapter_sign.py, audit_log.py, ...) chmod 0600;
canary.py now does too, via _harden_permissions. Proven by exercising the
POSIX branch directly, since the test skips on Windows.
MEDIUM — terminal escape injection. Verified: rich.markup.escape() passes a
raw ESC byte straight through (it only neutralises [...]), and load_manifest
accepted an OSC 52 sequence in a `secret`. The manifest is explicitly a
shareable artifact, so a hostile one is realistic, and the injected
sequence renders right above the MAJOR verdict it could obscure. Now
stripped via _for_terminal, mirroring data_doctor.py / shrink.py.
LOW — load_manifest had no entry cap. The 4 MB size cap alone still admits
tens of thousands of minimal entries, each costing a model forward pass in
check. Now bounded by the same _MAX_CANARIES the generators use.
Also closed a gap the review noted in passing: `check --output` embeds every
secret, so it is as sensitive as the manifest but carried no warning and no
permissions. It now gets both.
262 tests green (1 POSIX-only skip on Windows), ruff clean.
ECC code-review found a real bug and I reproduced it before fixing.
load_dataset runs _validate_vision_images / _validate_audio_files on the
primary dataset -- they exist to reject `{"image": "/etc/passwd"}` -- but
the replay file is loaded by _load_replay_rows, which did neither. A
genuinely llava-shaped replay row keeps its `image` value through
format_to_messages, and nothing gated data.replay on modality, so a
traversal path from the replay file reached PIL.Image.open in the trainer.
Verified rather than assumed: a chatml+image row is detected as chatml and
its image key is dropped (my first probe found 0 escapes, which is why the
shape matters), but a real llava row -> detect_format='llava' -> the
traversal path survives conversion intact. Both the vision and audio cases
now have failing-first tests; the vision guard is mutation-verified.
Fix: the replay file gets its own containment pass, resolving media against
the REPLAY file's directory (the old dataset's images live with the old
dataset) unless an explicit image_dir/audio_dir is configured.
Also from the same review:
- MEDIUM: `canary insert` wrote the poisoned dataset BEFORE the manifest, so
a manifest failure left canaried data on disk with nothing to identify the
secrets in it. Manifest is now written first; if the data write then
fails, the error says the manifest describes canaries that were never
inserted rather than leaving it looking authoritative.
- LOW: --threshold help said "MinHash similarity" but --semantic reuses it
for embedding cosine.
- LOW: added --replay-seed, which had no CLI override unlike its two sibling
flags.
260 new-file tests + 337 loader/vision/audio regression tests green.
Pins that the five new kernels stay importable without the [train] extra.
v0.71.0 split the deps deliberately; a stray top-level `import torch` in a
data module would silently undo that for every `soup data ...` user.
Includes a guard-the-guard test proving the AST walk really distinguishes a
top-level import from a lazy one inside a function, plus a check that the
CLI actually exposes `data topics` and `data canary`.
Full suite: 16127 passed, 126 skipped, 0 failed. Tests 16001 -> 16254.
Thin CLI overrides for data.replay / data.replay_ratio, mirroring the
--reward-hack-mitigation style.
_apply_replay_overrides rebuilds the config rather than mutating it in
place, and that IS the mechanism: rebuilding re-runs every cross-validator,
so `--replay` on task='dpo' hits _validate_replay_compat exactly as a YAML
value would. Mutating in place would let CLI flags bypass every gate --
mutation-verified: the in-place version fails 3 named tests. It also leaves
the caller's config untouched.
Declared the flags as plain str/float with a None default, matching this
file's existing convention (name/resume/annex_xi). train.py has
`from __future__ import annotations` but never imports Optional, and Typer
must resolve annotations at runtime -- Optional[str] raised NameError at
--help time, caught immediately by the help test.
All three load paths (local / remote / HF) previously derived their own
train/val split at three separate return sites. Extracted _finalize as the
single exit point and routed all three through it, so `soup sweep`,
`soup train` and `soup train --dry-run` cannot drift on replay behaviour.
103 existing loader tests still green.
Replay is mixed AFTER the split, into train ONLY -- val stays pure
new-task because it is the yardstick for the task being learned. Old-task
retention is measured externally with soup eval custom / soup ship, so no
new eval machinery lands here.
The replay file gets its OWN format detection: the old dataset may be
alpaca while the new one is sharegpt, so it cannot inherit
data_config.format. Its path is cwd-contained like every other data input.
Mutation-verified: mixing before the split leaks replay into val; letting
the replay file inherit the new file's format silently drops every row;
removing the containment check lets the path escape cwd. Each fails a
named test.
Sample a slice of an old dataset and interleave it into the new one, so
fine-tuning on a new task does not erase the previous one. Pure: no torch,
no I/O.
NAMED rehearsal.py, not replay.py: utils/replay.py already exists from
v0.34.0 (metric-history replay for `soup runs replay`, imported by
commands/runs.py + tui_app.py). The plan said "Create utils/replay.py",
which would have silently overwritten a shipped module and broken
`soup runs replay` and `soup tui`. The user-facing flag is still --replay.
Interleave, NOT concat: `new + replay` puts every replay row in one block
at the end, so the model sees them all in the final steps -- a second
mini-finetune, which is precisely the failure replay exists to prevent.
An empty replay pool is now a true no-op that returns the new rows
untouched. It previously still shuffled, so pointing --replay at an empty
file silently reordered your training data as a side effect.
All 5 behaviours mutation-verified to fail a named test: the r/(1-r) ratio
formula (the naive r*n gives 9.09% for a requested 10%), interleave-not-
concat, shortfall-reports-rather-than-upsamples (repeating rows changes
epoch semantics), seed determinism incl. seed=None meaning 0 not random,
and sampling without replacement.
data.replay / replay_ratio / replay_seed for continual-learning rehearsal,
plus the cross-validator gate. 540 existing schema tests still green.
Ratio semantics are pinned in the field description and by test: r is the
fraction of the FINAL mixed train set, so n_replay = round(r/(1-r)*n_new).
At r=0.1 over 1000 new rows that is 111 replay rows -> 1111 total -> 10.0%.
The naive r*n_new gives 9.09% and is wrong.
Gates, each mutation-verified to fail a named test:
- task in {sft, pretrain}: replay on dpo would be silently ignored
- packing / multipack rejected: both concatenate rows into fixed blocks, so
the ratio stops being meaningful at block boundaries -- reject rather than
silently mis-mix
- footgun: replay_ratio/replay_seed without data.replay silently no-op
Provenance needs no extra plumbing: data.replay* rides model_dump(), so the
experiment tracker, the registry's config_json, soup card and the repro
receipt already capture it. Per-row _replay keys would be a coin-flip bug --
sft.py computes remove_columns from dataset["train"][0], so whether the
column survives into TRL depends on whether row 0 happens to be a replay row.
insert: mix K canaries into a dataset + write the manifest. check: rank
each canary's loss against N never-inserted controls and report
OK/MINOR/MAJOR. Exit 0/2/1 mirrors soup diagnose + soup ship so CI can
gate on a leak.
The manifest is the sensitive artifact of this feature -- not the dataset.
It is cwd-contained, symlink-refused, atomically written, and the command
says out loud that it must not be committed alongside the data it
protects.
Tests cover the wiring the kernel cannot: that the rows actually inserted
carry the manifest's secrets, and that the canary/control loss split
follows the manifest length (compute_pair_losses is index-aligned, so an
off-by-one here would silently score canaries against each other).
Documented a real edge the test fixtures surfaced: an all-equal loss
fixture makes NO control strictly cheaper -> percentile 0.0 -> MAJOR. That
is the rule working correctly on unrealistic input, and ties are the safe
direction for a leak detector, so the docstring now warns against
"fixing" it with <=, which would let a memorized canary tying the cheapest
control read as typical. The fixtures were made realistic instead.
Generation, manifest I/O, and the exposure math that decides whether a
model memorized an inserted secret. Pure: the caller supplies the losses,
so the whole decision is testable on CPU with hand-written numbers.
Measurement is loss-vs-controls (Carlini et al., "The Secret Sharer"), not
greedy regurgitation: a model can memorize a canary and still not emit it
under greedy decoding, so "nothing came back" would be false reassurance --
the precise failure a leak-detection feature must not have. Controls are
drawn from the identical secret space AND share the carrier prompt, so a
loss gap measures the secret rather than the prompt.
The rule (single source of truth, mirroring ship_verdict.py):
percentile = |{control : loss(control) < loss(canary)}| / n_controls
memorized = percentile <= 0.01
MAJOR = any memorized | MINOR = any <= 0.10 | OK = otherwise
Edge cases that would each be a silent false-negative, all pinned:
zero (or all-NaN) controls REFUSE rather than return OK -- reporting OK
against nothing is the exact false reassurance this exists to prevent;
ties count as not-strictly-less; a NaN loss reports unknown and serializes
to null, never 0.0, which would read as the strongest possible leak.
Fixtures use INTEGER control losses so cheaper/n_controls is exact -- the
semdedup boundary test in this same release was vacuous because float32
never landed on the threshold. All 5 decision-rule mutations (both
boundaries, ties, the refusal, the NaN path) are verified to fail a named
test.
Canary exposure ranks ONE canary's loss against a control distribution, so
it needs per-item losses. compute_eval_loss returns only the mean and
silently compacts the list -- a skipped pair shifts every later index, so
it cannot be used for that.
compute_pair_losses returns one loss per input pair, index-aligned, with
nan for unusable pairs (empty target span) and nan losses. compute_eval_loss
becomes a thin mean over the non-nan entries: behaviour is unchanged and
all 60 existing consumer tests (live_eval / interference / tunability) stay
green.
Mutation-verified: reverting to the old "continue" (drop) behaviour makes
test_index_aligned_with_nan_for_skipped fail, so the alignment guarantee is
genuinely pinned.
Typer/Rich layer over the pure topics kernel: embed -> k-means ->
c-TF-IDF labels -> a coverage table ("82% code, 6% math") plus a gap
warning for thin clusters. New command file rather than growing
commands/data.py, which is already 3300+ lines; registered in cli.py
mirroring data_doctor.
Honest framing is in the help text, the table footer and the docstring:
labels are emergent term clusters, NOT a classification against a fixed
taxonomy, and there is no join to `soup eval coverage`.
Dataset-derived text is escape()d before it reaches the terminal, so a
crafted row cannot inject Rich markup into the table.
Caught while wiring: atomic_write_text takes (text, output_path) -- the
plan had the arguments reversed.
BERTopic-lite over embedding vectors: pure numpy, no torch, no I/O, so the
clustering and labelling decisions are testable on CPU with hand-built
input. Labels are emergent unsupervised term clusters, NOT a
classification against an ontology, and there is no join to
`soup eval coverage` (that compares an eval suite's scorer mix to a task
taxonomy -- a different axis). Docs say so plainly.
The mutation checks caught TWO more vacuous tests, both of my own planning:
1. c-TF-IDF: the fixture made the specific term MORE frequent in-cluster
than the filler ("python" 3x vs "the" 2x), so plain per-cluster TF
already picked it and deleting the idf term changed nothing -- 8/8
passed against a broken implementation. Rebuilt on 5 clusters of
["the","the","term_i"] where the filler dominates on raw frequency and
ONLY the inverse-cluster-frequency can demote it. The margin is
arithmetic, not luck: idf ratio 2.58 vs tf ratio 2.0.
2. kmeans determinism: two well-separated blobs converge to the same
partition from ANY init, so an unseeded RNG passed the same-seed
equality check (6 seeds -> 2 partitions). Rebuilt on unstructured points
where init decides (6 seeds -> 6 partitions).
Both now fail correctly when mutated. Each fixture has a companion
"guard the guard" test pinning the property that makes it discriminating,
so neither can silently drift back into vacuity.
Every "you're missing an extra, install it" hint rendered as
`pip install 'soup-cli'` -- Rich parsed [eval]/[onnx]/[serve]/[wandb]/...
as a markup tag and dropped it. The suggested command therefore installs
the base package WITHOUT the extra the user is missing, so following the
hint appears to succeed and the feature still fails. Pre-existing across
many releases. Same class as the v0.71.28 \[mcp] fix, which only fixed its
own call site.
17 sites escaped across 9 command modules. Typer help is affected too --
rich_markup_mode="rich" renders help through Rich, so `--semantic` shipped
as "Requires soup-cli." and --track-energy lost its [carbon] hint.
Deliberately NOT escaped: `raise ImportError("... 'soup-cli[mlx]'")`,
errors.append(...), docstrings and YAML samples never reach Rich, so a
backslash there would surface literally. An initial blanket sweep hit 48
sites; ~31 were of that kind and were reverted. If such an exception text
is ever printed via console.print, the fix belongs at the print site
(rich.markup.escape), not in the message.
Tests: pin the Rich behaviour both ways (eaten unescaped / survives
escaped), scan every markup-bearing hint line, render both Typer helps
end-to-end, and assert the non-Rich counter-case stays unescaped.
Adds an embedding-cosine backend to the shipped dedup command. MinHash
remains the default and its behaviour is unchanged (45 pre-existing dedup
tests still green).
Restructured dedup() so the datasketch import moves INTO the MinHash
branch. It previously sat at the top of the function, ahead of even the
file-exists check, so --semantic would have died with "datasketch not
installed" for a user who has [train] but not [data] -- a dependency the
semantic path never uses. Output resolution + cwd containment are now
resolved once, before the branch, and shared by both backends.
Also fixes a REAL user-facing Rich-markup bug in the two hints this
function prints: "pip install 'soup-cli[train]'" rendered as "pip install
'soup-cli'" because Rich ate [train] as a markup tag -- i.e. the suggested
command installs the package WITHOUT the extra the user is missing, so
following it appears to succeed and the feature still fails. Same class as
the v0.71.28 \[mcp] fix. 18 further unescaped sites survive elsewhere in
the codebase; swept separately.
Pure numpy over already-embedded L2-normalized vectors, so the whole
selection decision is testable on CPU with hand-built vectors and no
model. DedupReport.pairs records (dropped_idx, kept_idx, cosine) so a
drop names the nearest kept row that caused it -- that provenance is what
makes the upcoming MinHash-vs-semantic comparison auditable.
The planned boundary test was VACUOUS and the mutation check caught it:
arccos(0.8) -> cos() in float32 yields 0.800000011920929, strictly above
the threshold, so `>=` and `>` behaved identically and the boundary was
never exercised (18 passed against a deliberately broken `>`). Rebuilt the
fixture on a dyadic cosine that is exactly 0.5 in float32 ([1,0] vs
[0.5, sqrt(3)/2], still unit-norm) -- the mutation now fails as it must.
test_boundary_fixture_is_exact guards the fixture itself from drifting
back into vacuity.
Completes the embedding kernel started by the pooling gate. embed_texts
lazily imports torch/transformers/numpy so the module stays importable on
the light core (pip install soup-cli without the [train] extra).
_mean_pool masks padding before averaging: an unmasked hidden.mean(dim=1)
averages pad positions too and silently drags every vector toward the pad
embedding. Pinned by a mutation-verified test (padded 999.0 -> unmasked
mean ~334 vs the correct 2.0).
resolve_pooling gates embed_texts before any download, so an unverified
model is refused rather than fetched and mis-pooled.
Renamed fixture model ids in test_cls_pooling_config_is_refused and
test_refusal_names_what_it_found so the assertion words (cls/max) can
only come from the _NON_MEAN_POOLING_KEYS label, not from the model id
string echoed back by the generic fallback message. Verified by
mutation: deleting the _NON_MEAN_POOLING_KEYS loop now fails these
tests instead of passing by coincidence.
Added test_cls_precedence_over_contradictory_mean_flag pinning that
cls/max/etc. are checked before pooling_mode_mean_tokens, so a
self-contradictory config is still refused.
Made test_allowlisted_model_short_circuits and
test_allowlist_is_case_insensitive hermetic by monkeypatching
_fetch_pooling_config to raise if called, proving the allowlist
short-circuit never reaches the network.
Alpamys