diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index bf8a3c04df..16c21d2239 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -52,6 +52,34 @@ logger = get_logger(__name__) # ── Pre-compiled patterns for plan-without-action re-prompt ── +# Tool-action verbs used by _PLAN_LIST_FRAMING to distinguish plan-only +# numbered lists ("1. search the web for X", "1. query the internet") +# from answer numbered lists ("1. Search the left half", "1. Apple", +# "1. Write a poem"). Each lookup verb is gated on a freshness or +# web/internet/online target so ordinary answer prose like +# "binary search: 1. Search the left half" or "1. Find the bug" is +# preserved. The strong, unambiguous patterns (``web search``, +# ``query the web``, ``call a tool``, ``run python``) stay bare. +_TOOL_LOOKUP_TARGET = ( + r"(?:web|internet|online(?: sources?)?|" + r"current|latest|today['’]?s?|up[- ]to[- ]date|live)" +) +_TOOL_ACTION_VERBS = ( + r"web[ _-]?search|" + r"(?:search|look up|browse|google) (?:for )?(?:the |a |an )?" + rf"{_TOOL_LOOKUP_TARGET}|" + r"(?:query|consult) (?:the |a |an )?" + r"(?:web|internet|online(?: sources?)?)|" + r"fetch (?:the |a |an )?" + rf"{_TOOL_LOOKUP_TARGET}|" + r"(?:research|investigate|find|check|verify|compare|review) " + r"(?:for )?(?:the |a |an )?" + rf"{_TOOL_LOOKUP_TARGET}|" + r"(?:use|invoke|call) (?:the )?(?:python|search) tool|" + r"use python(?: tool)? to|" + r"call (?:a |the )?tool|run (?:python|the code)|execute (?:python|the code)" +) + # Forward-looking intent signals that indicate the model is # describing what it *will* do rather than giving a final answer. _INTENT_SIGNAL = re.compile( @@ -62,7 +90,7 @@ _INTENT_SIGNAL = re.compile( # appear frequently in direct answers / explanations. r"\b(i['\u2019](ll|m going to|m gonna)|i am (going to|gonna)|i will|i shall|let me|allow me)\b" r"|" - # Step/plan framing: "First ...", "Step 1:", "Here's my plan" + # 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"|" # "Now I" / "Next I" patterns @@ -71,6 +99,324 @@ _INTENT_SIGNAL = re.compile( ) _MAX_REPROMPTS = 3 +# Substantive answer artifacts. Re-prompt fires when the model emits +# intent-only language ("first I'll ...", "let me ...") without a tool +# call, but the same intent words appear in long explanations that +# accompany REAL code or markup. Without this guard, a complete reply +# like "First, let me set up pygame. ```python ... ```" trips the +# re-prompt and the next user-visible message wipes the code. We +# require ALL of (intent signal, length < _REPROMPT_MAX_CHARS, no +# answer artifact) to fire. +# +# Notes on the patterns: +# * `\r?\n` everywhere a newline is required so Windows-authored or +# CRLF-converted content still matches. +# * Code-fence info string is `[^\r\n]{0,200}` so common languages with +# digits / symbols (python3, c++, c#, objective-c, ts-node, ...) are +# all recognised; closing fence may be indented (` ``` ` inside a +# list or blockquote). +# * HTML branches require a closing `` so plan-only mentions of +# `` or `` do not bypass the re-prompt. +# * All `[\s\S]{...}?` runs are length-bounded so the search stays +# linear on adversarial input (CRLF spam, repeated `` etc.). +_CLOSED_CODE_FENCE = re.compile( + r"(?`{3,})(?!`)[^\r\n]{0,200}\r?\n[\s\S]{1,4000}?\r?\n[ \t]*(?P=bf)`*[ \t]*(?:\r?\n|\Z)" + r"|(?~{3,})(?!~)[^\r\n]{0,200}\r?\n[\s\S]{1,4000}?\r?\n[ \t]*(?P=tf)~*[ \t]*(?:\r?\n|\Z)", + re.IGNORECASE, +) +_CLOSED_MARKUP_ARTIFACT = re.compile( + r"(?:" + r"|", + re.IGNORECASE, +) +_HAS_ANSWER_ARTIFACT = re.compile( + # Closed backtick code fence (any markdown info string, optional indent + # on close). CommonMark allows opening fences of 3+ backticks; the + # closing fence must have at least as many delimiters, and the line + # must end cleanly (only trailing whitespace before newline / EOS), + # so spam like ``` ```not actually closed ``` does not count. + r"(?`{3,})(?!`)[^\r\n]{0,200}\r?\n[\s\S]{1,4000}?\r?\n[ \t]*(?P=bf)`*[ \t]*(?:\r?\n|\Z)" + # Closed tilde code fence; same 3+ rule (several models emit ~~~ when + # the body itself contains backticks). Opener anchored to the full + # run of tildes; closer accepts >= opener length per CommonMark. + r"|(?~{3,})(?!~)[^\r\n]{0,200}\r?\n[\s\S]{1,4000}?\r?\n[ \t]*(?P=tf)~*[ \t]*(?:\r?\n|\Z)" + # Complete HTML page; doctype prefix is optional. + r"|(?:" + # Complete SVG document. + r"|", + re.IGNORECASE, +) + +# Two or more numbered list items at column 0. Indent is spaces / tabs +# only so the regex stays linear on long whitespace runs. +_NUMBERED_LIST_ARTIFACT = re.compile( + r"(?:^|\r?\n)[ \t]*\d+\.[ \t]+\S.*?\r?\n[ \t]*\d+\.", +) + +# Markers that a numbered list is a plan (still re-promptable), not a +# final answer. Fires when an intent phrase from _INTENT_SIGNAL is +# followed anywhere in the short re-prompt candidate by a tool-action +# verb. The apostrophe in ``i['’]ll`` is required (no ``?``) so the +# regex does not accidentally match the word "ill". Without a tool- +# action verb the numbered list is treated as a completed answer +# artifact. The scan window is bounded at _REPROMPT_MAX_CHARS by the +# caller, so the lazy ``[\s\S]{0,2000}?`` quantifier stays linear. +_PLAN_LIST_FRAMING = re.compile( + r"\b(?:here['’]?s (?:my |the |a )?(?:plan|approach)|" + r"step \d+|first|" + r"i['’](?:ll|m going to|m gonna)|i am (?:going to|gonna)|" + r"i will|i shall|let me|allow me|now i|next i)\b" + r"[\s\S]{0,2000}?" + rf"\b(?:{_TOOL_ACTION_VERBS})\b", + re.IGNORECASE, +) + +# "Here's my plan" / "Here's my approach" are strong stand-alone plan +# signals: a possessive, first-person framing where the model is +# announcing what it WILL do. Treat the following numbered list as a +# plan regardless of the specific verbs each item uses, so stalls like +# ``Here's my plan: 1. Analyze 2. Draft`` still re-prompt. +_EXPLICIT_PLAN_HEADER = re.compile( + r"\bhere['’]?s (?:my |the |a )?(?:plan|approach)\b", + re.IGNORECASE, +) + +# Direct first-person intent + a tool/work verb that the model is about +# to perform + a numbered list. Catches stalls like +# ``First, I'll do this:\n1. Search ...`` or ``Let me do this:\n1. Parse +# the file ...`` where each list item is an action the model promised +# to take without actually invoking a tool. The first-person intent +# branch tolerates a broad set of work verbs (open/read/search/check/ +# review/inspect/etc.) because direct first-person announcements are +# strongly plan-like; the "First, ..." / "Step N:" branch stays +# narrow so algorithmic answers ("First, use binary search:") are +# preserved. +_DIRECT_NUMBERED_PLAN_FRAMING = re.compile( + r"(?:" + r"\b(?:i['’](?:ll|m going to|m gonna)|i am (?:going to|gonna)|" + r"i will|i shall|let me|allow me|now i|next i)\b" + r"[^\r\n]{0,160}" + r"\b(?:open|read|search|look (?:this |that |it |them )?up|browse|" + r"google|find|check|verify|compare|review|inspect|examine|" + r"visit|access|navigate|gather|collect|" + r"do (?:this|these|the following|it)|" + r"(?:take|follow|complete|perform) (?:these|the following) (?:steps|actions)|" + r"proceed|start|begin|" + r"create|build|implement|set up|add|calculate|compute|analy[sz]e|" + r"parse|load|run|execute|test)\b" + r"|" + r"\b(?:first|step \d+:?)\b" + r"[^\r\n]{0,160}" + r"\b(?:do (?:this|these|the following|it)|" + r"look (?:this |that |it |them )?up|" + r"proceed|start|begin|" + r"create|build|implement|set up|add|" + r"calculate|compute|analy[sz]e|parse|load|run|execute|test)\b" + r")" + r"[\s\S]{0,500}?" + r"(?:^|\r?\n)[ \t]*\d+\.", + re.IGNORECASE, +) + + +_FENCE_RUN_RE = re.compile( + r"(?`{3,})(?!`)|(?~{3,})(?!~)" +) + + +def _has_unclosed_code_fence(text: str) -> bool: + """True if ``text`` contains a code fence whose closer is missing. + + Each line is scanned with ``search`` so inline openers like + ``First. \\`\\`\\`python`` are tracked. To avoid reading prose + mentions of triple backticks as openers, an INLINE fence (fence + not at line start) is only accepted when its trailing characters + look like a clean CommonMark info-string token with no internal + whitespace. Column-0 fences always count, so multi-token info + strings like ``\\`\\`\\`python linenums=1`` still work. + """ + active_char: Optional[str] = None + active_len = 0 + for line in text.splitlines(): + m = _FENCE_RUN_RE.search(line) + if not m: + continue + fence = m.group("backticks") or m.group("tildes") + raw_trailing = line[m.end() :] + trailing = raw_trailing.strip() + ch = fence[0] + is_inline = bool(line[: m.start()].strip()) + # Inline + multi-word trailing or leading-space trailing both + # read as prose ("Use ``` to start", "Use ```python to open"). + if is_inline: + if raw_trailing and raw_trailing[0] == " " and trailing: + continue + if trailing and (" " in trailing or "\t" in trailing): + continue + if active_char is None: + active_char = ch + active_len = len(fence) + elif ch == active_char and len(fence) >= active_len and not trailing: + active_char = None + active_len = 0 + return active_char is not None + + +def _has_unclosed_markup_block(text: str) -> bool: + """True if ``text`` opens an / block without closing it. + + Either a missing close on the only block, OR a closed block followed + by a still-open block, qualifies. The check is unbalanced-count + based so half-finished output ALWAYS disqualifies the artifact path, + even when an earlier complete artifact is also present in the same + response. + """ + opens_html = len(re.findall(r"", text, re.IGNORECASE)) + if opens_html > closes_html: + return True + opens_svg = len(re.findall(r"", text, re.IGNORECASE)) + return opens_svg > closes_svg + + +# Matches the full span of an empty or +# skeleton. Plan-only mentions ("First, I'll create an +# skeleton") would otherwise look like a complete page. +_EMPTY_MARKUP_SKELETON = re.compile( + r"<(html|svg)\b[^>]*>\s*", + re.IGNORECASE, +) +_DOCTYPE_PREFIX = re.compile( + r"^", + re.IGNORECASE, +) + + +def _is_empty_markup_skeleton(matched: str) -> bool: + """True if ``matched`` is just an empty / + (optionally with a `` prefix and surrounding whitespace). + These read as plan-only mentions, not substantive answers.""" + candidate = _DOCTYPE_PREFIX.sub("", matched.strip(), count = 1).strip() + return _EMPTY_MARKUP_SKELETON.fullmatch(candidate) is not None + + +# A numbered list whose item lines start with a strong work / tool +# verb. Combined with first-person intent framing this catches stalls +# like "First, I'll:\n1. Load the CSV\n2. Compute the total" where the +# verbs sit in the list items rather than before the list. The verb +# list deliberately excludes ``search`` / ``look up`` / ``read`` / +# ``open`` / ``create`` / ``build`` etc. so ordinary algorithm or +# instructional answers ("1. Search the left half", "1. Read the +# docs") stay valid answers. +_LOCAL_ACTION_VERBS = ( + r"load|inspect|parse|" + r"calculate|compute|analy[sz]e|extract|" + r"run|execute|fetch|download|query|" + r"gather|collect|identify" +) +_NUMBERED_ACTION_ITEM = re.compile( + rf"(?:^|\r?\n)[ \t]*\d+\.[ \t]+(?:{_LOCAL_ACTION_VERBS})\b", + re.IGNORECASE, +) +# Direct first-person pronoun intent only. "First," and "Step N:" are +# intentionally excluded here because they appear in non-plan answers +# ("First, use binary search:") and would over-trigger the items-in- +# numbered-list cross-check. +_STRONG_INTENT_BEFORE_LIST = re.compile( + r"\b(?:i['’](?:ll|m going to|m gonna)|i am (?:going to|gonna)|" + r"i will|i shall|let me|allow me|now i|next i)\b", + re.IGNORECASE, +) + +# Bare first-person intent immediately followed by ``:`` and a numbered +# list whose first item begins with a work verb. Catches +# ``I'll:\n1. Open the URL`` and ``Let me:\n1. Parse the JSON`` where +# no work verb appears between the intent phrase and the list. The +# verb set here is broader than _LOCAL_ACTION_VERBS because the +# tight ``intent + : + newline + numbered`` shape is itself the strong +# signal that this is a tool stall. +_BARE_INTENT_NUMBERED_PLAN = re.compile( + r"\b(?:i['’](?:ll|m going to|m gonna)|i am (?:going to|gonna)|" + r"i will|i shall|let me|allow me|now i|next i)\s*:[ \t]*" + r"(?:\r?\n)[ \t]*\d+\.[ \t]+" + r"(?:open|read|search|" + r"look (?:this |that |it |them )?up|" + r"check|verify|create|build|add|set up|" + r"load|inspect|parse|calculate|compute|analy[sz]e|extract|run|execute|" + r"fetch|download|query|summari[sz]e|implement|generate|draft|write|" + r"visit|access|navigate|gather|collect|identify|update|edit)\b", + re.IGNORECASE, +) + + +def _looks_like_real_artifact(text: str) -> bool: + """Match _HAS_ANSWER_ARTIFACT but reject empty markup skeletons. + + Iterates every artifact match in ``text`` so an empty + skeleton followed by a real complete page still classifies as a + real artifact (the second match wins).""" + for m in _HAS_ANSWER_ARTIFACT.finditer(text): + if not _is_empty_markup_skeleton(m.group(0)): + return True + return False + + +def _has_answer_artifact(text: str) -> bool: + """True if ``text`` looks like a completed answer artifact. + + Code fences, complete HTML, and complete SVG count directly. A + numbered list counts only when there is no plan framing, so stalls + like ``Here's my plan:\\n1. search\\n2. summarise`` still re-prompt. + An explicit ``Here's my plan`` / ``Here's my approach`` header, or a + direct first-person ``I'll do this:\\n1. ...`` framing with a + work/tool verb before the list, also flags the list as a plan even + when no narrow tool-action verb appears in the items. Any unclosed + fence or unclosed `` / `` block disqualifies the + artifact path so half-finished output does not look like a final + answer, even when an earlier complete artifact is also present. + Empty `` / `` skeletons do not count. + """ + # Cross-strip closed artifacts before the unclosed-state checks so + # delimiter-like content INSIDE a complete code fence (e.g. + # `html = ''` literal in a Python snippet) or INSIDE complete + # HTML (e.g. a JS string containing backticks) does not falsely + # disqualify the artifact path. + text_without_closed_fences = _CLOSED_CODE_FENCE.sub("", text) + text_without_closed_markup = _CLOSED_MARKUP_ARTIFACT.sub("", text) + text_without_both = _CLOSED_MARKUP_ARTIFACT.sub("", text_without_closed_fences) + if _has_unclosed_code_fence(text_without_closed_markup): + return False + # When NO complete artifact has been emitted yet, count-based markup + # detection is reliable for spotting mid-stream output. Once a real + # artifact already exists, prose mentions of bare ```` / + # ```` tags in explanations are common (and would falsely + # unbalance the open/close count), so we rely on the closed-artifact + # path instead and skip the count check. + real_artifact = _looks_like_real_artifact(text) + if not real_artifact and _has_unclosed_markup_block(text_without_both): + return False + if real_artifact: + return True + if _NUMBERED_LIST_ARTIFACT.search(text): + if _EXPLICIT_PLAN_HEADER.search(text): + return False + if _DIRECT_NUMBERED_PLAN_FRAMING.search(text): + return False + if _BARE_INTENT_NUMBERED_PLAN.search(text): + return False + # First-person pronoun intent + numbered list where the items + # themselves start with a strong work verb ("First, I'll:\n + # 1. Load...\n2. Run...") is a plan stall, even when no work + # verb appears before the list. + if _STRONG_INTENT_BEFORE_LIST.search(text) and _NUMBERED_ACTION_ITEM.search( + text + ): + return False + return _PLAN_LIST_FRAMING.search(text) is None + return False + + # Without max_tokens, llama-server defaults to n_predict = n_ctx (up to # 262144 for Qwen3.5), producing many-minute zombie decodes when cancel # fails. t_max_predict_ms is a wall-clock backstop applied unconditionally, @@ -4810,15 +5156,48 @@ class LlamaCppBackend: # like "4" or "Hello!" won't trigger this. # Use content if available, otherwise fall back # to reasoning text (reasoning-only stalls). - _stripped = content_accum.strip() - if not _stripped: - _stripped = reasoning_accum.strip() - if ( + # Artifact check uses USER-VISIBLE text only. + # Reasoning is only user-visible when there are + # no content tokens (the branch above yields + # reasoning_accum as plain content in that + # case); otherwise reasoning stays hidden and + # an artifact inside it must NOT suppress the + # re-prompt. + # Strip orphan tool-call XML before measuring + # the visible answer. An ``...`` + # block that the route layer would scrub from the + # final visible message must not satisfy the + # artifact check. + _visible_raw = content_accum.strip() + _visible = ( + _strip_tool_markup(content_accum, final = True).strip() + if _visible_raw + else "" + ) + _reasoning = reasoning_accum.strip() + _stripped = _visible if _visible else _reasoning + # Cheap gates first so long final answers never + # pay the artifact-regex scan. The artifact + # check only runs when the candidate already + # passes length + intent + state checks. + _should_consider_reprompt = bool( tools and _reprompt_count < _MAX_REPROMPTS and 0 < len(_stripped) < _REPROMPT_MAX_CHARS and _INTENT_SIGNAL.search(_stripped) - ): + ) + if _should_consider_reprompt: + _artifact_text = ( + _visible + if _visible + else (_reasoning if not has_content_tokens else "") + ) + _visible_has_artifact = bool( + _artifact_text + ) and _has_answer_artifact(_artifact_text) + else: + _visible_has_artifact = False + if _should_consider_reprompt and not _visible_has_artifact: _reprompt_count += 1 logger.info( f"Re-prompt {_reprompt_count}/{_MAX_REPROMPTS}: " diff --git a/studio/backend/tests/test_llama_cpp_reprompt_guard.py b/studio/backend/tests/test_llama_cpp_reprompt_guard.py new file mode 100644 index 0000000000..c709ca0212 --- /dev/null +++ b/studio/backend/tests/test_llama_cpp_reprompt_guard.py @@ -0,0 +1,1300 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Tests for the plan-without-action re-prompt guard. + +The re-prompt path in ``LlamaCppEngine.chat_stream`` exists to nudge a +model that described what it *will* do (forward-looking language) +without actually calling a tool. Before the guard added in this PR, the +heuristic only checked ``len(content) < _REPROMPT_MAX_CHARS`` and the +intent regex, which over-fired on long-but-complete responses that +happened to contain phrases like "first" or "let me". Specifically, a +correct Python game answer of the form :: + + First, let me set up pygame. + ```python + import pygame; ... + ``` + +would still match (length < 2000, intent signal present) and the next +synthetic user turn ("STOP. Do NOT write code or explain.") wiped the +visible code from the conversation. + +The guard recognises completed code fences (any markdown info string, +indented closing fence allowed), complete HTML documents, and complete +SVGs as answer artifacts. A numbered list is an artifact only when the +response does NOT also contain plan framing ("Here's my plan", a tool- +action verb following intent phrasing, etc.), so plan-only stalls of +the form ``Here's my plan:\\n1. search\\n2. summarise`` still re-prompt. +""" + +from __future__ import annotations + +import sys +import types as _types +from pathlib import Path + +_BACKEND_DIR = str(Path(__file__).resolve().parent.parent) +if _BACKEND_DIR not in sys.path: + sys.path.insert(0, _BACKEND_DIR) + +# Inject minimal stand-ins ONLY when the real modules are unavailable. +# Using ``setdefault`` with a non-package ``ModuleType`` would otherwise +# poison ``sys.modules`` for any later test that does +# ``from loggers.handlers import ...`` (Python would raise "loggers is +# not a package" because the stub has no ``__path__``). +try: # noqa: E402 + import loggers # type: ignore # real backend package +except ModuleNotFoundError: + _loggers_stub = _types.ModuleType("loggers") + _loggers_stub.__path__ = [] # type: ignore[attr-defined] + _loggers_stub.get_logger = lambda name: __import__("logging").getLogger(name) + sys.modules["loggers"] = _loggers_stub + +try: # noqa: E402 + import structlog # type: ignore +except ModuleNotFoundError: + _structlog_stub = _types.ModuleType("structlog") + _structlog_stub.__path__ = [] # type: ignore[attr-defined] + _structlog_stub.get_logger = lambda *a, **k: __import__("logging").getLogger("stub") + sys.modules["structlog"] = _structlog_stub + +from core.inference.llama_cpp import ( # noqa: E402 + _HAS_ANSWER_ARTIFACT, + _INTENT_SIGNAL, + _NUMBERED_LIST_ARTIFACT, + _PLAN_LIST_FRAMING, + _has_answer_artifact, +) + + +# ── _INTENT_SIGNAL still matches plan-only stalls ────────────────── + + +def test_intent_signal_matches_plan_only_phrases(): + """Original behaviour is preserved: intent regex still matches the + plan-without-action phrases that motivated the re-prompt.""" + plan_only_samples = [ + "I'll search the web for that.", + "I will look that up.", + "I am going to search.", + "Let me search the web for the answer.", + "First, I need to look up the date.", + "Step 1: I'll search for the song list.", + "Now I need to call the tool.", + "Here's my plan: search for X.", + ] + for s in plan_only_samples: + assert _INTENT_SIGNAL.search(s), f"_INTENT_SIGNAL should match {s!r}" + + +def test_intent_signal_ignores_direct_answers(): + """Direct, complete answers do not match the intent regex.""" + direct_samples = [ + "4", + "Hello!", + "The answer is 42.", + "The capital of France is Paris.", + ] + for s in direct_samples: + assert not _INTENT_SIGNAL.search(s), f"_INTENT_SIGNAL must not match {s!r}" + + +# ── Code fence artifact detection ────────────────────────────────── + + +def test_artifact_regex_detects_closed_code_fence(): + """Closed Python code fence is an answer artifact.""" + text = "First, let me set up pygame.\n```python\nimport pygame\npygame.init()\n```" + assert _has_answer_artifact(text) + + +def test_artifact_regex_detects_non_alpha_info_strings(): + """Common languages with digits / symbols in the fence info string + (python3, c++, c#, objective-c, ts-node, bash-session) must all be + recognised as complete code answers.""" + samples = [ + "First, let me write it.\n```python3\nprint('hi')\n```", + "First, let me write it.\n```c++\nint main() { return 0; }\n```", + 'First, let me write it.\n```c#\nConsole.WriteLine("hi");\n```', + 'First, let me write it.\n```objective-c\nNSLog(@"hi");\n```', + "First, let me write it.\n```ts-node\nconsole.log('hi')\n```", + "First, let me script it.\n```bash-session\n$ echo hi\n```", + "First, let me show it.\n```python linenums=\"1\"\nprint('hi')\n```", + ] + for text in samples: + assert _has_answer_artifact(text), text + assert not _would_reprompt(text), text + + +def test_artifact_regex_detects_indented_close_fence(): + """A closing fence indented under a list / blockquote still counts. + Common when the model nests code in markdown structure.""" + text = "First, let me show:\n```python\nx = 1\n ```" + assert _has_answer_artifact(text) + + +def test_artifact_regex_detects_tilde_code_fence(): + """CommonMark also allows ``~~~`` fences. Models emit them when the + body itself contains backticks, e.g. shell or markdown.""" + samples = [ + "First, let me write it.\n~~~python\nprint('hi')\n~~~", + "First, let me show:\n~~~\nplain block\n~~~", + "Sure, here is the script.\n~~~bash\necho hi\n~~~", + ] + for text in samples: + assert _has_answer_artifact(text), text + assert not _would_reprompt(text), text + + +def test_artifact_regex_ignores_open_code_fence(): + """An UNCLOSED code fence is not yet a complete artifact.""" + text = "Let me set up pygame.\n```python\nimport pygame" + assert not _has_answer_artifact(text) + + +def test_artifact_regex_ignores_plain_text(): + """Plain conversational text contains no artifact.""" + text = "First, I will search for the songs that charted #3 in 2015." + assert not _has_answer_artifact(text) + + +# ── HTML artifact detection ──────────────────────────────────────── + + +def test_artifact_regex_detects_html_page(): + """Complete HTML pages (doctype optional, required) match.""" + text_a = "" + text_b = "Sure, here is the dashboard:\n..." + assert _has_answer_artifact(text_a) + assert _has_answer_artifact(text_b) + + +def test_artifact_regex_ignores_incomplete_html_mention(): + """A plan-only mention of / without close + must NOT be treated as a completed answer. Pre-fix the guard matched + bare `` skeleton, then add CSS and JavaScript.", + "First, I'll write a complete page with a button.", + "Let me design a structure for the dashboard.", + ] + for s in samples: + assert not _has_answer_artifact(s), s + + +# ── SVG artifact detection ───────────────────────────────────────── + + +def test_artifact_regex_detects_complete_svg(): + """A complete ... is an answer artifact.""" + text = ( + "Here is the sloth SVG:\n" + "" + "" + "" + "" + ) + assert _has_answer_artifact(text) + + +def test_artifact_regex_ignores_incomplete_svg(): + text = "Let me draw a sloth: bool: + """Return True if the re-prompt block at llama_cpp.py would fire.""" + from core.inference.llama_cpp import _REPROMPT_MAX_CHARS + + stripped = content.strip() + return bool( + 0 < len(stripped) < _REPROMPT_MAX_CHARS + and _INTENT_SIGNAL.search(stripped) + and not _has_answer_artifact(stripped) + ) + + +def test_no_reprompt_on_complete_python_game(): + """Response with intent phrasing + complete code does NOT re-prompt.""" + content = ( + "First, let me set up pygame.\n" + "```python\n" + "import pygame\n" + "pygame.init()\n" + "screen = pygame.display.set_mode((640, 480))\n" + "while True:\n" + " for e in pygame.event.get():\n" + " if e.type == pygame.QUIT: break\n" + "```" + ) + assert not _would_reprompt(content) + + +def test_no_reprompt_on_complete_svg(): + """Response with intent phrasing + complete SVG does NOT re-prompt.""" + content = ( + "Let me draw a cute sloth:\n" + "" + "" + "" + "" + "" + "" + ) + assert not _would_reprompt(content) + + +def test_no_reprompt_on_numbered_list_answer(): + """A list answer without plan framing does NOT re-prompt.""" + content = ( + "Here's my list of #3 hits:\n" + "1. Animals - Maroon 5\n" + "2. Take Me to Church - Hozier\n" + "3. Drag Me Down - One Direction\n" + ) + assert not _would_reprompt(content) + + +def test_reprompts_on_plan_only_stall(): + """Response that is purely a plan and no artifact STILL re-prompts.""" + content = "I'll search the web for the answer." + assert _would_reprompt(content) + + +def test_reprompts_on_intent_with_open_fence(): + """Open code fence is not a complete artifact, so we still re-prompt.""" + content = "First, let me write the code.\n```python\nimport" + assert _would_reprompt(content) + + +def test_reprompts_on_numbered_plan_only_stall(): + """Numbered plan ("Here's my plan: 1. search 2. summarise") STILL + re-prompts. Pre-fix the numbered-list artifact branch suppressed + the tool-call nudge, which contradicted the PR's stated invariant.""" + content = ( + "Here's my plan:\n" + "1. Search the web for the current Billboard Hot 100 2015 data.\n" + "2. Use python to categorise the matching songs." + ) + assert _would_reprompt(content) + + +def test_reprompts_on_intent_with_numbered_action_plan(): + """Numbered list where each item is an action (search, fetch, ...) + paired with intent phrasing is treated as a plan, not an answer.""" + content = ( + "First, I'll do these:\n" + "1. Search the web\n" + "2. Compare the sources\n" + "3. Answer concisely" + ) + assert _would_reprompt(content) + + +def test_reprompts_on_incomplete_html_intent(): + """A plan-only mention of without close STILL re-prompts.""" + content = "First, I'll create an skeleton, then add CSS." + assert _would_reprompt(content) + + +def test_plan_framing_requires_apostrophe_in_ill(): + """The ``i['’]ll`` plan-framing alternative requires an apostrophe so + the regex does not match the word "ill" (sick). Without this, a + numbered list near "ill" plus a freshness-gated lookup verb would + be misclassified as a plan and trigger a spurious re-prompt.""" + samples = [ + ("She is ill. Here is the list:\n1. Apple\n2. Orange\n3. Banana", False), + ("I'll search the web for X:\n1. step\n2. step", True), + ("I will search the latest docs:\n1. step\n2. step", True), + ] + for content, expected in samples: + got = _would_reprompt(content) + assert got == expected, f"{content!r} expected reprompt={expected} got {got}" + + +def test_reprompts_on_all_intent_form_numbered_action_plans(): + """``_PLAN_LIST_FRAMING`` must mirror every intent form that + ``_INTENT_SIGNAL`` accepts so numbered action plans phrased with + ``Allow me``, ``I'm going to``, ``I'm gonna``, ``I am gonna``, + ``I shall``, ``Now I``, ``Next I`` also re-prompt instead of being + silently classified as completed answers. Each sample pairs the + intent form with a freshness-gated lookup verb so the cross-check + against _TOOL_ACTION_VERBS succeeds.""" + samples = [ + "Allow me to do this:\n1. search the web for X\n2. fetch the latest result", + "I'm going to do this:\n1. search the latest docs\n2. fetch the current result", + "I'm gonna do this:\n1. search the web for X\n2. fetch the latest result", + "I am gonna do this:\n1. search the latest docs\n2. fetch the current result", + "I shall do this:\n1. search the web for X\n2. fetch the latest result", + "Now I will do these:\n1. search the web\n2. summarise", + "Next I will do these:\n1. fetch the latest chart\n2. compare", + ] + for s in samples: + assert _would_reprompt(s), s + + +def test_no_reprompt_on_plan_titled_final_answer_without_actions(): + """A final answer naturally titled ``Plan:`` / ``My plan:`` / + ``Approach:`` must NOT wipe. Bare ``Plan:`` / ``Approach:`` is + deliberately NOT an intent signal in _INTENT_SIGNAL because it + too often appears as a normal answer heading (lesson plan, meal + plan, business plan, project plan, ...).""" + samples = [ + "Plan:\n1. Warm-up: Students review fractions.\n2. Group practice.\n3. Assessment.", + "My plan:\n1. Breakfast: oatmeal and fruit.\n2. Lunch: rice bowl.\n3. Dinner: lentil soup.", + "The plan:\n1. Bring umbrellas.\n2. Pack snacks.\n3. Drive carefully.", + ] + for s in samples: + assert not _would_reprompt(s), s + + +def test_no_reprompt_on_bare_plan_header_action_stall(): + """Bare ``Plan:`` / ``Approach:`` headers paired with tool-action + verbs are NOT classified as plan stalls. Adding them as intent + markers caused false positives on legitimate plan answers; we + accept the smaller false negative (action plans titled only with + ``Plan:`` slip through) in exchange for not wiping valid answers. + Plan stalls that use an explicit first-person intent phrase ("I'll + search...", "First, I'll fetch...") are still caught.""" + samples = [ + "Plan:\n1. search the docs\n2. summarise the result", + "My plan:\n1. fetch the data\n2. verify the rows", + "The approach:\n1. look up the value\n2. compare versions", + ] + for s in samples: + assert not _would_reprompt(s), s + + +def test_no_reprompt_on_here_is_the_plan_prose_answer(): + """``Here is the plan you asked for. ...`` and similar prose + answers without action verbs must NOT wipe. The action-verb + lookahead on the ``Here is the plan`` intent branch filters them.""" + samples = [ + "Here is the plan you asked for. It is two pages long and covers Q4 goals.", + "Here are my steps in plain English. Step one is patience.", + "Here is a plan for the dinner party. Welcome, eat, dance.", + ] + for s in samples: + assert not _would_reprompt(s), s + + +# ── Cross-platform line endings ──────────────────────────────────── + + +def test_artifact_regex_handles_crlf_code_fence(): + """Windows / CRLF-converted content still detects a closed fence.""" + content = "First, let me code.\r\n```python\r\nimport sys\r\nprint('hi')\r\n```" + assert _has_answer_artifact(content) + + +def test_artifact_regex_handles_mixed_lf_crlf(): + """Mixed line endings (real-world: paste-and-edit on Windows).""" + content = "Here's the code:\r\n```python\nimport sys\r\n```" + assert _has_answer_artifact(content) + + +def test_no_reprompt_on_crlf_complete_python_game(): + """End-to-end CRLF: complete fence -> no re-prompt.""" + content = ( + "First, let me set up pygame.\r\n" + "```python\r\n" + "import pygame\r\n" + "pygame.init()\r\n" + "while True:\r\n" + " for e in pygame.event.get():\r\n" + " if e.type == pygame.QUIT: break\r\n" + "```" + ) + assert not _would_reprompt(content) + + +# ── ReDoS guards ─────────────────────────────────────────────────── + + +def test_no_backtrack_on_crlf_spam(): + """10K of `\\r\\n` repeats must complete fast. + + The numbered-list alternative previously used greedy `\\s*` which + O(n^2)-backtracked through embedded `\\r\\n` characters (~630 ms on + 10 KB). The current `[ \\t]*` indent restriction plus length-bounded + `[\\s\\S]{...}?` runs keep every alternative linear.""" + import time + + payload = "\r\n" * 5000 + t0 = time.time() + _has_answer_artifact(payload) + elapsed_ms = (time.time() - t0) * 1000 + assert elapsed_ms < 50, f"guard took {elapsed_ms:.1f}ms on 10KB CRLF spam" + + +def test_no_backtrack_on_open_html_spam(): + """Many `` close must still complete + quickly. Bounded `[\\s\\S]{0,4000}?` between the open and close caps + the scan per occurrence.""" + import time + + payload = "`` is retried at every `` with no ) plus a numbered + list must NOT be treated as a final answer; the markup is still + being streamed.""" + samples = [ + ( + "First, I'll draft a page.\n" + "\n" + "1. Section one.\n" + "2. Section two.\n" + ), + ("Let me design a chart.\n" "\n" "1. circle.\n" "2. rect."), + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_complete_html_with_trailing_prose_tag_still_counts(): + """A complete answer followed by prose that mentions + or tags (explanatory text) stays a complete artifact. The + unbalanced-tag count is skipped once a real artifact exists so + common explanatory prose does not falsely wipe valid answers.""" + samples = [ + "Here is the page:\n1\nUse the tag for the root.", + "Here is the SVG: Place it inside an page.", + ] + for content in samples: + assert _has_answer_artifact(content), content + assert not _would_reprompt(content), content + + +def test_reprompts_on_empty_html_or_svg_skeleton_mention(): + """```` / ```` with no body content is a + plan-only mention, not a substantive answer.""" + samples = [ + "First, I'll create an skeleton, then add CSS.", + "First, I'll draft a icon, then add shapes.", + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_no_reprompt_on_code_fence_containing_markup_literal(): + """A closed code fence whose body contains literal ````, + ````, ```` strings is still a complete code answer. + The unclosed-markup cross-check operates on text with closed + fences stripped out so code literals do not falsely trip it.""" + samples = [ + ( + "First, let me write the scraper.\n" + "```python\n" + "html = ''\n" + "svg = \"\"\n" + "print(html, svg)\n" + "```" + ), + ( + "First, let me write the parser.\n" + "```javascript\n" + "const open = '';\n" + "const fragment = '';\n" + "console.log(open, fragment);\n" + "```" + ), + ] + for content in samples: + assert _has_answer_artifact(content), content + assert not _would_reprompt(content), content + + +def test_reprompts_on_i_will_gather_identify_numbered_plan(): + """First-person + gather/identify verbs in list items is a tool + stall when the intent appears directly before the list.""" + content = "I'll:\n" "1. Gather the relevant files.\n" "2. Identify the issue." + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_no_reprompt_on_bare_i_need_to_clarification(): + """Bare ``I need to`` clarification or prose answers must NOT + trigger the re-prompt. The phrase is too common in plain answers.""" + samples = [ + "I need to know your operating system before giving the install command.", + "I need to be clear: the answer is Paris.", + 'The sentence is: "I need to leave early today."', + ] + for content in samples: + assert not _would_reprompt(content), content + + +def test_reprompts_on_visit_or_access_numbered_plan(): + """First-person + browser/navigation verbs (visit/access/navigate) + + numbered list is a tool stall.""" + samples = [ + ( + "I'll visit the official site:\n" + "1. Open the homepage.\n" + "2. Read the release notes.\n" + "3. Summarize." + ), + ( + "Let me access the GitHub repo:\n" + "1. Open the README.\n" + "2. Identify the install instructions." + ), + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_no_reprompt_on_inline_backtick_python_prose_after_code(): + """``Use ```python to start a Python fence.`` is prose after a + completed answer; it must NOT be treated as an unclosed fence.""" + content = ( + "Here is the snippet:\n" + "```python\n" + "print(1)\n" + "```\n" + "Use ```python to start a Python block in your reply." + ) + assert _has_answer_artifact(content) + assert not _would_reprompt(content) + + +def test_reprompts_on_take_follow_complete_steps_numbered_plan(): + """``I'll take/follow/complete these steps:`` + numbered list of + work items is a plan stall.""" + samples = [ + ( + "I'll take these steps:\n" + "1. Open the URL.\n" + "2. Read the page.\n" + "3. Summarize the answer." + ), + ( + "I will follow these steps:\n" + "1. Open the current docs.\n" + "2. Read the relevant section.\n" + "3. Answer." + ), + ( + "Let me complete these steps:\n" + "1. Read the uploaded CSV.\n" + "2. Check the totals." + ), + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_no_reprompt_on_prose_mention_of_triple_backticks_after_code(): + """Closed code fence followed by prose that describes triple- + backtick syntax (with leading space after the ticks) must NOT be + treated as an unclosed fence.""" + content = ( + "Here is the snippet:\n" + "```python\n" + "print(1)\n" + "```\n" + "Use ``` to start a markdown code fence in your reply." + ) + assert _has_answer_artifact(content) + assert not _would_reprompt(content) + + +def test_reprompts_on_direct_first_person_read_check_open_plan(): + """Direct first-person intent + open/read/check/review/inspect verbs + + numbered list is a tool stall. The broader verb set applies to + first-person intent only; bare ``First, ...`` and ``Step N: ...`` + keep their narrower verb whitelist.""" + samples = [ + ( + "Let me read the uploaded file:\n" + "1. Identify the columns.\n" + "2. Return the total." + ), + ("I will check the docs:\n" "1. Gather relevant sections.\n" "2. Answer."), + ( + "First, I'll review the repository:\n" + "1. Open the relevant file.\n" + "2. Read the implementation.\n" + "3. Suggest a fix." + ), + ( + "Let me examine the log file:\n" + "1. Open the log.\n" + "2. Read the errors.\n" + "3. Summarize." + ), + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_no_reprompt_on_html_with_inner_svg_or_self_closing_tag(): + """Complete answers that contain nested SVG / self-closing + tags are still complete pages. The unbalanced-count cross-check is + skipped when a real artifact already exists.""" + samples = [ + "", + "" + + "" + + "", + ] + for content in samples: + assert _has_answer_artifact(content), content + assert not _would_reprompt(content), content + + +def test_no_reprompt_on_complete_artifact_with_prose_tag_mention(): + """Complete code/markup artifacts followed by ordinary prose that + mentions ```` or ```` tags are not mid-stream output.""" + samples = [ + "hi\nUse the tag as the root.", + ( + "First, here is the SVG: \n" + "Put it inside an page if needed." + ), + ] + for content in samples: + assert _has_answer_artifact(content), content + assert not _would_reprompt(content), content + + +def test_no_reprompt_on_html_containing_backtick_literal(): + """A complete answer whose body contains a JS string with + literal backticks is still a complete page. The unclosed-fence + cross-check operates on text with closed markup stripped out.""" + content = ( + "First, here is the page.\n" + "" + ) + assert _has_answer_artifact(content) + assert not _would_reprompt(content) + + +def test_empty_markup_before_real_artifact_still_counts_real_artifact(): + """An empty / skeleton that PRECEDES a + real complete artifact must not hide it. _looks_like_real_artifact + iterates every match.""" + samples = [ + ( + "First, the minimal skeleton is . " + "Here is the full page:

