Skip to content

Commit 76ff913

Browse files
committed
fix(mlx): tools-aware prefix-cache boundary + incremental default-on
Agentic (tools) requests cold-re-prefilled the full ~48K system+tools head on every client compaction. The tools path stored the full prompt and matched via starts_with, so it only HIT on turn-to-turn extension; a compaction (rewritten early messages -> divergent tail) missed and re-prefilled from scratch (~98s for a 48K head). Two-part fix: - detect_system_tools_prefix_len[_from_turns]: snapshot the boundary at the rendered system+tools head (~47980 tok) instead of system-only (~22830), so the ~25K-token tool-schema block is inside the pinned boundary. Wired into all 4 tools call sites; render_tools_system_block made pub(crate). Safe: the store snapshots prompt_ids[..b] (real token prefix), so an off-by-a-token boundary can only under/over-cache, never corrupt KV. - DEFAULT_INCREMENTAL false -> true: incremental boundary caching now default-on (revert with LUMEN_MLX_PREFIX_INCREMENTAL=0). Verified on live 35B: second same-system/same-tools request forks the 1733-token head and prefills only the 20-token divergent tail (2357ms -> 102ms, 23x), output correct.
1 parent 82be0a6 commit 76ff913

3 files changed

Lines changed: 62 additions & 9 deletions

File tree

