Fix/web fetch reliability (#290)

* fix: improve web_fetch reliability — strip scripts, dynamic truncation, more tokens

- strip_html() now removes <script>, <style>, <template>, <noscript>
  element content instead of just their tags
- Truncation budget scales with context window (75% in chars, 50k floor)
  and takes from the beginning only instead of head+tail splice
- max_tokens bumped from 2000 to 8192 so thinking models don't starve
  the visible extraction answer
- reasoning_effort="low" on summarization call to avoid wasting tokens
- Empty responses and empty extractions now report as tool errors

* refactor: extract _utility_completion to fix reasoning_effort duplication

Callers previously had to pass reasoning_effort both as a direct keyword
(for commercial providers) and via _provider_extra_params (for local
model servers).  This duplication was easy to get wrong — web_fetch was
already missing the direct keyword.

_utility_completion threads it through both paths from a single call,
used by title generation, compaction, and web_fetch extraction.

* fix: disable thinking when max_tokens too small, cap extraction at 500k

_reasoning_params now returns empty dict when max_tokens can't fit a
thinking budget (e.g. title gen with max_tokens=200).  Previously
produced budget_tokens >= max_tokens which is an API error on
manual-thinking Anthropic models.

Also caps web_fetch content truncation at 500k chars — the dynamic
context-window calc was producing 3M chars on 1M-context models.

* fix: clamp utility max_tokens to model output limit, add strip_html tests

_utility_completion now clamps max_tokens to the model's advertised
max_output_tokens so small/local models don't reject 8192-token
requests.

Adds 8 tests for invisible element stripping (script, style, template,
noscript) including multiline, case-insensitive, and attribute cases.

* fix: mock get_capabilities in title retry tests for _utility_completion

_utility_completion calls _get_capabilities to clamp max_tokens.  The
existing title tests mocked _provider as a bare MagicMock, so
caps.max_output_tokens was a truthy MagicMock instead of an int.  Set
get_capabilities to return a real ModelCapabilities instance.

tests/test_html.py

@@ -37,3 +37,55 @@ class TestStripHtml:
     def test_self_closing_tags(self):
         result = strip_html("hello<br/>world")
         assert result == "helloworld"
+
+    # -- invisible element stripping -----------------------------------------
+
+    def test_strips_script_content(self):
+        html = "<p>before</p><script>var x = 1;</script><p>after</p>"
+        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 = "<style>.foo { color: red; }</style><p>visible</p>"
+        result = strip_html(html)
+        assert "color" not in result
+        assert "visible" in result
+
+    def test_strips_template_content(self):
+        html = "<template><div>hidden</div></template><p>shown</p>"
+        result = strip_html(html)
+        assert "hidden" not in result
+        assert "shown" in result
+
+    def test_strips_noscript_content(self):
+        html = "<noscript>Enable JS</noscript><p>content</p>"
+        result = strip_html(html)
+        assert "Enable JS" not in result
+        assert "content" in result
+
+    def test_strips_multiple_script_blocks(self):
+        html = "<script>a()</script><p>middle</p><script>b()</script>"
+        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 = "<script>\nfunction foo() {\n  return 1;\n}\n</script><p>ok</p>"
+        result = strip_html(html)
+        assert "function" not in result
+        assert "ok" in result
+
+    def test_strips_script_case_insensitive(self):
+        html = "<SCRIPT>code()</SCRIPT><p>text</p>"
+        result = strip_html(html)
+        assert "code()" not in result
+        assert "text" in result
+
+    def test_strips_script_with_attributes(self):
+        html = '<script type="text/javascript" src="app.js">init();</script><p>done</p>'
+        result = strip_html(html)
+        assert "init()" not in result
+        assert "done" in result

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

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 {}
 

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

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[^>]*>.*?</\1\s*>",
+    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)