Fix Gemma 4 GGUF OpenAI API streams#6476
Conversation
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for Gemma 4's native tool call format across the parser, tool healing, and routing modules, and introduces reasoning content extraction for GGUF streams. It also updates the stream item timeout handling to use an asyncio.timeout context manager to preserve the AnyIO cancel scope. Feedback on the changes suggests adding a missing docstring to _gemma_arguments_to_json in tool_healing.py to maintain byte-for-byte consistency with tool_call_parser.py and avoid AST verification failures.
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.
746463d to
6705053
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8b4efa837
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c877cc7d8
ℹ️ 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".
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd1e5eabe5
ℹ️ 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: 5c4e7b5365
ℹ️ 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: 3ca278cb79
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a57d3795a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 520df9fe9d
ℹ️ 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".
| different task" on Python 3.13 + httpx, which surfaced as a mid-stream | ||
| ``response.failed``. The streaming paths that take their response inline use | ||
| ``_SameTaskStreamingResponse`` directly for the same reason.""" | ||
| return _SameTaskStreamingResponse( |
There was a problem hiding this comment.
This changes every _sse_streaming_response caller to _SameTaskStreamingResponse, whose __call__ never listens on the ASGI receive channel. Several existing call sites still rely on Starlette's disconnect listener rather than their own watcher; for example the /inference/generate stream only awaits asyncio.to_thread(next, gen, ...) and has no Request to poll, so if a client disconnects during a long generation the cancel_event is not set and the backend keeps generating until the next send or completion. Please keep the same-task response limited to streams with explicit disconnect watchers or add equivalent cancellation for the shared helper's remaining callers.
Useful? React with 👍 / 👎.
| if not stripped or stripped[0] in '"{[': | ||
| out.append(element) | ||
| continue |
There was a problem hiding this comment.
When a Gemma-native call has an array-of-object argument, this branch preserves elements starting with { verbatim instead of recursively quoting their keys. Inputs such as <|tool_call>call:batch{items:[{path:a}]}<tool_call|> therefore reach json.loads as {"items":[{path:a}]}, fail parsing, and the entire tool call is dropped, even though arrays of objects are a valid/common tool schema shape. Please normalize object elements inside arrays the same way top-level object values are normalized.
Useful? React with 👍 / 👎.
| for m in _TC_GEMMA_START_RE.finditer(content): | ||
| end = _balanced_brace_end(content, m.end() - 1, gemma_quotes = True) | ||
| if end >= 0: | ||
| candidates.append((m.start(), end, "gemma", m)) |
There was a problem hiding this comment.
These new Gemma candidates are collected before the XML-style parser and are not filtered with _inside_open_parameter, so a literal marker inside an existing <function=...><parameter=code>... value is promoted to a separate tool call. For example, a Python call whose code/comment contains <|tool_call>call:terminal{command:ls}<tool_call|> now returns and executes terminal instead of treating that text as the Python argument. Please skip Gemma starts that fall inside an open XML parameter, matching the guard already used for nested <function= markers.
Useful? React with 👍 / 👎.
| if ( | ||
| not saw_done | ||
| and not saw_finish_reason | ||
| and not saw_stream_error | ||
| and not cancel_event.is_set() | ||
| ): |
There was a problem hiding this comment.
When an upstream stream ends cleanly after an include_usage chunk but omits data: [DONE], this branch first synthesizes a finish chunk and sets saw_finish_reason = True; the EOF guard below then skips because it also requires not saw_finish_reason, so the response closes without the [DONE] sentinel despite the comment promising finish -> usage -> [DONE]. Please make the EOF path emit [DONE] whenever saw_done is false, even if the finish chunk was already synthesized before a trailing usage chunk.
Useful? React with 👍 / 👎.
| aclose = getattr(self.body_iterator, "aclose", None) | ||
| if aclose is not None: | ||
| await aclose() |
There was a problem hiding this comment.
When ASGI send raises OSError after a chunk is produced, this path closes the body iterator with aclose(), which resumes async generators with GeneratorExit rather than CancelledError. The streaming generators' cancellation handlers therefore do not run unless the polling watcher already won the race, and their finally blocks cancel that watcher without calling api_monitor.finish(..., "cancelled"), leaving monitor entries active after common downstream disconnects. Please signal cancellation or finish the monitor on this send-disconnect path before closing the iterator.
Useful? React with 👍 / 👎.
Address five review findings on the Gemma tool-call and OpenAI passthrough
streaming paths:
- parse_tool_calls_from_text collected JSON and Gemma markers without the
_inside_open_parameter guard, so a marker embedded in an existing
<function=...><parameter=...> value was promoted to a separate tool call.
Candidates that start inside an open XML parameter are now skipped, matching
the guard the XML-style parser already applies.
- _quote_gemma_array_elements preserved array elements starting with { or [
verbatim, so an array of objects (items:[{path:a}]) or a nested array failed
json.loads and the whole call was dropped. Object and nested-array elements
are now normalised recursively.
- _openai_passthrough_stream synthesized a finish chunk before a trailing
usage-only chunk and set saw_finish_reason, which made the EOF guard skip the
[DONE] sentinel. The EOF path now emits [DONE] whenever the upstream omitted
it, even after a finish chunk was already synthesized.
- /generate/stream drove generation through asyncio.to_thread with no
disconnect watcher, so a client disconnect during a long generation went
unnoticed until the next send. It now runs _await_disconnect_then_cancel
against the request, matching the other local streaming endpoints.
- _SameTaskStreamingResponse closed the body iterator with aclose() on a
send-side disconnect, raising GeneratorExit so the generators' cancellation
handlers (which finish the api_monitor entry) never ran. It now throws
CancelledError, falling back to aclose() when athrow is unavailable.
Extends tests/test_gemma_tool_parse_edge_cases.py with array-of-objects,
nested-array, and marker-inside-XML-parameter cases.
|
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 |
for more information, see https://pre-commit.ci
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e58c3707d
ℹ️ 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".
|
|
||
| async def __call__(self, scope, receive, send) -> None: | ||
| try: | ||
| await self.stream_response(send) |
There was a problem hiding this comment.
Switching the shared SSE helper to _SameTaskStreamingResponse removes Starlette's receive-side disconnect listener. Callers that do not install their own watcher, such as _anthropic_tool_stream and _anthropic_plain_stream, only poll request.is_disconnected() before await asyncio.to_thread(next, gen, ...), so a client disconnect during prefill or a long generation/tool step will not set cancel_event or close the generator until another chunk is produced or a send fails. This lets local Anthropic streams keep consuming the decode slot after the client is gone; add the same disconnect watcher used by the chat streams or keep the receive listener for these callers.
Useful? React with 👍 / 👎.
| # A bare (unquoted) Gemma value ends at `}` or at a comma that begins the next | ||
| # `key:` pair. A comma NOT followed by a key token is part of the value (e.g. | ||
| # `location:New York, NY`), so it must not terminate the value. | ||
| _GEMMA_NEXT_KEY_RE = re.compile(r"\s*[\w-]+\s*:") |
There was a problem hiding this comment.
For bare Gemma string arguments, this delimiter treats any comma followed by \w+: as the start of the next key, so natural values like query:meet at 10:00, 11:00 tomorrow are parsed as {"query":"meet at 10:00","11":"00 tomorrow"} instead of preserving the query. This corrupts tool arguments for common time/URL/ratio text that appears after a comma; constrain the next-key check to actual identifier-shaped keys rather than numeric or arbitrary colon-bearing text.
Useful? React with 👍 / 👎.
Two follow-ups on the streaming and tool-parse paths: - _anthropic_tool_stream and _anthropic_plain_stream drove generation through asyncio.to_thread(next, gen, ...) and only polled is_disconnected() between events, so a client disconnect during prefill or a long generation/tool step held the decode slot until the next event or a failed send. Both now run the _await_disconnect_then_cancel watcher used by the other local streams, stop it in finally, and break promptly when cancel_event is set. - _GEMMA_NEXT_KEY_RE treated any comma followed by word-chars-then-colon as the next key, so a bare value such as "meet at 10:00, 11:00 tomorrow" was split into bogus keys. The next-key token must now be identifier-shaped (start with a letter or underscore), so a comma before a timestamp, ratio, or other numeric-then-colon text stays part of the value. Adds a timestamp-in-bare-value regression test.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
for more information, see https://pre-commit.ci
|
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d376756e9c
ℹ️ 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".
| if any(s <= start < e for s, e in consumed): | ||
| continue |
There was a problem hiding this comment.
When a Gemma-native call has an unquoted argument value that contains a literal tool-call marker, e.g. code:<|tool_call>call:terminal{command:ls}<tool_call|>, the outer candidate spans the inner marker but fails normalization at the inner }; because this guard only skips starts inside spans that were already successfully consumed, the inner marker is then accepted as a separate executable tool call. That violates the nested-marker protection added here and can run the wrong tool instead of preserving the outer argument; skip nested candidates based on all enclosing candidate spans or avoid accepting inner candidates when an earlier enclosing candidate failed to parse.
Useful? React with 👍 / 👎.
| athrow = getattr(self.body_iterator, "athrow", None) | ||
| if athrow is not None: | ||
| try: | ||
| await athrow(asyncio.CancelledError()) | ||
| except (asyncio.CancelledError, StopAsyncIteration, RuntimeError): |
There was a problem hiding this comment.
If the client disconnects before http.response.start is sent, stream_response() raises OSError before the body iterator has ever been advanced; calling athrow() on a fresh async generator closes it without entering its try/finally, so the cleanup inside generators (for example _tracker.__exit__() and _aclose_stream_resources() after an upstream passthrough response has already been opened) never runs. In that early-disconnect path this leaves stale cancel-registry entries and can leave an upstream llama-server stream generating until completion; add cleanup that is guaranteed to run even when the body iterator was never started.
Useful? React with 👍 / 👎.
| if cancel_event.is_set(): | ||
| break |
There was a problem hiding this comment.
In /generate/stream, when the new disconnect watcher sets cancel_event after a chunk was sent but before the next next(gen) call, this branch exits the loop without ever calling backend.reset_generation_state(); the finally also skips the reset because the event is already set. For the subprocess-backed generator, closing the Python generator at this point does not send the cancel command, so the model can keep decoding after the client has gone away; reset/cancel the backend before breaking on this event.
Useful? React with 👍 / 👎.
Three follow-ups on the tool-parse and streaming paths:
- parse_tool_calls_from_text only skipped markers that fell inside a span it
had already parsed successfully, so when an unquoted Gemma argument contained
a literal marker (code:<|tool_call>call:terminal{...}<tool_call|>) the outer
object failed to normalize, its span was never recorded, and the inner marker
was promoted to a standalone terminal call. Candidates nested inside any other
candidate's brace span are now skipped regardless of whether the enclosing
candidate parsed, so a marker in malformed outer data is never executed.
- /generate/stream skipped backend.reset_generation_state() when the disconnect
watcher set cancel_event between chunks: the loop broke and the finally's reset
is guarded on cancel_event being unset. A subprocess backend kept decoding
after the client left. The cancel-break path now resets the backend.
- _SameTaskStreamingResponse threw CancelledError / called aclose() on the body
iterator on a send-side disconnect, but neither runs the try/finally of a
generator that never started (early disconnect on http.response.start), so the
passthrough's eagerly-opened upstream httpx stream and cancel-registry entry
leaked. It now tracks whether the body started and, when it did not, runs an
optional unstarted_cleanup hook; the OpenAI passthrough wires it to close the
upstream resp/client and exit the cancel tracker.
Adds a nested-unquoted-marker regression test.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| _sf_tracker.__exit__(None, None, None) | ||
|
|
||
| if payload.stream: | ||
| return _sse_streaming_response(sf_tool_stream()) |
|
@codex review |
|
/gemini review |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Code Review
This pull request adds support for Gemma-native tool-call parsing and improves streaming robustness by replacing Starlette's stock StreamingResponse with a custom _SameTaskStreamingResponse to prevent task-hopping and ensure proper resource cleanup on client disconnects. It also introduces reasoning content extraction and splitting for GGUF/llama-server streams. Feedback is provided regarding the stream termination and timeout handling in _aiter_llama_stream_items, recommending a manual __anext__ loop with explicit response.aclose() calls in Python 3.13 environments to prevent GeneratorExit errors.
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.
| if ( | ||
| request is not None | ||
| and post_first_item_read_timeout_s is not None | ||
| and now - last_item_at < post_first_item_read_timeout_s | ||
| ): | ||
| continue |
There was a problem hiding this comment.
This condition, and the surrounding logic within the except StopAsyncIteration: block, incorrectly handles the termination of a stream. StopAsyncIteration signals that the upstream iterator has finished cleanly, not that it has timed out. Treating this as a potential httpx.ReadTimeout can lead to correctly finished streams being reported as errors.
The entire except StopAsyncIteration: block should be simplified to gracefully exit the loop. Additionally, in Python 3.13 environments using httpx and httpcore 1.0.x, when using a manual __anext__ loop, ensure you explicitly call response.aclose() before the iterator is closed to prevent RuntimeError: async generator ignored GeneratorExit during early exits or client disconnections.
References
- In Python 3.13 environments using
httpxandhttpcore1.0.x, avoid usingasync forto iterate overresponse.aiter_lines(). Instead, use a manual__anext__loop and explicitly callresponse.aclose()before the iterator is closed to preventRuntimeError: async generator ignored GeneratorExitduring early exits or client disconnections.
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…6476) (#6611) * Quote-aware Gemma strip, symmetric unstarted cleanup, ReDoS anchor Address review findings on the tool-strip and streaming paths: - strip_tool_call_markup stripped Gemma-native spans with a plain regex that stops at the first <tool_call|>, so a literal close marker inside a <|"|>-quoted argument truncated the span and leaked its suffix into visible text. A brace/quote-aware _strip_gemma_native_spans now removes complete spans (keeping an incomplete one unless final), matching the parser's own balance logic. - The Gemma close pattern this PR added (<\|tool_call>.*?<tool_call\|>) had no \Z fallback, so a run of unclosed markers backtracked from every open position (quadratic, and the streaming stripper re-scans per token). It is now anchored to (?:<tool_call|>|\Z) like routes/inference.py's _TOOL_XML_RE, linear with identical output on well-formed input. - _SameTaskStreamingResponse added unstarted_cleanup for the OpenAI passthrough, but the local GGUF/safetensors streams that enter _TrackedCancel before returning only unregister in the generator finally, which never runs if the client disconnects before the body iterator starts, leaking cancel-registry entries. Each such stream now passes unstarted_cleanup to exit its tracker. - __call__ reads _unstarted_cleanup via getattr so a response built through __new__ (the cancel-timing test) without __init__ does not raise AttributeError; the test also sets the attribute explicitly. - Document that the verbatim /v1/chat/completions passthrough delegates <think>/<|tool_call> splitting to llama-server (--jinja, --reasoning-format auto) and is intentionally not re-parsed locally, noting the llama.cpp dependency. Adds a regression test for the close-marker-inside-quoted-argument strip. * Tighten comments on the tool-strip and streaming paths Compress the verbose comment blocks added with the Gemma tool-call / streaming work to crisp one or two liners, drop restatements of obvious code, and shorten docstrings, keeping the load-bearing rationale (ReDoS anchor, quote-aware strip, unstarted-cleanup, llama.cpp passthrough dependency). Code is unchanged (verified comment-only via AST/ast signature, docstrings stripped). * Harden Gemma parse/strip: span-aware XML fallback and quote-aware streaming - Security: the XML fallback in parse_tool_calls_from_text scanned the whole content for <function=...> markers and only skipped those inside an open XML parameter, not those inside a collected JSON/Gemma candidate span. A balanced but unparsable Gemma call whose argument data contained XML tool markup (<|tool_call>call:outer{code:<function=terminal>...}<tool_call|>) therefore fell through to the fallback and returned an executable terminal call. The fallback now also excludes <function=> markers inside any candidate span, including ones that failed to parse. - strip_tool_call_markup no longer skips the generic Gemma regex after running the quote-aware _strip_gemma_native_spans, so a closed Gemma span the helper cannot match (malformed, e.g. <|tool_call>{"name":"x"}<tool_call|>) is still stripped instead of leaking its opener and payload into visible text. - _strip_gemma_native_spans stops at the first unbalanced start instead of re-scanning every later start to EOF, keeping it linear on a run of unclosed markers rather than quadratic. - The GGUF and safetensors streaming strippers run _strip_gemma_native_spans before the regex patterns, so a well-formed streamed call whose quoted argument contains a literal close marker no longer leaks its suffix into incremental display. Adds regression tests for the nested-XML escape and the malformed-span strip. * Avoid remainder copy in _strip_gemma_native_spans Match the Gemma close marker with re pos directly on the buffer instead of slicing tail = text[brace_end + 1:] on every span. The streaming strippers re-scan a growing cumulative buffer per token, so the per-span remainder copy was quadratic. Behavior is unchanged. * Exclude unclosed Gemma/JSON starts from the XML tool-call fallback The nested-XML guard only skipped <function=> markers inside recorded candidate spans, but a span is recorded only when the braces balance. An unbalanced call such as <|tool_call>call:outer{code:<function=terminal>... recorded no span, so the fallback still promoted the inner <function=> to an executable terminal call. Treat unclosed JSON/Gemma starts as exclusion spans through EOF before scanning. Standalone <function=> calls with no preceding unclosed start still parse. Regression tests added. * Skip doomed tool-strip passes to avoid quadratic rescans The lazy closed-pair strip patterns (<tool_call>.*?</tool_call>, <function=...>.*?</function>) rescan to EOF from every opener when their close token is absent, which is O(n^2) and re-runs per streamed token. Add strip_tool_patterns, which skips a pass whose close token is not present in the text; output is identical to the per-pattern loop (verified by fuzz), and a degenerate run drops from ~minutes to milliseconds. Used by strip_tool_call_markup and the GGUF/safetensors streaming strippers. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Use full tool-call envelopes to close nested-XML escape variants Key the parser and stripper off the full <|tool_call>...<tool_call|> / <tool_call>...</tool_call> envelope (start to close marker, searched after the braces; EOF if unclosed) instead of just the braces: - XML between the closing brace and the close marker (call:outer{broken:{x}}<function=terminal>...<tool_call|>) is now inside the envelope, so the fallback no longer promotes it to a tool call. - A balanced inner call inside an unclosed outer (call:outer{code:<|tool_call>call:terminal{...}<tool_call|>) is skipped via the envelope nested check, not just the XML fallback. - strip_tool_call_markup searches for the close marker after the braces, so junk before <tool_call|> is stripped through the close and text after it is preserved instead of truncated to EOF; a no-close run stops early (linear). Regression tests added; standalone XML and well-formed calls unaffected. * Fix non-final Gemma strip and missing-close recovery for PR #6611 Split the nested-skip from the XML fallback exclusion: nesting is decided by each marker's brace region, so a balanced call after one with a missing close marker is recovered instead of being swallowed to EOF. Only the XML fallback keeps the search-to-close envelope, so trailing nested markup still cannot escape as an executable call. Use a closed-only Gemma pattern in the non-final strip list so an incomplete block is preserved (matching the JSON and function paths); the final list keeps the close-or-EOF Gemma pattern in its original position, so streaming display output is byte-for-byte unchanged. Add regression tests for both cases. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Block gap-nested tool markers and fix XML strip order for PR #6611 Decide candidate nesting by a per-marker coverage region paired with a per-format stack (a close after the braces pops the nearest still-open marker of that format). A closed outer call now covers up to its own close marker, so a JSON or Gemma tool marker smuggled between the outer braces and that close is treated as data instead of being executed. An outer that balances but has no close of its own covers only its brace region, so a later sibling after an omitted close marker is still recovered (adjacent calls use an exclusive end bound so the next call is not misread as nested). Strip every closed pair (JSON, Gemma, function) before any to-EOF sweep, so a closed function call whose parameter text contains a bare Gemma opener is removed as a unit and the to-EOF sweep can no longer drop the visible text after the close. Add regression tests for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Strip closed tool blocks before the Gemma final sweep for PR #6611 The final display strip ran the quote-aware Gemma helper before the closed JSON/function patterns. A closed <tool_call>...</tool_call> or <function=...>...</function> block whose argument data held a call-form Gemma opener (e.g. a "<|tool_call>call:t{" string) was read as an incomplete Gemma span and truncated to EOF, dropping the block's close and any visible text after it. Strip closed JSON/function blocks first, so such a block is removed as a unit before the helper runs. Centralize the final strip order in a shared strip_tool_markup_final so strip_tool_call_markup and both streaming display wrappers (safetensors, llama_cpp) stay in sync, and apply the same closed-block pre-pass to the non-final path. Add regression tests for the JSON and function variants. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Recover XML/JSON siblings after a close-less tool marker for PR #6611 Two fixes so the XML fallback and marker coverage recover a later valid call after an earlier marker omits its close, matching the candidate loop: Reuse the candidate marker-coverage in the XML fallback instead of a separate search-to-close-or-EOF envelope. A balanced but close-less marker now covers only its brace region there too, so a following <function=...> sibling is recovered rather than filtered as nested data; an unbalanced marker still covers to EOF and a closed one still covers through its close, so nested XML stays blocked. Ignore a close token that falls inside another call's balanced braces when pairing closes in _marker_coverage. Such a token is that call's quoted argument data, so it no longer pops an earlier close-less marker and extends its coverage over a later valid sibling. Add regression tests for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Make the closed-block strip pre-pass Gemma-span-aware The final display strip ran the closed JSON/function regex pre-pass before removing Gemma-native spans, so a literal <function=...> quoted inside a Gemma argument plus any later </function> (a real call's close or even prose) was deleted across the Gemma boundary. That mangled the Gemma close marker, the quote-aware helper then saw an unclosed opener, and the whole visible tail after the call was truncated. The pre-pass now skips matches that start inside a complete Gemma span (that text is the span's argument data) and resumes scanning at the end of the covering span, so a real function-XML call after the Gemma call is still stripped. The original ordering rationale is preserved: a Gemma opener inside a JSON or function argument still cannot truncate that block, covered by regression tests for both directions. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim comments in the Gemma streaming and strip pipeline to essentials * Tighten comments in the Gemma strip and streaming disconnect paths * Fold marker-collection comment to two lines --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
3 participants
Summary
<think>output into OpenAI-compatiblereasoning_contentfor Chat Completions and Responses streams.<|tool_call>call:name{...}<tool_call|>blocks, including template-quoted strings, hyphenated args, Windows paths, and braces inside string values./v1/responsesstream reads, disconnect checks, and ASGI response sending in the same task to avoid AnyIO cancel-scope teardown errors.output_text, including truncated<think>streams.<tool_call|>close markers.Fixes #6471.
Tests
temp/pr-6476-venv/bin/python -m pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_tool_call_parser_strict.py studio/backend/tests/test_mcp_servers.py studio/backend/tests/test_responses_tool_passthrough.py studio/backend/tests/test_llama_route_timeouts.py studio/backend/tests/test_tool_xml_strip.py(267 passed)temp/pr-6476-venv/bin/python -m pytest studio/backend/tests/test_responses_tool_passthrough.py::TestResponsesStreamAdapter::test_stream_response_avoids_legacy_receive_watcher studio/backend/tests/test_llama_route_timeouts.py studio/backend/tests/test_tool_xml_strip.py(37 passed)temp/pr-6476-venv/bin/python -m py_compile studio/backend/routes/inference.py studio/backend/tests/test_responses_tool_passthrough.pytemp/pr-6476-py39/bin/python -m py_compile studio/backend/routes/inference.py studio/backend/tests/test_responses_tool_passthrough.py studio/backend/tests/test_llama_route_timeouts.py studio/backend/tests/test_tool_xml_strip.pygit diff --check/v1/chat/completionsand/v1/responses: HTTP 200, no raw<think>or<|tool_call>marker leakage; three concurrent/v1/responsesstreams completed.