From 7dbe18ab51cd7262e7588bd172a4976bc73a9084 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 19 May 2026 01:43:39 +0000 Subject: [PATCH] studio: line-anchor trailing-plan list items and ship a backend pytest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While drafting backend/tests/test_trailing_plan.py for the changes landed in b4e0985, the new tests surfaced a deeper false-positive the earlier regex tightening missed. For input "Let me explain:\n1. The function returns 42.\n\nThat's the answer." the previous `(?:\s*(?:[-*•]|\d+\.)\s+[^\n]+\n?)+` allowed the regex engine to backtrack and treat the in-prose "42." substring as a second list-item marker: iter 1 consumed "1. The function returns 4" and iter 2 consumed "2.\n\nThat's the answer." (with `\s+` greedily crossing the empty-line break). The pattern then satisfied `\s*\Z` and the buffer fired a spurious `Continue.` retry on what was already a fully-formed answer. Tighten the per-item boundary: - `[ \t]*` before the marker (no newlines): forces the marker to sit at the start of its own line. A mid-prose "42." cannot satisfy this because the engine cannot rewind past the preceding `\n` without invalidating the previous iteration's `[^\n]+\n` close. - `[ \t]+` between the marker and content: blocks an `\s+`-driven cross-newline reach into a closing paragraph. - `(?:\n|\Z)` at end of item: a real line break OR end of buffer. Preserves the "list at EOB with no trailing newline" case while eliminating the backtrack route. Land backend/tests/test_trailing_plan.py at the same time, covering: - `_TRAILING_PLAN_INTENT` "let me know" closer exclusion (cycle-3 fix). - `_TRAILING_PLAN_LIST` correctly fires on genuine trailing lists (dash, asterisk, unicode bullet, numeric). - `_TRAILING_PLAN_LIST` does NOT fire on list + closing paragraph, list + closing sentence, list embedded mid-text, or the "42." in-prose digit case. - `_TRAILING_PLAN_COLON` fires on bare trailing intent-colons only. - `_trailing_plan_hit` composite cases. - The 600-char window slicing. 31 cases, all pass. Pins the regex behaviour against future regressions inside the repo (the prior pin script lived only in the probing workspace, not the studio tree). --- studio/backend/core/inference/llama_cpp.py | 16 ++- studio/backend/tests/test_trailing_plan.py | 150 +++++++++++++++++++++ 2 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 studio/backend/tests/test_trailing_plan.py diff --git a/studio/backend/core/inference/llama_cpp.py b/studio/backend/core/inference/llama_cpp.py index 37c8d61f55..a1b7734301 100644 --- a/studio/backend/core/inference/llama_cpp.py +++ b/studio/backend/core/inference/llama_cpp.py @@ -73,15 +73,23 @@ _TRAILING_PLAN_INTENT = re.compile( r")[^.!?\n]*[.!?]?\s*$" ) _TRAILING_PLAN_LIST = re.compile( - # No `m` flag: terminal `\s*$` must match end-of-string, not end-of-line. - # With `m` an answer like "1. one\n2. two\n\nDone." would still match on - # the list block and trigger a spurious auto-continue. + # No `m` flag: the trailing anchor must match end-of-string, not + # end-of-line. With `m` an answer like "1. one\n2. two\n\nDone." would + # still match on the list block and trigger a spurious auto-continue. + # + # Anchors below also defend against a subtler backtrack: each list + # item starts with `[ \t]*` (horizontal whitespace only, NOT `\s*`) + # so the marker must sit on its own line, not be picked up mid-prose + # via a substring like "42." inside a sentence such as + # "Let me explain:\n1. The function returns 42.\n\nThat's the answer." + # The item content ends with `(?:\n|\Z)` so the iteration boundary + # is a real line break or end-of-buffer. r"(?i)" r"(?:let me|i['’]ll|i will|i['’]m going to|i am going to|" r"here['’]?s (?:my |the |a )?(?:plan|approach|steps?)|" r"as follows|the (?:plan|steps?) (?:is|are))" r"[^:\n]{0,160}:\s*\n" - r"(?:\s*(?:[-*•]|\d+\.)\s+[^\n]+\n?)+" + r"(?:[ \t]*(?:[-*•]|\d+\.)[ \t]+[^\n]+(?:\n|\Z))+" r"\s*\Z" ) _TRAILING_PLAN_COLON = re.compile( diff --git a/studio/backend/tests/test_trailing_plan.py b/studio/backend/tests/test_trailing_plan.py new file mode 100644 index 0000000000..d4c5cc7e07 --- /dev/null +++ b/studio/backend/tests/test_trailing_plan.py @@ -0,0 +1,150 @@ +# 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 mid-plan auto-continue regexes and ``_trailing_plan_hit``. + +The trailing-plan detector lives in ``core.inference.llama_cpp`` and decides +whether the model just stopped mid-plan (and therefore deserves a neutral +``Continue.`` re-prompt). False positives cost real tool calls and latency, +so the patterns get explicit coverage here. + +Bug history pinned by these tests: + +* ``"If you need anything else, let me know."`` matched ``_TRAILING_PLAN_INTENT`` + before the negative lookahead landed. +* ``"Here's my plan:\\n- a\\n- b\\n\\nDone, that should work."`` matched + ``_TRAILING_PLAN_LIST`` before the regex was switched off the ``m`` flag + and re-anchored with ``\\Z``. +""" + +from __future__ import annotations + +import pytest + +from core.inference.llama_cpp import ( + _TRAILING_PLAN_COLON, + _TRAILING_PLAN_INTENT, + _TRAILING_PLAN_LIST, + _trailing_plan_hit, +) + + +# ----- _TRAILING_PLAN_INTENT ------------------------------------------------- + + +@pytest.mark.parametrize( + "text,expected", + [ + # Closing phrases must NOT match (regression: "let me know") + ("If you need anything else, let me know.", False), + ("Let me know if I can help further.", False), + ("let me know!", False), + # Genuine mid-plan intent SHOULD match + ("Let me clone the repo.", True), + ("Let me check the file.", True), + ("Now let me run the tests.", True), + ("I'll now run the analyzer.", True), + ("I’ll now run the analyzer.", True), # curly apostrophe + ("I will now begin.", True), + # Unrelated trailing text must NOT match + ("Hello world.", False), + ("The answer is 42.", False), + ], +) +def test_trailing_plan_intent(text: str, expected: bool) -> None: + assert bool(_TRAILING_PLAN_INTENT.search(text)) is expected + + +# ----- _TRAILING_PLAN_LIST --------------------------------------------------- + + +@pytest.mark.parametrize( + "text,expected", + [ + # List block at end of buffer SHOULD match + ("Let me do this:\n- step one\n- step two\n", True), + ("Here's my plan:\n1. one\n2. two\n", True), + ("Here's my plan:\n1. one\n2. two\n \n", True), # trailing whitespace + # Unicode bullet at end of buffer SHOULD match + ("Let me try:\n• first\n• second\n", True), + # List followed by a closing sentence MUST NOT match (regression: + # the `m` flag in `(?ims)` previously let `\s*$` match end-of-line) + ( + "Here's my plan:\n- step one\n- step two\n\nDone, hope that helps.", + False, + ), + ( + "Let me walk through it:\n1. first\n2. second\n\nThat's everything.", + False, + ), + # Single-item numbered list followed by closing prose MUST NOT match + ( + "Let me explain:\n1. The function returns 42.\n\nThat's the answer.", + False, + ), + # List embedded mid-text (not trailing) MUST NOT match + ( + "Here's my plan:\n- step one\n- step two\nNow the conclusion follows.", + False, + ), + ], +) +def test_trailing_plan_list(text: str, expected: bool) -> None: + assert bool(_TRAILING_PLAN_LIST.search(text)) is expected + + +# ----- _TRAILING_PLAN_COLON -------------------------------------------------- + + +@pytest.mark.parametrize( + "text,expected", + [ + # Bare trailing colon SHOULD match + ("Let me check the repo:", True), + ("I'll now look at this:", True), + # Colon mid-sentence MUST NOT match + ("Let me check this: it should work fine.", False), + # Colon not in an intent-cue clause MUST NOT match + ("The result is:", False), + ], +) +def test_trailing_plan_colon(text: str, expected: bool) -> None: + assert bool(_TRAILING_PLAN_COLON.search(text)) is expected + + +# ----- _trailing_plan_hit composite ----------------------------------------- + + +@pytest.mark.parametrize( + "text,expected", + [ + # Any of the three sub-patterns triggers a hit + ("Now let me run the tests.", True), + ("Let me do this:\n- step one\n- step two\n", True), + ("Let me check the repo:", True), + # Negative cases that previously misfired + ("If you need anything else, let me know.", False), + ( + "Here's my plan:\n- step one\n- step two\n\nDone, hope that helps.", + False, + ), + # Short empty string is a no-op + ("", False), + (" ", False), + ], +) +def test_trailing_plan_hit(text: str, expected: bool) -> None: + assert _trailing_plan_hit(text) is expected + + +# ----- window slicing -------------------------------------------------------- + + +def test_trailing_plan_hit_respects_window() -> None: + """An intent cue further back than ``_TRAILING_PLAN_WINDOW`` must NOT + trigger a hit; only the tail of the response is inspected.""" + + # 800-char prefix of unrelated text, then a finalising sentence. + prefix = "lorem ipsum " * 80 # ~960 chars + text = f"Let me check the repo. {prefix}The result is 42." + assert _trailing_plan_hit(text) is False