-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Studio: Don't re-prompt finished answers in the tool loop #7505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
a1c3be8
2e51d87
6e7fcc0
4223f03
2253126
8f384e9
21b96b2
f161f67
f67c5b1
4824e33
b8a86ed
2c667f7
7be4415
c66c6de
d6073ce
fbe165e
2e3277b
dcf0de4
88784c0
eb59611
611aa05
4ca5e9e
9bf66ee
20fe689
a105c44
a9e3444
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,6 +92,8 @@ | |
| from core.inference.tool_call_parser import ( | ||
| MAX_ACT_REPROMPTS as _MAX_REPROMPTS, | ||
| REPROMPT_MAX_CHARS as _REPROMPT_MAX_CHARS, | ||
| is_reprompt_repeat as _is_reprompt_repeat, | ||
| is_reprompt_restatement as _is_reprompt_restatement, | ||
| is_short_intent_without_action as _is_short_intent_without_action, | ||
| reprompt_to_act_message as _reprompt_to_act_message, | ||
| ) | ||
|
|
@@ -339,12 +341,28 @@ def _finalize_reasoning_only_cumulative( | |
| # loop). Structured delta.tool_calls are grammar-bounded by llama-server; text | ||
| # parsed from content is not, so one runaway turn could fan out unbounded. | ||
| _MAX_TOOL_CALLS_PER_TURN = 8 | ||
| _FORCED_REPEAT_PLAN_SIGNAL = re.compile( | ||
| r"\b(?:i\s+will|i'll|let\s+me|going\s+to|need\s+to|call|use|run|search|fetch|render)\b", | ||
| re.I, | ||
| # Obligation phrasing INTENT_SIGNAL leaves alone ("I need to call ..."), paired with | ||
| # an action verb. Sentence-anchored: mid-sentence the same words are prose that names | ||
| # a tool ("The API I should invoke is foo() because ..."), and suppressing that loses | ||
| # a real answer. "should"/"must" sit outside the need|have|ought group because they | ||
| # take a bare infinitive. "invoke"/"query" stay out of the verb list: they read as | ||
| # technical prose far more often than as a stall. | ||
| _FORCED_PLAN_INTENT = re.compile( | ||
| r"(?:^|[.!?]\s+)\s*" | ||
| r"(?:i\s+(?:(?:need|have|ought)\s+to|should|must)|need\s+to|going\s+to|must|should)" | ||
| r"\s+(?:\w+\s+){0,2}?(?:call|use|run|search|fetch|render)\b", | ||
| re.I | re.M, | ||
| ) | ||
| _FINAL_ANSWER_SIGNAL = re.compile( | ||
| r"\b(?:final\s+answer|answer\s*:|here\s+is|here's|in\s+summary|result\s*:)\b", | ||
| r"\b(?:final\s+answer|answer\s*:|(?:the\s+)?answer\s+is|here\s+is|here's" | ||
| r"|in\s+summary|to\s+summari[sz]e|result\s*:)\b", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Refine this signal so phrases such as Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, and this one is mine: I added "the answer is" to Fixed in 611aa05 with a negative lookahead on that alternative (not / unavailable / unknown / unclear / missing). The pivot cases from last round still ship, and two missing-answer phrasings are pinned as positives in |
||
| re.I, | ||
| ) | ||
| # A plan that pivots ("I should call web_search, but Tokyo is the capital") has an | ||
| # answer attached, so the turn must survive. Leaking a plan sentence is cosmetic; | ||
| # dropping an answer is not, so the doubtful case keeps the output. | ||
| _ANSWER_PIVOT = re.compile( | ||
| r"\b(?:but|however|although|though|that\s+said|in\s+the\s+meantime|meanwhile)\b", | ||
| re.I, | ||
| ) | ||
|
|
||
|
|
@@ -436,14 +454,28 @@ def _held_rehearsal_tail_len(text: str, active_tools: list[dict]) -> int: | |
| return len(tail) if tail and _is_rehearsal_prefix(tail, active_tools) else 0 | ||
|
|
||
|
|
||
| def _should_suppress_forced_no_tool_output(text: str) -> bool: | ||
| """Suppress only repeated forced-turn planning text, not final answers.""" | ||
| def _should_suppress_forced_no_tool_output(text: str, previous: str = "") -> bool: | ||
| """Suppress only repeated forced-turn planning text, not final answers. | ||
|
|
||
| ``previous`` is the stall text that triggered the nudge, so a retry that | ||
| moved on can be told from one that just said the same thing again. | ||
| """ | ||
| stripped = text.strip() | ||
| if not stripped or len(stripped) >= _REPROMPT_MAX_CHARS: | ||
| return False | ||
| if _FINAL_ANSWER_SIGNAL.search(stripped): | ||
| return False | ||
| return _FORCED_REPEAT_PLAN_SIGNAL.search(stripped) is not None | ||
| plan = _FORCED_PLAN_INTENT.search(stripped) | ||
| if plan is not None: | ||
| # Only the plan itself is safe to drop; anything the turn pivots to after it | ||
| # is the answer the user is waiting for. | ||
| return _ANSWER_PIVOT.search(stripped[plan.end() :]) is None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On a forced GGUF retry, obligation plans that end with a pivot word but do not answer, such as Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and fixed in 9bf66ee. The pivot now has to carry text of its own: the pattern requires at least two more word tokens after the marker. That only ever adds suppression where there is demonstrably nothing after the pivot, so it does not weaken last round's fix. Both new negatives ("I should call web_search, though.", "I need to run the search, but") are pinned alongside the pivot cases that must still ship. |
||
| if not _is_short_intent_without_action(stripped): | ||
| return False | ||
| # INTENT_SIGNAL also fires on lead-ins to a real answer ("Now I have the results. | ||
| # The capital is Tokyo."), so a bare intent match is a stall only when the retry | ||
| # adds nothing. No ``previous`` keeps the standalone "is this a stall?" contract. | ||
| return not previous or _is_reprompt_restatement(stripped, previous) | ||
|
Comment on lines
+498
to
+503
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do not preserve every non-identical direct-intent retry as progress. For example, after nudging Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Closing this one: it reverses a finding from an earlier round of this same review, and the direction it asks for is the more damaging one. The restatement gate on this branch exists because "I will review the results now" and "Now I have the results. Tokyo is the capital." are not separable by pattern, which is why the branch asks whether the retry moved past the nudged text instead. Obligation plans keep their unconditional path; direct-intent plans do not, deliberately. A leaked plan sentence is cosmetic; a dropped answer is not. |
||
|
|
||
|
|
||
| # ── Pre-compiled patterns for GGUF shard detection ─────────── | ||
|
|
@@ -11241,14 +11273,18 @@ def _tool_succeeded(tool_name: str) -> bool: | |
| # direct answer ("4", "Hello!") won't match. Pattern shared with the | ||
| # safetensors loop (tool_call_parser.INTENT_SIGNAL). | ||
| _reprompt_count = 0 | ||
| # Budgeted apart from _reprompt_count so a pre-tool nudge can't spend it. | ||
| _post_tool_reprompts = 0 | ||
| # Text that triggered the last nudge; if the retry restates it, stop. | ||
| _last_reprompt_text = "" | ||
| # Gates ``max_tool_iterations`` on real tool turns (not the enlarged range) so reserved | ||
| # re-prompt slots don't extend the budget. Mirrors the safetensors guard. | ||
| _tool_iters_done = 0 | ||
| _forced_tool_call_pending = False | ||
|
|
||
| # Reserve extra iterations for re-prompts so they don't consume the | ||
| # caller's tool-call budget; only when tool iterations are allowed. | ||
| _extra = _MAX_REPROMPTS if max_tool_iterations > 0 else 0 | ||
| _extra = _MAX_REPROMPTS + 1 if max_tool_iterations > 0 else 0 | ||
| for iteration in range(max_tool_iterations + _extra): | ||
| if cancel_event is not None and cancel_event.is_set(): | ||
| return | ||
|
|
@@ -11874,12 +11910,10 @@ def _tool_succeeded(tool_name: str) -> bool: | |
| ) | ||
| if not _safety_tc: | ||
| # ── Re-prompt on plan-without-action ── | ||
| # If the model described its intent (forward-looking | ||
| # language) without calling a tool, nudge it to act. | ||
| # Fires at most once per request, only on short | ||
| # responses with intent signals -- "4" or "Hello!" | ||
| # won't trigger it. Use content if available, else | ||
| # fall back to reasoning text (reasoning-only stalls). | ||
| # Intent described without a tool call: nudge it to act. Up | ||
| # to _MAX_REPROMPTS times, only on short responses with intent | ||
| # signals -- "4" or "Hello!" won't trigger it. Uses content, | ||
| # else reasoning text (reasoning-only stalls). | ||
| _stripped = content_accum.strip() | ||
| if not _stripped: | ||
| _stripped = reasoning_accum.strip() | ||
|
|
@@ -11889,18 +11923,33 @@ def _tool_succeeded(tool_name: str) -> bool: | |
| r"(?i)\brender[_\s-]?html\b", | ||
| _stripped, | ||
| ) | ||
| # A post-tool stall still deserves a nudge, but each retry | ||
| # re-runs tools, so allow only one. RAG autoinject never lands | ||
| # in history, so _auto keeps a doc-grounded turn from reading | ||
| # as pre-tool (mirrors safetensors rag_autoinjected). | ||
| _already_acted = bool(_auto) or any( | ||
| record.executed for record in tool_controller.history | ||
| ) | ||
| if _already_acted: | ||
| _reprompt_used, _reprompt_cap = _post_tool_reprompts, 1 | ||
| else: | ||
| _reprompt_used, _reprompt_cap = _reprompt_count, _MAX_REPROMPTS | ||
| # None keeps the default-on re-prompt; False disables it. | ||
| if ( | ||
| auto_heal_tool_calls | ||
| and (nudge_tool_calls is None or nudge_tool_calls) | ||
| and active_tools | ||
| and not _render_html_already_done_intent | ||
| and _reprompt_count < _MAX_REPROMPTS | ||
| and _reprompt_used < _reprompt_cap | ||
| and not _is_reprompt_repeat(_stripped, _last_reprompt_text) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a request first hits a pre-tool plan-without-action, Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed, and fixed in 2253126. Reproduced on the head commit: with
|
||
| and _is_short_intent_without_action(_stripped) | ||
| ): | ||
| _reprompt_count += 1 | ||
| if _already_acted: | ||
| _post_tool_reprompts += 1 | ||
| _last_reprompt_text = _stripped | ||
| logger.info( | ||
| f"Re-prompt {_reprompt_count}/{_MAX_REPROMPTS}: " | ||
| f"Re-prompt {_reprompt_used + 1}/{_reprompt_cap}: " | ||
| f"model responded without calling tools " | ||
| f"({len(_stripped)} chars)" | ||
| ) | ||
|
|
@@ -11935,7 +11984,10 @@ def _tool_succeeded(tool_name: str) -> bool: | |
|
|
||
| if _forced_tool_call_pending: | ||
| _forced_tool_call_pending = False | ||
| if not _should_suppress_forced_no_tool_output(_stripped): | ||
| if not _should_suppress_forced_no_tool_output( | ||
| _stripped, | ||
| _last_reprompt_text, | ||
| ): | ||
| if cumulative_display: | ||
| forced_visible_text = _strip_tool_markup( | ||
| cumulative_display, | ||
|
|
@@ -12252,6 +12304,10 @@ def _invoke_tool(_output_callback, _decision = decision): | |
| _kb_search_count += 1 | ||
| completion = tool_controller.record_result(decision, result) | ||
| resolved_provisional_tool_call_ids.add(decision.tool_call_id) | ||
| # A real execution opens the post-tool phase; carrying the pre-tool | ||
| # stall text over would read the same sentence as a repeat and | ||
| # swallow the one post-tool nudge. | ||
| _last_reprompt_text = "" | ||
| # A tool ran this turn, so it counts against the caller's budget. | ||
| _turn_executed_real_tool = True | ||
| yield completion.tool_end_event() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ | |
| RAG_MAX_SEARCHES_PER_TURN, | ||
| RAG_SEARCH_CAP_NUDGE, | ||
| TOOL_XML_SIGNALS, | ||
| is_reprompt_repeat, | ||
| is_short_intent_without_action, | ||
| parse_tool_calls_from_text, | ||
| reprompt_to_act_message, | ||
|
|
@@ -559,6 +560,8 @@ def run_safetensors_tool_loop( | |
| final_attempt_done = False | ||
| next_call_id = 0 | ||
| reprompt_count = 0 | ||
| # Text that triggered the last nudge; if the retry restates it, stop (GGUF parity). | ||
| last_reprompt_text = "" | ||
| # A denied tool confirmation must not be answered with a plan-without-action | ||
| # re-prompt (which would raise the confirmation gate again). | ||
| tool_denied = False | ||
|
|
@@ -1009,9 +1012,11 @@ def _tool_succeeded(tool_name: str) -> bool: | |
| and not rag_autoinjected | ||
| and not tool_denied | ||
| and not any(record.executed for record in tool_controller.history) | ||
| and not is_reprompt_repeat(intent_text, last_reprompt_text) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a safetensors or MLX request has executed a tool and the next turn says something like Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not taking this one, at least not as part of this PR. The GGUF post-tool nudge is only safe because that loop can hide the retried turn: Porting it properly means adding a forced-retry flag plus buffering of visible output until the turn is classified, which is a state-machine change rather than a condition change, and larger than what this PR is doing. The repeat guard was worth wiring across because it is stateless and needs no buffering. Happy to see it as a follow-up, and the divergence is worth a comment in the loop so it is not read as an oversight. |
||
| and is_short_intent_without_action(intent_text) | ||
| ): | ||
| reprompt_count += 1 | ||
| last_reprompt_text = intent_text | ||
| logger.info( | ||
| "Safetensors re-prompt %d/%d: model responded without " | ||
| "calling tools (%d chars)", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -166,15 +166,31 @@ | |
|
|
||
|
|
||
| # ── Plan-without-action re-prompt (shared by the GGUF and safetensors loops) ── | ||
| # Verbs naming work this turn. Narrow on purpose: "install"/"add"/"open" belong to | ||
| # advice for the user, which must not be re-prompted. | ||
| _ACTION_VERB = ( | ||
| r"(?:search|check|look|find|fetch|get|call|use|run|query|invoke|analy[sz]e" | ||
| r"|review|inspect|read|gather|examine|retrieve|browse|consult|verify" | ||
| r"|confirm|compute|calculate|determine|identify|render)" | ||
| ) | ||
| # Forward-looking intent: the model says what it *will* do, not a final answer. | ||
| INTENT_SIGNAL = re.compile( | ||
| r"(?i)(" | ||
| # Direct intent ("I'll", "Let me"); lookahead drops negated forms | ||
| # ("I will not") so a refusal does not re-prompt. | ||
| r"\b(i['\u2019](ll|m going to|m gonna)|i am (going to|gonna)|i will|i shall|let me|allow me)\b(?!\s+(?:not|never)\b)" | ||
| r"(?im)(" | ||
| # Direct intent ("I'll"); lookahead drops negated forms ("I will not"). | ||
| r"\b(i['\u2019](ll|m going to|m gonna)|i am (going to|gonna)|i will|i shall)\b(?!\s+(?:not|never)\b)" | ||
| r"|" | ||
| # "let me know" hands control back rather than announcing an action. | ||
| r"\b(?:let me|allow me)\b(?!\s+(?:not|never|know)\b)" | ||
| r"|" | ||
| # Step/plan framing: "First ...", "Step 1:", "Here's my plan" | ||
| r"\b(?:first\b|step \d+:?|here['\u2019]?s (?:my |the |a )?(?:plan|approach))" | ||
| # Step/plan framing. "first" must open a sentence and be followed by a plan | ||
| # (pronoun, "my/our plan", or an action verb); otherwise it is prose ("The | ||
| # first line is blank.", "First place went to Alice") or advice to the user. | ||
| r"(?:^|[.!?]\s+)\s*(?:the\s+)?first\s+step\b" | ||
| r"|(?:^|[.!?]\s+)\s*first\s*[,:–—-]?\s+(?:my|our)\s+(?:plan|approach|step)\b" | ||
| r"|(?:^|[.!?]\s+)\s*first\s*[,:–—-]?\s+(?:i|we|let['’]?s|let us)\b" | ||
| r"|(?:^|[.!?]\s+)\s*first\s*[,:–—-]?\s+" + _ACTION_VERB + r"\b" | ||
|
Comment on lines
+197
to
+200
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Allow common Markdown and numbered-list prefixes before this sentence-opening pattern. Responses such as Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Closing this one. The premise does not hold: I checked the previous signal at d6073ce and On the gap itself: this PR narrows over-nudging, and adding new nudge triggers widens it in the opposite direction. Every widening of this regex so far has produced a false positive in the following round, and the cost of the miss is a leaked plan sentence rather than a lost answer. Worth doing separately with its own tests if it shows up in practice, not here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the model formats an imperative plan as a Markdown item such as Useful? React with 👍 / 👎. |
||
| r"|" | ||
| r"\b(?:step \d+:?|here['\u2019]?s (?:my |the |a )?(?:plan|approach))" | ||
| r"|" | ||
| r"\b(?:now i|next i)\b" | ||
| r")" | ||
|
|
@@ -190,6 +206,55 @@ def is_short_intent_without_action(text: str) -> bool: | |
| return 0 < len(stripped) < REPROMPT_MAX_CHARS and INTENT_SIGNAL.search(stripped) is not None | ||
|
|
||
|
|
||
| # Leading marks are kept unless they are quotes or brackets, so ".NET" survives; | ||
| # stripping all non-word chars would collapse "C++" and "C#" to the same token. | ||
| _REPEAT_TRAIL_PUNCT = ".,;:!?\"'`()[]{}<>‘’“”" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a forced GGUF retry corrects an intent-like response using a standalone comparison operator, this punctuation set removes the correction entirely. For example, “Now I think the value is 5” and “Now I think the value is < 5” both normalize to the same text because the standalone Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reproduced and fixed in 2e3277b. The normaliser now keeps the original token whenever stripping would empty it, which covers the whole class ("<", ">", "->", "..."), not just the comparison operators. Pinned in |
||
| _REPEAT_LEAD_PUNCT = "\"'`([{‘“" | ||
| # Wording that can drift between two attempts without the attempt changing. | ||
| _REPEAT_FILLER = frozenset( | ||
| {"a", "an", "the", "now", "then", "just", "so", "ok", "okay", "please", "also", "again"} | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do not remove filler words from every position in the response: when a retry corrects a search target from Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed and fixed in 611aa05. Articles are out of the filler set entirely: whether "the" changes the referent is not something this comparison can judge, and the failure direction of guessing wrong is losing the retry. The set is now discourse fillers only (now/then/just/so/ok/okay/please/also/again), which is what motivated it in the first place ("I will now summarize the findings" vs "I will summarize the findings now" still compares equal). Pinned in |
||
|
|
||
|
|
||
| def _normalize_for_repeat(text: str) -> str: | ||
| words = [] | ||
| for word in text.lower().split(): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a retry changes only a case-sensitive target, such as correcting Useful? React with 👍 / 👎. |
||
| stripped = word.rstrip(_REPEAT_TRAIL_PUNCT).lstrip(_REPEAT_LEAD_PUNCT) | ||
| # Keep marks-only tokens: "value is 5" and "value is < 5" differ, and | ||
| # dropping the "<" threw the corrected attempt away. | ||
| words.append(stripped or word) | ||
| return " ".join(words) | ||
|
|
||
|
|
||
| # A nudge that just gets the same answer back has not worked, so stop there. | ||
| def is_reprompt_repeat(text: str, previous: str) -> bool: | ||
| if not previous: | ||
| return False | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a safetensors/MLX model repeats the same pre-tool intent after a nudge, Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair, and fixed in 21b96b2. The PR added the shared helper but wired only one consumer, so the same repeated stall cost three generations on safetensors/MLX and one on GGUF.
One existing test needed updating: |
||
| a, b = _normalize_for_repeat(text), _normalize_for_repeat(previous) | ||
| if not a or not b: | ||
| return False | ||
| if a == b: | ||
| return True | ||
| ta = [word for word in a.split() if word not in _REPEAT_FILLER] | ||
| tb = [word for word in b.split() if word not in _REPEAT_FILLER] | ||
| if len(ta) < 4 or len(tb) < 4: | ||
| return False # too short for overlap to mean anything | ||
| # Ordered content-word sequence, not a similarity ratio: any ratio is length | ||
| # dependent (one corrected token in a 50-word plan still scored 0.98), and order | ||
| # matters since "cats not dogs" and "dogs not cats" share every word. | ||
| return ta == tb | ||
|
|
||
|
|
||
| # Stricter sibling of ``is_reprompt_repeat``: exact equality, since this discards the | ||
| # turn. An appended answer would clear the fuzzy bar, and deletions flip meaning | ||
| # ("is not supported" -> "is supported"). | ||
| def is_reprompt_restatement(text: str, previous: str) -> bool: | ||
| if not previous: | ||
| return False | ||
| a, b = _normalize_for_repeat(text), _normalize_for_repeat(previous) | ||
| return bool(a) and a == b | ||
|
|
||
|
|
||
| def reprompt_to_act_message(tool_hint: str) -> str: | ||
| """The user message appended when re-prompting a plan-without-action turn.""" | ||
| return ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a forced GGUF retry, obligation stalls such as
I need to look up the release notes noworNeed to check the docsno longer match this narrowed forced-plan pattern because the action verb must be one ofcall/use/run/search/fetch/render. Since the shared intent signal also leavesneed to ...phrasing alone,_should_suppress_forced_no_tool_outputreturns false and exposes the hidden retry as the final answer; cover the same unambiguous work verbs used by the plan nudge here too.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing this one. "no longer match" is not accurate:
call/use/run/search/fetch/renderhas been the verb list on this pattern since it was introduced, andlook/checkwere never in it, so nothing regressed here.More importantly, the direction is the one you asked me to reverse earlier in this review.
invokeandquerywere removed from this list because they read as technical prose far more often than as a stall, and "I should invoke foo() because it supports streaming" was being discarded.lookandcheckbehave the same way: "I need to check the documentation, it says Tokyo" has no pivot marker to save it, so widening the list here drops answers, which your own P1 two rounds ago established is the failure mode that matters.The asymmetry is deliberate.
_ACTION_VERBin the shared intent signal can afford a broad list because a false positive there only spends a nudge. This pattern discards the turn, so it stays narrow.