fix(server): greedy sampling branch drops per-request dry_sequence_breakers when temperature <= 0 #1102

Summary

build_sampling_config in src/execution/sampling.rs has two branches. The sampled branch (temperature > 0.0) forwards every DRY field, but the greedy branch (temperature <= 0.0) forwards four of the five DRY fields and omits dry_sequence_breakers, so it falls back to the empty vector in SamplingConfig::greedy(). DRY is not gated on temperature, so a request such as {"temperature": 0, "dry_multiplier": 0.8, "dry_sequence_breakers": [198]} still runs the DRY penalty, but with no breakers. The backward match never stops at a boundary token, match_len grows past the intended window, and the penalty (dry_multiplier * dry_base.powi(match_len - dry_allowed_length)) is larger than requested. At temperature 0 the argmax can then land on a different token.

Background

• src/execution/sampling.rs:52 defines build_sampling_config, branching on temperature <= 0.0 at line 53.
• Sampled branch: src/execution/sampling.rs:84 threads dry_sequence_breakers: params.dry_sequence_breakers.
• Greedy branch (lines 54-71) threads dry_multiplier, dry_base, dry_allowed_length, dry_penalty_last_n, then ends with ..SamplingConfig::greedy() and does not set dry_sequence_breakers.
• SamplingConfig::greedy() at src/lib/mlxcel-core/src/generate.rs:863 sets dry_sequence_breakers: Vec::new() (line 875).
• src/lib/mlxcel-core/src/sampling.rs:389 applies DRY whenever config.dry_multiplier > 0.0 && !token_history.is_empty(), independent of temperature.
• src/lib/mlxcel-core/src/sampling.rs:846 uses config.dry_sequence_breakers.contains(&window[p1]) as the loop's break condition; an empty vector means the match never breaks at a boundary.

The comment at src/execution/sampling.rs:64 shows the greedy branch was written to thread XTC through precisely because it applies regardless of temperature. DRY is in the same class and did not get the same treatment. The server accepts the field per request on both request shapes (src/server/types/request.rs:601 and :978), so the path is reachable.

Proposed Solution

Add dry_sequence_breakers: params.dry_sequence_breakers, to the greedy branch, alongside the four DRY fields it already forwards. If instead the intent is that DRY is greedy-inert, the greedy branch should drop all five DRY fields rather than one, and that would change existing behavior for temperature: 0 with dry_multiplier > 0, so it needs a stronger justification.

Implementation Notes

• SamplingConfig::greedy() sets top_k: 1, top_p: 1.0; this change must not touch those. Greedy determinism is unaffected because DRY is a logits pre-processing step and the change only makes an explicitly requested penalty behave as documented.
• src/execution/sampling_tests.rs:70 currently asserts config.dry_sequence_breakers == Vec::<i32>::new() inside build_sampling_config_uses_greedy_defaults_when_temperature_is_zero, pinning the current behavior. It carries no comment justifying it, unlike the XTC assertion nearby, and does not assert the four DRY fields the greedy branch does forward, so it reads as a transcription artifact rather than a deliberate contract. Update it (or, if the drop is deliberate, drop the other four and add a comment saying why).
• The CLI cannot reach this path: mlxcel generate hardcodes dry_sequence_breakers: Vec::new() at src/commands/generate.rs:912 and has no flag to set it. This is a server-only defect.

Acceptance Criteria

• temperature: 0 + dry_multiplier > 0 + non-empty dry_sequence_breakers produces a SamplingConfig whose breakers match the request.
• src/execution/sampling_tests.rs:70 is updated (or, if the drop is deliberate, the four other DRY fields are dropped too and the test grows a comment explaining why).
• build_sampling_config_uses_greedy_defaults_when_temperature_is_zero also asserts dry_multiplier / dry_base / dry_allowed_length / dry_penalty_last_n, so the greedy DRY contract is stated in one place.
• cargo test --workspace --profile test-fast --features metal,accelerate execution::sampling is green.

Original Suggestion

Title: fix(server): per-request dry_sequence_breakers is silently dropped when temperature <= 0

Problem / Background

build_sampling_config in src/execution/sampling.rs:52 branches on temperature.
The sampled branch (temperature > 0.0) threads every DRY field through:

dry_multiplier: params.dry_multiplier,
dry_base: params.dry_base,
dry_allowed_length: params.dry_allowed_length,
dry_penalty_last_n: params.dry_penalty_last_n,
dry_sequence_breakers: params.dry_sequence_breakers,   // line 84

