diff --git a/changedetectionio/llm/response_parser.py b/changedetectionio/llm/response_parser.py index 6c30603ef..5f3f453d2 100644 --- a/changedetectionio/llm/response_parser.py +++ b/changedetectionio/llm/response_parser.py @@ -16,6 +16,14 @@ _POSITIONAL_SELECTOR_RE = re.compile( r'nth-child|nth-of-type|:eq\(|\[\d+\]|\/\/\*\[\d', re.IGNORECASE ) +# Reasoning models (DeepSeek-R1, Qwen reasoning, etc.) wrap their scratchpad in tags. +# Three shapes have to be handled, because the scratchpad routinely contains JSON of its own +# ("initially I thought {"important": false}, but..."), so leaving any of it in place lets +# _extract_json lock onto a discarded intermediate answer instead of the real one. +_THINK_BLOCK_RE = re.compile(r'.*?', re.DOTALL | re.IGNORECASE) +_THINK_TAIL_RE = re.compile(r'^.*', re.DOTALL | re.IGNORECASE) +_THINK_OPEN_RE = re.compile(r'', re.IGNORECASE) + def _to_bool(value, default: bool = False) -> bool: """Safely coerce boolean values from LLM responses. @@ -33,10 +41,24 @@ def _to_bool(value, default: bool = False) -> bool: def _extract_json(raw: str) -> str: - """Strip reasoning blocks, markdown fences, and extract the first JSON object.""" + """Strip reasoning blocks, markdown fences, and extract the first JSON object. + + Raises: + ValueError: the response opens a reasoning block it never closes, i.e. it was cut + off mid-thought (usually by max_tokens) and contains no answer at all. Callers + in evaluator.py catch this and fall back safely - for diff evaluation that + means passing the change through as important rather than silently dropping it. + """ raw = raw.strip() - # Strip ... blocks emitted by reasoning models (DeepSeek-R1, Qwen reasoning, etc.) - raw = re.sub(r'.*?', '', raw, flags=re.DOTALL | re.IGNORECASE).strip() + # Well-formed scratchpads. + raw = _THINK_BLOCK_RE.sub('', raw).strip() + # Some providers/chat templates emit the opening tag themselves and only the closer comes + # back over the wire, so anything up to the last closer is still scratchpad. + raw = _THINK_TAIL_RE.sub('', raw).strip() + # An opener with no closer means the response was truncated part-way through reasoning. + # There is no answer to find; the only JSON present would be a discarded intermediate one. + if _THINK_OPEN_RE.search(raw): + raise ValueError('LLM response contains an unterminated reasoning block (truncated?)') # Remove ```json ... ``` or ``` ... ``` fences raw = re.sub(r'^```(?:json)?\s*', '', raw, flags=re.MULTILINE) raw = re.sub(r'\s*```$', '', raw, flags=re.MULTILINE) diff --git a/changedetectionio/tests/llm/test_response_parser.py b/changedetectionio/tests/llm/test_response_parser.py index f6a098869..79fbdd5a2 100644 --- a/changedetectionio/tests/llm/test_response_parser.py +++ b/changedetectionio/tests/llm/test_response_parser.py @@ -4,6 +4,8 @@ Unit tests for changedetectionio/llm/response_parser.py All functions are pure — no external dependencies needed. """ +import pytest + from changedetectionio.llm.response_parser import ( _extract_json, parse_eval_response, @@ -58,6 +60,67 @@ class TestExtractJson: assert '' not in result +class TestReasoningBlockEdgeCases: + """A reasoning scratchpad usually contains JSON of its own, so any leftover scratchpad + lets _extract_json return a discarded intermediate answer. Every shape below carries a + misleading `"important": false` in the scratchpad and the real verdict outside it.""" + + def test_closing_tag_only_is_still_stripped(self): + # Several providers/chat templates inject the opening tag themselves, so only the + # closer comes back over the wire. + raw = ( + 'My first read was {"important": false, "summary": "nothing"}\n' + '\n' + '{"important": true, "summary": "Price dropped"}' + ) + assert _extract_json(raw) == '{"important": true, "summary": "Price dropped"}' + assert parse_eval_response(raw) == { + 'important': True, + 'summary': 'Price dropped', + } + + def test_thinking_tag_variant_is_stripped(self): + raw = ( + 'weighing {"important": false, "summary": "no"}\n' + '{"important": true, "summary": "Price dropped"}' + ) + assert parse_eval_response(raw)['important'] is True + + def test_multiple_reasoning_blocks_are_stripped(self): + raw = ( + 'step one' + '{"important": false, "summary": "no"}' + '{"important": true, "summary": "Price dropped"}' + ) + assert parse_eval_response(raw)['important'] is True + + def test_unterminated_reasoning_block_raises(self): + # Truncated by max_tokens mid-thought: the only JSON present is the abandoned guess, + # so returning it would silently invert the verdict. Raise instead and let + # evaluator.py's handler fall back to "important" rather than dropping the change. + raw = ( + '\n' + 'First guess: {"important": false, "summary": "nothing"}\n' + 'But actually the price dropped, so' + ) + with pytest.raises(ValueError, match='unterminated reasoning block'): + _extract_json(raw) + + def test_unterminated_block_propagates_out_of_parse_eval_response(self): + """Deliberately NOT swallowed. parse_eval_response's own fallback is + important=False, which suppresses the notification - the opposite of what + evaluator.py wants on failure ("don't suppress the notification"). Letting + ValueError escape routes it to that handler instead. Do not add ValueError to + the except tuple in parse_eval_response.""" + raw = 'truncated mid-thought {"important": false}' + with pytest.raises(ValueError): + parse_eval_response(raw) + + def test_response_with_no_reasoning_block_is_untouched(self): + raw = '{"important": true, "summary": "plain"}' + assert _extract_json(raw) == raw + + class TestParseEvalResponse: def test_valid_important_true(self): raw = '{"important": true, "summary": "Price dropped from $500 to $400"}'