Skip to content

feat: add N-gram loop detection with Gemma 4 family default-on#433

Merged
inureyes merged 4 commits into
mainfrom
feature/issue-432-loop-detection
Jun 24, 2026
Merged

feat: add N-gram loop detection with Gemma 4 family default-on#433
inureyes merged 4 commits into
mainfrom
feature/issue-432-loop-detection

Conversation

@inureyes

Copy link
Copy Markdown
Member

Summary

Adds vLLM-style N-gram repetition / loop detection that ends a generation early when the raw generated token stream collapses into a short repeated pattern, and turns it on by default for the Gemma 4 family. This breaks the upstream, weights-level Gemma 4 token-repetition collapse (google-deepmind/gemma#622, vllm-project/vllm#40080) where generation degenerates into a single repeated token or short fragment that fills the token budget, most often inside the thought channel and amplified by tool declarations or grammar-constrained decoding. Sampling penalties (repeat_penalty, DRY) cannot recover once the logits collapse, so distribution shaping is not enough; this is a stop condition on the raw stream instead.

Design

The detector is generic and model-family agnostic; the activation policy lives in the server control plane.

  • mlxcel-core gains a loop_detection module: LoopDetectionConfig (vLLM field names max_pattern_size / min_pattern_size / min_count, all default 0 = disabled) and the pure, unit-testable detect_repetition_loop(generated, cfg). For each pattern size p in min_pattern_size..=max_pattern_size it checks whether the last p * min_count tokens are a single p-length block repeated min_count times. This catches single-token loops (p=1) and short multi-token loops (p=2,3,...). It runs only when enabled, so a model that does not opt in pays nothing.
  • SamplingConfig gains a loop_detection: LoopDetectionConfig field defaulting to a zero-overhead no-op, mirroring how token_bias / stop_token_ids preserve the bit-exact baseline.
  • The detector is invoked at every decode stop site (the four decode loops in generate.rs and the three scheduler decode paths) right after each token is pushed to generated_tokens, alongside the existing eos/length checks, so it also catches loops inside the reasoning channel and tool-call JSON, not just the final answer. A new FinishReason::RepetitionLoop is added with its SequenceState transitions; the OpenAI wire finish_reason stays "stop" (an early, non-error stop, matching vLLM), and the scheduler logs an info line when it fires.

Activation policy (issue "Preferred activation surface")

Default-on for the Gemma 4 family with no configuration required, implemented in request_options::resolve_loop_detection (the server request to SamplingConfig assembly), keyed off the loaded model type resolved once at startup (ModelType::Gemma4 | Gemma4VLM | Gemma4Unified). Precedence, highest first:

  1. Explicit per-request fields (max_pattern_size / min_pattern_size / min_count on the OpenAI chat SamplingParams); a client may tune or disable (set max_pattern_size=0) but never has to send anything.
  2. Global operator override MLXCEL_LOOP_DETECTION (off / on / MIN,MAX,COUNT), usable on any model to force-enable, tune, or force-disable.
  3. Gemma 4 family default-on with the conservative built-in threshold min_pattern_size=1, max_pattern_size=20, min_count=4, unconditional (does not require tools or a structured-output request, so plain Gemma 4 chat is covered).
  4. Disabled for every non-Gemma-4 model, preserving the exact baseline output.

What changed

  • src/lib/mlxcel-core/src/loop_detection.rs (new) + loop_detection_tests.rs (new): generic detector and config, with thorough unit tests.
  • src/lib/mlxcel-core/src/generate.rs, lib.rs: SamplingConfig.loop_detection field, re-exports, and the detector call at all four decode loops.
  • src/server/batch/sequence.rs: FinishReason::RepetitionLoop + transition-table entries + integration-style tests that drive the real scheduler guard to the new finish reason.
  • src/server/batch/scheduler.rs: the loop guard at the three decode paths plus an info log when it fires; RepetitionLoop treated as a healthy cache-donation finish.
  • src/server/request_options.rs (+ tests), config.rs, startup.rs: the resolution helper, the loop_detection global override (MLXCEL_LOOP_DETECTION), and the model_is_gemma4_family startup flag.
  • src/server/types/request.rs, src/server/routes/chat.rs, src/server/routes/native_completion.rs, src/execution/sampling.rs, src/distributed/disaggregated/serving_protocol.rs: thread the new fields through the request and sampling-config surfaces (the disaggregated handoff keeps the disabled baseline; serializing the config across the prefill/decode split is a noted follow-up).
  • docs/environment-variables.md, docs/supported-models.md: document the params, the default-on policy, the override surfaces, and the precedence.

Test plan

  • cargo test -p mlxcel-core --lib loop_detection:: (14 passed): detector boundaries (fires at exactly min_count, not before; multi-token loops; clamp / normalization; disabled no-op).
  • cargo test --lib server::request_options (12 passed): resolution precedence, family default-on without amplifier, non-Gemma disabled, per-request and global overrides.
  • cargo test --lib server::batch::sequence (30 passed): includes a deterministic integration test that drives the real scheduler stop guard (detect_repetition_loop + transition_to) to FinishReason::RepetitionLoop.
  • cargo check --lib --tests and cargo clippy --lib --tests -- -D warnings clean; cargo fmt applied.
  • Real-model smoke on models/gemma-4-e2b-it-qat-4bit (release mlxcel-server, --jinja --parallel 1):
    • (a) Family default-on, no per-request loop fields and no tools: a forced repeated tail ended early with finish_reason=stop, completion_tokens=173 of 300, and the server logged loop detection: ending generation early (repetition loop) generated=173. A normal tool-laden generation completed coherently (no regression).
    • (b) Non-Gemma models/qwen3-0.6b-4bit, same prompt: detection disabled by default, ran to finish_reason=length (300 tokens), no firing. Baseline preserved.
    • (c) Gemma 4 with per-request max_pattern_size: 0: detection disabled even for the family, ran to finish_reason=length (300 tokens), no firing. Override precedence confirmed.

Closes #432

Gemma 4 has an upstream, weights-level token-repetition collapse (google-deepmind/gemma#622, vllm-project/vllm#40080) where generation degenerates into a single repeated token or short fragment that fills the token budget, most often inside the thought channel and amplified by tool declarations or grammar-constrained decoding. Sampling penalties (repeat_penalty, DRY) cannot recover once the logits collapse, so this adds a vLLM-style N-gram repetition detector that ends a generation early when the raw generated stream collapses into a short repeated pattern.

Changes:

- mlxcel-core gains a generic, model-agnostic loop_detection module: LoopDetectionConfig (vLLM field names max_pattern_size / min_pattern_size / min_count, all default 0 = disabled) and the pure detect_repetition_loop helper, plus a loop_detection field on SamplingConfig that defaults to a zero-overhead no-op preserving the bit-exact baseline for every model that does not opt in.
- The detector is invoked at every decode stop site (the four decode loops in generate.rs and the three scheduler decode paths) right after each token is pushed, alongside the existing eos/length checks. A new FinishReason::RepetitionLoop is added with its SequenceState transitions; the OpenAI wire finish_reason stays "stop" (an early, non-error stop, matching vLLM), and the scheduler logs an info line when it fires.
- Server policy lives in the control plane, not in mlxcel-core: the Gemma 4 family (Gemma4 / Gemma4VLM / Gemma4Unified) is default-on with the built-in conservative threshold (min_pattern_size=1, max_pattern_size=20, min_count=4), with no per-user configuration and no end-user toggle. Precedence, highest first: explicit per-request fields, then the global MLXCEL_LOOP_DETECTION env override, then the Gemma 4 family default-on, then disabled. Non-Gemma models stay disabled, preserving the bit-exact baseline.
- Adds the three vLLM fields to the OpenAI chat SamplingParams (per-request tune/disable, including max_pattern_size=0 to opt out) and the MLXCEL_LOOP_DETECTION operator override. Documented in docs/environment-variables.md and docs/supported-models.md.

Validated with unit tests (detector boundaries, resolution precedence, family default-on), a deterministic integration test that drives the real scheduler stop logic to FinishReason::RepetitionLoop, and a real-model smoke on gemma-4-e2b-it-qat-4bit: a forced repeated tail under the family default-on ended early with finish_reason "stop" (173/300 tokens, "loop detection: ending generation early" logged), the same request with max_pattern_size=0 ran to length, and qwen3-0.6b-4bit (non-Gemma) was unaffected.
@inureyes inureyes added type:enhancement New features, capabilities, or significant additions priority:high High priority area:inference Generation, sampling, decoding (incl. speculative, DRY) status:review Under review labels Jun 24, 2026
inureyes added 2 commits June 25, 2026 08:07
…unconditional

The pr-reviewer (issue #432) flagged stale internal comments in config.rs, startup.rs, and execution/sampling.rs that described the Gemma 4 family loop-detection auto-enable as gated on the documented amplifiers (tools or a json_schema response_format). The implemented policy is unconditional engine-level default-on for the family, so plain Gemma 4 chat is covered too. Comment-only, no behavior change.
detect_repetition_loop runs once per decode step, and a per-request max_pattern_size (OpenAI chat field) or the MLXCEL_LOOP_DETECTION global-env triple both build a LoopDetectionConfig with no upper bound. With min_count=2 and a large max_pattern_size the per-step tail scan grows to O(n^2) in the generated length and O(n^3) across a long generation, so a single crafted request can pin the worker thread for the life of the generation. The Gemma 4 family default (max=20) was already safe; only the client/operator override surfaces were exposed.

Cap the effective scanned pattern size at MAX_EFFECTIVE_PATTERN_SIZE (64) inside the detector, the single chokepoint every path flows through (per-request, global-env, and family-default), so the bound applies uniformly without touching each entry point. effective_min_pattern_size now clamps to the capped maximum so a huge configured max paired with a min above the cap still yields a well-formed range. Per-step work is now bounded regardless of the configured values, and patterns longer than the cap (well beyond the 1-20 token collapses seen in practice, google-deepmind/gemma#622) are simply not scanned. Adds detector tests covering the huge-max case, the cap boundary, and the min-above-cap no-panic case.
@inureyes

Copy link
Copy Markdown
Member Author

Security and performance review

Reviewed the N-gram loop-detection change (detector, server resolution, request/env override surfaces, scheduler/sequence integration, finish-reason wiring). One HIGH finding addressed; all other concerns verified safe.

HIGH (fixed): unbounded effective max_pattern_size -> CPU-DoS

detect_repetition_loop runs once per decode step. A per-request max_pattern_size (OpenAI chat field) and the MLXCEL_LOOP_DETECTION global-env triple both build a LoopDetectionConfig with no upper bound. With min_count=2 and a large max_pattern_size, the per-step tail scan is O(n^2) in the generated length and O(n^3) across a long generation, so a single crafted request can pin the worker thread. The Gemma 4 family default (max=20) was already safe.

Fix (commit 91d499d21): cap the effective scanned pattern size at MAX_EFFECTIVE_PATTERN_SIZE = 64 inside the detector, the single chokepoint every path flows through (per-request, global-env, family-default), so the bound applies uniformly without touching each entry point. effective_min_pattern_size now clamps to the capped maximum so a huge configured max paired with a min above the cap still yields a well-formed scan range. Patterns longer than 64 tokens (well beyond the 1-20 token collapses reported in google-deepmind/gemma#622) are no longer scanned. Added three detector tests: huge-max still fires and terminates, the cap boundary (length-64 fires, length-65 does not), and the min-above-cap no-panic case.

Verified safe (no change needed)

  • No-op when disabled: is_enabled() short-circuits before touching the slice; the token stream is passed by reference (no clone/alloc). Non-Gemma models and max_pattern_size=0 keep the bit-exact baseline.
  • Bounded tail / no panic: the scan reads generated[len-window..] only after a len < window guard (no underflow), p.checked_mul(count) guards overflow, and p >= 1 so chunks_exact never panics.
  • Finish-reason wiring: FinishReason::RepetitionLoop maps to the wire finish_reason="stop" via completion_tokens < max_tokens (matches vLLM), has its state-transition entries, and is correctly grouped as a healthy cache-donation finish (prompt-cache reuse keys on the exact token prefix and the KV up to the committed loop tokens is mathematically valid, so no poisoning).
  • Input validation / fail-safe: request fields are Option<usize> (malformed -> clean 422); the MLXCEL_LOOP_DETECTION parser returns None on malformed input with a warning (no panic), failing safe to the per-family policy.

Verification (narrow targets only)

  • cargo test -p mlxcel-core --lib loop_detection:: -> 17 passed
  • cargo clippy --lib --tests -- -D warnings -> clean

No remaining CRITICAL/HIGH items.

Add 7 tests for `resolve_loop_detection_env` in `startup_tests.rs`, covering all parser branches: unset returns `None`; the `off`/`0`/`none`/`false`/`disabled` keywords produce `Some(disabled())`; the `on`/`default`/`true`/`enabled` keywords produce the recommended threshold; comma-separated and colon-separated numeric triples are parsed correctly; a malformed value and a wrong-field-count value each return `None`.

Also extend the "Sampling parameters" section of `docs/python-client.md` to name the vLLM-compatible loop-detection fields (`max_pattern_size` / `min_pattern_size` / `min_count`) alongside the other non-OpenAI knobs so they are discoverable through the client reference.
@inureyes

Copy link
Copy Markdown
Member Author

PR finalization complete

Summary

Tests added (7 new, in src/server/startup_tests.rs): covered all branches of the MLXCEL_LOOP_DETECTION env parser (resolve_loop_detection_env), which was the only public-surface gap. Branches: unset returns None; off/0/none/false/disabled force-disable; on/default/true/enabled apply the recommended threshold; comma-separated and colon-separated numeric triples are parsed and ordered correctly; a malformed value and a wrong-field-count value both return None without panicking.

Documentation updated (docs/python-client.md): added the vLLM-compatible per-request loop-detection fields (max_pattern_size / min_pattern_size / min_count) to the "Sampling parameters" discoverability list alongside the other non-OpenAI knobs. The MLXCEL_LOOP_DETECTION env var section, Gemma 4 default-on policy, and precedence table in docs/environment-variables.md and docs/supported-models.md were already complete and accurate.

Lint/format: cargo fmt made no changes; cargo clippy --lib --tests -- -D warnings was clean before and after.

// Used by: annotations: detect_repetition_loop already carries the correct annotation covering both call sites (generate.rs and server/batch/scheduler.rs).

No translations to update: docs/en/ and docs/ko/ do not exist in the repo (the mkdocs config references them as a future destination).

Verification output

  • cargo check --lib --tests: finished in ~10 s, 0 errors
  • cargo clippy --lib --tests -- -D warnings: finished in ~0.2 s, 0 warnings
  • cargo fmt: no changes
  • cargo test -p mlxcel-core --lib loop_detection::: 17 passed
  • cargo test --lib server::request_options: 12 passed
  • cargo test --lib server::batch::sequence: 30 passed
  • cargo test --lib server::startup::tests::loop_detection: 7 passed

Benchmark CSVs under benchmarks/pylm_*.csv were not staged, stashed, or touched.

Commit: 39be67d

@inureyes inureyes added status:done Completed and removed status:review Under review labels Jun 24, 2026
@inureyes
inureyes merged commit f6b1e2b into main Jun 24, 2026
5 checks passed
@inureyes
inureyes deleted the feature/issue-432-loop-detection branch June 24, 2026 23:28
inureyes added a commit that referenced this pull request Jun 25, 2026
The initial v0.3.3 cut (4d5f4fb) was never tagged, and 16 more commits
landed on main afterward. Fold those fixes into the v0.3.3 CHANGELOG and
debian/changelog entries and set the release date to 2026-06-25:

- N-gram loop detection (#433) and Nemotron-H Nano Omni audio input (#443)
- Prefill masks sized from the live window under --max-kv-size trim
  (#418, #420, #422, #431)
- Double-transpose crash on mlx-community conv checkpoints (#429)
- conv1d/conv2d and nemotron audio-encoder convs fallible at the FFI
  boundary (#434, #439)
- Gemma 4 audio placed in the user turn (#438, #440)
- mistral4 MLA backbone routing and 2D MoE token flattening (#423, #425)
- Bump actions/checkout from 6 to 7 (#395)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) priority:high High priority status:done Completed type:enhancement New features, capabilities, or significant additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add generation loop/repetition detection to break Gemma 4 token-repetition collapse under tool/grammar-constrained decoding

1 participant