The greedy branch (temperature <= 0.0, lines 53-71) threads four of those five,
dry_multiplier, dry_base, dry_allowed_length, dry_penalty_last_n, and omits
dry_sequence_breakers, so it falls to ..SamplingConfig::greedy()
(src/lib/mlxcel-core/src/generate.rs:863-885), whose line 875 is:

dry_sequence_breakers: Vec::new(),

Nothing repopulates it downstream. BatchScheduler::enqueue_request mutates two
sampling fields after build_sampling_config returns, sampling.token_bias
(src/server/batch/scheduler.rs:3057) and sampling.xtc_special_token_ids
(:3087), and the breakers are not among them. The only other assignment to the
field anywhere in src/ is in a test
(src/distributed/disaggregated/serving_protocol_tests.rs:220).

DRY is not gated on temperature. src/lib/mlxcel-core/src/sampling.rs:389 applies it
whenever the multiplier is positive:

let last_logits = if config.dry_multiplier > 0.0 && !token_history.is_empty() {
    apply_dry_penalty(&last_logits, token_history, config)

and the breakers are the loop's termination condition
(src/lib/mlxcel-core/src/sampling.rs:846):

if config.dry_sequence_breakers.contains(&window[p1]) {
    break;
}

With the vector empty the backward match never breaks at a boundary token, so match_len
grows past what the caller asked for and the penalty
(dry_multiplier * dry_base.powi(match_len - dry_allowed_length)) is exponentially
larger than intended. At temperature 0 the argmax then lands on a different token.

The comment at src/execution/sampling.rs:64 shows the branch was written with exactly
this asymmetry in mind for XTC ("applied regardless of temperature, so the greedy branch
threads it through too"). DRY is in the same class and was not given the same treatment.

Affected surface

The server, not the CLI. mlxcel generate has no --dry-sequence-breakers flag and
hardcodes Vec::new() at src/commands/generate.rs:912, so it cannot reach this path.
The server accepts the field per request on both shapes:

• src/server/types/request.rs:601 (/v1/completions-style)
• src/server/types/request.rs:978 (native /completion)

and passes it into the overrides at src/server/routes/chat.rs:1198 and
src/server/routes/native_completion.rs:285, reaching
src/server/request_options.rs:239.

So {"temperature": 0, "dry_multiplier": 0.8, "dry_sequence_breakers": [198]} is a
supported request whose breakers do nothing.

Reproduction

Unit level, no model needed:

let mut params = sample_params();      // src/execution/sampling_tests.rs:17
params.temperature = 0.0;              // params.dry_sequence_breakers == vec[10, 20]
let config = build_sampling_config(params);
assert_eq!(config.dry_sequence_breakers, vec[10, 20]);  // fails: got []

Note that src/execution/sampling_tests.rs:70 currently asserts the opposite:

assert_eq!(config.dry_sequence_breakers, Vec::<i32>::new());

That line pins the current behavior. It carries no comment justifying it, unlike the
XTC assertion five lines below which does explain itself, and the same test does not
assert the four DRY fields the greedy branch does thread through, so the assertion
reads as an artifact of transcribing the branch rather than a deliberate contract.
Whoever picks this up should decide which reading is right and record it either way.

Proposed solution

Add dry_sequence_breakers: params.dry_sequence_breakers, to the greedy branch,
alongside the other four DRY fields it already forwards.

If instead the intent is that DRY is greedy-inert, then the greedy branch should drop
all five DRY fields, not one, but that would change existing behavior for anyone
running temperature: 0 with dry_multiplier > 0, so it needs a stronger argument.

Acceptance criteria

• temperature: 0 + dry_multiplier > 0 + non-empty dry_sequence_breakers
  produces a SamplingConfig whose breakers match the request.
• src/execution/sampling_tests.rs:70 is updated (or, if the drop is deliberate,
  the four other DRY fields are dropped too and the test grows a comment saying why).
• build_sampling_config_uses_greedy_defaults_when_temperature_is_zero also asserts
  dry_multiplier / dry_base / dry_allowed_length / dry_penalty_last_n, so the
  branch's DRY contract is stated in one place instead of half-stated.
• cargo test --workspace --profile test-fast --features metal,accelerate execution::sampling
  is green.

Technical considerations

SamplingConfig::greedy() sets top_k: 1, top_p: 1.0; this change must not touch
those. Greedy determinism is unaffected: DRY is a logits pre-processing step, and the
change only makes an explicitly-requested penalty behave as documented.