fix(llm): handle reasoning blocks that aren't a well-formed <think>...</think> pair (#4349)

Follow-up to #4340. The <think> stripper added there only matches a closed pair, but a
reasoning scratchpad routinely contains JSON of its own ("initially I thought
{"important": false}, but..."), so any leftover scratchpad lets _extract_json lock onto
a discarded intermediate answer. Three shapes slipped through, all of which inverted the
verdict to important=false and therefore silently suppressed the notification:

- opener stripped by the provider/chat template, only </think> comes back over the wire
- <thinking> spelled out in full
- unterminated block, i.e. the response was cut off mid-thought by max_tokens

The first two are now stripped. The third raises ValueError, because a truncated response
holds no answer at all - only the abandoned guess. Raising routes it to the existing
handler in evaluator.py, which passes the change through as important rather than dropping
it; parse_eval_response deliberately does not catch ValueError, since its own fallback
(important=False) would suppress the notification instead.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

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 <think> 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'<think(?:ing)?>.*?</think(?:ing)?>', re.DOTALL | re.IGNORECASE)
+_THINK_TAIL_RE = re.compile(r'^.*</think(?:ing)?>', re.DOTALL | re.IGNORECASE)
+_THINK_OPEN_RE = re.compile(r'<think(?:ing)?>', 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 <think> ... </think> blocks emitted by reasoning models (DeepSeek-R1, Qwen reasoning, etc.)
-    raw = re.sub(r'<think>.*?</think>', '', 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)

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 '<think>' 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'
+            '</think>\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 = (
+            '<thinking>weighing {"important": false, "summary": "no"}</thinking>\n'
+            '{"important": true, "summary": "Price dropped"}'
+        )
+        assert parse_eval_response(raw)['important'] is True
+
+    def test_multiple_reasoning_blocks_are_stripped(self):
+        raw = (
+            '<think>step one</think>'
+            '<think>{"important": false, "summary": "no"}</think>'
+            '{"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 = (
+            '<think>\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 = '<think>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"}'