fix(server): wire --dry-sequence-breaker through to the sampler

[inureyes]

Summary

Both server binaries expose --dry-sequence-breaker, and the value flowed from the CLI into ServerStartupConfig and stopped there. There was no tokenization step, no default_dry_sequence_breakers on ServerConfig, no fallback in build_server_generate_options, and no /props entry. An operator got a clean startup, a flag accepted without warning, and DRY matching that ran straight through the breakers they configured.

Related issues

Closes #1103

Type of change

• fix (bug fix)

Why wiring it rather than removing it

The issue offered both. Wiring wins because llama.cpp compatibility is a stated goal and the flag sits in the help text of two binaries, and because the failure was silent in both directions: nothing told the operator the flag was inert, and nothing told them the penalty was stronger than they configured. That second half is the part worth stressing. The breakers are the DRY backward match's termination condition, not a switch, so running DRY without them lets match_len grow past the intended boundary and the penalty comes out at or above what was asked for (the same mechanism as #1102, from the other end).

What changed

src/server/dry_breakers.rs (new, 136 lines). resolve_dry_sequence_breakers(tokenizer, raw) -> Result<Vec<i32>>, plus the private unescape_breaker. Modelled on src/server/cors.rs, which is the existing precedent in this tree for "validate an operator string at startup and name the offending value".

The resolution point. The flag takes token strings; SamplingConfig and the HTTP field take token IDs. The conversion needs the tokenizer, which is not loaded when build_server_config runs, so it cannot live there. build_server_config leaves the new field empty with a comment stating why, and run_server fills it immediately after load_tokenizer returns, then logs the resolved IDs at INFO alongside the strings they came from.

The resolver discounts the tokenizer's normalizer. Encoding a breaker on its own is the obvious implementation and is wrong for every SentencePiece-derived checkpoint: their tokenizer.json carries a Prepend "▁" normalizer, usually in a Sequence with Replace " " -> "▁". Eleven of the 162 checkpoints in the local model store have it (Mixtral, three Phi-3 variants, two MiniCPM, two LLaVA, Idefics2, InternLM3, ERNIE 4.5). On Mixtral, encode("\n") returns two tokens even though the newline is a single vocabulary entry at id 13, so startup would have refused to boot on the example in this flag's own help text. Worse, encode(" ") normalizes to "▁▁" and returns ONE token, the double-space entry at id 259: it passes the length check, startup succeeds, /props reports a plausible id, and the operator silently gets a breaker they never asked for. encode_breaker therefore encodes anchor + breaker and subtracts the anchor's own encoding, so what is measured is the breaker's contribution rather than the normalizer's fixed prefix. When the anchor merges with the breaker or encodes to nothing, the bare encoding is used.

Fail closed on anything unrepresentable. A breaker that does not encode to exactly one token fails startup with a message naming it and reporting the count it got. Single-token is not a policy choice: the sampler's check is config.dry_sequence_breakers.contains(&window[p1]) over a Vec<i32>, so a multi-token breaker has no representation at all. A string that encodes to nothing is the same class of failure and is rejected identically. A flag that was set but produced nothing usable is an error rather than a silent no-op, mirroring parse_allowed_origins.

Empty entries are skipped, blank ones are not. --dry-sequence-breaker 'a,' splits to ["a", ""] on the clap delimiter, so the trailing empty is a typo rather than a breaker. The test is is_empty(), deliberately not trim().is_empty(): a single space is a legitimate single-token breaker in most vocabularies, and trimming would silently discard it.

Escape handling, without which the flag's own documentation would fail startup. --dry-sequence-breaker '\n' reaches the process as the two characters \ and n, because no common shell expands an escape inside quotes, and the help text has advertised "\n" and "\t" as its examples since the flag was added. Reading the value literally would reject the documented usage, or worse accept it as whatever unrelated single token \n happens to encode to. \n, \t, \r and \\ are interpreted; every other backslash sequence is preserved exactly as typed, so a breaker that genuinely contains a backslash is never silently rewritten, and \\ is the escape hatch.

