mirror of
https://github.com/dgtlmoon/changedetection.io.git
synced 2026-09-26 23:35:51 +00:00
fix(llm): string boolean parsing, reasoning model <think> stripping, and timestamp diff annotation (#4340)
- Simplify _to_bool using changedetectionio.strtobool.strtobool - Strip <think>...</think> reasoning blocks in _extract_json for reasoning models - Fix _annotate_moved_lines short-circuit so standalone relative timestamps are always annotated - Add comprehensive unit tests in test_response_parser and test_prompt_builder
This commit is contained in:
@@ -9,39 +9,44 @@ from .bm25_trim import trim_to_relevant
|
||||
|
||||
_AGO_RE = re.compile(r'^\d+\s+\w+\s+ago$', re.IGNORECASE)
|
||||
|
||||
SNAPSHOT_CONTEXT_CHARS = 30_000 # current page state excerpt sent alongside the diff
|
||||
SNAPSHOT_CONTEXT_CHARS = 30_000 # current page state excerpt sent alongside the diff
|
||||
|
||||
|
||||
def _annotate_moved_lines(diff_text: str) -> str:
|
||||
"""
|
||||
Pre-process a unified diff to mark lines that appear on both the + and - sides
|
||||
as [MOVED] rather than genuinely added/removed. This prevents the LLM from
|
||||
as [MOVED] (~ prefix) rather than genuinely added/removed. This prevents the LLM from
|
||||
incorrectly classifying repositioned content as new or deleted.
|
||||
Also marks standalone relative timestamps (e.g. '3 hours ago') as ~ trivial.
|
||||
|
||||
Lines are compared after stripping leading +/- and whitespace so that
|
||||
indentation changes don't prevent matching.
|
||||
"""
|
||||
lines = diff_text.splitlines()
|
||||
added_texts = {l[1:].strip().lower() for l in lines if l.startswith('+') and l[1:].strip()}
|
||||
removed_texts = {l[1:].strip().lower() for l in lines if l.startswith('-') and l[1:].strip()}
|
||||
moved_texts = added_texts & removed_texts
|
||||
|
||||
if not moved_texts:
|
||||
return diff_text
|
||||
added_texts = {
|
||||
line[1:].strip().lower() for line in lines if line.startswith('+') and line[1:].strip()
|
||||
}
|
||||
removed_texts = {
|
||||
line[1:].strip().lower() for line in lines if line.startswith('-') and line[1:].strip()
|
||||
}
|
||||
moved_texts = added_texts & removed_texts
|
||||
|
||||
result = []
|
||||
has_changes = False
|
||||
for line in lines:
|
||||
if line.startswith(('+', '-')):
|
||||
bare = line[1:].strip().lower()
|
||||
if bare in moved_texts or _AGO_RE.match(line[1:].strip()):
|
||||
result.append(f'~{line[1:]}') # ~ prefix = moved/reordered/trivial, skip
|
||||
has_changes = True
|
||||
continue
|
||||
result.append(line)
|
||||
return '\n'.join(result)
|
||||
return '\n'.join(result) if has_changes else diff_text
|
||||
|
||||
|
||||
def build_eval_prompt(intent: str, diff: str, current_snapshot: str = '',
|
||||
url: str = '', title: str = '') -> str:
|
||||
def build_eval_prompt(
|
||||
intent: str, diff: str, current_snapshot: str = '', url: str = '', title: str = ''
|
||||
) -> str:
|
||||
"""
|
||||
Build the user message for a diff evaluation call.
|
||||
The system prompt is kept separate (see build_eval_system_prompt).
|
||||
@@ -131,8 +136,9 @@ def build_preview_system_prompt() -> str:
|
||||
)
|
||||
|
||||
|
||||
def build_change_summary_prompt(diff: str, custom_prompt: str,
|
||||
current_snapshot: str = '', url: str = '', title: str = '') -> str:
|
||||
def build_change_summary_prompt(
|
||||
diff: str, custom_prompt: str, current_snapshot: str = '', url: str = '', title: str = ''
|
||||
) -> str:
|
||||
"""
|
||||
Build the user message for an AI Change Summary call.
|
||||
The user supplies their own instructions (custom_prompt); this wraps them
|
||||
|
||||
@@ -9,16 +9,34 @@ text. This module handles those cases gracefully.
|
||||
import json
|
||||
import re
|
||||
|
||||
from changedetectionio.strtobool import strtobool
|
||||
|
||||
# Positional selectors are fragile — reject them even if the LLM generates them
|
||||
_POSITIONAL_SELECTOR_RE = re.compile(
|
||||
r'nth-child|nth-of-type|:eq\(|\[\d+\]|\/\/\*\[\d',
|
||||
re.IGNORECASE
|
||||
r'nth-child|nth-of-type|:eq\(|\[\d+\]|\/\/\*\[\d', re.IGNORECASE
|
||||
)
|
||||
|
||||
|
||||
def _to_bool(value, default: bool = False) -> bool:
|
||||
"""Safely coerce boolean values from LLM responses.
|
||||
|
||||
Handles native booleans, truthy/falsy integers (1/0), and string booleans
|
||||
("true", "false", "yes", "no", "1", "0") using strtobool.
|
||||
Avoids Python's bool("false") -> True bug on stringified JSON booleans.
|
||||
"""
|
||||
if value is None:
|
||||
return default
|
||||
try:
|
||||
return strtobool(value)
|
||||
except (ValueError, AttributeError):
|
||||
return default
|
||||
|
||||
|
||||
def _extract_json(raw: str) -> str:
|
||||
"""Strip markdown fences and extract the first JSON object."""
|
||||
"""Strip reasoning blocks, markdown fences, and extract the first JSON object."""
|
||||
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()
|
||||
# Remove ```json ... ``` or ``` ... ``` fences
|
||||
raw = re.sub(r'^```(?:json)?\s*', '', raw, flags=re.MULTILINE)
|
||||
raw = re.sub(r'\s*```$', '', raw, flags=re.MULTILINE)
|
||||
@@ -36,7 +54,7 @@ def parse_eval_response(raw: str) -> dict:
|
||||
try:
|
||||
data = json.loads(_extract_json(raw))
|
||||
return {
|
||||
'important': bool(data.get('important', False)),
|
||||
'important': _to_bool(data.get('important'), default=False),
|
||||
'summary': str(data.get('summary', '')).strip(),
|
||||
}
|
||||
except (json.JSONDecodeError, AttributeError):
|
||||
@@ -52,7 +70,7 @@ def parse_preview_response(raw: str) -> dict:
|
||||
try:
|
||||
data = json.loads(_extract_json(raw))
|
||||
return {
|
||||
'found': bool(data.get('found', False)),
|
||||
'found': _to_bool(data.get('found'), default=False),
|
||||
'answer': str(data.get('answer', '')).strip(),
|
||||
}
|
||||
except (json.JSONDecodeError, AttributeError):
|
||||
@@ -67,7 +85,7 @@ def parse_setup_response(raw: str) -> dict:
|
||||
"""
|
||||
try:
|
||||
data = json.loads(_extract_json(raw))
|
||||
needs = bool(data.get('needs_prefilter', False))
|
||||
needs = _to_bool(data.get('needs_prefilter'), default=False)
|
||||
selector = data.get('selector') or None
|
||||
|
||||
# Sanitise: reject positional selectors
|
||||
|
||||
@@ -3,16 +3,37 @@ Unit tests for changedetectionio/llm/prompt_builder.py
|
||||
|
||||
All functions are pure — no external dependencies needed.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from changedetectionio.llm.prompt_builder import (
|
||||
_annotate_moved_lines,
|
||||
build_eval_prompt,
|
||||
build_eval_system_prompt,
|
||||
build_setup_prompt,
|
||||
build_setup_system_prompt,
|
||||
SNAPSHOT_CONTEXT_CHARS,
|
||||
)
|
||||
|
||||
|
||||
class TestAnnotateMovedLines:
|
||||
def test_annotate_moved_lines_marks_reordered_content(self):
|
||||
diff = "- Item Alpha\n+ Item Beta\n+ Item Alpha\n- Item Beta"
|
||||
annotated = _annotate_moved_lines(diff)
|
||||
assert "~ Item Alpha" in annotated
|
||||
assert "~ Item Beta" in annotated
|
||||
|
||||
def test_annotate_standalone_timestamp_without_moved_lines(self):
|
||||
# Even when there are NO moved lines, standalone relative timestamps must be annotated
|
||||
diff = "- 2 hours ago\n+ 3 hours ago\n+ Genuine new article headline"
|
||||
annotated = _annotate_moved_lines(diff)
|
||||
assert "~ 2 hours ago" in annotated
|
||||
assert "~ 3 hours ago" in annotated
|
||||
assert "+ Genuine new article headline" in annotated
|
||||
|
||||
def test_unrelated_diff_remains_unchanged(self):
|
||||
diff = "- Old price: $100\n+ New price: $80"
|
||||
annotated = _annotate_moved_lines(diff)
|
||||
assert annotated == diff
|
||||
|
||||
|
||||
class TestBuildEvalPrompt:
|
||||
def test_contains_intent(self):
|
||||
prompt = build_eval_prompt(intent='Alert on price drops', diff='- $500\n+ $400')
|
||||
|
||||
@@ -3,10 +3,11 @@ 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,
|
||||
parse_preview_response,
|
||||
parse_setup_response,
|
||||
)
|
||||
|
||||
@@ -37,6 +38,25 @@ class TestExtractJson:
|
||||
result = _extract_json(raw)
|
||||
assert '"important"' in result
|
||||
|
||||
def test_strips_reasoning_think_tags(self):
|
||||
raw = (
|
||||
'<think>\n'
|
||||
'Let us consider if {"important": false} is right. Actually, yes.\n'
|
||||
'</think>\n'
|
||||
'{"important": true, "summary": "Price fell to $300"}'
|
||||
)
|
||||
result = _extract_json(raw)
|
||||
assert result == '{"important": true, "summary": "Price fell to $300"}'
|
||||
|
||||
def test_strips_reasoning_think_tags_with_code_fence(self):
|
||||
raw = (
|
||||
'<think>\nThinking about the price change.\n</think>\n'
|
||||
'```json\n{"important": false, "summary": "Cosmetic change only"}\n```'
|
||||
)
|
||||
result = _extract_json(raw)
|
||||
assert '"important"' in result
|
||||
assert '<think>' not in result
|
||||
|
||||
|
||||
class TestParseEvalResponse:
|
||||
def test_valid_important_true(self):
|
||||
@@ -51,12 +71,36 @@ class TestParseEvalResponse:
|
||||
assert result['important'] is False
|
||||
assert 'date counter' in result['summary']
|
||||
|
||||
def test_string_false_evaluates_to_false(self):
|
||||
raw = '{"important": "false", "summary": "No relevant changes found"}'
|
||||
result = parse_eval_response(raw)
|
||||
assert result['important'] is False
|
||||
assert result['summary'] == 'No relevant changes found'
|
||||
|
||||
def test_string_true_evaluates_to_true(self):
|
||||
raw = '{"important": "true", "summary": "Price updated"}'
|
||||
result = parse_eval_response(raw)
|
||||
assert result['important'] is True
|
||||
assert result['summary'] == 'Price updated'
|
||||
|
||||
def test_markdown_fenced_response(self):
|
||||
raw = '```json\n{"important": true, "summary": "New job posted"}\n```'
|
||||
result = parse_eval_response(raw)
|
||||
assert result['important'] is True
|
||||
assert result['summary'] == 'New job posted'
|
||||
|
||||
def test_reasoning_model_response_parsed_correctly(self):
|
||||
raw = (
|
||||
'<think>\n'
|
||||
'1. Checking diff: {"important": false} was our initial thought.\n'
|
||||
'2. However the price dropped from $100 to $80.\n'
|
||||
'</think>\n'
|
||||
'{"important": true, "summary": "Price dropped by $20"}'
|
||||
)
|
||||
result = parse_eval_response(raw)
|
||||
assert result['important'] is True
|
||||
assert result['summary'] == 'Price dropped by $20'
|
||||
|
||||
def test_malformed_json_falls_back_to_safe_default(self):
|
||||
result = parse_eval_response('this is not json at all')
|
||||
assert result['important'] is False
|
||||
@@ -71,6 +115,11 @@ class TestParseEvalResponse:
|
||||
result = parse_eval_response(raw)
|
||||
assert result['important'] is True
|
||||
|
||||
def test_falsy_integer_coerced_to_bool(self):
|
||||
raw = '{"important": 0, "summary": "no"}'
|
||||
result = parse_eval_response(raw)
|
||||
assert result['important'] is False
|
||||
|
||||
def test_summary_stripped_of_whitespace(self):
|
||||
raw = '{"important": false, "summary": " no match "}'
|
||||
result = parse_eval_response(raw)
|
||||
@@ -88,6 +137,32 @@ class TestParseEvalResponse:
|
||||
assert result['summary'] == 'skip'
|
||||
|
||||
|
||||
class TestParsePreviewResponse:
|
||||
def test_valid_found_true(self):
|
||||
raw = '{"found": true, "answer": "Price is $49.99"}'
|
||||
result = parse_preview_response(raw)
|
||||
assert result['found'] is True
|
||||
assert result['answer'] == 'Price is $49.99'
|
||||
|
||||
def test_valid_found_false(self):
|
||||
raw = '{"found": false, "answer": "Item not listed"}'
|
||||
result = parse_preview_response(raw)
|
||||
assert result['found'] is False
|
||||
assert result['answer'] == 'Item not listed'
|
||||
|
||||
def test_string_false_in_preview(self):
|
||||
raw = '{"found": "false", "answer": "Not found"}'
|
||||
result = parse_preview_response(raw)
|
||||
assert result['found'] is False
|
||||
assert result['answer'] == 'Not found'
|
||||
|
||||
def test_preview_with_think_tags(self):
|
||||
raw = '<think>Looking for price...</think>\n{"found": true, "answer": "$19.99"}'
|
||||
result = parse_preview_response(raw)
|
||||
assert result['found'] is True
|
||||
assert result['answer'] == '$19.99'
|
||||
|
||||
|
||||
class TestParseSetupResponse:
|
||||
def test_no_prefilter_needed(self):
|
||||
raw = '{"needs_prefilter": false, "selector": null, "reason": "intent is global"}'
|
||||
@@ -95,8 +170,15 @@ class TestParseSetupResponse:
|
||||
assert result['needs_prefilter'] is False
|
||||
assert result['selector'] is None
|
||||
|
||||
def test_string_false_in_setup(self):
|
||||
raw = '{"needs_prefilter": "false", "selector": null, "reason": "global"}'
|
||||
result = parse_setup_response(raw)
|
||||
assert result['needs_prefilter'] is False
|
||||
|
||||
def test_semantic_selector_accepted(self):
|
||||
raw = '{"needs_prefilter": true, "selector": "footer", "reason": "intent references footer"}'
|
||||
raw = (
|
||||
'{"needs_prefilter": true, "selector": "footer", "reason": "intent references footer"}'
|
||||
)
|
||||
result = parse_setup_response(raw)
|
||||
assert result['needs_prefilter'] is True
|
||||
assert result['selector'] == 'footer'
|
||||
|
||||
Reference in New Issue
Block a user