From bb33346642bddcfcb94c9ff13a724269fb734c62 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 19 May 2026 02:57:56 +0000 Subject: [PATCH] studio: drop redundant boundary flag on post-tool status, do reset at tool_end Codex 02:42Z on #5549 caught that flagging the post-tool empty-status event with boundary=True (cycle-12 commit 610c387) double-handles the cursor reset in the Anthropic streaming path. AnthropicStreamEmitter._handle_tool_end already: - closes the open tool_use block, - emits tool_result, - increments block_index, - opens a fresh text block, and - resets _prev_text = "". When llama_cpp.py then yielded boundary=True on the very next event, _handle_boundary fired _close_block + _open_text_block on that freshly opened (still-empty) text block. Result: every tool call produced a spurious content_block_stop + content_block_start pair before the post-tool model text streamed. Fix: - llama_cpp.py: drop boundary=True from the post-tool status emit (line ~4768). Keep boundary=True only at the auto-continue site (line ~4500), which has no preceding tool_end to do the cursor work. - routes/inference.py OpenAI-compat tool stream: mirror the Anthropic semantics by resetting prev_text on BOTH tool_start AND tool_end, so the post-tool empty-status no longer needs to do it. Add backend/tests/test_anthropic_messages.py::TestAnthropicStreamEmitter:: test_post_tool_empty_status_does_not_double_close as a regression test: content -> tool_start -> tool_end -> empty status -> content must not bump block_index past tool_end's increment, and the post-tool content must land in the text block tool_end opened. 95 tests pass across the anthropic + trailing-plan suites. --- studio/backend/core/inference/llama_cpp.py | 12 ++++-- studio/backend/routes/inference.py | 14 ++++++- .../backend/tests/test_anthropic_messages.py | 41 +++++++++++++++++++ 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index afb3c4a428..44e12c2b32 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -4757,9 +4757,15 @@ class LlamaCppBackend: conversation.append(tool_msg) # Clear tool status badge before next generation iteration. - # boundary=True: the model is about to start a fresh turn - # so cumulative-text adapters must reset their cursor. - yield {"type": "status", "text": "", "boundary": True} + # We do NOT mark this as a boundary: the preceding + # tool_end event already opened a fresh text block and + # reset the cumulative cursor in Anthropic streaming + # (see AnthropicStreamEmitter._handle_tool_end), and the + # OpenAI-compat path resets prev_text on tool_start. A + # second boundary here would close the freshly opened + # text block (empty) and reopen it, producing a spurious + # content_block_stop/start pair on every tool call. + yield {"type": "status", "text": ""} # Continue the loop to let model respond with context continue diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 24b08f031b..cb86b3bd7d 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -2481,8 +2481,18 @@ async def openai_chat_completions( continue if event["type"] in ("tool_start", "tool_end"): - if event["type"] == "tool_start": - prev_text = "" + # Both endpoints of a tool call begin a fresh + # cumulative-text window: tool_start because + # the model's next visible content restarts + # cumulative-from-zero in the post-tool turn, + # and tool_end because after we emit the + # tool result, the model's next iteration + # produces its own fresh cumulative stream + # (the post-tool empty-status event is just + # a UI badge clear; it does NOT carry a + # boundary flag, so this is the place to + # reset prev_text on tool_end). + prev_text = "" yield f"data: {json.dumps(event)}\n\n" continue diff --git a/studio/backend/tests/test_anthropic_messages.py b/studio/backend/tests/test_anthropic_messages.py index e3a5bc6358..31077b996b 100644 --- a/studio/backend/tests/test_anthropic_messages.py +++ b/studio/backend/tests/test_anthropic_messages.py @@ -649,6 +649,47 @@ class TestAnthropicStreamEmitter: parsed = json.loads(nxt[0].split("data: ")[1]) assert parsed["delta"]["text"] == "second turn" + def test_post_tool_empty_status_does_not_double_close(self): + """After tool_end already opens a fresh text block, the post-tool + empty-status event emitted by llama_cpp.py (line 4766) must NOT + close that fresh block. Otherwise every tool call produces a + spurious empty content_block_stop + content_block_start pair + before the model's post-tool text arrives. Regression test for + PR 5549 codex 02:42Z.""" + + e = AnthropicStreamEmitter() + e.start("msg_1", "m") + e.feed({"type": "content", "text": "pre"}) + e.feed( + { + "type": "tool_start", + "tool_name": "t", + "tool_call_id": "tc_1", + "arguments": {}, + } + ) + e.feed( + { + "type": "tool_end", + "tool_name": "t", + "tool_call_id": "tc_1", + "result": "ok", + } + ) + block_after_tool = e.block_index + # Post-tool empty status (no boundary flag): should produce zero + # SSE events and leave block_index unchanged. The previous + # behaviour was to close+reopen, which produced a duplicate + # empty content block. + out = e.feed({"type": "status", "text": ""}) + assert out == [] + assert e.block_index == block_after_tool + # Next content delta lands in the same fresh text block that + # tool_end opened. + nxt = e.feed({"type": "content", "text": "post"}) + parsed = json.loads(nxt[0].split("data: ")[1]) + assert parsed["delta"]["text"] == "post" + def test_empty_status_without_boundary_does_not_close_block(self): """A non-boundary empty-status event (UI badge clear at normal stream end, draining fallbacks, final status yields in