Fix Studio reasoning channel rendering - #7121
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for native reasoning channels (such as Gemma's native thought-channel protocol) and normalizes them into canonical tags during streaming across both Transformers and MLX backends. It also improves streaming error handling and client-safe error propagation. The review feedback highlights a few issues: a fallback loop in _selected_chat_template_strings that breaks prematurely on exceptions instead of continuing, potential infinite loop risks in the custom ReasoningChannelNormalizer parser, and unreachable dead code in mlx_inference.py where the generation_failed flag is set right before an exception is re-raised.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d6a4c4410
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes Studio’s streaming/rendering of native reasoning-channel output (notably Gemma-style <|channel>thought ... <channel|>) by detecting markers from the active chat template, normalizing native channel text into <think>...</think>, and ensuring streamed backend errors are surfaced as errors (not assistant text).
Changes:
- Detect reasoning-channel markers from the request’s selected chat template and propagate that metadata into streaming backends.
- Normalize native reasoning-channel output into Studio’s canonical
<think>...</think>format across safetensors/CUDA, MLX text, and MLX VLM streaming. - Harden streaming error propagation (typed streamed errors + thread failures) so errors don’t leak as model text.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| studio/backend/core/inference/chat_template_helpers.py | Adds active-template-based detection of native reasoning markers plus stream normalizers and render metadata plumbing. |
| studio/backend/core/inference/inference.py | Updates Transformers streaming to use a protocol-aware streamer and propagate typed generation/thread errors. |
| studio/backend/core/inference/mlx_inference.py | Carries template metadata into MLX generation and normalizes native reasoning channels for text/VLM. |
| studio/backend/core/inference/orchestrator.py | Adds public error support and an exception form to propagate streamed errors across generator boundaries. |
| studio/backend/routes/inference.py | Treats GenStreamError as an error event (not content) across multiple streaming routes and improves error propagation. |
| studio/backend/tests/test_think_prefill_reemit.py | Adds unit coverage for template-based marker detection + normalizer behavior + native-template fallback metadata. |
| studio/backend/tests/test_*.py | Adds streaming regression tests for native reasoning normalization and error handling across backends. |
Comments suppressed due to low confidence (2)
studio/backend/core/inference/chat_template_helpers.py:1
- This uses truthiness (
if native_prompt:) to decide whether the native render succeeded. If the native template render legitimately returns an empty prompt (e.g., edge-case message set / template behavior),\"\"is falsy and the function will incorrectly fall back toformatted_prompt. Prefer checking explicitly forNone(the documented failure sentinel), e.g.if native_prompt is not None:(and forChatTemplateRenderResult, checknative_prompt is not Nonerather than.prompttruthiness).
# SPDX-License-Identifier: AGPL-3.0-only
studio/backend/routes/inference.py:1
- There’s now an inconsistency in the generic internal-error string:
_friendly_errorreturns\"An internal error occurred\"while_friendly_gen_stream_errorfalls back to\"An internal error occurred.\"(with a period). This can create inconsistent UI output and brittle tests/string matching. Consider standardizing on one exact message (including punctuation) across both helpers.
# SPDX-License-Identifier: AGPL-3.0-only
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Verified live on CUDA with Tools + thinking path: fixed.
Error-propagation hardening: confirmed end to end. Two blockers. 1. CI red on all four backend "(Python 3.x)" jobs
Sibling tests guard this with inf = pytest.importorskip("core.inference.inference")2. Plain (no-tools) thinking chat unchanged for gemma-4 on CUDA/safetensorsOutput is byte-identical to main and still leaks Cause:
The model emits the channel protocol in plain thinking mode, so the normalizer would handle it if detection also consulted the model's native or thinking template on this path. The PR description reads as a general Studio-chat fix. As written it does not hold for default CUDA chat. |
|
Thanks, both blockers should be addressed in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4db534874
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6488f2c69f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
fdef271 to
4708c64
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4708c64a8e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9824d2f6cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ac5f8be8d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Thanks @Lyxot, here is a final test. I verified your fix end to end on CUDA with My follow-up commits narrow marker detection to the template each request actually renders, re-gate think parsing on reasoning capability, and pass tool context into detection. All three are no-ops when the template lacks the channel markers; scanning the 24 model templates in my HF cache trips detection only for the four gemma-4 variants. Merging now. |
Summary
This PR fixes Studio inference rendering for models that emit native reasoning-channel markers, especially Gemma-family checkpoints that can output
<|channel>thought/<channel|>style control text. Studio should present that content as structured reasoning (<think>...</think>) instead of leaking raw channel markers into the final assistant message.Problem
Some Gemma-style native chat templates encode reasoning through channel delimiters instead of the
<think>format Studio already understands. When those models generated through Studio, the raw native markers could appear in chat output, for example internal channel text,thoughtlabels, or turn delimiters. The issue was visible in Studio chat for Gemma 4 MLX checkpoints, but the underlying rendering problem sat in the shared Studio inference path rather than in a single model card.Root Cause
Studio previously inferred whether to preserve or strip special tokens from tokenizer metadata, model-level hints, or the original tokenizer state. That was not the same as the exact chat template used to render the current request. Native template fallback could select a channel-based prompt while the downstream streamer still inspected the original tokenizer/template and missed the protocol. The opposite could also happen: token metadata could make Studio preserve special tokens for requests that were not actually using native reasoning channels.
After generation started, CUDA/safetensors and MLX streaming paths treated native channel text as ordinary assistant text. That meant the backend had no consistent place to convert native reasoning-channel output into Studio's reasoning representation or to strip response-boundary control tokens safely.
What Changed
<think>...</think>reasoning format.Scope And Behavior Boundaries
This is intended to affect only Studio inference output rendering for native reasoning-channel templates. It does not change training, save/export, GGUF/llama.cpp execution, remote provider APIs, model loading, or tokenizer files. Non-native templates continue through the existing normal text decoding path with
skip_special_tokens=Truebehavior where applicable.Models with ordinary
<think>output are not converted through this native-channel normalizer. Harmony/gpt-oss streams keep their dedicated reasoning parser. Cached template checks covered Gemma, Qwen, GLM, DeepSeek, FunctionGemma, and no-template/base-style cases to reduce the chance of false-positive activation.Reviewer Notes
The main review surface is the handoff between chat-template selection and streaming. The important invariant is that if native-channel fallback renders the prompt, the downstream stream normalizer must see the same marker metadata; if the selected template does not contain native-channel markers, the normal decoding path should remain unchanged.
The stream normalizer is intentionally protocol-based rather than model-name based. That keeps the fix applicable to Gemma-family native channel templates without special-casing a specific checkpoint name.
Validation
Focused regression suite:
Result:
Expanded local validation also passed before opening the PR:
Result:
Additional checks:
unsloth/gemma-4-E4B-it-UD-MLX-4bitreturned clean output for both thinking enabled and disabled, without raw channel marker leakage.