Skip to content

fix(server): --dry-sequence-breakers is parsed and stored but never reaches the sampler #1103

Closed

Description

@rapsealk

Summary

Both server binaries expose a --dry-sequence-breaker(s) flag. The value flows from the CLI into ServerStartupConfig and then stops: there is no tokenization step, no default_dry_sequence_breakers field on ServerConfig, no fallback in build_server_generate_options, and no /props entry. An operator who sets the flag gets a clean startup and DRY matching that runs straight through the breakers they configured, with nothing in the logs or /props to reveal that the flag is inert.

Background

  • mlxcel serve defines the flag at src/main.rs:1261 (field dry_sequence_breakers: Vec<String>, #[arg(long, value_delimiter = ',')]).
  • mlxcel-server defines it at src/bin/mlx_server.rs:618 (#[arg(long = "dry-sequence-breaker", value_delimiter = ',')], singular).
  • The value flows into ServerCliInput (src/server/cli_input.rs:121), is copied into ServerStartupConfig (src/server/cli_input.rs:589), and lands on the field at src/server/startup.rs:170.
  • In src/server/startup.rs, dry_sequence_breakers appears only at the declaration (170) and the Default initializer (452). A tokenizer is loaded at startup (startup.rs:1866) but is never used to convert the breaker strings to token IDs.
  • src/server/config.rs:283-286 declares default_dry_multiplier, default_dry_base, default_dry_allowed_length, default_dry_penalty_last_n, and no default_dry_sequence_breakers.
  • src/server/request_options.rs:239: dry_sequence_breakers: overrides.dry_sequence_breakers.unwrap_or_default(). Every neighbouring DRY field uses .unwrap_or(config.default_*); this one has no server default to fall back on, so a request that omits the field always gets an empty vector.
  • src/server/routes/props.rs:41-44 reports the four DRY fields and cannot report the breakers because no config field holds them.

There is a type mismatch the wiring must bridge: the CLI takes Vec<String> (token strings, matching llama.cpp's --dry-sequence-breaker), while SamplingConfig and the HTTP request field take Vec<i32> (token IDs, src/server/types/request.rs:601). The conversion needs the tokenizer, so it is not a one-line copy.

Proposed Solution

Wire it (preferred) or remove it. Wiring is preferred since llama.cpp compatibility is a stated goal and the flag is in the help text of two binaries:

  1. Add default_dry_sequence_breakers: Vec<i32> to ServerConfig.
  2. Tokenize ServerStartupConfig::dry_sequence_breakers at startup, where the tokenizer is already loaded, into that field. A breaker string that does not encode to exactly one token should fail startup with a clear message rather than being silently dropped or expanded, matching the posture --allowed-origins takes for a malformed origin.
  3. Use it as the fallback at src/server/request_options.rs:239, matching the unwrap_or(config.default_*) shape of every neighbouring DRY field.
  4. Add dry_sequence_breakers to the /props payload.

Removing the flag instead is acceptable but needs a CHANGELOG.md note, since it is user-visible surface on two binaries.

Implementation Notes

Acceptance Criteria

  • --dry-sequence-breakers set at startup affects generation for a request that does not carry its own dry_sequence_breakers.
  • A per-request dry_sequence_breakers still overrides the server default.
  • A breaker string that does not tokenize to a single token fails startup with a message naming the offending string.
  • /props reports the resolved value.
  • Tests cover: default applied, request override wins, and the startup rejection.

Original Suggestion

Title: fix(server): --dry-sequence-breakers is parsed and stored but never reaches the sampler

Problem / Background

Both server binaries expose a --dry-sequence-breakers flag:

  • mlxcel serve — src/main.rs:1261, /// DRY sequence breaker token strings (e.g. "\n", "\t")
  • mlxcel-server — src/bin/mlx_server.rs:619, same flag

The value flows into ServerCliInput (src/server/cli_input.rs:121), is copied into
ServerStartupConfig at src/server/cli_input.rs:589, and lands on the field declared
at src/server/startup.rs:170.

That is the end of the trail. grep -rn dry_sequence_breakers src/server/startup.rs
returns only the declaration (170) and the Default initializer (452). There is no
tokenization step turning the strings into token IDs, and ServerConfig has no
default_dry_sequence_breakers field at all — src/server/config.rs:283-286 declares
default_dry_multiplier, default_dry_base, default_dry_allowed_length and
default_dry_penalty_last_n, and stops there.

build_server_generate_options consequently has no server default to fall back on
(src/server/request_options.rs:239):

dry_sequence_breakers: overrides.dry_sequence_breakers.unwrap_or_default(),

A request that omits the field gets an empty vector regardless of what the operator
passed on the command line.

The same gap shows in the /props response: src/server/routes/props.rs:41-44 reports
dry_multiplier, dry_base, dry_allowed_length and dry_penalty_last_n, and cannot
report the breakers because no config field holds them.

Impact

An operator running

mlxcel-server -m Qwen3.5-0.8B-4bit --dry-multiplier 0.8 --dry-sequence-breakers '\n,\t'

gets a clean startup, a flag accepted without warning, and DRY matching that runs straight
through newlines. There is no log line and no /props entry that would reveal it. The
failure is silent in both directions: nothing tells the operator the flag is inert, and
nothing tells them the penalty is stronger than they configured.

Note the type mismatch that the wiring has to bridge: the CLI takes Vec<String>
(token strings, matching llama.cpp's --dry-sequence-breaker), while SamplingConfig
and the HTTP request field both take Vec<i32> (token IDs, see
src/server/types/request.rs:601). That conversion needs the tokenizer, which is why
the wiring is not a one-line copy.

Proposed solution

Either wire it or remove it. Wiring is preferred, since llama.cpp compatibility is a
stated goal and the flag is already in the help text of two binaries:

  1. Add default_dry_sequence_breakers: Vec<i32> to ServerConfig.
  2. Tokenize ServerStartupConfig::dry_sequence_breakers at startup, where the tokenizer
    is already loaded, into that field. A breaker string that does not encode to exactly
    one token should fail startup with a clear message rather than being silently dropped
    or silently expanded — the same posture --allowed-origins takes for a malformed
    origin (docs/environment-variables.md, MLXCEL_ALLOWED_ORIGINS row).
  3. Use it as the fallback at src/server/request_options.rs:239, matching the
    unwrap_or(config.default_*) shape of every neighbouring DRY field.
  4. Add "dry_sequence_breakers" to the /props payload.

Removing the flag instead is acceptable but needs a note in CHANGELOG.md, since it is
user-visible surface on two binaries.

Acceptance criteria

  • --dry-sequence-breakers set at startup affects generation for a request that does
    not carry its own dry_sequence_breakers.
  • A per-request dry_sequence_breakers still overrides the server default.
  • A breaker string that does not tokenize to a single token fails startup with a
    message naming the offending string.
  • /props reports the resolved value.
  • Tests cover: default applied, request override wins, and the startup rejection.

Technical considerations

Depends on #1102 if you validate at temperature: 0 — until that lands, the
greedy path drops the breakers regardless of where they came from. Validate at
temperature > 0, or land #1102 first.

Activity

  1. rapsealk commented on Aug 12, 2026

    @rapsealk
    MemberAuthor

    Related: #1109 covers the flag spelling half of this — mlxcel serve uses --dry-sequence-breakers (plural) while mlxcel-server uses --dry-sequence-breaker (singular). This issue covers the wiring. Either can land first; the second should rebase rather than re-deciding the primary name.

  2. added
    type:bugBug fixes, error corrections, or issue resolutions
    and removed on Aug 12, 2026
  3. added a commit that references this issue on Aug 20, 2026
    5d08289
  4. added a commit that references this issue on Oct 27, 2026
    c2b84d4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:mediumMedium prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions