From e1271816df214c73fde771e01708c45035e45e41 Mon Sep 17 00:00:00 2001 From: Ming Wen Date: Wed, 3 Jun 2026 08:43:45 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix(openai):=20normalise=20OpenRouter=20non?= =?UTF-8?q?-stream=20message.reasoning=20=E2=86=92=20reasoning=5Fcontent?= =?UTF-8?q?=20(#648)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenRouter (and some OpenAI-compatible aggregators) return a reasoning model's chain-of-thought at `message.reasoning` on the non-stream path, NOT the DeepSeek-canonical `message.reasoning_content`. `OpenAiResponseMessage` was a closed struct that only deserialized `reasoning_content`, so `message.reasoning` was dropped at decode. A reasoning model (e.g. z-ai/glm-4.6 via OpenRouter) that emits its whole answer as reasoning then reached the customer with BOTH `content` AND `reasoning_content` empty — observed on the real-chain e2e (status 200, completion_tokens=230, empty customer fields). The streaming path already handles this via the per-key `reasoning_field` override, but that lift is streaming/`delta`-only (extract_reasoning_field hard-rejects non-`delta` paths), so non-stream had no path to surface it. Fix (auto-normalize, no per-key config needed): - Capture `message.reasoning` on OpenAiResponseMessage (was dropped). - In response_into_chat_response, lift it into the canonical `reasoning_content` extra slot when canonical reasoning_content is absent/empty. The DeepSeek-canonical field takes precedence when both are present; empty `reasoning` adds no noise field (same skip-empty rule as #466). Symmetric with the existing non-stream reasoning_content capture. Non-OpenRouter upstreams omit `reasoning` (serde default) → unaffected. Tests: 3 new unit tests (OpenRouter reasoning→canonical; canonical-wins precedence; empty-reasoning adds no field) + the existing 8 reasoning tests pass. clippy + fmt clean; sibling crates (azure-openai, proxy) build. --- crates/aisix-provider-openai/src/wire.rs | 133 +++++++++++++++++++++-- 1 file changed, 126 insertions(+), 7 deletions(-) diff --git a/crates/aisix-provider-openai/src/wire.rs b/crates/aisix-provider-openai/src/wire.rs index d39ca993..442622a7 100644 --- a/crates/aisix-provider-openai/src/wire.rs +++ b/crates/aisix-provider-openai/src/wire.rs @@ -176,6 +176,17 @@ pub struct OpenAiResponseMessage { /// `choices[0].message.reasoning_content`. #[serde(default)] pub reasoning_content: Option, + /// OpenRouter (and some other OpenAI-compatible aggregators) put a + /// reasoning model's chain-of-thought at `message.reasoning` — + /// NOT the DeepSeek-canonical `message.reasoning_content` (#648). + /// Captured here so `response_into_chat_response` can normalise it + /// into the canonical `reasoning_content` slot; without this, an + /// OpenRouter reasoning model that returns its whole answer as + /// reasoning surfaces empty `content` AND empty `reasoning_content` + /// to the customer. Default so non-OpenRouter upstreams (which omit + /// the field) parse unaffected. + #[serde(default)] + pub reasoning: Option, } // `#[serde(default)]` at the container level so an upstream that omits @@ -245,13 +256,24 @@ pub fn response_into_chat_response(mut raw: OpenAiResponse) -> ChatResponse { // streaming path's `delta.reasoning_content`. Skip empty // strings so a model that returns `""` doesn't add a noise // field. - if let Some(reasoning) = c.message.reasoning_content { - if !reasoning.is_empty() { - extra.insert( - "reasoning_content".to_string(), - serde_json::Value::String(reasoning), - ); - } + // + // #648: normalise OpenRouter's `message.reasoning` into the same + // canonical `reasoning_content` slot. The DeepSeek-canonical + // `reasoning_content` takes precedence when present; otherwise we + // fall back to OpenRouter's `reasoning`. Without this an + // OpenRouter reasoning model that emits its whole answer as + // reasoning (empty `content`) reaches the customer with BOTH + // fields empty. + let reasoning_text = c + .message + .reasoning_content + .filter(|s| !s.is_empty()) + .or(c.message.reasoning.filter(|s| !s.is_empty())); + if let Some(reasoning) = reasoning_text { + extra.insert( + "reasoning_content".to_string(), + serde_json::Value::String(reasoning), + ); } ( ChatMessage { @@ -753,6 +775,103 @@ mod tests { ); } + /// #648: OpenRouter puts a reasoning model's chain-of-thought at + /// `message.reasoning` (not the DeepSeek-canonical + /// `message.reasoning_content`). The non-stream path must normalise it + /// into the canonical `reasoning_content` slot, so an OpenRouter + /// reasoning model that returns its whole answer as reasoning (empty + /// `content`) does NOT reach the customer with both fields empty. + #[test] + fn non_streaming_openrouter_reasoning_normalises_to_reasoning_content() { + let body = r#"{ + "id": "gen-or", + "object": "chat.completion", + "model": "z-ai/glm-4.6", + "choices": [{ + "index": 0, + "message": { + "role": "assistant", + "content": "", + "reasoning": "Step 1: parse. Step 2: answer. The capital is Paris." + }, + "finish_reason": "stop" + }], + "usage": {"prompt_tokens": 11, "completion_tokens": 230, "total_tokens": 241} + }"#; + let raw: OpenAiResponse = serde_json::from_str(body).unwrap(); + let out = response_into_chat_response(raw); + let reasoning = out + .message + .extra + .get("reasoning_content") + .expect("OpenRouter `message.reasoning` must normalise into canonical reasoning_content (#648)") + .as_str() + .unwrap(); + assert_eq!( + reasoning, + "Step 1: parse. Step 2: answer. The capital is Paris." + ); + } + + /// #648: when an upstream sends BOTH the canonical `reasoning_content` + /// AND OpenRouter's `reasoning`, the canonical field wins (no + /// double-capture, no clobber). + #[test] + fn non_streaming_canonical_reasoning_content_takes_precedence_over_reasoning() { + let body = r#"{ + "id": "gen-both", + "object": "chat.completion", + "model": "some-compat-model", + "choices": [{ + "index": 0, + "message": { + "role": "assistant", + "content": "ok", + "reasoning_content": "canonical thoughts", + "reasoning": "aggregator thoughts" + }, + "finish_reason": "stop" + }], + "usage": {"prompt_tokens": 5, "completion_tokens": 5, "total_tokens": 10} + }"#; + let raw: OpenAiResponse = serde_json::from_str(body).unwrap(); + let out = response_into_chat_response(raw); + let reasoning = out + .message + .extra + .get("reasoning_content") + .expect("reasoning_content must be present") + .as_str() + .unwrap(); + assert_eq!( + reasoning, "canonical thoughts", + "canonical reasoning_content must win over OpenRouter `reasoning`", + ); + } + + /// #648: an empty `reasoning` (alongside empty/absent reasoning_content) + /// must NOT add a noise field — same skip-empty rule as #466. + #[test] + fn non_streaming_empty_reasoning_adds_no_extra_field() { + let body = r#"{ + "id": "gen-empty", + "object": "chat.completion", + "model": "z-ai/glm-4.6", + "choices": [{ + "index": 0, + "message": {"role": "assistant", "content": "hi", "reasoning": ""}, + "finish_reason": "stop" + }], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2} + }"#; + let raw: OpenAiResponse = serde_json::from_str(body).unwrap(); + let out = response_into_chat_response(raw); + assert!( + !out.message.extra.contains_key("reasoning_content"), + "empty `reasoning` must not add a reasoning_content field", + ); + } + /// PR #442 audit MEDIUM-1: a hybrid OpenAI-compat upstream that /// sends BOTH a nested `prompt_tokens_details.cached_tokens: 0` /// AND a non-zero top-level `prompt_cache_hit_tokens` must not let From bf3f008e7532ed274bd66fa396eeaaeb4e561417 Mon Sep 17 00:00:00 2001 From: Ming Wen Date: Wed, 3 Jun 2026 08:56:07 +0800 Subject: [PATCH 2/2] test: add discriminating fall-through guard for non-stream reasoning normalisation The empty-canonical-falls-through case is the one branch exercising the real precedence logic (`reasoning_content.filter(!empty).or(reasoning)`): an empty `reasoning_content` ("") alongside a real OpenRouter `reasoning` must surface `reasoning`. This test FAILS against the pre-fix canonical-only code, so it's a genuine regression guard (#648). --- crates/aisix-provider-openai/src/wire.rs | 37 ++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/aisix-provider-openai/src/wire.rs b/crates/aisix-provider-openai/src/wire.rs index 442622a7..25eed4dc 100644 --- a/crates/aisix-provider-openai/src/wire.rs +++ b/crates/aisix-provider-openai/src/wire.rs @@ -849,6 +849,43 @@ mod tests { ); } + /// #648: an EMPTY canonical `reasoning_content` ("") alongside a real + /// OpenRouter `reasoning` must fall through to `reasoning` — the empty + /// canonical must not be treated as "present and winning". This is the + /// one branch with real precedence logic (`.filter(!empty).or(...)`), so + /// it's the discriminating guard: it FAILS against the pre-fix + /// canonical-only code (which ignored `reasoning` entirely). + #[test] + fn non_streaming_empty_canonical_falls_through_to_openrouter_reasoning() { + let body = r#"{ + "id": "gen-fallthrough", + "object": "chat.completion", + "model": "z-ai/glm-4.6", + "choices": [{ + "index": 0, + "message": { + "role": "assistant", + "content": "", + "reasoning_content": "", + "reasoning": "real openrouter thoughts" + }, + "finish_reason": "stop" + }], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2} + }"#; + let raw: OpenAiResponse = serde_json::from_str(body).unwrap(); + let out = response_into_chat_response(raw); + assert_eq!( + out.message + .extra + .get("reasoning_content") + .expect("empty canonical reasoning_content must fall through to OpenRouter `reasoning` (#648)") + .as_str() + .unwrap(), + "real openrouter thoughts", + ); + } + /// #648: an empty `reasoning` (alongside empty/absent reasoning_content) /// must NOT add a noise field — same skip-empty rule as #466. #[test]