Honor direct chat no-think requests - #647
Conversation
Validation * Validation tier: Tier 2 - narrow OpenAI-compatible direct chat request normalization and host forwarding conformance for issue Mesh-LLM#636. * git fetch --no-tags origin main:refs/remotes/origin/main: PASS, updated origin/main to c7948d7. * git rebase origin/main: PASS. * git diff --check origin/main...HEAD: PASS, no output * git diff --check: PASS, no output * git diff --cached --check: PASS, no output * cargo fmt --all -- --check: PASS * cargo test -p openai-frontend normalize_chat --lib: PASS, 3 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_reasoning_effort_none_is_canonicalized_before_forwarding --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_existing_template_kwargs_survive_forwarding_rewrite --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p skippy-server top_level_reasoning_effort_none_turns_off_chat_template_thinking --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo check -p mesh-llm: PASS * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal /opt/homebrew/bin/cargo-clippy clippy -p openai-frontend -p skippy-server -p mesh-llm-host-runtime --all-targets -- -D warnings: PASS * Ledger: not applicable - not required for selected validation tier/change family. * Version: not applicable - OpenAI-compatible request normalization only; no release/version sync required. * Not run: live MiniMax/Qwen direct-model smoke - no local GGUF/runtime endpoint was available; deterministic forwarding and skippy template-option coverage prove the code path. Rollback * git revert HEAD
|
This might conflict with #645 - there's some work in there to correct think tags via the middleware layered approach, which I think is going to be more "canonical" for tweaking model output from backends to shape it If that PR doesn't already entirely fix the issue, we might consider layering select changes from this one into the surfaces it adds instead |
|
One thing I noticed: |
Validation * Validation tier: Tier 2R - narrow post-review correction for direct chat reasoning precedence in OpenAI-compatible request forwarding. * git diff --check: PASS, no output * git diff --cached --check: PASS, no output * cargo fmt --all: PASS * cargo fmt --all -- --check: PASS * cargo test -p openai-frontend normalize_chat_reasoning_enabled_false_wins_over_nested_effort --lib: FAIL before fix, reproduced reviewer edge case. * cargo test -p openai-frontend normalize_chat --lib: PASS, 4 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_reasoning_enabled_false_wins_over_nested_effort_before_forwarding --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_reasoning_effort_none_is_canonicalized_before_forwarding --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_existing_template_kwargs_survive_forwarding_rewrite --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo test -p skippy-server top_level_reasoning_effort_none_turns_off_chat_template_thinking --lib: PASS, 1 passed * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal cargo check -p mesh-llm: PASS * LLAMA_STAGE_BUILD_DIR=/Users/Funtland/Downloads/mesh-llm/.deps/llama-build/build-stage-abi-metal /opt/homebrew/bin/cargo-clippy clippy -p openai-frontend -p skippy-server -p mesh-llm-host-runtime --all-targets -- -D warnings: PASS * Ledger: not applicable - not required for selected validation tier/change family. * Version: not applicable - post-review compatibility precedence correction only; no release/version sync required. * Not run: live MiniMax/Qwen direct-model smoke - not required for this deterministic precedence correction. * Not run: two-node mesh or agent harness - not required for selected Tier 2R correction. Rollback * git revert HEAD
|
Thanks both, I looked through this carefully and updated the PR. For the precedence edge case: For the #645 overlap: I kept this PR scoped to request-side compatibility normalization for the direct forwarding/proxy path. #645 still feels like the right home for the broader guardrail/output-contract layer, but this fix covers the raw forwarded request shape before any guardrail middleware is involved. The PR body is updated with the scope and validation, and CI is green on the final SHA. |
|
LGTM |
Summary
Direct OpenAI-compatible chat requests now preserve explicit no-thinking intent when they are forwarded to downstream model backends.
This canonicalizes caller intent such as
reasoning_effort: "none",reasoning.enabled=false, provider thinking aliases, andthinking_budget=0intochat_template_kwargs.enable_thinking=falsebefore forwarding. Existingchat_template_kwargsvalues remain authoritative and are not overwritten.Why
Issue #636 showed that direct pinned-model requests could still surface
<think>content even when the caller explicitly requested no thinking.The typed Skippy path already understands these options, but the host proxy forwarding path could pass the original request body through without translating the no-thinking intent into the backend-compatible chat-template shape. That meant behavior depended on every downstream backend understanding every caller-specific reasoning knob.
What changed
/v1/chat/completionsand translated/v1/responsesbodies intochat_template_kwargs.enable_thinkingwhen explicit thinking/no-thinking intent is present.chat_template_kwargsoverrides and unrelated request fields.reasoning.enabled=falsewins overreasoning.effort="low"before forwarding.reasoning_effort: "none"request shape.Scope relative to #645
This PR stays scoped to request-side compatibility normalization before the direct forwarding/proxy path sends a body to a downstream OpenAI-compatible backend.
#645 is the broader guardrail/output-contract layer and should continue to own tool rescue, structured-output retries, and output cleanup. This PR does not try to duplicate that middleware path, and it should remain useful even while #645 settles because it does not depend on opt-in guardrails.
Compatibility
This is an HTTP-body-only compatibility fix.
No protobuf, gossip, mesh protocol, catalog, or Skippy ABI changes. Older peers and existing model-serving paths should continue to ignore unknown request fields as before.
Branch integrity
mainorigin/main@c7948d7181f26ed8cf99f9389a7deb8988162a320 behind / 2 aheada915c43199bbee9046c1e412ef2e24cf5f1ddebaValidation
git fetch --no-tags origin main:refs/remotes/origin/main: PASS, origin/main atc7948d7181f26ed8cf99f9389a7deb8988162a32.git diff --check: PASS, no outputgit diff --cached --check: PASS, no outputcargo fmt --all: PASScargo fmt --all -- --check: PASScargo test -p openai-frontend normalize_chat_reasoning_enabled_false_wins_over_nested_effort --lib: FAIL before fix, reproduced reviewer edge case.cargo test -p openai-frontend normalize_chat --lib: PASS, 4 passedLLAMA_STAGE_BUILD_DIR=<repo>/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_reasoning_enabled_false_wins_over_nested_effort_before_forwarding --lib: PASS, 1 passedLLAMA_STAGE_BUILD_DIR=<repo>/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_reasoning_effort_none_is_canonicalized_before_forwarding --lib: PASS, 1 passedLLAMA_STAGE_BUILD_DIR=<repo>/.deps/llama-build/build-stage-abi-metal cargo test -p mesh-llm-host-runtime chat_existing_template_kwargs_survive_forwarding_rewrite --lib: PASS, 1 passedLLAMA_STAGE_BUILD_DIR=<repo>/.deps/llama-build/build-stage-abi-metal cargo test -p skippy-server top_level_reasoning_effort_none_turns_off_chat_template_thinking --lib: PASS, 1 passedLLAMA_STAGE_BUILD_DIR=<repo>/.deps/llama-build/build-stage-abi-metal cargo check -p mesh-llm: PASSLLAMA_STAGE_BUILD_DIR=<repo>/.deps/llama-build/build-stage-abi-metal /opt/homebrew/bin/cargo-clippy clippy -p openai-frontend -p skippy-server -p mesh-llm-host-runtime --all-targets -- -D warnings: PASSa915c43199bbee9046c1e412ef2e24cf5f1ddeba(PR Builds, PR Docker Build, and PR Quality Checks).Rollback
Revert this PR.
DB downgrade: not applicable.
Data repair: not applicable.
Operational caveats: none known.
Residual risk
The original hosted-model symptom should still be confirmed against the affected live endpoint. This PR proves the deterministic request-normalization and forwarding path locally, but does not claim to solve unrelated
<think>leakage outside that path.