Studio: Don't re-prompt finished answers in the tool loop (#7505)
* don't re-prompt finished answers in the tool loop * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * keep a separate post-tool reprompt budget and tighten the intent regexes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reset the repeat guard after a tool runs and suppress 'I should call ...' forced stalls * Cover 'must' in forced-retry suppression, keep appended answers, and count RAG autoinject as a prior tool run * Anchor obligation suppression to sentence starts and wire the repeat guard into the safetensors loop * Keep deletions out of restatement and nudge pronoun-free first-step plans * Tighten repeat similarity, anchor subjectless plans, and restore first-step plan forms * Keep first-person plan framing and punctuation-bearing terms out of repeat detection * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep leading term punctuation, accept colon-delimited first steps, and drop invoke/query from suppression * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments on the plan-without-action re-prompt guards * Compare plans by token sequence, suppress subjectless modals, and accept dash-delimited first steps * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: narrow the first-step plan match and make repeat detection content-based Restrict the bare "First, <word>" intent alternative to a pronoun, an explicit plan, or an investigative verb, so ordinal prose ("First place went to Alice") and user-facing advice ("First, install the package") no longer count as a plan without action. Keep punctuation-only tokens in the repeat comparison, so "the value is 5" and "the value is < 5" stay distinct, and compare content-word sequences instead of a similarity ratio: any ratio is length-dependent, so one corrected token in a 54-token plan still scored 0.98 and cost the model its remaining nudge. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tighten comments in the plan-without-action re-prompt path * studio: keep a forced retry that pivots from a plan to an answer The obligation-plan branch discarded the whole turn, so a retry such as "I should call web_search, but the answer is Tokyo." reached the user as nothing at all. Suppress the plan only when nothing follows it: a pivot after the match keeps the output, and _FINAL_ANSWER_SIGNAL now recognises "the answer is" and "to summarise" alongside "answer:". Leaking a plan sentence is cosmetic, dropping an answer is not, so the doubtful case now resolves towards shipping the turn. * studio: keep articles in repeat comparison and exclude missing-answer phrasing Articles are not filler: dropping them made "search for The Who" and "search for Who" compare equal, so a corrected target ended the nudge. _FINAL_ANSWER_SIGNAL matched "the answer is not in the provided context", which announces a missing answer, so the plan behind it shipped as the final response instead of being suppressed. Negated forms are now excluded. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tighten the pivot and final-answer signals, drop filler-insensitive repeats The purpose clause in "call web_search to summarize the results" matched the final-answer signal, so the plan shipped instead of being suppressed; that alternative is gone. A pivot word now has to carry text of its own, since "I should call web_search, though." answers nothing. Repeat detection no longer ignores filler words. No word is reliably filler: dropping them to absorb rewording also absorbed the target ("OK Go" became "Go"). A missed repeat costs one nudge out of the cap; a false one strands the plan unexecuted. * studio: exempt offers of help, and add a measured accuracy floor Offering to help hands control back exactly like the existing "let me know" exemption. On a corpus of real model turns, "I'll do my best to help" and "allow me to assist" close a clarification request and never precede a tool call, but they were read as intent and re-prompted. "help you" keeps its plan reading when an action verb follows it. The new test scores the classifier against 300 turns captured from three local GGUF models, each one a finished answer: the turn called no tool, and three regenerations behind the production nudge produced no tool call either. Over those turns, wasted nudges go from 36 (12.0%) on main to 5 (1.7%), and retries whose text would be discarded from 60 (20.2%) to 1 (0.3%). Until now these patterns were tuned on hand-written example sentences, which cannot show how often the classifier is right on real output. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
This commit is contained in:
parent
7b211c30fe
commit
22493242a3
7 changed files with 1149 additions and 25 deletions
|
|
@ -1487,7 +1487,418 @@ def test_internal_reprompt_attempts_do_not_duplicate_visible_text(monkeypatch):
|
|||
|
||||
content_texts = [event.get("text", "") for event in events if event.get("type") == "content"]
|
||||
assert content_texts == ["I will use render_html now."]
|
||||
# Each retry restates the last, so the loop gives up: initial + 2 re-prompts.
|
||||
assert len(payloads) == 3 < _MAX_REPROMPTS + 1
|
||||
|
||||
|
||||
def test_post_tool_stall_still_nudged_after_a_pre_tool_reprompt(monkeypatch):
|
||||
"""The post-tool nudge has its own budget, so an earlier stall can't spend it."""
|
||||
|
||||
streams = [
|
||||
[_sse({"content": "I will search the web now."}), _done()],
|
||||
[
|
||||
_sse(
|
||||
{
|
||||
"tool_calls": [
|
||||
{
|
||||
"index": 0,
|
||||
"id": "call_first",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"arguments": json.dumps({"query": "red square"}),
|
||||
},
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
_done(),
|
||||
],
|
||||
[_sse({"content": "Let me summarize the results."}), _done()],
|
||||
[_sse({"content": "Final answer: the square is red."}), _done()],
|
||||
]
|
||||
payloads: list[dict] = []
|
||||
backend = _make_backend(monkeypatch, streams, payloads)
|
||||
|
||||
calls: list[tuple[str, dict]] = []
|
||||
|
||||
def fake_execute_tool(name, arguments, **_kwargs):
|
||||
calls.append((name, arguments))
|
||||
return "Search results: red is #f00."
|
||||
|
||||
monkeypatch.setattr("core.inference.tools.execute_tool", fake_execute_tool)
|
||||
|
||||
tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"description": "Search the web.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
},
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
events = list(
|
||||
backend.generate_chat_completion_with_tools(
|
||||
messages = [{"role": "user", "content": "Make a red square."}],
|
||||
tools = tools,
|
||||
max_tool_iterations = 2,
|
||||
)
|
||||
)
|
||||
|
||||
assert len(payloads) == 4
|
||||
assert len(calls) == 1
|
||||
nudges = [
|
||||
message
|
||||
for message in payloads[-1]["messages"]
|
||||
if message.get("role") == "user" and "call web_search now" in message.get("content", "")
|
||||
]
|
||||
assert len(nudges) == 2
|
||||
content_texts = [event.get("text", "") for event in events if event.get("type") == "content"]
|
||||
assert content_texts[-1] == "Final answer: the square is red."
|
||||
|
||||
|
||||
def test_post_tool_reprompt_budget_is_one(monkeypatch):
|
||||
"""The post-tool nudge fires once; a second stall is surrendered as the answer."""
|
||||
|
||||
streams = [
|
||||
[
|
||||
_sse(
|
||||
{
|
||||
"tool_calls": [
|
||||
{
|
||||
"index": 0,
|
||||
"id": "call_first",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"arguments": json.dumps({"query": "red square"}),
|
||||
},
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
_done(),
|
||||
],
|
||||
[_sse({"content": "Let me summarize the results."}), _done()],
|
||||
[_sse({"content": "Now I will check the sources."}), _done()],
|
||||
]
|
||||
payloads: list[dict] = []
|
||||
backend = _make_backend(monkeypatch, streams, payloads)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"core.inference.tools.execute_tool",
|
||||
lambda *_a, **_k: "Search results: red is #f00.",
|
||||
)
|
||||
|
||||
tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"description": "Search the web.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
},
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
list(
|
||||
backend.generate_chat_completion_with_tools(
|
||||
messages = [{"role": "user", "content": "Make a red square."}],
|
||||
tools = tools,
|
||||
max_tool_iterations = 2,
|
||||
)
|
||||
)
|
||||
|
||||
assert len(payloads) == 3
|
||||
|
||||
|
||||
def test_repeat_guard_resets_after_a_tool_runs(monkeypatch):
|
||||
"""A tool execution opens a new phase, so the same intent text is nudged again.
|
||||
|
||||
Without the reset the pre-tool stall text still sits in the repeat tracker and
|
||||
the identical post-tool stall is surrendered as the visible final answer.
|
||||
"""
|
||||
|
||||
stall = "I will search the web now."
|
||||
streams = [
|
||||
[_sse({"content": stall}), _done()],
|
||||
[
|
||||
_sse(
|
||||
{
|
||||
"tool_calls": [
|
||||
{
|
||||
"index": 0,
|
||||
"id": "call_first",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"arguments": json.dumps({"query": "red square"}),
|
||||
},
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
_done(),
|
||||
],
|
||||
[_sse({"content": stall}), _done()],
|
||||
[_sse({"content": "Final answer: the square is red."}), _done()],
|
||||
]
|
||||
payloads: list[dict] = []
|
||||
backend = _make_backend(monkeypatch, streams, payloads)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"core.inference.tools.execute_tool",
|
||||
lambda *_a, **_k: "Search results: red is #f00.",
|
||||
)
|
||||
|
||||
tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"description": "Search the web.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
},
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
events = list(
|
||||
backend.generate_chat_completion_with_tools(
|
||||
messages = [{"role": "user", "content": "Make a red square."}],
|
||||
tools = tools,
|
||||
max_tool_iterations = 2,
|
||||
)
|
||||
)
|
||||
|
||||
assert len(payloads) == 4
|
||||
content_texts = [event.get("text", "") for event in events if event.get("type") == "content"]
|
||||
assert content_texts[-1] == "Final answer: the square is red."
|
||||
|
||||
|
||||
def test_restatement_keeps_deletions_that_change_the_answer():
|
||||
"""A dropped word can invert the meaning, so a subset is not a restatement."""
|
||||
|
||||
from core.inference.tool_call_parser import is_reprompt_restatement
|
||||
from core.inference.llama_cpp import _should_suppress_forced_no_tool_output as suppress
|
||||
|
||||
previous = "Now I think the feature is not supported in version 1."
|
||||
corrected = "Now I think the feature is supported in version 1."
|
||||
assert not is_reprompt_restatement(corrected, previous)
|
||||
assert not suppress(corrected, previous)
|
||||
|
||||
stall = "I'll search for that now."
|
||||
assert is_reprompt_restatement(stall, stall)
|
||||
assert is_reprompt_restatement("Understood. " + stall, "Understood, " + stall)
|
||||
assert not is_reprompt_restatement(stall + " Tokyo.", stall)
|
||||
|
||||
|
||||
def test_forced_turn_suppression_covers_obligation_phrasing():
|
||||
from core.inference.llama_cpp import _should_suppress_forced_no_tool_output as suppress
|
||||
for stall in (
|
||||
"I need to use render_html now",
|
||||
"Need to call web_search",
|
||||
"I will summarize the results now",
|
||||
"I have to run the search first",
|
||||
"I should call web_search now",
|
||||
"I should use render_html now",
|
||||
# Plain modals take a bare infinitive, not the need|have|ought "to" group.
|
||||
"I must call web_search now",
|
||||
"I must use render_html now",
|
||||
"I must run the search first",
|
||||
# Subjectless plans open a new sentence just as often as a new line.
|
||||
"Okay. Need to call web_search now.",
|
||||
"Understood. Going to search now.",
|
||||
# Subjectless modals, not just subjectless semi-modals.
|
||||
"Must call web_search now.",
|
||||
"Should search the web now.",
|
||||
# A missing answer is not a final answer: the plan behind it is still a stall.
|
||||
"I should call web_search because the answer is not in the provided context",
|
||||
"I must run the search since the answer is unknown so far",
|
||||
# A pivot with nothing behind it answers nothing.
|
||||
"I should call web_search, though.",
|
||||
"I need to run the search, but",
|
||||
# A purpose clause is part of the plan, not a summary of results.
|
||||
"I need to call web_search to summarize the results",
|
||||
):
|
||||
assert suppress(stall), f"leaked {stall!r}"
|
||||
|
||||
for answer in (
|
||||
"You need to install the package first.",
|
||||
"The square is red.",
|
||||
"Here is the summary of what I found.",
|
||||
"Run `pip install unsloth` to get started.",
|
||||
"I should mention that the square is red.",
|
||||
# Obligation phrasing mid-sentence is prose that happens to name a tool.
|
||||
"The API I should invoke is foo() because it supports streaming.",
|
||||
"The tool I need to use is documented here.",
|
||||
# "invoke"/"query" read as technical prose far more often than as a stall.
|
||||
"I should invoke foo() because it supports streaming.",
|
||||
"I should query the cache first for a faster path.",
|
||||
"You should call your bank about the charge.",
|
||||
# Second person is the user's obligation, not the model's plan.
|
||||
"You must call your bank about the charge.",
|
||||
"I must admit the square is red.",
|
||||
# A plan that pivots to an answer must ship the answer with it.
|
||||
"I should call web_search, but the answer is Tokyo.",
|
||||
"I need to call web_search. The answer is Tokyo.",
|
||||
"I should call web_search to confirm, but Tokyo is the capital of Japan.",
|
||||
"I must run the search, however the result is already known: 42.",
|
||||
):
|
||||
assert not suppress(answer), f"dropped {answer!r}"
|
||||
|
||||
|
||||
def test_forced_turn_intent_lead_in_needs_a_restatement_to_be_dropped():
|
||||
"""A bare intent match is a stall only when the retry restates the nudge.
|
||||
|
||||
``INTENT_SIGNAL`` fires on lead-ins that introduce a real answer ("Now I
|
||||
have the results. ..."), so matching it alone would discard the answer.
|
||||
"""
|
||||
from core.inference.llama_cpp import _should_suppress_forced_no_tool_output as suppress
|
||||
|
||||
stall = "I will summarize the results now"
|
||||
answer = "Now I have the search results. The capital of Japan is Tokyo."
|
||||
|
||||
# Restating the nudged text is still a stall.
|
||||
assert suppress(stall, stall)
|
||||
assert suppress("Understood. " + stall, "Understood, " + stall)
|
||||
# Progress past the nudged text keeps the answer, lead-in and all.
|
||||
assert not suppress(answer, stall)
|
||||
assert not suppress("Step 3: done. Tokyo is the capital.", stall)
|
||||
# Near-repeat is enough to stop nudging, never enough to drop the turn.
|
||||
assert not suppress(stall + ": Tokyo.", stall)
|
||||
# An obligation plan is a stall on its own, no previous text needed.
|
||||
assert suppress("I must call web_search now", answer)
|
||||
|
||||
|
||||
def test_forced_turn_answer_with_an_intent_lead_in_survives_after_a_tool(monkeypatch):
|
||||
"""The post-tool retry answers behind a lead-in; the answer must still ship.
|
||||
|
||||
The nudge budget is spent, so the reply lands on the suppression branch.
|
||||
``INTENT_SIGNAL`` matches its "Now I ..." opener, and dropping it on that
|
||||
alone left the user with the stall and no answer at all.
|
||||
"""
|
||||
|
||||
answer = "Now I have the results. The capital of Japan is Tokyo."
|
||||
streams = [
|
||||
[
|
||||
_sse(
|
||||
{
|
||||
"tool_calls": [
|
||||
{
|
||||
"index": 0,
|
||||
"id": "call_first",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"arguments": json.dumps({"query": "capital of Japan"}),
|
||||
},
|
||||
}
|
||||
]
|
||||
}
|
||||
),
|
||||
_done(),
|
||||
],
|
||||
[_sse({"content": "Let me summarize what I found."}), _done()],
|
||||
[_sse({"content": answer}), _done()],
|
||||
]
|
||||
payloads: list[dict] = []
|
||||
backend = _make_backend(monkeypatch, streams, payloads)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"core.inference.tools.execute_tool",
|
||||
lambda *_a, **_k: "Search results: Tokyo.",
|
||||
)
|
||||
|
||||
tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"description": "Search the web.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
},
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
events = list(
|
||||
backend.generate_chat_completion_with_tools(
|
||||
messages = [{"role": "user", "content": "What is the capital of Japan?"}],
|
||||
tools = tools,
|
||||
max_tool_iterations = 2,
|
||||
)
|
||||
)
|
||||
|
||||
assert len(payloads) == 3
|
||||
content_texts = [event.get("text", "") for event in events if event.get("type") == "content"]
|
||||
assert content_texts[-1] == answer
|
||||
|
||||
|
||||
def test_forced_turn_answer_with_an_intent_lead_in_survives_pre_tool(monkeypatch):
|
||||
"""Same guarantee once the pre-tool nudge budget is spent on distinct stalls."""
|
||||
|
||||
answer = "Now I see the data clearly. Tokyo is the capital."
|
||||
streams = [
|
||||
[_sse({"content": text}), _done()]
|
||||
for text in (
|
||||
"I will look that up for you.",
|
||||
"Now I have the search results. The capital of Japan is Tokyo.",
|
||||
"Now I can confirm it. Japan's capital city is Tokyo.",
|
||||
answer,
|
||||
)
|
||||
]
|
||||
payloads: list[dict] = []
|
||||
backend = _make_backend(monkeypatch, streams, payloads)
|
||||
|
||||
def fake_execute_tool(name, arguments, **_kwargs):
|
||||
raise AssertionError(f"unexpected tool execution: {name} {arguments}")
|
||||
|
||||
monkeypatch.setattr("core.inference.tools.execute_tool", fake_execute_tool)
|
||||
|
||||
tools = [
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "web_search",
|
||||
"description": "Search the web.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string"}},
|
||||
"required": ["query"],
|
||||
},
|
||||
},
|
||||
}
|
||||
]
|
||||
|
||||
events = list(
|
||||
backend.generate_chat_completion_with_tools(
|
||||
messages = [{"role": "user", "content": "What is the capital of Japan?"}],
|
||||
tools = tools,
|
||||
max_tool_iterations = 2,
|
||||
)
|
||||
)
|
||||
|
||||
# Initial turn plus the three pre-tool nudges.
|
||||
assert len(payloads) == _MAX_REPROMPTS + 1
|
||||
content_texts = [event.get("text", "") for event in events if event.get("type") == "content"]
|
||||
assert content_texts[-1] == answer
|
||||
|
||||
|
||||
def test_forced_reprompt_plain_final_answer_is_visible(monkeypatch):
|
||||
|
|
@ -2084,6 +2495,51 @@ def test_confirm_tool_calls_skips_gguf_rag_autoinject(monkeypatch):
|
|||
assert any(event.get("type") == "content" and event.get("text") == "Done." for event in events)
|
||||
|
||||
|
||||
def test_rag_autoinject_counts_as_a_prior_tool_execution(monkeypatch):
|
||||
"""Autoinjected retrieval runs before the controller, so history stays empty.
|
||||
|
||||
Without counting it the turn reads as pre-tool and gets the full re-prompt
|
||||
budget, repeating the expensive retrieval the post-tool cap exists to stop.
|
||||
"""
|
||||
|
||||
stall = "I will summarize the retrieved passages now."
|
||||
streams = [
|
||||
[_sse({"content": stall}), _done()],
|
||||
[_sse({"content": "Still working on the summary."}), _done()],
|
||||
[_sse({"content": "Final answer: the passages describe Tokyo."}), _done()],
|
||||
]
|
||||
payloads: list[dict] = []
|
||||
backend = _make_backend(monkeypatch, streams, payloads)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"core.inference.tools.build_rag_autoinject",
|
||||
lambda *_a, **_k: {
|
||||
"events": [],
|
||||
"messages": [{"role": "user", "content": "Retrieved passage: Tokyo."}],
|
||||
},
|
||||
)
|
||||
|
||||
events = list(
|
||||
backend.generate_chat_completion_with_tools(
|
||||
messages = [{"role": "user", "content": "summarize the docs"}],
|
||||
tools = [{"type": "function", "function": {"name": "search_knowledge_base"}}],
|
||||
max_tool_iterations = 2,
|
||||
rag_scope = {"thread_id": "t1"},
|
||||
)
|
||||
)
|
||||
|
||||
# Initial turn plus one retry; read as pre-tool it would spend the full budget.
|
||||
assert len(payloads) == 2, payloads
|
||||
nudges = [
|
||||
message
|
||||
for message in payloads[-1]["messages"]
|
||||
if message.get("role") == "user"
|
||||
and "call search_knowledge_base now" in message.get("content", "")
|
||||
]
|
||||
assert len(nudges) == 1, nudges
|
||||
assert events
|
||||
|
||||
|
||||
def test_confirm_tool_calls_deny_skips_gguf_tool_and_retry_can_execute(monkeypatch):
|
||||
same_call = _structured_tool_call("python", {"code": "print(1)"}, "call_py")
|
||||
streams = [
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue