perf(agent): precompile response and skill-scan regexes (#33208)
strip_think_blocks passed the same response-scrubbing strings through re's pattern dispatcher on every response. Skills Guard repeated the same work for 121 patterns against every scanned line. Compile the existing expressions once and reuse Pattern.sub/search. Keep each generic tool-call tag in its own paired expression so mismatched openers retain their payload while existing stray-closer cleanup remains unchanged. Part of #33208 Salvaged from #32713 by @ErnestHysa. Co-authored-by: ErnestHysa <takis312@hotmail.com>
This commit is contained in:
parent
3b53a3c560
commit
9421c5afdf
|
|
@ -52,6 +52,54 @@ logger = logging.getLogger(__name__)
|
|||
_MAX_AUTH_REFRESH_ATTEMPTS = 2
|
||||
|
||||
|
||||
_REASONING_BLOCK_PATTERNS = (
|
||||
re.compile(r'<think>.*?</think>', re.DOTALL | re.IGNORECASE),
|
||||
re.compile(r'<thinking>.*?</thinking>', re.DOTALL | re.IGNORECASE),
|
||||
re.compile(r'<reasoning>.*?</reasoning>', re.DOTALL | re.IGNORECASE),
|
||||
re.compile(
|
||||
r'<REASONING_SCRATCHPAD>.*?</REASONING_SCRATCHPAD>',
|
||||
re.DOTALL | re.IGNORECASE,
|
||||
),
|
||||
re.compile(r'<thought>.*?</thought>', re.DOTALL | re.IGNORECASE),
|
||||
)
|
||||
|
||||
_TOOL_CALL_BLOCK_PATTERNS = (
|
||||
re.compile(r'<tool_call\b[^>]*>.*?</tool_call>', re.DOTALL | re.IGNORECASE),
|
||||
re.compile(r'<tool_calls\b[^>]*>.*?</tool_calls>', re.DOTALL | re.IGNORECASE),
|
||||
re.compile(r'<tool_result\b[^>]*>.*?</tool_result>', re.DOTALL | re.IGNORECASE),
|
||||
re.compile(
|
||||
r'<function_call\b[^>]*>.*?</function_call>',
|
||||
re.DOTALL | re.IGNORECASE,
|
||||
),
|
||||
re.compile(
|
||||
r'<function_calls\b[^>]*>.*?</function_calls>',
|
||||
re.DOTALL | re.IGNORECASE,
|
||||
),
|
||||
)
|
||||
|
||||
_NAMED_FUNCTION_BLOCK_PATTERN = re.compile(
|
||||
r'(?:(?<=^)|(?<=[\n\r.!?:]))[ \t]*'
|
||||
r'<function\b[^>]*\bname\s*=[^>]*>'
|
||||
r'(?:(?:(?!</function>).)*)</function>',
|
||||
re.DOTALL | re.IGNORECASE,
|
||||
)
|
||||
|
||||
_UNTERMINATED_REASONING_BLOCK_PATTERN = re.compile(
|
||||
r'(?:^|\n)[ \t]*<(?:think|thinking|reasoning|thought|REASONING_SCRATCHPAD)\b[^>]*>.*$',
|
||||
re.DOTALL | re.IGNORECASE,
|
||||
)
|
||||
|
||||
_ORPHAN_REASONING_TAG_PATTERN = re.compile(
|
||||
r'</?(?:think|thinking|reasoning|thought|REASONING_SCRATCHPAD)>\s*',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
_STRAY_TOOL_CALL_CLOSER_PATTERN = re.compile(
|
||||
r'</(?:tool_call|tool_calls|tool_result|function_call|function_calls|function)>\s*',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def _ra():
|
||||
"""Lazy ``run_agent`` reference for test-patch routing."""
|
||||
import run_agent
|
||||
|
|
@ -826,62 +874,31 @@ def strip_think_blocks(agent, content: str) -> str:
|
|||
# 1. Closed tag pairs — case-insensitive for all variants so
|
||||
# mixed-case tags (<THINK>, <Thinking>) don't slip through to
|
||||
# the unterminated-tag pass and take trailing content with them.
|
||||
content = re.sub(r'<think>.*?</think>', '', content, flags=re.DOTALL | re.IGNORECASE)
|
||||
content = re.sub(r'<thinking>.*?</thinking>', '', content, flags=re.DOTALL | re.IGNORECASE)
|
||||
content = re.sub(r'<reasoning>.*?</reasoning>', '', content, flags=re.DOTALL | re.IGNORECASE)
|
||||
content = re.sub(r'<REASONING_SCRATCHPAD>.*?</REASONING_SCRATCHPAD>', '', content, flags=re.DOTALL | re.IGNORECASE)
|
||||
content = re.sub(r'<thought>.*?</thought>', '', content, flags=re.DOTALL | re.IGNORECASE)
|
||||
for _pattern in _REASONING_BLOCK_PATTERNS:
|
||||
content = _pattern.sub('', content)
|
||||
# 1b. Tool-call XML blocks (openclaw/openclaw#67318). Handle the
|
||||
# generic tag names first — they have no attribute gating since
|
||||
# a literal <tool_call> in prose is already vanishingly rare.
|
||||
for _tc_name in ("tool_call", "tool_calls", "tool_result",
|
||||
"function_call", "function_calls"):
|
||||
content = re.sub(
|
||||
rf'<{_tc_name}\b[^>]*>.*?</{_tc_name}>',
|
||||
'',
|
||||
content,
|
||||
flags=re.DOTALL | re.IGNORECASE,
|
||||
)
|
||||
for _pattern in _TOOL_CALL_BLOCK_PATTERNS:
|
||||
content = _pattern.sub('', content)
|
||||
# 1c. <function name="...">...</function> — Gemma-style standalone
|
||||
# tool call. Only strip when the tag sits at a block boundary
|
||||
# (start of text, after a newline, or after sentence-ending
|
||||
# punctuation) AND carries a name="..." attribute. This keeps
|
||||
# prose mentions like "Use <function> to declare" safe.
|
||||
content = re.sub(
|
||||
r'(?:(?<=^)|(?<=[\n\r.!?:]))[ \t]*'
|
||||
r'<function\b[^>]*\bname\s*=[^>]*>'
|
||||
r'(?:(?:(?!</function>).)*)</function>',
|
||||
'',
|
||||
content,
|
||||
flags=re.DOTALL | re.IGNORECASE,
|
||||
)
|
||||
content = _NAMED_FUNCTION_BLOCK_PATTERN.sub('', content)
|
||||
# 2. Unterminated reasoning block — open tag at a block boundary
|
||||
# (start of text, or after a newline) with no matching close.
|
||||
# Strip from the tag to end of string. Fixes #8878 / #9568
|
||||
# (MiniMax M2.7 leaking raw reasoning into assistant content).
|
||||
content = re.sub(
|
||||
r'(?:^|\n)[ \t]*<(?:think|thinking|reasoning|thought|REASONING_SCRATCHPAD)\b[^>]*>.*$',
|
||||
'',
|
||||
content,
|
||||
flags=re.DOTALL | re.IGNORECASE,
|
||||
)
|
||||
content = _UNTERMINATED_REASONING_BLOCK_PATTERN.sub('', content)
|
||||
# 3. Stray orphan open/close tags that slipped through.
|
||||
content = re.sub(
|
||||
r'</?(?:think|thinking|reasoning|thought|REASONING_SCRATCHPAD)>\s*',
|
||||
'',
|
||||
content,
|
||||
flags=re.IGNORECASE,
|
||||
)
|
||||
content = _ORPHAN_REASONING_TAG_PATTERN.sub('', content)
|
||||
# 3b. Stray tool-call closers. (We do NOT strip bare <function> or
|
||||
# unterminated <function name="..."> because a truncated tail
|
||||
# during streaming may still be valuable to the user; matches
|
||||
# OpenClaw's intentional asymmetry.)
|
||||
content = re.sub(
|
||||
r'</(?:tool_call|tool_calls|tool_result|function_call|function_calls|function)>\s*',
|
||||
'',
|
||||
content,
|
||||
flags=re.IGNORECASE,
|
||||
)
|
||||
content = _STRAY_TOOL_CALL_CLOSER_PATTERN.sub('', content)
|
||||
return content
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -418,6 +418,25 @@ class TestStripThinkBlocks:
|
|||
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("text", "expected"),
|
||||
[
|
||||
(
|
||||
"before <tool_call>{x}</function_call> after",
|
||||
"before <tool_call>{x}after",
|
||||
),
|
||||
(
|
||||
"before <function_calls>{x}</tool_calls> after",
|
||||
"before <function_calls>{x}after",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_mismatched_generic_tool_tags_preserve_opener_and_payload(
|
||||
self, agent, text, expected
|
||||
):
|
||||
assert agent._strip_think_blocks(text) == expected
|
||||
|
||||
|
||||
class TestExtractReasoning:
|
||||
def test_reasoning_field(self, agent):
|
||||
msg = _mock_assistant_msg(reasoning="thinking hard")
|
||||
|
|
@ -5805,4 +5824,3 @@ class TestMemoryContextSanitization:
|
|||
assert "stale observation" not in result
|
||||
assert "how is the honcho working" in result
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -523,6 +523,11 @@ THREAT_PATTERNS = [
|
|||
"instructs agent to send data to a URL"),
|
||||
]
|
||||
|
||||
_COMPILED_THREAT_PATTERNS = [
|
||||
(re.compile(pattern, re.IGNORECASE), pid, severity, category, description)
|
||||
for pattern, pid, severity, category, description in THREAT_PATTERNS
|
||||
]
|
||||
|
||||
# Structural limits for skill directories
|
||||
MAX_FILE_COUNT = 50 # skills shouldn't have 50+ files
|
||||
MAX_TOTAL_SIZE_KB = 1024 # 1MB total is suspicious for a skill
|
||||
|
|
@ -594,11 +599,11 @@ def scan_file(file_path: Path, rel_path: str = "") -> List[Finding]:
|
|||
seen = set() # (pattern_id, line_number) for deduplication
|
||||
|
||||
# Regex pattern matching
|
||||
for pattern, pid, severity, category, description in THREAT_PATTERNS:
|
||||
for pattern, pid, severity, category, description in _COMPILED_THREAT_PATTERNS:
|
||||
for i, line in enumerate(lines, start=1):
|
||||
if (pid, i) in seen:
|
||||
continue
|
||||
if re.search(pattern, line, re.IGNORECASE):
|
||||
if pattern.search(line):
|
||||
seen.add((pid, i))
|
||||
matched_text = line.strip()
|
||||
if len(matched_text) > 120:
|
||||
|
|
|
|||
Loading…
Reference in New Issue