Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a1c3be8
don't re-prompt finished answers in the tool loop
NilayYadav Jul 26, 2026
2e51d87
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 27, 2026
6e7fcc0
keep a separate post-tool reprompt budget and tighten the intent regexes
NilayYadav Jul 28, 2026
4223f03
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2026
2253126
Reset the repeat guard after a tool runs and suppress 'I should call …
danielhanchen Jul 28, 2026
8f384e9
Cover 'must' in forced-retry suppression, keep appended answers, and …
danielhanchen Jul 28, 2026
21b96b2
Anchor obligation suppression to sentence starts and wire the repeat …
danielhanchen Jul 28, 2026
f161f67
Keep deletions out of restatement and nudge pronoun-free first-step p…
danielhanchen Jul 28, 2026
f67c5b1
Tighten repeat similarity, anchor subjectless plans, and restore firs…
danielhanchen Jul 28, 2026
4824e33
Keep first-person plan framing and punctuation-bearing terms out of r…
danielhanchen Jul 28, 2026
b8a86ed
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2026
2c667f7
Keep leading term punctuation, accept colon-delimited first steps, an…
danielhanchen Jul 28, 2026
7be4415
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2026
c66c6de
Tighten comments on the plan-without-action re-prompt guards
danielhanchen Jul 28, 2026
d6073ce
Compare plans by token sequence, suppress subjectless modals, and acc…
danielhanchen Jul 28, 2026
fbe165e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2026
2e3277b
studio: narrow the first-step plan match and make repeat detection co…
danielhanchen Jul 28, 2026
dcf0de4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2026
88784c0
studio: tighten comments in the plan-without-action re-prompt path
danielhanchen Jul 28, 2026
eb59611
studio: keep a forced retry that pivots from a plan to an answer
danielhanchen Jul 28, 2026
611aa05
studio: keep articles in repeat comparison and exclude missing-answer…
danielhanchen Jul 28, 2026
4ca5e9e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 28, 2026
9bf66ee
studio: tighten the pivot and final-answer signals, drop filler-insen…
danielhanchen Jul 28, 2026
20fe689
Merge remote-tracking branch 'origin/main' into r7505
danielhanchen Jul 29, 2026
a105c44
studio: exempt offers of help, and add a measured accuracy floor
danielhanchen Jul 29, 2026
a9e3444
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 29, 2026
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
94 changes: 77 additions & 17 deletions studio/backend/core/inference/llama_cpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -339,12 +341,32 @@ 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",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include look-up plans in forced retry suppression

On a forced GGUF retry, obligation stalls such as I need to look up the release notes now or Need to check the docs no longer match this narrowed forced-plan pattern because the action verb must be one of call/use/run/search/fetch/render. Since the shared intent signal also leaves need to ... phrasing alone, _should_suppress_forced_no_tool_output returns 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 👍 / 👎.

Copy link
Copy Markdown
Member

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/render has been the verb list on this pattern since it was introduced, and look/check were never in it, so nothing regressed here.

More importantly, the direction is the one you asked me to reverse earlier in this review. invoke and query were 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. look and check behave 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_VERB in 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.

re.I | re.M,
)
# "the answer is not in the context" announces a *missing* answer, so the negated
# forms are excluded or the plan behind them would ship as the final response.
_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*:|here\s+is|here's|in\s+summary|result\s*:"
r"|(?:the\s+)?answer\s+is(?!\s+(?:not|unavailable|unknown|unclear|missing)\b))\b",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat modified missing-answer phrases as unfinished

When a forced retry says, for example, The answer is still unknown, so I should call web_search, the negative lookahead does not see unknown immediately after is, so this new alternative classifies the whole turn as a final answer. _should_suppress_forced_no_tool_output consequently returns before recognizing the remaining tool plan and exposes an unfinished response; the same happens with phrases such as the answer is probably unavailable, so the missing-answer check needs to account for intervening modifiers.

Useful? React with 👍 / 👎.

Comment on lines +380 to +381

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require answer content after the answer is

