From 7fe1adbf581037b2dd82c2651dc2245d662e09f7 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Tue, 26 May 2026 11:18:03 +0000 Subject: [PATCH] Studio: per-card web_search result + shell_call output fallback (OpenAI) Two empty-output bugs in the OpenAI Responses tool-result rendering that showed up clearly when a single prompt invoked 9 web_search + 4 code_execution + 1 image_generation in one turn. Reproduction shape in the SQLite-stored chat history: - 8 of 9 web_search tool-call records had result == "" (the cards rendered as empty cards in the thread) - 4 of 4 code_execution (shell_call) records were missing the result key entirely (NoneType), so the cards that showed "Ran cat ..." style commands displayed the command line but no output panel at all - image_generation worked, as did the very last web_search of the run Root causes in studio/backend/core/inference/external_provider.py: 1. web_search_call's tool_end emitted result: "" by design, with the intent of overwriting only the LAST call at response.completed with the full citation list (the source-pill extractor on the frontend flatMaps across every web_search result, so a single non-empty result is enough for the trailing source pills). Side effect: every intermediate card renders empty in the thread. Fix: seed each call's own tool_end result with "Searching: " so the per-card text is never empty, then keep the last-call overwrite path so the source-pill extractor still works. Falls back to empty when the model emits an action with no query, so the existing last-call path stays unchanged for that edge. 2. shell_call's tool_start was emitted from response.output_item.done for the call item, but tool_end lived in the separate response.output_item.done handler for shell_call_output. When OpenAI's Responses stream bundles the output array onto the shell_call item's own done event (no separate shell_call_output item), the previous handler emitted tool_start with no following tool_end. The card spun on "running" indefinitely and stored as NoneType in the thread DB. Fix: when the shell_call's done event carries an embedded output list, emit tool_end immediately from that. Track tool_end_emitted on the shell_calls map so a subsequent shell_call_output event (some streams ship both) is skipped instead of double-completing the card. A final flush at response.completed emits tool_end for any orphan shell_call that received neither bundled output nor a separate output event, so cards always finalise. Tests (studio/backend/tests/test_openai_tool_result_fallbacks.py, 6 new): - web_search: three calls, each card's result is its own Searching: query (no empties) - web_search: last call still gets the aggregated citation block when url_citations arrive (pins the overwrite path) - web_search: empty action.query falls back to result == "" (no junk Searching: placeholder) - shell_call: bundled output on done emits a single tool_end with that output as the result text - shell_call: bundled-then-separate output does not double-emit tool_end (subsequent shell_call_output is skipped) - shell_call: orphan call with neither bundled nor separate output is flushed at response.completed so the card finalises 15/15 tests green when combined with the existing 9 in test_openai_code_execution.py. Pre-commit + ruff format clean. Scope: OpenAI Responses-API code path only. The Anthropic native Messages-API path (_stream_anthropic) is untouched, as is the local llama-server path. Local-model behaviour cannot regress because the edited handlers only fire inside the OpenAI cloud branch. --- .../core/inference/external_provider.py | 75 +++- .../test_openai_tool_result_fallbacks.py | 328 ++++++++++++++++++ 2 files changed, 395 insertions(+), 8 deletions(-) create mode 100644 studio/backend/tests/test_openai_tool_result_fallbacks.py diff --git a/studio/backend/core/inference/external_provider.py b/studio/backend/core/inference/external_provider.py index f34ef5fd62..f1bbd8bdf4 100644 --- a/studio/backend/core/inference/external_provider.py +++ b/studio/backend/core/inference/external_provider.py @@ -3873,14 +3873,20 @@ class ExternalProviderClient: ), } ) + # Seed result with the call's own query so + # each card shows "Searching: " + # instead of an empty panel. The last call + # is still overwritten at response.completed + # with the aggregated citation list (used + # by the source-pill extractor). + per_call_result = ( + f"Searching: {query}" if query else "" + ) yield _emit_tool_event( { "type": "tool_end", "tool_call_id": item_id, - # Empty result — the last call gets - # overwritten with citations at - # response.completed. - "result": "", + "result": per_call_result, } ) elif item.get("type") == "shell_call": @@ -3908,7 +3914,11 @@ class ExternalProviderClient: ) shell_calls.setdefault( item_id, - {"commands": [], "output": None}, + { + "commands": [], + "output": None, + "tool_end_emitted": False, + }, ) shell_calls[item_id]["commands"] = ( list(commands) @@ -3926,6 +3936,27 @@ class ExternalProviderClient: }, } ) + # Fallback: some Responses streams ship the + # output bundled on the shell_call item's + # done event instead of as a separate + # shell_call_output. If so, emit tool_end + # now so the card never stays in "running". + embedded_output = item.get("output") + if ( + isinstance(embedded_output, list) + and embedded_output + ): + shell_calls[item_id]["output"] = embedded_output + shell_calls[item_id]["tool_end_emitted"] = True + yield _emit_tool_event( + { + "type": "tool_end", + "tool_call_id": item_id, + "result": _format_shell_output( + embedded_output + ), + } + ) elif item.get("type") == "shell_call_output": # `call_id` links back to the shell_call's # `id`, which is what we used as the @@ -3936,8 +3967,16 @@ class ExternalProviderClient: item.get("call_id") or item.get("id") or "" ) output = item.get("output") or [] + # Skip if the fallback above already emitted + # tool_end for this call from the bundled + # output, so the card is not re-completed. + if shell_calls.get(call_id, {}).get( + "tool_end_emitted" + ): + continue if call_id in shell_calls: shell_calls[call_id]["output"] = output + shell_calls[call_id]["tool_end_emitted"] = True result_text = _format_shell_output(output) yield _emit_tool_event( { @@ -4099,9 +4138,10 @@ class ExternalProviderClient: # parseSourcesFromResult flatMaps every # web_search tool-call result, so a single # non-empty result is enough to surface the - # whole source-pill set at the message tail — - # no need to fan out across every card (which - # would just duplicate the same pills). + # whole source-pill set at the message tail. + # Earlier per-call results carry their own + # "Searching: " text so the cards are + # never empty. if web_search_calls and all_url_citations: last_id = list(web_search_calls.keys())[-1] blocks: list[str] = [] @@ -4119,6 +4159,25 @@ class ExternalProviderClient: "result": "\n---\n".join(blocks), } ) + # Final flush: any shell_call that never got + # an output event needs a tool_end so the card + # transitions out of "running". Emit with the + # accumulated output (may be empty) so the + # frontend renders "(no output)" rather than + # spinning indefinitely. + for sc_id, sc_state in shell_calls.items(): + if sc_state.get("tool_end_emitted"): + continue + yield _emit_tool_event( + { + "type": "tool_end", + "tool_call_id": sc_id, + "result": _format_shell_output( + sc_state.get("output") or [] + ), + } + ) + sc_state["tool_end_emitted"] = True chunk = { "id": completion_id, "object": "chat.completion.chunk", diff --git a/studio/backend/tests/test_openai_tool_result_fallbacks.py b/studio/backend/tests/test_openai_tool_result_fallbacks.py new file mode 100644 index 0000000000..55325a1f64 --- /dev/null +++ b/studio/backend/tests/test_openai_tool_result_fallbacks.py @@ -0,0 +1,328 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Regression tests for OpenAI Responses tool-result rendering. + +Two bug classes covered: + +1. web_search: every call's tool_end carried `result: ""`, leaving every + card except the last one empty in the chat thread. Now each call seeds + its own `Searching: ` text so the cards always render content; + the last call is still overwritten at response.completed with the + aggregated citation list (consumed by the source-pill extractor). + +2. shell_call (code_execution): when OpenAI bundles the output on the + shell_call item's `response.output_item.done` event instead of as a + separate `shell_call_output`, the previous handler never emitted + tool_end and the card stayed in "running" with no output. The fallback + now emits tool_end from the bundled output. A final flush at + response.completed catches shell_call ids that received neither. +""" + +import asyncio +import json + +import httpx + +from core.inference import external_provider as ep_mod +from core.inference.external_provider import ExternalProviderClient + + +def _drive(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +async def _collect(agen): + out = [] + async for line in agen: + out.append(line) + return out + + +def _mock_http_client(monkeypatch, handler): + transport = httpx.MockTransport(handler) + monkeypatch.setattr(ep_mod, "_http_client", httpx.AsyncClient(transport = transport)) + + +def _make_client(base_url: str = "https://api.openai.com/v1") -> ExternalProviderClient: + return ExternalProviderClient( + provider_type = "openai", + base_url = base_url, + api_key = "sk-test", + ) + + +def _openai_sse(events: list[dict]) -> bytes: + chunks: list[str] = [] + for event in events: + chunks.append(f"event: {event['type']}") + chunks.append(f"data: {json.dumps(event)}") + chunks.append("") + return ("\n".join(chunks) + "\n").encode("utf-8") + + +def _tool_events(lines: list[str]) -> list[dict]: + out: list[dict] = [] + for line in lines: + if not line.startswith("data:"): + continue + raw = line[len("data:") :].strip() + if not raw or raw == "[DONE]": + continue + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + continue + if isinstance(parsed, dict) and "_toolEvent" in parsed: + out.append(parsed["_toolEvent"]) + return out + + +def _drive_stream(sse_events, enabled_tools, monkeypatch): + def handler(request): + return httpx.Response( + 200, + content = _openai_sse(sse_events), + headers = {"content-type": "text/event-stream"}, + ) + + _mock_http_client(monkeypatch, handler) + + async def run(): + client = _make_client() + return await _collect( + client._stream_openai_responses( + messages = [{"role": "user", "content": "x"}], + model = "gpt-5.5", + temperature = 0.7, + top_p = 0.95, + max_tokens = 4096, + enable_thinking = None, + reasoning_effort = None, + enabled_tools = enabled_tools, + ) + ) + + return _drive(run()) + + +# ── web_search per-card result ───────────────────────────────────────── + + +def test_web_search_each_call_carries_its_own_query_as_result(monkeypatch): + """Three search calls, no citations. Each card must render with its + own `Searching: ` text; none should be empty.""" + sse_events = [ + { + "type": "response.output_item.done", + "item": { + "type": "web_search_call", + "id": "ws_1", + "action": {"query": "popular animals 2026"}, + }, + }, + { + "type": "response.output_item.done", + "item": { + "type": "web_search_call", + "id": "ws_2", + "action": {"query": "most loved animals poll"}, + }, + }, + { + "type": "response.output_item.done", + "item": { + "type": "web_search_call", + "id": "ws_3", + "action": {"query": "tiger ranking"}, + }, + }, + {"type": "response.completed", "response": {}}, + ] + lines = _drive_stream(sse_events, ["web_search"], monkeypatch) + events = _tool_events(lines) + ends = [e for e in events if e["type"] == "tool_end"] + by_id = {e["tool_call_id"]: e for e in ends} + assert by_id["ws_1"]["result"] == "Searching: popular animals 2026" + assert by_id["ws_2"]["result"] == "Searching: most loved animals poll" + assert by_id["ws_3"]["result"] == "Searching: tiger ranking" + + +def test_web_search_last_call_overwritten_with_citations(monkeypatch): + """Pin the existing behaviour: the last call still gets the + aggregated citation list (the source-pill extractor depends on this). + Earlier calls keep their per-call `Searching:` text.""" + sse_events = [ + { + "type": "response.output_item.done", + "item": { + "type": "web_search_call", + "id": "ws_1", + "action": {"query": "first query"}, + }, + }, + { + "type": "response.output_item.done", + "item": { + "type": "web_search_call", + "id": "ws_2", + "action": {"query": "second query"}, + }, + }, + { + "type": "response.output_text.annotation.added", + "annotation": { + "type": "url_citation", + "url": "https://example.com/a", + "title": "Example A", + }, + }, + {"type": "response.completed", "response": {}}, + ] + lines = _drive_stream(sse_events, ["web_search"], monkeypatch) + events = _tool_events(lines) + ends = [e for e in events if e["type"] == "tool_end"] + by_id: dict = {} + # Keep the LAST tool_end per id (the citation overwrite for ws_2). + for e in ends: + by_id[e["tool_call_id"]] = e + # First call keeps its own query. + assert by_id["ws_1"]["result"] == "Searching: first query" + # Last call gets overwritten with the citation block. + assert "Title: Example A" in by_id["ws_2"]["result"] + assert "URL: https://example.com/a" in by_id["ws_2"]["result"] + + +def test_web_search_empty_query_falls_back_to_empty_result(monkeypatch): + """Defensive: if the action carries no query, do not write a junk + `Searching:` placeholder; leave the result empty so the existing + last-call overwrite path is unchanged.""" + sse_events = [ + { + "type": "response.output_item.done", + "item": { + "type": "web_search_call", + "id": "ws_only", + "action": {}, + }, + }, + {"type": "response.completed", "response": {}}, + ] + lines = _drive_stream(sse_events, ["web_search"], monkeypatch) + events = _tool_events(lines) + ends = [e for e in events if e["type"] == "tool_end"] + assert len(ends) == 1 + assert ends[0]["result"] == "" + + +# ── shell_call output fallbacks ──────────────────────────────────────── + + +def test_shell_call_emits_tool_end_when_output_bundled_on_done(monkeypatch): + """Some Responses streams ship the output array embedded on the + shell_call item's done event instead of as a separate + shell_call_output. The fallback must emit tool_end from that bundled + output so the card never stays in "running".""" + sse_events = [ + { + "type": "response.output_item.done", + "item": { + "type": "shell_call", + "id": "scall_bundled", + "action": {"commands": ["echo hi"]}, + "output": [ + { + "stdout": "hi\n", + "stderr": "", + "outcome": {"type": "exit", "exit_code": 0}, + } + ], + }, + }, + {"type": "response.completed", "response": {}}, + ] + lines = _drive_stream(sse_events, ["code_execution"], monkeypatch) + events = _tool_events(lines) + starts = [e for e in events if e["type"] == "tool_start"] + ends = [e for e in events if e["type"] == "tool_end"] + assert len(starts) == 1 + assert starts[0]["tool_call_id"] == "scall_bundled" + assert len(ends) == 1 + assert ends[0]["tool_call_id"] == "scall_bundled" + assert "hi" in ends[0]["result"] + + +def test_shell_call_bundled_then_separate_output_does_not_double_emit(monkeypatch): + """If the bundled output ALREADY emitted tool_end and a separate + shell_call_output event arrives afterwards (some streams do both), + the second one is skipped so the card is not re-completed.""" + sse_events = [ + { + "type": "response.output_item.done", + "item": { + "type": "shell_call", + "id": "scall_both", + "action": {"commands": ["echo bundle"]}, + "output": [ + { + "stdout": "bundle\n", + "stderr": "", + "outcome": {"type": "exit", "exit_code": 0}, + } + ], + }, + }, + { + "type": "response.output_item.done", + "item": { + "type": "shell_call_output", + "id": "scout_both", + "call_id": "scall_both", + "output": [ + { + "stdout": "should not double-emit\n", + "stderr": "", + "outcome": {"type": "exit", "exit_code": 0}, + } + ], + }, + }, + {"type": "response.completed", "response": {}}, + ] + lines = _drive_stream(sse_events, ["code_execution"], monkeypatch) + events = _tool_events(lines) + ends = [e for e in events if e["type"] == "tool_end"] + assert len(ends) == 1 + assert ends[0]["tool_call_id"] == "scall_both" + assert "bundle" in ends[0]["result"] + assert "should not double-emit" not in ends[0]["result"] + + +def test_shell_call_final_flush_on_completed_when_no_output_event(monkeypatch): + """shell_call gets tool_start but no shell_call_output arrives and + the done event has no bundled output either. The final flush at + response.completed must emit tool_end so the card finalises.""" + sse_events = [ + { + "type": "response.output_item.added", + "item": { + "type": "shell_call", + "id": "scall_orphan", + "action": {"commands": ["true"]}, + }, + }, + { + "type": "response.output_item.done", + "item": { + "type": "shell_call", + "id": "scall_orphan", + "action": {"commands": ["true"]}, + "status": "completed", + }, + }, + {"type": "response.completed", "response": {}}, + ] + lines = _drive_stream(sse_events, ["code_execution"], monkeypatch) + events = _tool_events(lines) + ends = [e for e in events if e["type"] == "tool_end"] + assert any(e["tool_call_id"] == "scall_orphan" for e in ends)