crates/lumen-mlx/src/lib.rs

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3449,7 +3449,7 @@ impl MlxQwen35Backend {
34493449
let grammar = self.build_qwen35_tool_grammar(tools, tool_choice);
34503450
let prefix_key = auto_prefix_key(messages);
34513451
let incremental_boundary = self
3452-
.detect_system_prefix_len(messages)
3452+
.detect_system_tools_prefix_len(messages, tools)
34533453
.ok()
34543454
.filter(|&b| b > 0 && b < prompt_ids.len());
34553455
self.chat_with_tools_impl(
@@ -3492,7 +3492,7 @@ impl MlxQwen35Backend {
34923492
let grammar = self.build_qwen35_tool_grammar(tools, tool_choice);
34933493
let prefix_key = auto_prefix_key(messages);
34943494
let incremental_boundary = self
3495-
.detect_system_prefix_len(messages)
3495+
.detect_system_tools_prefix_len(messages, tools)
34963496
.ok()
34973497
.filter(|&b| b > 0 && b < prompt_ids.len());
34983498
self.chat_with_tools_impl(
@@ -3528,7 +3528,7 @@ impl MlxQwen35Backend {
35283528
let grammar = self.build_qwen35_tool_grammar(tools, tool_choice);
35293529
let prefix_key = auto_prefix_key_from_turns(turns);
35303530
let incremental_boundary = self
3531-
.detect_system_prefix_len_from_turns(turns)
3531+
.detect_system_tools_prefix_len_from_turns(turns, tools)
35323532
.ok()
35333533
.filter(|&b| b > 0 && b < prompt_ids.len());
35343534
self.chat_with_tools_impl(
@@ -3568,7 +3568,7 @@ impl MlxQwen35Backend {
35683568
let grammar = self.build_qwen35_tool_grammar(tools, tool_choice);
35693569
let prefix_key = auto_prefix_key_from_turns(turns);
35703570
let incremental_boundary = self
3571-
.detect_system_prefix_len_from_turns(turns)
3571+
.detect_system_tools_prefix_len_from_turns(turns, tools)
35723572
.ok()
35733573
.filter(|&b| b > 0 && b < prompt_ids.len());
35743574
self.chat_with_tools_impl(
@@ -4194,6 +4194,52 @@ impl MlxQwen35Backend {
41944194
Ok(sys_ids.len())
41954195
}
41964196

4197+
/// Tools-aware variant of [`Self::detect_system_prefix_len`]: returns the
4198+
/// token length of the **system + rendered-tools** head — the stable prefix
4199+
/// every same-system/same-tools request shares, which is what the agentic
4200+
/// chat path actually re-uses across turns and client compactions. The
4201+
/// plain system-only boundary leaves the ~25K-token tool-schema block out
4202+
/// of the snapshot, so it gets cold-prefilled every divergent turn; this
4203+
/// captures it. Mirrors exactly what `format_qwen3_chat_with_tools_*` emits
4204+
/// before the first body turn (`render_tools_system_block`), so
4205+
/// `prompt_ids[..len]` is a strict prefix. Falls back to the system-only
4206+
/// boundary when there are no tools.
4207+
fn detect_system_tools_prefix_len(
4208+
&self,
4209+
messages: &[(String, String)],
4210+
tools: &[crate::chat_io::ToolDef<'_>],
4211+
) -> Result<usize> {
4212+
if tools.is_empty() {
4213+
return self.detect_system_prefix_len(messages);
4214+
}
4215+
let leading_system = match messages.first() {
4216+
Some((role, text)) if role == "system" && !text.is_empty() => Some(text.as_str()),
4217+
_ => None,
4218+
};
4219+
let block = crate::qwen3_5_tools::render_tools_system_block(tools, leading_system);
4220+
let ids = self.encode(&block)?;
4221+
Ok(ids.len())
4222+
}
4223+
4224+
/// `detect_system_tools_prefix_len` for the structured-history shape.
4225+
fn detect_system_tools_prefix_len_from_turns(
4226+
&self,
4227+
turns: &[crate::chat_io::ChatTurn<'_>],
4228+
tools: &[crate::chat_io::ToolDef<'_>],
4229+
) -> Result<usize> {
4230+
use crate::chat_io::ChatTurn;
4231+
if tools.is_empty() {
4232+
return self.detect_system_prefix_len_from_turns(turns);
4233+
}
4234+
let leading_system = match turns.first() {
4235+
Some(ChatTurn::System(s)) if !s.is_empty() => Some(*s),
4236+
_ => None,
4237+
};
4238+
let block = crate::qwen3_5_tools::render_tools_system_block(tools, leading_system);
4239+
let ids = self.encode(&block)?;
4240+
Ok(ids.len())
4241+
}
4242+
41974243
/// Drop a prefix-cache entry by key, releasing its master snapshot.
41984244
/// Returns true if the entry existed.
41994245
pub fn drop_prefix_cache(&mut self, key: &str) -> bool {

crates/lumen-mlx/src/prefix_cache.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@ use std::time::{Duration, Instant};
3838
/// Default per-key branch cap when `LUMEN_MLX_PREFIX_CACHE_BRANCHES` is unset.
3939
const DEFAULT_BRANCHES: usize = 4;
4040

41-
/// Default for incremental chunked-prefill boundary caching (Phase 0). OFF so
42-
/// the cold-MISS path is byte-identical to the pre-Phase-0 single-prefill until
43-
/// explicitly enabled via `LUMEN_MLX_PREFIX_INCREMENTAL`.
44-
const DEFAULT_INCREMENTAL: bool = false;
41+
/// Default for incremental chunked-prefill boundary caching (Phase 0). **ON**:
42+
/// on a cold MISS the stable head (system + rendered tools, via the tools-aware
43+
/// boundary in `lib.rs::detect_system_tools_prefix_len*`) is snapshotted as a
44+
/// `pinned_boundary` branch so a later same-system/same-tools but divergent-tail
45+
/// prompt (e.g. a client compaction/resume) FORKS the ~48K head instead of
46+
/// cold-prefilling it again (~98s → ~1-2s on the agentic path). Set
47+
/// `LUMEN_MLX_PREFIX_INCREMENTAL=0` to revert to the byte-identical single-prefill.
48+
const DEFAULT_INCREMENTAL: bool = true;
4549

4650
/// The minimal runner surface the prefix cache needs. A backend implements this
4751
/// for its runner type — e.g. a blanket impl over a richer internal `Runner`

crates/lumen-mlx/src/qwen3_5_tools.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ const TOOL_INSTRUCTION_BLOCK: &str = "\n\nIf you choose to call a function ONLY
8585
/// Render the system prefix containing the `<tools>` block + instruction.
8686
/// Optional existing system content is appended after the IMPORTANT block,
8787
/// separated by `\n\n` — matches Qwen's template behavior.
88-
fn render_tools_system_block(tools: &[ToolDef<'_>], extra_system: Option<&str>) -> String {
88+
pub(crate) fn render_tools_system_block(
89+
tools: &[ToolDef<'_>],
90+
extra_system: Option<&str>,
91+
) -> String {
8992
let mut s = String::new();
9093
s.push_str("<|im_start|>system\n");
9194
s.push_str("# Tools\n\nYou have access to the following functions:\n\n<tools>");

0 commit comments

Comments
 (0)