When a forced retry says something like I should call web_search to learn what the answer is., this alternative matches the embedded question even though no answer was provided. _should_suppress_forced_no_tool_output consequently returns before checking _FORCED_PLAN_INTENT and exposes the unfinished tool plan as the final response; require answer content after is or exclude embedded what the answer is clauses.

Useful? React with 👍 / 👎.

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. The pivot has to
# carry text of its own: "I should call web_search, though." answers nothing.
_ANSWER_PIVOT = re.compile(
r"\b(?:but|however|although|though|that\s+said|in\s+the\s+meantime|meanwhile)\b"
r"[\W_]*(?:\w+[\W_]+){1,}\w",
Comment on lines +388 to +390

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify that pivot text is an answer rather than another plan

On a forced retry such as I should call web_search, but first I need to formulate the query., this pattern accepts the words after but solely because there are at least two of them. _should_suppress_forced_no_tool_output then preserves the whole turn even though the pivot contains only another unfinished plan, leaking the hidden retry as the final response; the pivot guard needs to distinguish substantive answers from continued intent phrasing.

Useful? React with 👍 / 👎.

re.I,
)

Expand Down Expand Up @@ -436,14 +458,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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require answer text after pivot words

On a forced GGUF retry, obligation plans that end with a pivot word but do not answer, such as I should call web_search, though., hit _FORCED_PLAN_INTENT and then this pivot check returns false for suppression, so the hidden no-tool retry is emitted as the final response. Require substantive answer text after the pivot marker before preserving the turn, otherwise these unfinished retry plans leak instead of being dropped.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 9bf66ee. "I should call web_search, though." matched the pivot and shipped as the final response.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Suppress changed plan-only retries after the nudge budget

Do not preserve every non-identical direct-intent retry as progress. For example, after nudging Let me summarize the results, a retry of I will review the results now is still only a plan, but it is not an obligation-style _FORCED_PLAN_INTENT and is not an exact restatement, so this branch surfaces it as the final GGUF response once the post-tool cap or pre-tool budget is exhausted. The previous suppression recognized I will plans, so this can newly leave users with another unfinished promise instead of an answer.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 INTENT_SIGNAL fires on lead-ins that introduce a real answer ("Now I have the search results. The capital of Japan is Tokyo."). Suppressing on a bare intent match dropped those answers, which is what test_forced_turn_intent_lead_in_needs_a_restatement_to_be_dropped pins. Your P1 last round made the same point from the other side, and I agreed with it: suppression means the user gets nothing at all.

"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 ───────────
Expand Down Expand Up @@ -11241,14 +11277,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
Expand Down Expand Up @@ -11874,12 +11914,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()
Expand All @@ -11889,18 +11927,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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset repeat tracking after a tool runs

When a request first hits a pre-tool plan-without-action, _last_reprompt_text is set. If the retry then calls a tool and the next model turn repeats that same short intent (for example, “I will search the web now.” before and after the search result), _already_acted selects the fresh post-tool budget but this guard rejects the nudge as a repeat. Because _forced_tool_call_pending was cleared when the tool executed, the repeated plan is streamed/returned as the final answer instead of using the one post-tool corrective retry. Fresh evidence: this revision separated _post_tool_reprompts, but this new repeat guard still shares _last_reprompt_text across the pre- and post-tool phases.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, and fixed in 2253126.

Reproduced on the head commit: with I will search the web now. said both before and after the search, the loop stopped after 3 generations and surrendered the repeated stall as the visible answer instead of spending the post-tool nudge.

_last_reprompt_text is now cleared right after tool_controller.record_result, so a real execution opens a clean post-tool phase. Same script now runs 4 generations, logs Re-prompt 1/1, and ends on the final answer. Regression test: test_repeat_guard_resets_after_a_tool_runs.

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)"
)
Expand Down Expand Up @@ -11935,7 +11988,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,
Expand Down Expand Up @@ -12252,6 +12308,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()
Expand Down
5 changes: 5 additions & 0 deletions studio/backend/core/inference/safetensors_agentic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the post-tool nudge budget to safetensors

