Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions studio/backend/core/inference/llama_cpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
)
from core.inference.tool_loop_controller import (
ToolLoopController,
append_deferred_nudges,
tool_event_provenance,
)
from state.tool_approvals import (
Expand Down Expand Up @@ -10107,6 +10108,9 @@ def _tool_succeeded(tool_name: str) -> bool:

assistant_msg: dict = {"role": "assistant", "content": content_text}
assistant_appended = False
# 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");
# a Mistral-style call carries its own id and would open a duplicate. Reuse
Expand Down Expand Up @@ -10149,14 +10153,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()]
Expand Down Expand Up @@ -10275,6 +10279,8 @@ def _invoke_tool(_output_callback, _decision = decision):
if _forced_tool_call_pending:
_forced_tool_call_pending = False

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:
Expand Down
10 changes: 8 additions & 2 deletions studio/backend/core/inference/safetensors_agentic.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
)
from core.inference.tool_loop_controller import (
ToolLoopController,
append_deferred_nudges,
coerce_tool_arguments,
status_for_tool,
tool_event_provenance,
Expand Down Expand Up @@ -1099,6 +1100,9 @@ def _tool_succeeded(tool_name: str) -> bool:

assistant_msg: dict = {"role": "assistant", "content": content_text}
assistant_appended = False
# 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 []:
func = tc.get("function", {}) or {}
Expand Down Expand Up @@ -1127,12 +1131,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()]
Expand Down Expand Up @@ -1243,6 +1247,8 @@ def _invoke_tool(_output_callback, _decision = decision):
yield completion.tool_end_event()
conversation.append(completion.tool_message())

append_deferred_nudges(conversation, deferred_noop_msgs)

# Clear the status badge before the next turn.
yield {"type": "status", "text": ""}

Expand Down
20 changes: 16 additions & 4 deletions studio/backend/core/inference/tool_loop_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,17 @@ 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 one deduped ``role=user`` message.

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:
conversation.append({"role": "user", "content": "\n\n".join(contents)})
Comment thread
oobabooga marked this conversation as resolved.


def _tool_name_from_schema(tool: Mapping[str, Any]) -> str:
function = tool.get("function")
if not isinstance(function, Mapping):
Expand All @@ -277,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."
Expand All @@ -291,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."
)

Expand Down
74 changes: 74 additions & 0 deletions studio/backend/tests/test_llama_cpp_tool_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,80 @@ 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
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):
same_turn_render_calls = [
_sse(
Expand Down
55 changes: 55 additions & 0 deletions studio/backend/tests/test_safetensors_tool_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -2843,6 +2843,61 @@ 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(
[
['<tool_call>{"name":"web_search","arguments":{"query":"x"}}</tool_call>'],
[
'<tool_call>{"name":"web_search","arguments":{"query":"x"}}</tool_call>'
'<tool_call>{"name":"python","arguments":{"code":"print(1)"}}</tool_call>'
],
["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
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]] = []
captured_tool_names: list[list[str]] = []
Expand Down
28 changes: 27 additions & 1 deletion studio/backend/tests/test_tool_loop_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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}}

Expand Down Expand Up @@ -111,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"
Expand Down Expand Up @@ -165,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() == []

Expand Down
Loading