-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(studio): gate external tool nudges and preserve retry context #9125
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
Open
Biotrioo
wants to merge
12
commits into
unslothai:main
Choose a base branch
from
Biotrioo:fix/8907-external-tool-nudge
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
0415d12
fix(studio): gate external tool nudges and preserve retry context
Biotrioo 23155ef
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 772037a
Merge branch 'main' into fix/8907-external-tool-nudge
Biotrioo 30d38e7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 80d01cb
fix(studio): preserve external nudge policy context
Imagineer99 d24e2a7
Merge remote-tracking branch 'origin/main' into takeover/9125-rebased
danielhanchen c05cb16
studio: move the continuation-merge fix out to its own PR
danielhanchen 899eae5
studio: classify the external stall on the text the retry replays
danielhanchen 98d00d9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 4f077a7
Tighten the comment added by this change
danielhanchen a916f6a
Drop the duplicate stalled-model default test
danielhanchen 0abe73b
Fall back to the reasoning block when a stall strips empty
danielhanchen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # SPDX-License-Identifier: AGPL-3.0-only | ||
| # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 | ||
|
|
||
| """Regression guards for external Studio tool-call nudging (#8907 follow-up).""" | ||
|
|
||
| import inspect | ||
| from pathlib import Path | ||
|
|
||
| from core.inference.studio_tool_loop import ToolLoopPolicy, stream_with_studio_tools | ||
|
|
||
|
|
||
| def test_external_tool_loop_accepts_and_gates_nudge_flag(): | ||
| assert "nudge_tool_calls" in ToolLoopPolicy.__dataclass_fields__ | ||
| src = inspect.getsource(stream_with_studio_tools) | ||
| assert "nudge_enabled(policy.nudge_tool_calls)" in src | ||
|
|
||
|
|
||
| def test_external_route_forwards_request_nudge_flag(): | ||
| from routes import inference as routes_inference | ||
|
|
||
| external_src = inspect.getsource(routes_inference._proxy_to_external_provider) | ||
| assert external_src.count("nudge_tool_calls = payload.nudge_tool_calls") == 2 | ||
| codex_policy = external_src.split("CodexToolPolicy(", 1)[1].split("if studio_tool_payloads", 1)[ | ||
| 0 | ||
| ] | ||
| assert "nudge_tool_calls = payload.nudge_tool_calls" in codex_policy | ||
|
|
||
|
|
||
| def test_frontend_forwards_nudge_setting_to_external_tools(): | ||
| studio = Path(__file__).resolve().parents[2] | ||
| adapter = (studio / "frontend/src/features/chat/api/chat-adapter.ts").read_text( | ||
| encoding = "utf-8" | ||
| ) | ||
| # One local-model request and one external local-tool request. | ||
| assert adapter.count("nudge_tool_calls: runtime.nudgeToolCalls") >= 2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For external models that emit a reasoning-only stall using Magistral-style
[THINK]...[/THINK]markup,strip_tool_markup(..., final=True)removes the entire block, soreplayable_answeris empty andis_short_intent_without_actionnever triggers the enabled nudge. This differs from the referenced_reprompt_intent_text, which deliberately falls back to reasoning when there is no visible answer, and causes these models to stop after planning to use a tool instead of receiving the retry; classify with the reasoning-aware fallback while keeping the separately stripped text for replay.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.
Confirmed and fixed. My first probe made this look like a non-issue, because the string I tried classified
Falsebefore stripping as well. Shorter phrasings do diverge:So the nudge really was being dropped for reasoning-only stalls that main still nudges.
I matched
_reprompt_intent_textrather than only classifying differently: it classifies and replays the same string, and for a reasoning-only stall that string is the think block itself. Classifying the reasoning while replaying the stripped text would have put back the empty-replay case this PR exists to fix.Ordering carries the correctness here. The fallback runs after
strip_tool_markup, so a turn that is only an unpromotable call block has no think span, stays empty, and is still not nudged:Both cases are now pinned by tests. The reasoning-only test fails without the change and passes with it (
1 failed, 2 passedthen3 passed); the suite is 54 passed.Fixed in 0abe73b.