Studio: stop the model from replying twice when it refuses#5775
Merged
danielhanchen merged 1 commit intoMay 26, 2026
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the regular expression in llama_cpp.py to include a negative lookahead that drops negated forms (like "not" or "never") so that model refusals do not trigger a re-prompt. The reviewer suggested expanding this negative lookahead to cover other common refusal phrasings such as "decline", "refuse", or "be unable" to make the detection more robust.
Comment on lines
+63
to
+65
| # Negative lookahead drops negated forms ("I will not", "I'll never") | ||
| # so a refusal doesn't trigger a 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)" |
Contributor
There was a problem hiding this comment.
While the negative lookahead for not and never successfully handles the most common refusal patterns, models can also refuse using other common formal phrasings such as "decline", "refuse", or "be unable".
Expanding the negative lookahead to cover these variations makes the refusal detection more robust and prevents duplicate responses when the model refuses using these alternative phrasings.
Suggested change
| # Negative lookahead drops negated forms ("I will not", "I'll never") | |
| # so a refusal doesn't trigger a 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)" | |
| # Negative lookahead drops negated or refusal forms ("I will not", "I'll never", "I will decline", "I will be unable") | |
| # so a refusal doesn't trigger a 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|decline|declining|refuse|refusing|be\s+unable)\b)" |
Contributor
Author
There was a problem hiding this comment.
A refusal-verb list is bypassed by one intervening word ("I will respectfully decline", "I will have to decline"), so it never converges. The general case belongs in the "don't re-prompt after a final answer" path (the #5714 direction), not in this regex. Keeping it to "not"/"never", which handles the common case.
3 tasks
danielhanchen
added a commit
that referenced
this pull request
May 27, 2026
MLX CI on Mac M1 + Backend CI (both Repo tests CPU and Python 3.10/11/12/13) have been red on every push to main for days. None of the underlying code is wrong; three test files have stale anchors / assertions left behind by PR #5537 (max_steps bump) and PR #5775 (composer + provision-desktop-auth). 1. tests/studio/run_real_mlx_smoke.py:393 PR #5537 bumped max_steps from 7 to 30 for seed-robust convergence but left `assert len(losses_per_step) == 7`. With logging_steps=1 the callback fires once per step; 30 entries, not 7. Track config.max_steps so the gate auto-follows future bumps. 2. tests/studio/test_composer_rtl_bidi_attribute.py:29 PR #5775 changed the composer aria-label from the literal `aria-label="Message input"` to a JSX ternary `aria-label={overlay ? "Image edit instructions" : "Message input"}`. Anchor on the inner string literal `"Message input"` instead. 3. studio/backend/tests/test_desktop_auth.py:487 The guarded_import in test_provision_desktop_auth_writes_secret_and_creates_db_without_backend_deps blocks any import whose name == "utils", including the relative `from .utils import echo` inside typer._click.decorators (typer 0.25+). Gate the block on level == 0 so only absolute imports of `utils` / `auth` / `fastapi` / `structlog` are rejected; relative imports inside third-party packages pass through. All three tests pass locally; the MLX one is a mechanical 7->config.max_steps swap and will be exercised by MLX CI on this PR.
danielhanchen
added a commit
that referenced
this pull request
May 27, 2026
MLX CI on Mac M1 + Backend CI (both Repo tests CPU and Python 3.10/11/12/13) have been red on every push to main for days. None of the underlying code is wrong; three test files have stale anchors / assertions left behind by PR #5537 (max_steps bump) and PR #5775 (composer + provision-desktop-auth). 1. tests/studio/run_real_mlx_smoke.py:393 PR #5537 bumped max_steps from 7 to 30 for seed-robust convergence but left `assert len(losses_per_step) == 7`. With logging_steps=1 the callback fires once per step; 30 entries, not 7. Track config.max_steps so the gate auto-follows future bumps. 2. tests/studio/test_composer_rtl_bidi_attribute.py:29 PR #5775 changed the composer aria-label from the literal `aria-label="Message input"` to a JSX ternary `aria-label={overlay ? "Image edit instructions" : "Message input"}`. Anchor on the inner string literal `"Message input"` instead. 3. studio/backend/tests/test_desktop_auth.py:487 The guarded_import in test_provision_desktop_auth_writes_secret_and_creates_db_without_backend_deps blocks any import whose name == "utils", including the relative `from .utils import echo` inside typer._click.decorators (typer 0.25+). Gate the block on level == 0 so only absolute imports of `utils` / `auth` / `fastapi` / `structlog` are rejected; relative imports inside third-party packages pass through. All three tests pass locally; the MLX one is a mechanical 7->config.max_steps swap and will be exercised by MLX CI on this PR.
danielhanchen
added a commit
that referenced
this pull request
May 27, 2026
…d CI) (#5803) * tests: unblock three stale assertions broken on main MLX CI on Mac M1 + Backend CI (both Repo tests CPU and Python 3.10/11/12/13) have been red on every push to main for days. None of the underlying code is wrong; three test files have stale anchors / assertions left behind by PR #5537 (max_steps bump) and PR #5775 (composer + provision-desktop-auth). 1. tests/studio/run_real_mlx_smoke.py:393 PR #5537 bumped max_steps from 7 to 30 for seed-robust convergence but left `assert len(losses_per_step) == 7`. With logging_steps=1 the callback fires once per step; 30 entries, not 7. Track config.max_steps so the gate auto-follows future bumps. 2. tests/studio/test_composer_rtl_bidi_attribute.py:29 PR #5775 changed the composer aria-label from the literal `aria-label="Message input"` to a JSX ternary `aria-label={overlay ? "Image edit instructions" : "Message input"}`. Anchor on the inner string literal `"Message input"` instead. 3. studio/backend/tests/test_desktop_auth.py:487 The guarded_import in test_provision_desktop_auth_writes_secret_and_creates_db_without_backend_deps blocks any import whose name == "utils", including the relative `from .utils import echo` inside typer._click.decorators (typer 0.25+). Gate the block on level == 0 so only absolute imports of `utils` / `auth` / `fastapi` / `structlog` are rejected; relative imports inside third-party packages pass through. All three tests pass locally; the MLX one is a mechanical 7->config.max_steps swap and will be exercised by MLX CI on this PR. * [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>
rhsCZ
pushed a commit
to rhsCZ/unsloth
that referenced
this pull request
May 27, 2026
…k-glm-kimi Resolves three conflicts against the updated 5620 base (which itself merged main after main moved on with PRs unslothai#5735 / unslothai#5775 / unslothai#5803 etc. touching the same routes/inference.py and tool_call_parser.py surface): * studio/backend/core/inference/tool_call_parser.py ``_TOOL_CLOSED_PATS``: kept 5624's full set (Mistral pre-v11 array, Mistral v11+ name{json}, DeepSeek envelope, Kimi section) on top of the 3-pattern base. New 5620 base reverted to the 3 base patterns because main never carried the tool-format extensions. * studio/backend/routes/inference.py Merged the two regex bodies: kept 5624's elaborate python_tag ``(?:[^<]|<(?!\|))*`` clause and the new-family closed-pair patterns (DeepSeek envelope, Kimi section). DROPPED the inlined Mistral patterns in favour of the base's ``_strip_tool_xml`` helper which delegates Mistral handling to the parser module's ``_strip_mistral_closed_calls`` -- the non-greedy ``\{.*?\}`` form truncates at the first ``}`` of a nested JSON arg, so balanced brace/bracket scanning is correct here. Also kept the base's orphan-close and tail-only ``</parameter>`` patterns from the speculative-buffer split boundary work. Net: 9 call sites continue to use ``_strip_tool_xml(...)`` (Mistral-safe). * studio/backend/tests/test_safetensors_tool_loop.py ``TestRoutesPythonTagStrip``: kept the base's wording for the section header (the two were near-identical) and switched the helper ``_strip`` back to ``_strip_tool_xml`` since the helper is restored. Also retargeted ``test_pr5624_regressions.py``'s routes-layer strip tests to ``_strip_tool_xml`` for consistency with the restored helper. Tests: pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py studio/backend/tests/test_pr5624_regressions.py -q -> 170 passed in 1.93s pytest studio/backend/tests/ -q -k 'not gpu and not llama_cpp_integration' -> 2034 passed, 15 failed (pre-existing on the 5620 base; same set as before the merge: test_training_worker_flash_attn, test_desktop_auth, test_studio_api integration shims).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When a GGUF model in Studio refuses a request, it can reply twice in one turn: e.g. "I will not respond to that." then "I cannot fulfill this request.", each with its own thinking block.
Cause: the agentic loop re-prompts the model when it announces a plan ("I'll...", "Let me...") without calling a tool, detected by the
_INTENT_SIGNALregex. Itsi willpattern also matches refusals like "I will not...", so a refusal is misread as a plan and triggers a second generation.Fix: a negative lookahead so negated forms ("I will not", "I'll never") no longer match. Genuine plans like "I will search" still re-prompt.
Only fires with tools enabled. Related: #5714, #5549, #5706 rework the same path for other cases; none cover refusals.
Reproduction
gemma-4-E2B-it-GGUF) and turn on tools ("Think, Search, Code").