From 068e129cb2d7e3a495ffb306b5402fc7902f3005 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sat, 23 May 2026 16:42:39 +0000 Subject: [PATCH] fix(mtp): gate legacy --draft-min with --draft-max on chained ngram Codex flagged that the legacy chained mtp+ngram path emits --draft-min 48 while --draft-max is suppressed and later reused for MTP (spec_draft_n_max, typically 2/3). That produces an inverted legacy ngram range (--draft-min 48 --draft-max 2/3) on affected binaries, which can break or effectively disable ngram-mod for auto CPU MTP loads and forced mtp+ngram requests. Both --draft-min and --draft-max are generic flags on legacy llama-server builds, so either one would race with MTP's own values. Gate the pair together: when chain_with_mtp=True on the legacy flavor we drop both flags and rely on MTP's emission for the chained range. Standalone ngram still emits both, preserving a valid min<=max window. Updated test_build_ngram_mod_flags_legacy_chained_omits_draft_max (now omits_draft_min_and_max) and added a min<=max guard on the standalone case. Full suite (test_llama_cpp_mtp_detection + test_llama_server_args) passes locally: 229 / 229. --- studio/backend/core/inference/llama_cpp.py | 21 ++++++------- .../tests/test_llama_cpp_mtp_detection.py | 30 ++++++++++++------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index f58bd7750c..bc84c5f9f0 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -523,17 +523,18 @@ def _build_ngram_mod_flags( # Legacy llama.cpp before the spec arg rename: same knobs lived # under --spec-ngram-size-n (lookup length) and the generic # --draft-min / --draft-max (ngram size N range). - out = [ - "--spec-ngram-size-n", - str(n_match), - "--draft-min", - str(n_min), - ] + out = ["--spec-ngram-size-n", str(n_match)] if not chain_with_mtp: - # Only safe to set --draft-max here when MTP is NOT in the - # same emission; otherwise this duplicates MTP's --draft-max - # and last-wins clobbers the MTP draft length. - out.extend(["--draft-max", str(n_max)]) + # Gate --draft-min AND --draft-max together. Both flags are + # generic on legacy binaries, so emitting either in the same + # invocation as MTP would race with MTP's own --draft-min / + # --draft-max (typically 2/3 from spec_draft_n_max). Earlier + # we suppressed only --draft-max, which produced an inverted + # legacy range (--draft-min 48 --draft-max 2/3) and disabled + # ngram-mod entirely. Keeping the pair together preserves a + # valid range when ngram is standalone and avoids the clobber + # when it is chained with MTP. + out.extend(["--draft-min", str(n_min), "--draft-max", str(n_max)]) return out return [] diff --git a/studio/backend/tests/test_llama_cpp_mtp_detection.py b/studio/backend/tests/test_llama_cpp_mtp_detection.py index bd5e1307e3..8bb890c0b1 100644 --- a/studio/backend/tests/test_llama_cpp_mtp_detection.py +++ b/studio/backend/tests/test_llama_cpp_mtp_detection.py @@ -1269,28 +1269,36 @@ def test_canonicalize_spec_mode_none_aliases_map_to_off(value): # ---- Bug C: legacy chained MTP+ngram does not duplicate --draft-max ---- -def test_build_ngram_mod_flags_legacy_chained_omits_draft_max(): - """When chaining ngram-mod alongside MTP on a legacy llama-server, - --draft-max collides with MTP's --draft-max (same flag, different - semantic in old builds). The chain_with_mtp flag must suppress the - ngram-mod --draft-max so MTP's draft length wins.""" +def test_build_ngram_mod_flags_legacy_chained_omits_draft_min_and_max(): + """Legacy chained ngram+MTP must drop BOTH --draft-min and --draft-max. + Both are generic flags on legacy binaries, so MTP's own --draft-min / + --draft-max (e.g. 2/3 from spec_draft_n_max) would race with ngram's + larger size-N range. Suppressing only --draft-max produced an inverted + legacy range (--draft-min 48 --draft-max 2/3) that disabled ngram-mod + on affected builds; gate them together. --spec-ngram-size-n is a + distinct flag and must remain so ngram-mod still tunes the chain.""" caps = {"ngram_mod_flavor": "legacy"} chained = _build_ngram_mod_flags(caps, chain_with_mtp = True) assert ( "--draft-max" not in chained ), f"chain_with_mtp=True must drop --draft-max on legacy; got {chained}" - # The other two knobs must still be present so ngram-mod actually - # tunes the chain. + assert ( + "--draft-min" not in chained + ), f"chain_with_mtp=True must drop --draft-min on legacy; got {chained}" assert "--spec-ngram-size-n" in chained - assert "--draft-min" in chained -def test_build_ngram_mod_flags_legacy_standalone_keeps_draft_max(): - """When NOT chaining, --draft-max is the ngram-mod size-N max and must - still be emitted on legacy.""" +def test_build_ngram_mod_flags_legacy_standalone_keeps_draft_min_and_max(): + """Standalone ngram on legacy must emit --draft-min AND --draft-max as + a valid range (size-N min/max) so ngram-mod actually has a window.""" caps = {"ngram_mod_flavor": "legacy"} standalone = _build_ngram_mod_flags(caps, chain_with_mtp = False) + assert "--draft-min" in standalone assert "--draft-max" in standalone + # The range must not be inverted: min <= max. + i_min = standalone.index("--draft-min") + i_max = standalone.index("--draft-max") + assert int(standalone[i_min + 1]) <= int(standalone[i_max + 1]) def test_build_ngram_mod_flags_new_flavor_always_emits_distinct_names():