diff --git a/tests/test_html.py b/tests/test_html.py index f941008d..a852d808 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -37,3 +37,55 @@ class TestStripHtml: def test_self_closing_tags(self): result = strip_html("hello
world") assert result == "helloworld" + + # -- invisible element stripping ----------------------------------------- + + def test_strips_script_content(self): + html = "

before

after

" + result = strip_html(html) + assert "var x" not in result + assert "before" in result + assert "after" in result + + def test_strips_style_content(self): + html = "

visible

" + result = strip_html(html) + assert "color" not in result + assert "visible" in result + + def test_strips_template_content(self): + html = "

shown

" + result = strip_html(html) + assert "hidden" not in result + assert "shown" in result + + def test_strips_noscript_content(self): + html = "

content

" + result = strip_html(html) + assert "Enable JS" not in result + assert "content" in result + + def test_strips_multiple_script_blocks(self): + html = "

middle

" + result = strip_html(html) + assert "a()" not in result + assert "b()" not in result + assert "middle" in result + + def test_strips_multiline_script(self): + html = "

ok

" + result = strip_html(html) + assert "function" not in result + assert "ok" in result + + def test_strips_script_case_insensitive(self): + html = "

text

" + result = strip_html(html) + assert "code()" not in result + assert "text" in result + + def test_strips_script_with_attributes(self): + html = '

done

' + result = strip_html(html) + assert "init()" not in result + assert "done" in result diff --git a/tests/test_session.py b/tests/test_session.py index eb20bc99..64201994 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -759,6 +759,8 @@ class TestTitleRetry: """_generate_title resets _title_generated on failure.""" def test_title_generated_reset_on_failure(self, tmp_db): + from turnstone.core.providers._protocol import ModelCapabilities + session = _make_session() session._title_generated = True session.messages = [ @@ -767,6 +769,7 @@ class TestTitleRetry: ] # Mock provider to raise session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() session._provider.create_completion.side_effect = RuntimeError("API error") session._generate_title() @@ -774,6 +777,8 @@ class TestTitleRetry: assert session._title_generated is False def test_title_generated_stays_true_on_success(self, tmp_db): + from turnstone.core.providers._protocol import ModelCapabilities + session = _make_session() session._title_generated = True session.messages = [ @@ -783,6 +788,7 @@ class TestTitleRetry: result = MagicMock() result.content = "Test Title" session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() session._provider.create_completion.return_value = result with patch("turnstone.core.session.update_workstream_title"): @@ -793,6 +799,8 @@ class TestTitleRetry: def test_title_skipped_after_resume_changes_ws_id(self, tmp_db): """If ws_id changes (via resume) during title generation, discard the result.""" + from turnstone.core.providers._protocol import ModelCapabilities + session = _make_session() session._title_generated = True session.messages = [ @@ -803,6 +811,7 @@ class TestTitleRetry: result = MagicMock() result.content = "Test Title" session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() session._provider.create_completion.return_value = result # Simulate resume() changing ws_id while title generation is in flight diff --git a/turnstone/core/providers/_anthropic.py b/turnstone/core/providers/_anthropic.py index 587abe3c..19686bc5 100644 --- a/turnstone/core/providers/_anthropic.py +++ b/turnstone/core/providers/_anthropic.py @@ -543,9 +543,13 @@ class AnthropicProvider: if extra_params and "thinking_budget_tokens" in extra_params: budget = extra_params["thinking_budget_tokens"] if budget > 0: - # Budget must leave room for the response + # Budget must be strictly less than max_tokens (API requirement). + # If max_tokens is too small to fit even a minimal thinking + # budget alongside the response, disable thinking entirely. if budget >= max_tokens: - budget = max(1024, max_tokens - 1024) + budget = max_tokens - 1024 + if budget < 1: + return {} return {"thinking": {"type": "enabled", "budget_tokens": budget}} return {} diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 9987f760..ad8e5624 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -924,10 +924,8 @@ class ChatSession: if asst_msg: snippet += f"\nAssistant: {asst_msg}" snippet += "\n\nTitle:" - result = self._provider.create_completion( - client=self.client, - model=self.model, - messages=[ + result = self._utility_completion( + [ { "role": "system", "content": ( @@ -942,9 +940,6 @@ class ChatSession: {"role": "user", "content": snippet}, ], max_tokens=200, - temperature=0.3, - reasoning_effort="low", - extra_params=self._provider_extra_params(reasoning_effort="low"), ) raw = (result.content or "").strip() # Take first line, strip quotes @@ -1281,6 +1276,33 @@ class ChatSession: return {"chat_template_kwargs": kwargs} return None + def _utility_completion( + self, + messages: list[dict[str, Any]], + *, + max_tokens: int = 4096, + temperature: float = 0.3, + reasoning_effort: str = "low", + ) -> CompletionResult: + """Run a lightweight internal completion (title gen, compaction, extraction). + + Threads ``reasoning_effort`` through both the direct keyword (for + commercial providers) and ``extra_params`` (for local model servers) + so callers don't need to duplicate it. ``max_tokens`` is clamped to + the model's advertised output limit so small models don't error. + """ + caps = self._get_capabilities() + clamped = min(max_tokens, caps.max_output_tokens) if caps.max_output_tokens else max_tokens + return self._provider.create_completion( + client=self.client, + model=self.model, + messages=messages, + max_tokens=clamped, + temperature=temperature, + reasoning_effort=reasoning_effort, + extra_params=self._provider_extra_params(reasoning_effort=reasoning_effort), + ) + # -- tool search helpers -------------------------------------------------- def _get_active_tools(self) -> list[dict[str, Any]] | None: @@ -2482,14 +2504,9 @@ class ChatSession: result: CompletionResult | None = None for attempt in range(self._MAX_RETRIES + 1): try: - result = self._provider.create_completion( - client=self.client, - model=self.model, - messages=summary_msgs, + result = self._utility_completion( + summary_msgs, max_tokens=summary_max_tokens, - temperature=0.3, - reasoning_effort="low", - extra_params=self._provider_extra_params(reasoning_effort="low"), ) break except Exception as e: @@ -6167,26 +6184,30 @@ class ChatSession: return call_id, msg if not text.strip(): - return call_id, "(empty response from URL)" + msg = "Error: fetch returned empty response" + self._report_tool_result(call_id, "web_fetch", msg, is_error=True) + return call_id, msg original_len = len(text) self.ui.on_info(f"fetched {original_len} chars, extracting...") - # Phase 2: truncate for summarization context - max_content = 50_000 + # Phase 2: truncate for summarization context. + # Reserve ~25% of the context window for the extraction prompt + # overhead (system message, URL, question) and response tokens. + # Convert token budget to chars using the calibrated ratio. + max_content = int(self.context_window * self._chars_per_token * 0.75) + max_content = min(max(max_content, 50_000), 500_000) # 50k–500k if len(text) > max_content: - text = ( - text[: max_content // 2] - + f"\n\n... [{len(text) - max_content} chars omitted] ...\n\n" - + text[-(max_content // 2) :] - ) + # Prefer the beginning — page content is usually top-heavy. + text = text[:max_content] + f"\n\n... [{len(text) - max_content} chars truncated] ...\n" - # Phase 3: summarization API call + # Phase 3: summarization API call. + # Use a generous max_tokens so thinking models don't starve the + # visible answer, and pass reasoning_effort="low" to avoid wasting + # budget on deep reasoning for a simple extraction task. try: - result = self._provider.create_completion( - client=self.client, - model=self.model, - messages=[ + result = self._utility_completion( + [ { "role": "system", "content": ( @@ -6206,11 +6227,12 @@ class ChatSession: ), }, ], - max_tokens=2000, + max_tokens=8192, temperature=0.2, - extra_params=self._provider_extra_params(), ) - answer = result.content or "(no answer)" + answer = result.content or "" + if not answer: + answer = "Error: extraction returned no answer" except Exception as e: answer = f"Extraction failed (page was fetched but summarization errored): {e}" @@ -6218,7 +6240,7 @@ class ChatSession: call_id, "web_fetch", answer, - is_error=answer.startswith("Extraction failed"), + is_error=answer.startswith(("Error:", "Extraction failed")), ) return call_id, answer diff --git a/turnstone/core/web.py b/turnstone/core/web.py index 97da24c3..a5dab5b5 100644 --- a/turnstone/core/web.py +++ b/turnstone/core/web.py @@ -6,14 +6,20 @@ import socket from html import unescape as _html_unescape from urllib.parse import urlparse +_RE_INVISIBLE = re.compile( + r"<(script|style|template|noscript)\b[^>]*>.*?", + re.DOTALL | re.IGNORECASE, +) _RE_TAGS = re.compile(r"<[^>]+>") _RE_WS = re.compile(r"[ \t]+") _RE_BLANKLINES = re.compile(r"\n{3,}") def strip_html(html: str) -> str: - """Convert HTML to plain text: strip tags, decode entities, collapse whitespace.""" - text = _RE_TAGS.sub("", html) + """Convert HTML to plain text: strip invisible elements, tags, decode entities.""" + # Remove elements whose content should never appear as text + text = _RE_INVISIBLE.sub("", html) + text = _RE_TAGS.sub("", text) text = _html_unescape(text) text = _RE_WS.sub(" ", text) text = _RE_BLANKLINES.sub("\n\n", text)