When a safetensors or MLX request has executed a tool and the next turn says something like Let me summarize the results now., this conditional still requires both not rag_autoinjected and no executed controller record, so it skips the nudge entirely and returns the unfinished plan. The GGUF path added by this commit instead grants exactly one post-tool retry; wire the same separate post-tool budget into this loop so the advertised behavior does not depend on the inference backend.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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: _suppress_visible_output = _forced_tool_call_pending drops every content chunk of the retry, and _should_suppress_forced_no_tool_output decides at the end whether any of it reaches the user. The safetensors loop has no equivalent state, so its stall text has already streamed to the user by the time the nudge decision is made. Granting it a post-tool nudge without that machinery would show the plan, then show the retry, which is a worse outcome than the missing nudge.

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)",
Expand Down
63 changes: 57 additions & 6 deletions studio/backend/core/inference/tool_call_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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" 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Accept list markers before First-step plans

Allow common Markdown and numbered-list prefixes before this sentence-opening pattern. Responses such as - First, search the web, * First: check the documentation, and 1) First, fetch the URL matched the previous first signal but no longer match any alternative here, so both inference loops treat these ordinary formatted plans as final answers instead of nudging the model to make the described tool call.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 - First, search the web, * First: check the documentation and 1) First, fetch the URL did not match it either. The sentence anchor has always been (?:^|[.!?]\s+)\s*, and a list marker is not whitespace, so these never reached the first branch. This is a pre-existing gap, not something the narrowing introduced.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Accept Markdown bullets before first-step plans

When the model formats an imperative plan as a Markdown item such as - First, search the web, none of these narrowed arms match because they require first immediately after the line start or sentence punctuation. The previous generic \bfirst\b arm detected this common plan form, but both tool loops now return it as the final response instead of nudging the model to perform the search; allow list markers before the sentence-opening first.

Useful? React with 👍 / 👎.

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))"
r"\b(?:step \d+:?|here['\u2019]?s (?:my |the |a )?(?:plan|approach))"
r"|"
r"\b(?:now i|next i)\b"
r")"
Expand All @@ -190,6 +206,41 @@ 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 = ".,;:!?\"'`()[]{}<>‘’“”"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve standalone comparison operators in repeat checks

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 < token becomes empty; the retry is then considered an exact restatement and _should_suppress_forced_no_tool_output discards the corrected response. Preserve semantically meaningful operators such as < and > during normalization.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced and fixed in 2e3277b. _normalize_for_repeat("Now I think the value is < 5") returned "now i think the value is 5", identical to the unbounded text, so is_reprompt_restatement was True and the corrected answer was discarded.

The normaliser now keeps the original token whenever stripping would empty it, which covers the whole class ("<", ">", "->", "..."), not just the comparison operators. Pinned in test_reprompt_repeat_keeps_standalone_operator_tokens against both is_reprompt_repeat and is_reprompt_restatement.

_REPEAT_LEAD_PUNCT = "\"'`([{‘“"


def _normalize_for_repeat(text: str) -> str:
words = []
for word in text.lower().split():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve case when comparing re-prompt targets

When a retry changes only a case-sensitive target, such as correcting src/Foo.py to src/foo.py on Linux, lowercasing the entire response makes the two plans compare equal. The new repeat guard then stops further nudging; in the GGUF loop the corrected retry is also suppressed, so the requested action can be stranded without executing. Normalize case only for known intent boilerplate, or otherwise retain case in arguments and identifiers.

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.
# Exact after normalisation, deliberately. Every relaxation tried here lost a real
# correction: a similarity ratio is length dependent (one changed token in a 50-word
# plan still scored 0.98), a set ignores order ("cats not dogs"), and ignoring filler
# words eats the target itself ("The Who", "OK Go"). A missed repeat costs one nudge
# out of MAX_ACT_REPROMPTS; a false one strands the plan unexecuted.
def is_reprompt_repeat(text: str, previous: str) -> bool:
return is_reprompt_restatement(text, previous)


# Same comparison, different decision: this one discards the turn. An appended answer
# must not match, 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 (
Expand Down
Loading
Loading