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.
This commit is contained in:
parent
0cc69de210
commit
bb33346642
3 changed files with 62 additions and 5 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue