feat(moa): default to no-think on workers + reducer (escape hatch for callers) - #620
feat(moa): default to no-think on workers + reducer (escape hatch for callers)#620michaelneale wants to merge 10 commits into
Conversation
When the gateway forwards a request to a remote peer via QUIC and the upstream pre-commit phase fails (tunnel open / request forward / response probe), the failure becomes `RouteAttemptResult::RetryableUnavailable`. Today the outer routing loop retries on the next available target, which fixes the case where multiple peers serve the same model but does nothing when only one peer serves it: a single transient QUIC path teardown kills the request, even though iroh typically reconnects within ~1s on a fresh connection. This is the dominant failure mode on asymmetric / lossy direct paths (corporate firewalls, hotel wifi, Tailscale interfering with NAT traversal, M4-style incoming filter rules). Real-world meshes will see this constantly. Implementation: * Splits `route_remote_attempt` into a `_once` helper plus a wrapper that retries once with a 750ms backoff if the first attempt returned `RetryableUnavailable`. 750ms is below user-perceptible-stall and matches the typical iroh reconnect window. * Retry is only safe on `RetryableUnavailable` because that variant is exclusively set BEFORE any bytes are written to the client TCP stream (probe failed, tunnel open failed, buffered-request forward failed). Any post-commit error or non-network outcome falls through unchanged. * Pure-function `should_retry_remote_attempt` extracted for unit testing; 5 tests pin the retry policy (yes on unavailable; no on delivered / timeout / context-overflow / client-disconnected). Live lab validation pending on a 3-node mesh where the M4 \u2194 mini path is consistently asymmetric. Baseline before the fix was direct_mini 27% success over 1h40m. Expect the curl_fail subset (pre-commit drops) to recover to ~100% with this change; http_err 429 subset (mini's own admission control) is unaffected by design.
For `model: "mesh"`, the OpenAI-style reasoning knobs
(`reasoning_effort: "none"`, `reasoning: { enabled: false }`,
`enable_thinking: false`, `thinking_budget: 0`, `chat_template_kwargs.enable_thinking`,
and the THINKING_BOOLEAN_ALIASES) were silently dropped in MoA. The fast
worker's 256-token budget then got burned inside an unclosed `<think>`
block on reasoning models and never reached the actual answer.
Live lab on a 3-node mesh (M4 Qwen2.5-3B + studio MiniMax-M2.5 + mini
Qwen3.5-9B), "reply with one short word" prompt:
| Run | default thinking | reasoning_effort: none |
|-----|--------------------------|------------------------|
| 1 | 32s, leaked "Thinking Process: 1. Analyze..." | 1s, "Okay." |
| 2 | 17s, "Yes" | 2s, "Hello" |
| 3 | 32s, leaked reasoning prose | 1s, "Hello" |
Worker log on the slow runs shows Qwen3.5-9B returning 1700+ char
payloads at 31s \u2014 the full runaway think block. On the no-think runs
all three workers finish in <1.3s with 4-5 char clean answers.
Implementation:
* `SamplingParams` gains `enable_thinking: Option<bool>` and a
builder-style `with_thinking(...)` helper. `None` (default) is
"don't override" \u2014 callers without a preference see no behavior
change.
* New `backend::apply_enable_thinking(body, hint)` helper centralises
the wire shape: injects `chat_template_kwargs.enable_thinking`
(canonical llama.cpp chat-template knob) and `reasoning_effort:
"none"` when disabled. Merges into existing
`chat_template_kwargs` instead of clobbering.
* `HttpBackend`, `LocalModelBackend`, and `RemoteModelBackend` all
call it after building their request bodies.
* `GatewayConfig` gains `enable_thinking: Option<bool>` so the choice
flows in one place to every worker (`SamplingParams::worker().with_thinking(...)`)
AND the hedged reducer (`SamplingParams::reducer().with_thinking(...)`).
* `moa_gateway.rs::try_handle_moa` extracts the override from the
inbound request body via the new `extract_enable_thinking_override`
helper, which mirrors every shape that
`openai_frontend::common::normalize_reasoning_template_options`
recognises (so MoA users get the same surface as direct callers).
Tests:
* mesh-mixture-of-agents lib: 98 pass (6 new for apply_enable_thinking
and SamplingParams::with_thinking).
* mesh-mixture-of-agents/tests/sim_enable_thinking_propagation.rs (new):
3 mock-backend integration tests pinning that
chat_template_kwargs.enable_thinking reaches every worker AND that
no spurious fields appear when no override is requested.
* mesh-llm-host-runtime lib: 1478 pass (10 new for
extract_enable_thinking_override covering every JSON shape).
* clippy --all-targets -D warnings: clean.
* cargo fmt --all -- --check: clean.
Compat:
* No mesh wire-protocol change. The new fields travel inside the
existing chat-completion request JSON over the QUIC tunnel; peers
on older binaries simply forward the body to llama.cpp which
already understands `chat_template_kwargs`. Additive only, safe
across mixed-version meshes.
* No skippy ABI change. No plugin protocol change.
* MoA callers without a preference get the same behavior as before
this commit \u2014 `enable_thinking: None` is the default.
Lab follow-up: a new `mesh_no_think` probe in the stability lab
(`/tmp/lab/probe-stable.sh`) hits `/v1/chat/completions` with
`reasoning_effort: "none"` so we can A/B latency and quality over
hours of probe traffic.
There was a problem hiding this comment.
Pull request overview
This PR improves Mixture-of-Agents (MoA) request fidelity by carrying a caller’s “thinking / reasoning enabled” preference from the inbound OpenAI-style request through the MoA gateway into every worker and the reducer, ensuring consistent behavior across fan-out and reduce steps.
Changes:
- Add
GatewayConfig::enable_thinking: Option<bool>and propagate it into worker + reducerSamplingParamsviaSamplingParams::with_thinking(...). - Add
apply_enable_thinking(...)to injectchat_template_kwargs.enable_thinking(andreasoning_effort: "none"when disabled) into outbound backend request bodies, merging with existingchat_template_kwargs. - Add/extend tests to pin propagation behavior, plus introduce a same-target retry wrapper for remote routing attempts and add a lab-notes markdown file.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| MIC_LAB_NOTES.md | Adds QUIC stability investigation lab notes and retry design context. |
| crates/mesh-mixture-of-agents/tests/sim_worker_accounting.rs | Updates test config initialization to include enable_thinking: None. |
| crates/mesh-mixture-of-agents/tests/sim_tool_result_routes_to_reducer.rs | Updates test config initialization to include enable_thinking: None. |
| crates/mesh-mixture-of-agents/tests/sim_tool_call_text_not_passed_as_content.rs | Updates test config initialization to include enable_thinking: None. |
| crates/mesh-mixture-of-agents/tests/sim_enable_thinking_propagation.rs | New simulated integration tests asserting thinking override reaches all workers/reducer and is absent when unset. |
| crates/mesh-mixture-of-agents/tests/sim_all_workers_fail.rs | Updates test config initialization to include enable_thinking: None. |
| crates/mesh-mixture-of-agents/src/reducer.rs | Propagates thinking override into reducer hedged calls. |
| crates/mesh-mixture-of-agents/src/lib.rs | Propagates enable_thinking into every worker call and reducer escalation path. |
| crates/mesh-mixture-of-agents/src/backend.rs | Adds SamplingParams.enable_thinking, with_thinking, and apply_enable_thinking injection into outbound backend bodies. |
| crates/mesh-llm-host-runtime/src/network/openai/transport.rs | Wraps remote routing attempts with a one-shot same-target retry on RetryableUnavailable. |
| crates/mesh-llm-host-runtime/src/network/openai/moa_gateway.rs | Extracts thinking override from inbound JSON and injects it into the MoA GatewayConfig; applies injection in local/remote MoA backends. |
Comments suppressed due to low confidence (1)
crates/mesh-llm-host-runtime/src/network/openai/moa_gateway.rs:105
reasoning_effortis interpreted as "enabled" for any string other than "none". On the direct OpenAI surface this value is an enum (ReasoningEffort) and unknown strings are typically rejected; here they would silently enable thinking. To keep MoA behavior consistent, consider parsing the value viaserde_json::from_value::<openai_frontend::ReasoningEffort>(...)(and either ignore/400 on parse failure) rather than treating any non-"none" string as valid.
// reasoning_effort: "none" / "low" / etc.
if let Some(effort) = obj.get("reasoning_effort").and_then(|v| v.as_str()) {
result = Some(effort != "none");
}
| if r.get("enabled") == Some(&serde_json::Value::Bool(false)) | ||
| || r.get("effort").and_then(|v| v.as_str()) == Some("none") | ||
| || r.get("max_tokens").and_then(|v| v.as_u64()) == Some(0) | ||
| { | ||
| result = Some(false); | ||
| } else if r.get("enabled") == Some(&serde_json::Value::Bool(true)) | ||
| || r.get("effort").is_some() | ||
| || r.get("max_tokens").is_some() | ||
| { |
| // chat_template_kwargs.enable_thinking is the canonical knob the | ||
| // llama.cpp templates read. Merge into any existing object instead | ||
| // of clobbering. | ||
| let kwargs = obj | ||
| .entry("chat_template_kwargs".to_string()) | ||
| .or_insert_with(|| json!({})); | ||
| if let Some(kwargs_obj) = kwargs.as_object_mut() { | ||
| kwargs_obj.insert("enable_thinking".to_string(), json!(enable)); | ||
| } | ||
|
|
| //! Background — see #617 / `~/Desktop/think.md`. For `model: "mesh"`, the | ||
| //! OpenAI-style reasoning knobs were being silently dropped, so the MoA | ||
| //! fast worker (256-token budget) burned its entire budget inside an | ||
| //! unclosed `<think>` block and never reached the answer. The fix | ||
| //! propagates the caller's preference through `GatewayConfig::enable_thinking` | ||
| //! down to `SamplingParams` and the backends. |
| /// Single attempt against a remote host: open tunnel, forward request, | ||
| /// probe response. Returns `RouteAttemptResult` describing what happened. | ||
| /// | ||
| /// `route_remote_attempt` (below) wraps this with a one-shot same-target | ||
| /// retry on `RetryableUnavailable`, because real-world meshes have | ||
| /// asymmetric/lossy direct paths and a single transient QUIC path drop | ||
| /// shouldn't doom a request when iroh typically reconnects within ~1s. | ||
| async fn route_remote_attempt_once( |
Honest testing readoutWanted to be explicit about how much real-world testing this PR has had so far, in case the original description read more confidently than it should: Verified live, on a clean 3-node private mesh (M4 Qwen2.5-3B + studio MiniMax-M2.5 + mini Qwen3.5-9B), release-build binary:
Verified via unit/integration tests, not live:
Not yet verified:
The smoke result was big enough that I felt OK opening the PR, but the long-horizon and live-tool-path numbers are still TBD. |
…asked
Per Mic: MoA shouldn't have to ship behind a UI toggle. The MoA gateway
now defaults to `enable_thinking = Some(false)` for every `model: "mesh"`
request. Callers can still opt-in by passing any recognised reasoning
knob explicitly (`reasoning_effort: "low"`, `enable_thinking: true`,
etc.) — but the default is reasoning off.
Rationale:
* Workers are short-budget internal slots, not user-facing reasoning
steps. The fast worker has a 256-token budget that doesn't fit
`<think>...</think>` + answer.
* The reducer doesn't want reasoning prose as candidate input.
* MoA chat UX is the worst case for thinking models — every turn pays
the reasoning latency penalty without giving the user the reasoning
output.
Implementation:
* Extracted the policy into a tiny pure function,
`effective_enable_thinking_for_moa(&body) -> Option<bool>`, that
returns `extract_enable_thinking_override(body).or(Some(false))`.
* 4 unit tests cover the contract: silent caller → no-think; explicit
disable → no-think; explicit enable → thinking on (escape hatch);
tool turn → still no-think by default.
* The existing `extract_enable_thinking_override` tests are unchanged
(they test the parser, not the gateway policy).
Live verification on the 3-node lab mesh (M4 + studio + mini), release
binary:
* `model=mesh` with no knobs: 3/3 clean short answers ("Hello", "Okay",
"Hello"), zero think leakage.
* `model=mesh` + `reasoning_effort: "low"`: thinking turns back on, raw
"Thinking Process:" prose appears as expected.
* `model=mesh` + `tools=[read_file]`: tool_calls path unchanged,
`finish_reason: tool_calls`, correct args. No-think default applies.
Tests: 1486 host-runtime lib pass (4 new), all mesh-mixture-of-agents
tests pass, clippy + fmt clean.
| /// Returns `None` when the caller hasn't expressed a preference, leaving | ||
| /// each worker's default behavior alone. | ||
| /// MoA's opinionated default: workers do not think unless the caller | ||
| /// explicitly asks for it. Workers are short-budget internal slots, not | ||
| /// user-facing reasoning steps. The fast worker's 256-token budget is |
| //! Background — see #617 / `~/Desktop/think.md`. For `model: "mesh"`, the | ||
| //! OpenAI-style reasoning knobs were being silently dropped, so the MoA | ||
| //! fast worker (256-token budget) burned its entire budget inside an | ||
| //! unclosed `<think>` block and never reached the answer. The fix | ||
| //! propagates the caller's preference through `GatewayConfig::enable_thinking` | ||
| //! down to `SamplingParams` and the backends. |
| Each node advertises three addresses in its invite token: the iroh relay URL, its public IP/port via STUN-like discovery, and its LAN IP/port. Decoded: | ||
|
|
||
| ``` | ||
| M4 id=cf2d4edab1... addrs=[relay, 180.181.228.108:36188, 192.168.86.172:56727] | ||
| Mini id=462bcc96fb... addrs=[relay, 180.181.228.108:36119, 192.168.86.60:63108] | ||
| Studio id=d0782f712e... addrs=[relay, 180.181.228.108:0, 192.168.86.24:61252] |
| /// How long to wait between a pre-commit failure and the same-target | ||
| /// retry. iroh typically reopens a fresh connection within ~1s after a | ||
| /// path teardown; 750ms gives that recovery time without making the | ||
| /// retry feel sluggish on the client. | ||
| const REMOTE_ATTEMPT_RETRY_BACKOFF: std::time::Duration = std::time::Duration::from_millis(750); | ||
|
|
||
| /// Decide whether a remote attempt result is safe to retry against the | ||
| /// same host. Extracted as a pure function so we can unit-test the | ||
| /// retry policy without spinning up a real QUIC endpoint. | ||
| fn should_retry_remote_attempt(first: &RouteAttemptResult) -> bool { | ||
| matches!(first, RouteAttemptResult::RetryableUnavailable) | ||
| } |
…able-thinking * origin/main: Cover asymmetric KV cache width pricing (#594)
Resolved a non-substantive test-list conflict between #620 and #621 \u2014 both branches appended new tests at the end of moa_gateway.rs's `mod tests`. Both blocks retained. No behavioural overlap; the changes are independent. This branch now carries both for combined lab validation. 1489 host-runtime lib tests pass, clippy + fmt clean.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/mesh-llm-host-runtime/src/network/openai/moa_gateway.rs:380
build_moa_config’s comment saystry_handle_moaonly overridesenable_thinkingwhen the caller has expressed a preference, buttry_handle_moanow always setsconfig.enable_thinkingviaeffective_enable_thinking_for_moa(defaulting toSome(false)when silent). Please adjust the comment to reflect that MoA now defaults to no-think unless explicitly enabled.
// Defaults to leaving each model's thinking behavior alone.
// `try_handle_moa` overrides this from the inbound request body
// when the caller has expressed a preference
// (`reasoning_effort: "none"`, `enable_thinking: false`, etc.).
enable_thinking: None,
| /// Pull the caller's "disable / enable thinking" preference out of an | ||
| /// inbound chat-completion or responses JSON body. Mirrors the same | ||
| /// shapes that `openai_frontend::common::normalize_reasoning_template_options` | ||
| /// recognises so MoA users get the same surface as direct callers. | ||
| /// | ||
| /// Recognised inputs (any one is enough): | ||
| /// * `reasoning_effort: "none"` (off) or any non-`"none"` value (on) | ||
| /// * `reasoning: { enabled: false }` (off) / `{ enabled: true }` (on) | ||
| /// * `reasoning: { effort: "none" }` / `{ max_tokens: 0 }` (off) | ||
| /// * Any of `THINKING_BOOLEAN_ALIASES` as a top-level field with bool | ||
| /// * `thinking_budget: 0` (off) | ||
| /// * `chat_template_kwargs.enable_thinking` (or any alias) as bool | ||
| /// | ||
| /// Returns `None` when the caller hasn't expressed a preference, leaving | ||
| /// each worker's default behavior alone. | ||
| /// MoA's opinionated default: workers do not think unless the caller | ||
| /// explicitly asks for it. Workers are short-budget internal slots, not | ||
| /// user-facing reasoning steps. The fast worker's 256-token budget is | ||
| /// far too small to fit `<think>…</think>` + answer, and the reducer | ||
| /// doesn't want reasoning prose as candidate input. | ||
| /// | ||
| /// The caller can still explicitly enable thinking (e.g. for | ||
| /// experimentation) via any of the recognised knobs — see | ||
| /// [`extract_enable_thinking_override`]. When no preference is | ||
| /// expressed, MoA picks for them: off. | ||
| fn effective_enable_thinking_for_moa(body: &serde_json::Value) -> Option<bool> { | ||
| extract_enable_thinking_override(body).or(Some(false)) | ||
| } |
| /// How long to wait between a pre-commit failure and the same-target | ||
| /// retry. iroh typically reopens a fresh connection within ~1s after a | ||
| /// path teardown; 750ms gives that recovery time without making the | ||
| /// retry feel sluggish on the client. | ||
| const REMOTE_ATTEMPT_RETRY_BACKOFF: std::time::Duration = std::time::Duration::from_millis(750); | ||
|
|
||
| /// Decide whether a remote attempt result is safe to retry against the | ||
| /// same host. Extracted as a pure function so we can unit-test the | ||
| /// retry policy without spinning up a real QUIC endpoint. | ||
| fn should_retry_remote_attempt(first: &RouteAttemptResult) -> bool { | ||
| matches!(first, RouteAttemptResult::RetryableUnavailable) | ||
| } | ||
|
|
||
| async fn route_remote_attempt( | ||
| node: &mesh::Node, | ||
| tcp_stream: &mut TcpStream, | ||
| host_id: iroh::EndpointId, | ||
| prefetched: &[u8], | ||
| retry_context_overflow: bool, | ||
| response_adapter: ResponseAdapter, | ||
| ) -> RouteAttemptResult { | ||
| let first = route_remote_attempt_once( | ||
| node, | ||
| tcp_stream, | ||
| host_id, | ||
| prefetched, | ||
| retry_context_overflow, | ||
| response_adapter, | ||
| ) | ||
| .await; | ||
|
|
||
| // Only `RetryableUnavailable` is safe to retry against the same | ||
| // host. We only ever set it BEFORE any bytes are written to the | ||
| // client TCP stream (probe failed, tunnel open failed, buffered | ||
| // request forward failed). Anything else has either succeeded, | ||
| // partially committed bytes to the client, or has a different | ||
| // remediation (timeout = genuinely slow upstream; context overflow | ||
| // = same failure will recur on retry). | ||
| if !should_retry_remote_attempt(&first) { | ||
| return first; | ||
| } | ||
|
|
||
| tracing::info!( | ||
| "API proxy: pre-commit failure to host {} — retrying once after {}ms", | ||
| host_id.fmt_short(), | ||
| REMOTE_ATTEMPT_RETRY_BACKOFF.as_millis() | ||
| ); | ||
| tokio::time::sleep(REMOTE_ATTEMPT_RETRY_BACKOFF).await; | ||
|
|
Same reason as the matching commit on micn/moa-grace-on-diverse-answers: these tracking files don't belong in canonical repo and were making the PR diff look like it touched QUIC / keep-alive / discovery code when it doesn't. Code change is strictly MoA application layer.
|
Superseded by #629 which combines this PR + #624 into a single MoA chat improvement (40s → ~1.2s p50 on public mesh). The two changes were inseparable in the user journey, and the combined branch is cleaner (no lab notes, no accidental QUIC retry code from earlier merges). Old branch left in place for git history; safe to delete. |
For
model: "mesh"(MoA), the OpenAI reasoning knobs were silently dropped, so reasoning models thought regardless. The 256-token fast-worker budget got burned inside an unclosed<think>block and never reached the answer; the reducer's quality dropped when candidate answers leaked reasoning prose.This PR makes MoA opinionated: workers and the reducer do NOT think by default. The caller can still explicitly enable thinking for an individual request (escape hatch), but for the chat UI's
Auto(which routes through MoA) and any othermodel: "mesh"call without explicit knobs, MoA picks: reasoning off.Closes #617.
Why opinionated, not opt-in
Workers in MoA are short-budget internal slots, not user-facing reasoning steps. The fast worker has a 256-token budget that doesn't fit
<think>\u2026</think>+ answer. The reducer doesn't want reasoning prose as candidate input. And the chat UX is the worst case for thinking models — every turn pays the reasoning latency penalty without showing the user the reasoning output. Asking every MoA caller to remember a flag is the wrong shape; MoA should just do the right thing.The escape hatch covers experimentation: any of
reasoning_effort: "low"(or any non-"none" value),reasoning: { enabled: true },enable_thinking: true, etc. turns it back on for that request.Implementation
SamplingParamsgainsenable_thinking: Option<bool>and a builder-stylewith_thinking(...)helper.backend::apply_enable_thinking(body, hint)injectschat_template_kwargs.enable_thinking(canonical llama.cpp knob) andreasoning_effort: "none"when disabled. Merges into any existingchat_template_kwargs.HttpBackend,LocalModelBackend, andRemoteModelBackendcall it after building their request bodies.GatewayConfiggainsenable_thinking: Option<bool>so the choice flows in one place to every worker (SamplingParams::worker().with_thinking(...)) AND the hedged reducer.effective_enable_thinking_for_moa(body) -> Option<bool>encodes the policy:extract_enable_thinking_override(body).or(Some(false)). Easy to unit-test, easy to flip if we change our minds.Live verification on 3-node lab mesh
Same prompt ("reply with one short word"), same mesh, release binary:
model=mesh, no knobs (3 runs)model=mesh,reasoning_effort: "low"model=mesh,tools=[read_file]finish_reason: tool_calls, args correct, no-think default applied.For comparison, the pre-fix smoke (also on the same mesh) showed 32s/17s/32s turns on the same prompt with raw reasoning prose leaking in 2/3.
Tests
apply_enable_thinkingandSamplingParams::with_thinking).tests/sim_enable_thinking_propagation.rs(new): 3 mock-backend integration tests pinning thatchat_template_kwargs.enable_thinkingreaches every worker AND that no spurious fields appear when no override is requested.extract_enable_thinking_overridecovering every JSON shape (reasoning_effort,reasoning: { enabled / effort / max_tokens },enable_thinking, all THINKING_BOOLEAN_ALIASES,thinking_budget: 0,chat_template_kwargs).effective_enable_thinking_for_moacovering the opinionated default + the escape hatch + the tool path.cargo clippy --all-targets -- -D warnings: clean.cargo fmt --all -- --check: clean.Compat
chat_template_kwargs. Additive only, safe across mixed-version meshes.reasoning_effort: "low"or similar) restores the old behavior for that caller. The chat UI is the main consumer ofmodel=meshand benefits from this change.Follow-up
Lab will keep running with a
mesh_no_thinkprobe alongsidemesh_chatfor longer-horizon A/B. Tomorrow report will include latency distribution under a few hours of mixed traffic.