From 546faeb038fed57f63f81a434bce023d345dee24 Mon Sep 17 00:00:00 2001 From: oobabooga <112222186+oobabooga@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:58:57 -0300 Subject: [PATCH 1/3] Studio: don't drop parallel tool calls after an internal no-op --- studio/backend/core/inference/llama_cpp.py | 13 +++- .../core/inference/safetensors_agentic.py | 13 +++- .../core/inference/tool_loop_controller.py | 14 ++++ .../backend/tests/test_llama_cpp_tool_loop.py | 70 +++++++++++++++++++ .../tests/test_safetensors_tool_loop.py | 51 ++++++++++++++ .../tests/test_tool_loop_controller.py | 17 +++++ 6 files changed, 174 insertions(+), 4 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index e797cfe22a7..cc31eaf18c0 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -82,6 +82,7 @@ ) from core.inference.tool_loop_controller import ( ToolLoopController, + append_deferred_nudges, tool_event_provenance, ) from state.tool_approvals import ( @@ -10107,6 +10108,11 @@ def _tool_succeeded(tool_name: str) -> bool: assistant_msg: dict = {"role": "assistant", "content": content_text} assistant_appended = False + # No-op nudges (duplicate / disabled / render_html_repeat) are held + # here and appended after the batch's tool results, so they never split + # an assistant's tool_calls from their results. A no-op no longer aborts + # the batch, so legitimate parallel calls after it still run. + deferred_noop_msgs: list = [] # The text-path provisional card uses the parser's default id ("call_0"); # a Mistral-style call carries its own id and would open a duplicate. Reuse @@ -10149,14 +10155,14 @@ def _tool_succeeded(tool_name: str) -> bool: "provenance": decision.provenance, } completion = tool_controller.record_noop(decision) - conversation.append(completion.model_message()) + deferred_noop_msgs.append(completion.model_message()) if _forced_tool_call_pending: _forced_tool_call_pending = False logger.info( "Suppressed local GGUF tool call as internal no-op: " f"action={decision.action} tool={decision.tool_name}" ) - break + continue if not assistant_appended: assistant_msg["tool_calls"] = [decision.as_assistant_tool_call()] @@ -10275,6 +10281,9 @@ def _invoke_tool(_output_callback, _decision = decision): if _forced_tool_call_pending: _forced_tool_call_pending = False + # Deliver the deferred no-op nudges after every tool result. + append_deferred_nudges(conversation, deferred_noop_msgs) + # Close provisional cards not resolved by execution/no-op handling. for _pid, _pname in provisional_started_tool_calls.items(): if _pid not in resolved_provisional_tool_call_ids: diff --git a/studio/backend/core/inference/safetensors_agentic.py b/studio/backend/core/inference/safetensors_agentic.py index f4c243d1bf3..4ceec9a6923 100644 --- a/studio/backend/core/inference/safetensors_agentic.py +++ b/studio/backend/core/inference/safetensors_agentic.py @@ -57,6 +57,7 @@ ) from core.inference.tool_loop_controller import ( ToolLoopController, + append_deferred_nudges, coerce_tool_arguments, status_for_tool, tool_event_provenance, @@ -1099,6 +1100,11 @@ def _tool_succeeded(tool_name: str) -> bool: assistant_msg: dict = {"role": "assistant", "content": content_text} assistant_appended = False + # No-op nudges (duplicate / disabled / render_html_repeat) are held here and + # appended after the batch's tool results, so they never split an assistant's + # tool_calls from their results. A no-op no longer aborts the batch, so + # legitimate parallel calls after it still run. + deferred_noop_msgs: list = [] for tc in tool_calls or []: func = tc.get("function", {}) or {} @@ -1127,12 +1133,12 @@ def _tool_succeeded(tool_name: str) -> bool: "provenance": decision.provenance, } completion = tool_controller.record_noop(decision) - conversation.append(completion.model_message()) + deferred_noop_msgs.append(completion.model_message()) logger.info( "Suppressed local safetensors tool call as internal no-op: " f"action={decision.action} tool={decision.tool_name}" ) - break + continue if not assistant_appended: assistant_msg["tool_calls"] = [decision.as_assistant_tool_call()] @@ -1243,6 +1249,9 @@ def _invoke_tool(_output_callback, _decision = decision): yield completion.tool_end_event() conversation.append(completion.tool_message()) + # Deliver the deferred no-op nudges after every tool result. + append_deferred_nudges(conversation, deferred_noop_msgs) + # Clear the status badge before the next turn. yield {"type": "status", "text": ""} diff --git a/studio/backend/core/inference/tool_loop_controller.py b/studio/backend/core/inference/tool_loop_controller.py index f595531b907..fb64836561e 100644 --- a/studio/backend/core/inference/tool_loop_controller.py +++ b/studio/backend/core/inference/tool_loop_controller.py @@ -266,6 +266,20 @@ def strip_result_for_model(result: str) -> str: return result +def append_deferred_nudges(conversation: list, msgs: Sequence[dict]) -> None: + """Append a batch's no-op nudges as a single ``role=user`` message after the + tool results. + + Held until here (rather than appended inline) so a no-op never splits an + assistant's ``tool_calls`` from their ``role=tool`` results, and merged into + one message (deduped, order-preserving) so parallel suppressions don't stack + extra user turns. + """ + contents = list(dict.fromkeys(msg["content"] for msg in msgs)) + if contents: + conversation.append({"role": "user", "content": "\n\n".join(contents)}) + + def _tool_name_from_schema(tool: Mapping[str, Any]) -> str: function = tool.get("function") if not isinstance(function, Mapping): diff --git a/studio/backend/tests/test_llama_cpp_tool_loop.py b/studio/backend/tests/test_llama_cpp_tool_loop.py index c3161c57145..ea4edf4ff2d 100644 --- a/studio/backend/tests/test_llama_cpp_tool_loop.py +++ b/studio/backend/tests/test_llama_cpp_tool_loop.py @@ -1061,6 +1061,76 @@ def fake_execute_tool(name, arguments, **_kwargs): ] +def test_same_turn_duplicate_does_not_drop_later_parallel_call(monkeypatch): + # One batch: search(a), search(a) [duplicate], search(b). The duplicate is an + # internal no-op, but the distinct search(b) after it must still run, and the + # no-op nudge must land after the tool results rather than splitting them. + batch = [ + _sse( + { + "tool_calls": [ + { + "index": 0, + "id": "call_a1", + "type": "function", + "function": {"name": "web_search", "arguments": json.dumps({"query": "a"})}, + }, + { + "index": 1, + "id": "call_a2", + "type": "function", + "function": {"name": "web_search", "arguments": json.dumps({"query": "a"})}, + }, + { + "index": 2, + "id": "call_b", + "type": "function", + "function": {"name": "web_search", "arguments": json.dumps({"query": "b"})}, + }, + ] + } + ), + _done(), + ] + final_stream = [_sse({"content": "Final answer."}), _done()] + payloads: list[dict] = [] + backend = _make_backend(monkeypatch, [batch, final_stream], payloads) + + calls: list[dict] = [] + + def fake_execute_tool(name, arguments, **_kwargs): + calls.append(arguments) + return "search-result" + + monkeypatch.setattr("core.inference.tools.execute_tool", fake_execute_tool) + + events = list( + backend.generate_chat_completion_with_tools( + messages = [{"role": "user", "content": "search"}], + tools = [{"type": "function", "function": {"name": "web_search"}}], + max_tool_iterations = 3, + ) + ) + + # Both distinct calls ran; the duplicate did not (old `break` dropped search(b)). + assert calls == [{"query": "a"}, {"query": "b"}] + assert [e.get("tool_call_id") for e in events if e.get("type") == "tool_end"] == [ + "call_a1", + "call_b", + ] + + # The next generation's conversation must be well-formed: the assistant lists + # only the executed calls (no orphan for the duplicate), the two tool results + # follow contiguously, and the no-op nudge lands after them, never between. + conv = payloads[1]["messages"] + asst = next(m for m in conv if m["role"] == "assistant" and m.get("tool_calls")) + assert [tc.get("id") for tc in asst["tool_calls"]] == ["call_a1", "call_b"] + after = conv[conv.index(asst) + 1 :] + assert [m["role"] for m in after[:2]] == ["tool", "tool"] + assert [m.get("tool_call_id") for m in after[:2]] == ["call_a1", "call_b"] + assert after[2]["role"] == "user" # deferred duplicate nudge, after the results + + def test_same_turn_repeated_render_html_does_not_emit_second_provisional_start(monkeypatch): same_turn_render_calls = [ _sse( diff --git a/studio/backend/tests/test_safetensors_tool_loop.py b/studio/backend/tests/test_safetensors_tool_loop.py index eae1a75161e..27ac369b69d 100644 --- a/studio/backend/tests/test_safetensors_tool_loop.py +++ b/studio/backend/tests/test_safetensors_tool_loop.py @@ -2843,6 +2843,57 @@ def fake_single_turn(messages): ] assert len(duplicate_nudges) == 1 + def test_same_turn_duplicate_does_not_drop_later_parallel_call(self): + # Turn 1 runs search(x). Turn 2's batch is [search(x) duplicate, python]: + # the duplicate is a no-op, but python after it must still run, and the + # no-op nudge must land after python's result rather than splitting it. + captured_messages: list[list[dict]] = [] + turns = iter( + [ + ['{"name":"web_search","arguments":{"query":"x"}}'], + [ + '{"name":"web_search","arguments":{"query":"x"}}' + '{"name":"python","arguments":{"code":"print(1)"}}' + ], + ["final"], + ] + ) + + def fake_single_turn(messages, active_tools = None): + captured_messages.append([dict(m) for m in messages]) + chunks = next(turns) + acc = "" + for chunk in chunks: + acc += chunk + yield acc + + exec_fn = FakeExecuteTool(["search-x", "py-result"]) + _collect_events( + run_safetensors_tool_loop( + single_turn = fake_single_turn, + messages = [{"role": "user", "content": "hi"}], + tools = [ + {"type": "function", "function": {"name": "web_search"}}, + {"type": "function", "function": {"name": "python"}}, + ], + execute_tool = exec_fn, + max_tool_iterations = 4, + ) + ) + + # Turn-1 search and turn-2 python both ran; the turn-2 duplicate search did not. + assert exec_fn.calls == [ + ("web_search", {"query": "x"}), + ("python", {"code": "print(1)"}), + ] + + conv = captured_messages[-1] + turn2 = [m for m in conv if m.get("role") == "assistant" and m.get("tool_calls")][-1] + assert [tc["function"]["name"] for tc in turn2["tool_calls"]] == ["python"] + after = conv[conv.index(turn2) + 1 :] + assert after[0]["role"] == "tool" and after[0]["content"] == "py-result" + assert after[1]["role"] == "user" # deferred duplicate nudge, after the result + def test_duplicate_tool_call_internal_noop_allows_distinct_followup_tool(self): captured_messages: list[list[dict]] = [] captured_tool_names: list[list[str]] = [] diff --git a/studio/backend/tests/test_tool_loop_controller.py b/studio/backend/tests/test_tool_loop_controller.py index 0e8ae798af1..74e9b402b4a 100644 --- a/studio/backend/tests/test_tool_loop_controller.py +++ b/studio/backend/tests/test_tool_loop_controller.py @@ -13,6 +13,7 @@ from core.inference.tool_loop_controller import ( ToolLoopController, + append_deferred_nudges, canonical_tool_call_key, coerce_tool_arguments, status_for_tool, @@ -21,6 +22,22 @@ ) +def test_append_deferred_nudges_merges_deduped_into_one_message(): + conversation = [{"role": "assistant", "tool_calls": [1]}, {"role": "tool", "content": "r"}] + nudges = [ + {"role": "user", "content": "duplicate"}, + {"role": "user", "content": "duplicate"}, # dropped: same content + {"role": "user", "content": "disabled foo"}, + ] + append_deferred_nudges(conversation, nudges) + # One user message, after the results, with distinct contents joined. + assert conversation[2:] == [{"role": "user", "content": "duplicate\n\ndisabled foo"}] + # Empty is a no-op. + before = list(conversation) + append_deferred_nudges(conversation, []) + assert conversation == before + + def _tool(name: str) -> dict: return {"type": "function", "function": {"name": name}} From 4a466fe59df6644f0381e7998fed5a9177effc56 Mon Sep 17 00:00:00 2001 From: oobabooga <112222186+oobabooga@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:11:09 -0300 Subject: [PATCH 2/3] Trim redundant no-op deferral comments --- studio/backend/core/inference/llama_cpp.py | 7 ++----- studio/backend/core/inference/safetensors_agentic.py | 7 ++----- studio/backend/core/inference/tool_loop_controller.py | 9 +++------ 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index cc31eaf18c0..4aeedafea53 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -10108,10 +10108,8 @@ def _tool_succeeded(tool_name: str) -> bool: assistant_msg: dict = {"role": "assistant", "content": content_text} assistant_appended = False - # No-op nudges (duplicate / disabled / render_html_repeat) are held - # here and appended after the batch's tool results, so they never split - # an assistant's tool_calls from their results. A no-op no longer aborts - # the batch, so legitimate parallel calls after it still run. + # Collect no-op nudges and flush them after the batch, so a no-op + # doesn't abort it and drop the parallel calls that follow. deferred_noop_msgs: list = [] # The text-path provisional card uses the parser's default id ("call_0"); @@ -10281,7 +10279,6 @@ def _invoke_tool(_output_callback, _decision = decision): if _forced_tool_call_pending: _forced_tool_call_pending = False - # Deliver the deferred no-op nudges after every tool result. append_deferred_nudges(conversation, deferred_noop_msgs) # Close provisional cards not resolved by execution/no-op handling. diff --git a/studio/backend/core/inference/safetensors_agentic.py b/studio/backend/core/inference/safetensors_agentic.py index 4ceec9a6923..43b72110ff0 100644 --- a/studio/backend/core/inference/safetensors_agentic.py +++ b/studio/backend/core/inference/safetensors_agentic.py @@ -1100,10 +1100,8 @@ def _tool_succeeded(tool_name: str) -> bool: assistant_msg: dict = {"role": "assistant", "content": content_text} assistant_appended = False - # No-op nudges (duplicate / disabled / render_html_repeat) are held here and - # appended after the batch's tool results, so they never split an assistant's - # tool_calls from their results. A no-op no longer aborts the batch, so - # legitimate parallel calls after it still run. + # Collect no-op nudges and flush them after the batch, so a no-op doesn't + # abort it and drop the parallel calls that follow. deferred_noop_msgs: list = [] for tc in tool_calls or []: @@ -1249,7 +1247,6 @@ def _invoke_tool(_output_callback, _decision = decision): yield completion.tool_end_event() conversation.append(completion.tool_message()) - # Deliver the deferred no-op nudges after every tool result. append_deferred_nudges(conversation, deferred_noop_msgs) # Clear the status badge before the next turn. diff --git a/studio/backend/core/inference/tool_loop_controller.py b/studio/backend/core/inference/tool_loop_controller.py index fb64836561e..52847ed5e72 100644 --- a/studio/backend/core/inference/tool_loop_controller.py +++ b/studio/backend/core/inference/tool_loop_controller.py @@ -267,13 +267,10 @@ def strip_result_for_model(result: str) -> str: def append_deferred_nudges(conversation: list, msgs: Sequence[dict]) -> None: - """Append a batch's no-op nudges as a single ``role=user`` message after the - tool results. + """Append a batch's no-op nudges as one deduped ``role=user`` message. - Held until here (rather than appended inline) so a no-op never splits an - assistant's ``tool_calls`` from their ``role=tool`` results, and merged into - one message (deduped, order-preserving) so parallel suppressions don't stack - extra user turns. + Deferred to after the batch's tool results so a no-op never splits an + assistant's ``tool_calls`` from their ``role=tool`` results. """ contents = list(dict.fromkeys(msg["content"] for msg in msgs)) if contents: From 230e1ce20d6273d3c08344bd280a9e1bf06f8daf Mon Sep 17 00:00:00 2001 From: oobabooga <112222186+oobabooga@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:27:15 -0300 Subject: [PATCH 3/3] Clarify deferred no-op nudges --- studio/backend/core/inference/tool_loop_controller.py | 9 +++++---- studio/backend/tests/test_llama_cpp_tool_loop.py | 4 ++++ studio/backend/tests/test_safetensors_tool_loop.py | 4 ++++ studio/backend/tests/test_tool_loop_controller.py | 11 ++++++++++- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/studio/backend/core/inference/tool_loop_controller.py b/studio/backend/core/inference/tool_loop_controller.py index 52847ed5e72..f7ed450d118 100644 --- a/studio/backend/core/inference/tool_loop_controller.py +++ b/studio/backend/core/inference/tool_loop_controller.py @@ -288,8 +288,9 @@ def _tool_name_from_schema(tool: Mapping[str, Any]) -> str: def _noop_result(reason: NoopReason, tool_name: str) -> str: if reason == "duplicate": return ( - "The previous tool request was not executed because this exact " - "tool call already completed successfully. Do not repeat the same " + f"One earlier request to call tool '{tool_name}' in this batch was " + "not executed because an identical call had already completed " + "successfully. Do not repeat the same " "tool call. Continue with a different enabled tool if that would " "materially help, or provide the final answer if you have enough " "information." @@ -302,8 +303,8 @@ def _noop_result(reason: NoopReason, tool_name: str) -> str: "the requested final note or answer." ) return ( - f"The previous tool request was not executed because tool " - f"'{tool_name}' is not enabled for this request. Provide the " + f"One earlier request to call tool '{tool_name}' in this batch was " + "not executed because that tool is not enabled for this request. Provide the " "final answer now without calling more tools." ) diff --git a/studio/backend/tests/test_llama_cpp_tool_loop.py b/studio/backend/tests/test_llama_cpp_tool_loop.py index ea4edf4ff2d..bd2c0085890 100644 --- a/studio/backend/tests/test_llama_cpp_tool_loop.py +++ b/studio/backend/tests/test_llama_cpp_tool_loop.py @@ -1129,6 +1129,10 @@ def fake_execute_tool(name, arguments, **_kwargs): assert [m["role"] for m in after[:2]] == ["tool", "tool"] assert [m.get("tool_call_id") for m in after[:2]] == ["call_a1", "call_b"] assert after[2]["role"] == "user" # deferred duplicate nudge, after the results + assert after[2]["content"].startswith( + "One earlier request to call tool 'web_search' in this batch was not executed" + ) + assert "previous tool request" not in after[2]["content"].lower() def test_same_turn_repeated_render_html_does_not_emit_second_provisional_start(monkeypatch): diff --git a/studio/backend/tests/test_safetensors_tool_loop.py b/studio/backend/tests/test_safetensors_tool_loop.py index 27ac369b69d..e3633de2892 100644 --- a/studio/backend/tests/test_safetensors_tool_loop.py +++ b/studio/backend/tests/test_safetensors_tool_loop.py @@ -2893,6 +2893,10 @@ def fake_single_turn(messages, active_tools = None): after = conv[conv.index(turn2) + 1 :] assert after[0]["role"] == "tool" and after[0]["content"] == "py-result" assert after[1]["role"] == "user" # deferred duplicate nudge, after the result + assert after[1]["content"].startswith( + "One earlier request to call tool 'web_search' in this batch was not executed" + ) + assert "previous tool request" not in after[1]["content"].lower() def test_duplicate_tool_call_internal_noop_allows_distinct_followup_tool(self): captured_messages: list[list[dict]] = [] diff --git a/studio/backend/tests/test_tool_loop_controller.py b/studio/backend/tests/test_tool_loop_controller.py index 74e9b402b4a..496c30ac136 100644 --- a/studio/backend/tests/test_tool_loop_controller.py +++ b/studio/backend/tests/test_tool_loop_controller.py @@ -128,6 +128,10 @@ def test_successful_duplicate_is_internal_noop_and_keeps_remaining_tools(): assert not duplicate.should_execute assert not duplicate.emit_visible_events duplicate_nudge = completion.model_message()["content"] + assert duplicate_nudge.startswith( + "One earlier request to call tool 'web_search' in this batch was not executed" + ) + assert "previous tool request" not in duplicate_nudge.lower() assert "already completed successfully" in duplicate_nudge assert "different enabled tool" in duplicate_nudge assert completion.model_message()["role"] == "user" @@ -182,7 +186,12 @@ def test_empty_enabled_tool_list_blocks_all_tool_calls(): assert decision.action == "disabled" assert not decision.emit_visible_events assert completion.model_message()["role"] == "user" - assert "not enabled" in completion.model_message()["content"] + disabled_nudge = completion.model_message()["content"] + assert disabled_nudge.startswith( + "One earlier request to call tool 'web_search' in this batch was not executed" + ) + assert "previous tool request" not in disabled_nudge.lower() + assert "not enabled" in disabled_nudge assert controller.force_final_answer assert controller.active_tools() == []