fix(server): Gemma 4 default-on loop detection falsely truncates markdown tables; raise min_count and gate activation to tool/grammar-constrained requests #967

Follow-up to #432, which added the vLLM-style repetition loop detection and enabled it by default for the Gemma 4 family. The current calibration falsely truncates correct output; this issue fixes the threshold and gates the default-on activation to the conditions that actually amplify the collapse.

Symptom (verified reproduction)

• Model: models/gemma-4-12b-it-4bit (config.json model_type: gemma4_unified), served by mlxcel-server, plain chat request (no tools, no response_format).
• A Korean prompt asking for an intuitive explanation of calculus produces a long markdown answer that ends with a summary table. Generation terminates mid-table, exactly after the alignment row | :--- | :--- | :--- | :--- | (the header row has 5 columns but only 4 alignment cells are emitted before the cut).
• Fully reproducible: asking the model to continue makes it re-emit the table header plus alignment row and it cuts at the identical position every time.
• The client sees a normal stop. The server log shows loop detection: ending generation early (repetition loop) at info level at each cut.

Root cause (verified)

1. Add generation loop/repetition detection to break Gemma 4 token-repetition collapse under tool/grammar-constrained decoding #432 added vLLM-style tail n-gram repetition detection (detect_repetition_loop, src/lib/mlxcel-core/src/loop_detection.rs:145) and the server control plane enables it unconditionally for the Gemma 4 family: LOOP_DETECTION_RECOMMENDED = LoopDetectionConfig::new(1, 20, 4) at src/server/request_options.rs:33, applied via resolve_loop_detection (src/server/request_options.rs:107) when config.model_is_gemma4_family is true (src/server/startup.rs:943, detect_gemma4_family). Precedence: per-request override, then MLXCEL_LOOP_DETECTION env override, then family default-on.
2. With the checkpoint's own tokenizer.json, the markdown alignment row | :--- | :--- | :--- | :--- | tokenizes as '|' (236909) followed by the 3-token block [' :' (1017), '---' (7243), ' |' (1109)] repeated exactly 4 times.
3. Replaying detect_repetition_loop over that stream token by token: with min_count=4 and pattern sizes 1..=20 it triggers at pattern size 3 immediately after the 4th | (1109), which is precisely where the user-visible output ends.
4. The batch scheduler then finishes the sequence with FinishReason::RepetitionLoop (src/server/batch/scheduler.rs:6603), so the truncation looks like a normal stop to clients.

So this is not an EOS handling bug; it is a false positive of the repetition guard. Any markdown table whose alignment row repeats the same :--- | cell 4 or more times trips it. Other legitimate short repetitions also qualify at min_count=4, for example ㅋㅋㅋㅋㅋㅋㅋㅋ (4 repeats of a 2-character token), !!!!, and ASCII/box-drawing divider runs.

Why the threshold is wrong (asymmetry argument)

A false positive truncates a correct answer mid-table, a user-visible correctness bug that is hard to diagnose (finish looks normal). A late detection merely emits a few extra garbage tokens: a genuine Gemma 4 collapse (upstream weights-level defect, google-deepmind/gemma#622) repeats until the token budget is exhausted, so raising min_count from 4 to 12 delays the stop by at most (12 - 4) * 20 = 160 tokens in the worst case while eliminating the entire class of small-table false positives (tables up to 11 identical alignment cells pass). #432 itself called min_count=4 "a starting point". Full opt-in (vLLM default) is explicitly NOT wanted: #432 requires an engine-level default so downstream desktop-app integrators need no request plumbing or settings UI. Keep default-on, fix the calibration.

Proposed fix (implement both parts)

Part 1, threshold recalibration (required):

• Change LOOP_DETECTION_RECOMMENDED from LoopDetectionConfig::new(1, 20, 4) to LoopDetectionConfig::new(1, 20, 12) in src/server/request_options.rs.
• This constant is also the MLXCEL_LOOP_DETECTION=on force-enable value; update the doc comments at src/server/request_options.rs:28 and src/server/startup.rs:960 (the env help text says min=1, max=20, count=4) and any docs that state the triple: docs/environment-variables.md, docs/supported-models.md, docs/python-client.md.

Part 2, gate the family default-on to the amplification conditions (required in this issue, was "Acceptable" surface in #432):

• Add generation loop/repetition detection to break Gemma 4 token-repetition collapse under tool/grammar-constrained decoding #432's reproduction states "No tools in the request: always clean"; the collapse is strongly amplified by tool declarations and/or grammar-constrained decoding. Therefore restrict the Gemma 4 family auto-enable to requests that carry tools or response_format: json_schema. Plain chat and plain completion requests get detection disabled by default, which would have prevented this false positive entirely.
• Wiring: the chat route already sees request.tools and request.response_format (src/server/routes/chat.rs, around lines 112, 273-280, 326, and the RequestOptionOverrides construction near line 1177 where loop_detection_request is built). Add a boolean (for example request_uses_constrained_decoding) to RequestOptionOverrides (src/server/request_options.rs:37) and change resolve_loop_detection so the family default-on branch requires family_default_on && request_uses_constrained_decoding. Keep the existing precedence intact: explicit per-request override wins, then MLXCEL_LOOP_DETECTION (both off and on/triple forms apply to all requests unconditionally, unchanged), then the gated family default, else disabled.
• Audit all RequestOptionOverrides construction sites (chat, native_completion, and any other routes plus src/distributed/disaggregated/serving_protocol.rs if it carries these options) so the flag is set consistently; completion endpoints without tools/response_format resolve to disabled.

Escape hatches (unchanged): per-request vLLM-style fields (max_pattern_size, min_pattern_size, min_count) and MLXCEL_LOOP_DETECTION env (off / on / MIN,MAX,COUNT) keep working exactly as today, so an operator who wants the old aggressive behavior can set MLXCEL_LOOP_DETECTION=1,20,4.

Tests

• Regression test with the real token stream: [107, 236909] followed by [1017, 7243, 1109] repeated 4 times must NOT trigger detect_repetition_loop under the new recommended config; the same block repeated 12 times must trigger. Also keep a single-token collapse case (for example one id repeated 12 times) triggering.
• Update the existing threshold expectations in src/server/request_options_tests.rs (they assert the recommended triple and family default-on behavior, see lines around 161-234).
• New resolve_loop_detection gating tests: gemma4 + tools => recommended config; gemma4 + response_format json_schema => recommended config; gemma4 plain chat => disabled; non-gemma4 + tools => disabled; MLXCEL_LOOP_DETECTION=on => enabled even for plain chat; per-request override wins over everything including the gate.

Acceptance Criteria (integration required, not just unit changes)

• With a real gemma-4-12b-it-4bit checkpoint served by mlxcel-server, a plain chat prompt that elicits a markdown table with 5 or more columns streams the complete table and finishes without FinishReason::RepetitionLoop.
• The Add generation loop/repetition detection to break Gemma 4 token-repetition collapse under tool/grammar-constrained decoding #432 protection still works end to end: under a tools-bearing request, a synthetic or reproduced repetition collapse is still cut early with FinishReason::RepetitionLoop and the info log line.
• cargo test --release passes; cargo clippy --all-targets -- -D warnings and cargo fmt --check clean.
• Docs listed above updated to describe the new triple and the gated activation policy.