src/server/request_options.rs. unwrap_or_default() becomes unwrap_or_else(|| config.default_dry_sequence_breakers.clone()), matching the unwrap_or(config.default_*) shape of every neighbouring DRY field.

src/server/routes/props.rs. The payload gains dry_sequence_breakers, reported as resolved token IDs (what the sampler compares against, and what a per-request value overrides). The payload construction moves into default_generation_settings(&ServerConfig) so the reported field set can be asserted without standing up an AppState.

Diagnostics, since a resolved id cannot be checked by eye. The rejection message renders each token as "piece"=id, which is what makes a normalizer artifact visible, and no longer suggests "\n" as the remedy, since that may be exactly the value that failed. Startup logs the decoded pieces alongside the ids. The "no usable breaker" error explains that the flag splits on commas, so an operator who passed a bare comma is not told their entries were empty.

Help text on both binaries, kept byte-identical so #1109's description-parity assertion stays green. It now states that the flag sets a server-wide default a request can override, that each value must encode to one token or startup fails, and which escapes are interpreted.

Test plan

• cargo test --profile test-fast --features cuda --lib server::dry_breakers (15 passed)
• cargo test --profile test-fast --features cuda --lib server::routes::props (4 passed)
• cargo test --profile test-fast --features cuda --lib server::request_options (39 passed, up from 35)
• cargo test --profile test-fast --features cuda --test cli_help_consistency (25 passed, confirming the --dry-sequence-breaker prose stayed byte-identical on both binaries after the help-text edits)
• cargo test --profile test-fast --features cuda --lib server::cli_input (93 passed)
• cargo test --profile test-fast --features cuda --lib execution::sampling (3 passed)
• cargo clippy --profile test-fast --features cuda --lib --bins --tests -- -D warnings clean
• cargo fmt --all -- --check clean
• cargo test --workspace --profile test-fast and cargo deny check not run here; no dependency changes.
• No real-checkpoint validation. The resolver is exercised against a purpose-built vocabulary instead (see below); the acceptance criteria are all at the configuration boundary.

On the test fixture

MlxcelTokenizer::stub_with_byte_fallback() turned out unusable here: with no BPE merges, Hello tokenizes to nothing rather than to its vocabulary entry, so it cannot express "exactly one token". The tests build a small tokenizer whose vocabulary is single characters (a, b, newline, tab, space), which makes all three outcomes the resolver must distinguish reachable without a checkpoint: one token, more than one, and none. fixture_tokenizer_behaves_as_the_tests_assume pins those four encodings, so a fixture change fails there with a clear message instead of making the real assertions look wrong.

That fixture was necessary and not sufficient, which is worth recording. Its encode is a pure per-character vocabulary lookup, precisely the tokenizer shape in which the normalizer defect above cannot occur. Every test passed, none of them vacuously, and the resolver was still wrong for eleven local checkpoints. A second fixture now carries the Prepend "▁" / Replace " " -> "▁" pair with merges that make ▁a and ▁▁ real entries, and prepend_normalizer_fixture_reproduces_both_bare_encoding_defects asserts the bare encoding still exhibits both defects, so the two tests guarding the fix are demonstrably guarding something.

Coverage: the escape handling at the string level and end to end through tokenization ('\n' resolving to the newline token id); the prepend-normalizer cases in both directions (a newline that must resolve rather than fail, and a space that must not resolve to the double-space token); the server default applying when a request omits the field; a per-request value overriding it; an explicitly empty request list disabling it; a serde-level assertion that an absent field and an explicit [] deserialize differently, which is the layer where they could actually collapse; the startup rejection naming the offending string, its token count, and its decoded pieces; a whitespace-only breaker surviving; and the /props payload's full key set.

Pre-existing failure, not from this PR

server::startup::muse_glimmer_startup_guard_tests::muse_glimmer_startup_allows_baseline_and_keeps_video_disabled fails when run alongside its siblings and passes in isolation. Its sibling muse_glimmer_startup_rejects_xla_backend_selection sets MLXCEL_BACKEND=xla and restores it under a crate-wide env lock the baseline test does not take, so the baseline test can observe the variable mid-window. The file was last touched by #1101 and is not in this diff.