Hello

" + ), + ( + "First, the icon skeleton is . " + "Here is the full SVG: " + "" + ), + ] + for content in samples: + assert _has_answer_artifact(content), content + assert not _would_reprompt(content), content + + +def test_doctype_empty_html_skeleton_still_reprompts(): + """```` is an empty skeleton even with + a doctype prefix; the artifact check must reject it.""" + content = ( + "First, I'll create a skeleton, then add CSS." + ) + assert not _has_answer_artifact(content) + assert _would_reprompt(content) + + +def test_reprompts_on_bare_intent_colon_numbered_plan(): + """Bare ``I'll:`` / ``Let me:`` immediately followed by a numbered + list of work verbs is a tool stall regardless of whether the verbs + sit before or in the list.""" + samples = [ + "I'll:\n1. Open the URL.\n2. Read the page.\n3. Summarize the answer.", + "First, I'll:\n1. Create the Python file.\n2. Build the game loop.\n3. Test it.", + "Let me:\n1. Parse the JSON.\n2. Calculate the average.", + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_reprompts_on_intent_plus_numbered_action_items(): + """``First, I'll:\\n1. Load CSV\\n2. Compute total`` is a plan stall: + the work verbs are in the list ITEMS even though no work verb + appears between the intent phrase and the list. Verbs are taken + from the narrow _LOCAL_ACTION_VERBS set (load, parse, calculate, + compute, analyze, run, execute, fetch, download, query, inspect, + extract). Bare ``read`` / ``search`` / ``check`` are NOT in the + set because they appear in non-plan answers too.""" + samples = [ + "First, I'll:\n1. Load the uploaded CSV.\n2. Compute the total revenue.", + "Let me:\n1. Parse the JSON.\n2. Calculate the average.", + "Now I:\n1. Inspect the file.\n2. Analyze the rows.", + "I'll:\n1. Fetch the latest data.\n2. Compute the totals.", + ] + for content in samples: + assert _would_reprompt(content), content + + +def test_reprompts_on_numbered_compare_or_review_lookup_plan(): + """Freshness-gated ``compare`` / ``review`` lookups read as tool + plans and STILL re-prompt as numbered plans.""" + samples = [ + "Here's my plan:\n1. Compare the latest release sources.\n2. Summarise.", + "First, I'll do this:\n1. Review the current documentation.\n2. Answer.", + ] + for s in samples: + assert _would_reprompt(s), s + + +def test_no_reprompt_on_first_use_binary_search_answer(): + """``First, use binary search:`` is an ordinary algorithm answer. + ``use`` is not in the direct-numbered-plan verb whitelist so the + following list stays an answer.""" + content = ( + "First, use binary search:\n" + "1. Search the left half.\n" + "2. Search the right half." + ) + assert _has_answer_artifact(content) + assert not _would_reprompt(content) + + +def test_reprompts_when_later_fence_is_open_after_closed_fence(): + """A response with a complete code fence followed by a SECOND, + unclosed fence is still mid-stream and must re-prompt. The + `_has_unclosed_code_fence` cross-check must short-circuit even + after `_HAS_ANSWER_ARTIFACT` finds the first complete fence.""" + content = ( + "First, let me provide two files:\n" + "```python\n" + "print('main')\n" + "```\n" + "```python\n" + "print('utils')" + ) + assert not _has_answer_artifact(content) + assert _would_reprompt(content) + + +def test_open_fence_with_inner_numbered_list_still_reprompts(): + """A response that opens a code fence and emits numbered lines INSIDE + must NOT count those lines as a completed numbered-list answer.""" + samples = [ + ( + "First, let me write it.\n" + "```text\n" + "1. Install dependencies\n" + "2. Run the app" + ), + ("Let me draft a checklist.\n" "````markdown\n" "1. step one\n" "2. step two"), + ] + for content in samples: + assert not _has_answer_artifact(content), content + assert _would_reprompt(content), content + + +def test_hidden_reasoning_artifact_still_reprompts(): + """When content tokens were emitted but content_accum is empty (a + streaming oddity) and reasoning hides a complete artifact, the user + sees nothing, so the re-prompt MUST still fire.""" + from core.inference.llama_cpp import _REPROMPT_MAX_CHARS + + content_accum = "" + reasoning_accum = ( + "First, let me draft it.\n" "```python\n" "print('hidden answer')\n" "```" + ) + has_content_tokens = True # content existed but was stripped + + visible = content_accum.strip() + reasoning = reasoning_accum.strip() + stripped = visible if visible else reasoning + artifact_text = ( + visible if visible else (reasoning if not has_content_tokens else "") + ) + would_reprompt = bool( + 0 < len(stripped) < _REPROMPT_MAX_CHARS + and _INTENT_SIGNAL.search(stripped) + and not (artifact_text and _has_answer_artifact(artifact_text)) + ) + assert would_reprompt