Studio: harden re-prompt artifact regex for CRLF + catastrophic backtracking
Two robustness fixes for the `_HAS_ANSWER_ARTIFACT` regex from the parent commit, both caught by a thorough simulation suite covering Linux/Mac/Windows line-ending portability and adversarial inputs. 1. **CRLF line endings.** The original `\n` literals missed Windows- authored or CRLF-converted content (model echoing a pasted prompt, etc.). Replaced with `\r?\n` everywhere a newline is required, so closed code fences, numbered lists, and end-to-end re-prompt decisions all work on `\r\n` as well as `\n`. 2. **Catastrophic backtracking on whitespace spam.** The numbered-list alternative `(?:^|\r?\n)\s*\d+\.\s+\S.*?\r?\n\s*\d+\.` was O(n^2) on long whitespace runs: `\s*` greedy + `\d+` failing + `\s` matching `\r\n` led to repeated backtracking through the newline characters. Measured at ~630ms for 10KB of `\r\n` repeats. Fix: restrict the post-newline indent to `[ \t]*` (spaces / tabs only). After `\r?\n` we are at column 0 and only spaces / tabs are a sensible leading indent for a list item; greedy whitespace was never needed. New worst case on the same input: <1ms (1000x speedup). Added 5 in-tree tests: - test_artifact_regex_handles_crlf_code_fence - test_artifact_regex_handles_crlf_numbered_list - test_artifact_regex_handles_mixed_lf_crlf - test_no_backtrack_on_crlf_spam (asserts <50ms on 10KB \r\n) - test_no_reprompt_on_crlf_complete_python_game All 18 reprompt-guard tests pass. All 253 llama_cpp-related tests pass. Out-of-tree simulation suite (84 tests) passes on both Python 3.12 and Python 3.13 inside isolated uv venvs.
This commit is contained in:
parent
cb6ebc032a
commit
2db8b81854
2 changed files with 65 additions and 2 deletions
|
|
@ -79,12 +79,19 @@ _MAX_REPROMPTS = 3
|
|||
# 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.
|
||||
#
|
||||
# `\r?\n` is used everywhere a newline is required so Windows-authored or
|
||||
# CRLF-converted content still matches. The numbered-list indent uses
|
||||
# `[ \t]*` (spaces / tabs only) rather than `\s*` so the regex stays
|
||||
# linear on long whitespace runs -- greedy `\s*` + failing `\d+` caused
|
||||
# O(n^2) backtracking through embedded `\r\n` characters on adversarial
|
||||
# inputs.
|
||||
_HAS_ANSWER_ARTIFACT = re.compile(
|
||||
r"```[a-zA-Z]*\n[\s\S]+?\n```" # closed code fence
|
||||
r"```[a-zA-Z]*\r?\n[\s\S]+?\r?\n```" # closed code fence
|
||||
r"|<!doctype\b" # HTML page
|
||||
r"|<html\b"
|
||||
r"|<svg\b[\s\S]*?</svg>" # complete SVG
|
||||
r"|(?:^|\n)\s*\d+\.\s+\S.*?\n\s*\d+\.", # 2+ numbered list items
|
||||
r"|(?:^|\r?\n)[ \t]*\d+\.[ \t]+\S.*?\r?\n[ \t]*\d+\.", # 2+ numbered list items
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -205,3 +205,59 @@ 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)
|
||||
|
||||
|
||||
# ── 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.search(content), (
|
||||
"CRLF (\\r\\n) line endings inside a code fence must still match"
|
||||
)
|
||||
|
||||
|
||||
def test_artifact_regex_handles_crlf_numbered_list():
|
||||
"""CRLF numbered list also matches."""
|
||||
content = "Here's the plan:\r\n1. one\r\n2. two\r\n"
|
||||
assert _HAS_ANSWER_ARTIFACT.search(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.search(content)
|
||||
|
||||
|
||||
def test_no_backtrack_on_crlf_spam():
|
||||
"""10K of `\\r\\n` repeats must complete fast.
|
||||
|
||||
Pre-fix the numbered-list alternative `(?:^|\\r?\\n)\\s*\\d+\\.` would
|
||||
O(n^2)-backtrack on this kind of input (measured at ~630ms for 10KB
|
||||
of `\\r\\n` repeats). The post-fix `[ \\t]*` indent restriction
|
||||
keeps it linear.
|
||||
"""
|
||||
import time
|
||||
payload = "\r\n" * 5000
|
||||
t0 = time.time()
|
||||
_HAS_ANSWER_ARTIFACT.search(payload)
|
||||
elapsed_ms = (time.time() - t0) * 1000
|
||||
assert elapsed_ms < 50, f"regex took {elapsed_ms:.1f}ms on 10KB CRLF spam"
|
||||
|
||||
|
||||
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), (
|
||||
"CRLF-encoded complete fence must also suppress the re-prompt"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue