perf: harden content extractor - async fetch, single parse, batch cap

- Pages are fetched with httpx.AsyncClient under real connect (3s) and read timeouts on the event loop; only the CPU-bound Trafilatura parse runs in the thread pool. trafilatura.fetch_url previously ran inside the worker thread with no caller-side timeout control, so an asyncio.wait_for timeout abandoned the thread while it kept downloading for up to ~30s. - Trafilatura now runs ONCE per document via bare_extraction (text and metadata together). The old path parsed three times: extract() for text, extract(output_format='xml') whose result was discarded, and bare_extraction for metadata. - extract_batch caps full-page extractions per call (default 8, configurable); overflow URLs return unsuccessful results so the web leg falls back to the search snippet instead of fanning out unbounded downloads per search. - Responses over 5MB are truncated before parsing; thread-pool queue depth is logged for backpressure visibility. Verified live against a real URL (fetch + single-parse extraction OK). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbFZyDvYksazX6nYQYZ67L
2026-07-14 14:29:19 +02:00
parent c17c623936
commit 69e6a01e65
3 changed files with 316 additions and 190 deletions
@@ -1,11 +1,26 @@
 """Tests for ContentExtractor client."""

 import pytest
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import AsyncMock, patch
+
+import httpx

 from src.clients.content_extractor import ContentExtractor
 from src.models.content import ContentExtractionResult

+TEST_HTML = "<html><body><article>Test</article></body></html>"
+
+
+def _doc(text, **meta):
+    """bare_extraction-style result dict."""
+    return {
+        "text": text,
+        "title": meta.get("title"),
+        "author": meta.get("author"),
+        "date": meta.get("date"),
+        "language": meta.get("language"),
+    }
+

@pytest.fixture
 def content_extractor():
@@ -20,6 +35,9 @@ class TestContentExtractor:
        """Test ContentExtractor initialization."""
        assert content_extractor.timeout == 5
        assert content_extractor.max_length == 2000
+        assert content_extractor.max_urls_per_batch == (
+            ContentExtractor.DEFAULT_MAX_URLS_PER_BATCH
+        )

    @pytest.mark.asyncio
    async def test_extract_success(self, content_extractor):
@@ -27,31 +45,48 @@ class TestContentExtractor:
        test_url = "https://example.com/article"
        test_content = "This is the extracted article content."

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.fetch_url.return_value = "<html><body>Test</body></html>"
-            mock_traf.extract.return_value = test_content
-            mock_traf.bare_extraction.return_value = {
-                "title": "Test Article",
-                "author": "John Doe",
-                "date": "2024-01-15",
-                "language": "en"
-            }
+        with patch.object(
+            content_extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ), patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction.return_value = _doc(
+                test_content,
+                title="Test Article",
+                author="John Doe",
+                date="2024-01-15",
+                language="en",
+            )

            result = await content_extractor.extract(test_url)

            assert result.success is True
            assert result.url == test_url
            assert result.content == test_content
+            assert result.title == "Test Article"
            assert result.error is None

+    @pytest.mark.asyncio
+    async def test_extraction_runs_once_per_url(self, content_extractor):
+        """Trafilatura must parse the document exactly ONCE (the old code
+        ran extract() twice plus bare_extraction — three full parses)."""
+        with patch.object(
+            content_extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ), patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction.return_value = _doc("content")
+
+            await content_extractor.extract("https://example.com/a")
+
+            assert mock_traf.bare_extraction.call_count == 1
+            mock_traf.extract.assert_not_called()
+            mock_traf.fetch_url.assert_not_called()  # httpx fetches now
+
    @pytest.mark.asyncio
    async def test_extract_fetch_failure(self, content_extractor):
        """Test extraction when URL fetch fails."""
        test_url = "https://example.com/nonexistent"

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.fetch_url.return_value = None
-
+        with patch.object(
+            content_extractor, "_fetch", AsyncMock(return_value=None)
+        ):
            result = await content_extractor.extract(test_url)

            assert result.success is False
@@ -59,14 +94,44 @@ class TestContentExtractor:
            assert result.content == ""
            assert "Failed to fetch URL" in result.error

+    @pytest.mark.asyncio
+    async def test_extract_fetch_timeout(self, content_extractor):
+        """A slow server hits the httpx timeout instead of pinning a
+        worker thread on a blind download."""
+        test_url = "https://example.com/slow-server"
+
+        with patch.object(
+            content_extractor,
+            "_fetch",
+            AsyncMock(side_effect=httpx.ReadTimeout("read timeout")),
+        ):
+            result = await content_extractor.extract(test_url)
+
+            assert result.success is False
+            assert "timed out" in result.error.lower()
+
+    @pytest.mark.asyncio
+    async def test_extract_network_error(self, content_extractor):
+        """Connection errors return a failed result, not an exception."""
+        with patch.object(
+            content_extractor,
+            "_fetch",
+            AsyncMock(side_effect=httpx.ConnectError("refused")),
+        ):
+            result = await content_extractor.extract("https://example.com/down")
+
+            assert result.success is False
+            assert "Failed to fetch URL" in result.error
+
    @pytest.mark.asyncio
    async def test_extract_no_content(self, content_extractor):
        """Test extraction when page has no extractable content."""
        test_url = "https://example.com/empty"

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.fetch_url.return_value = "<html><body></body></html>"
-            mock_traf.extract.return_value = None
+        with patch.object(
+            content_extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ), patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction.return_value = None

            result = await content_extractor.extract(test_url)

@@ -80,10 +145,10 @@ class TestContentExtractor:
        # Content longer than max_length (2000)
        long_content = "x" * 3000

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.fetch_url.return_value = "<html><body>Test</body></html>"
-            mock_traf.extract.return_value = long_content
-            mock_traf.bare_extraction.return_value = {}
+        with patch.object(
+            content_extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ), patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction.return_value = _doc(long_content)

            result = await content_extractor.extract(test_url)

@@ -97,13 +162,13 @@ class TestContentExtractor:
        test_urls = [
            "https://example.com/article1",
            "https://example.com/article2",
-            "https://example.com/article3"
+            "https://example.com/article3",
        ]

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.fetch_url.return_value = "<html><body>Test</body></html>"
-            mock_traf.extract.return_value = "Extracted content"
-            mock_traf.bare_extraction.return_value = {}
+        with patch.object(
+            content_extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ), patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction.return_value = _doc("Extracted content")

            results = await content_extractor.extract_batch(test_urls)

@@ -113,8 +178,32 @@ class TestContentExtractor:
                assert result.success is True

    @pytest.mark.asyncio
-    async def test_extract_timeout(self):
-        """Test extraction timeout handling."""
+    async def test_extract_batch_caps_full_page_extractions(self):
+        """URLs beyond the per-call cap are skipped (callers fall back to
+        the search snippet) instead of fanning out unbounded downloads."""
+        extractor = ContentExtractor(timeout=5, max_length=2000, max_urls_per_batch=2)
+        test_urls = [f"https://example.com/{i}" for i in range(5)]
+
+        with patch.object(
+            extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ) as mock_fetch, patch(
+            "src.clients.content_extractor.trafilatura"
+        ) as mock_traf:
+            mock_traf.bare_extraction.return_value = _doc("content")
+
+            results = await extractor.extract_batch(test_urls)
+
+            assert len(results) == 5
+            assert mock_fetch.await_count == 2
+            assert [r.url for r in results] == test_urls
+            assert all(r.success for r in results[:2])
+            for skipped in results[2:]:
+                assert skipped.success is False
+                assert "cap" in skipped.error
+
+    @pytest.mark.asyncio
+    async def test_extract_parse_timeout(self):
+        """Test extraction (parse) timeout handling."""
        import time

        test_url = "https://example.com/slow"
@@ -122,12 +211,14 @@ class TestContentExtractor:
        # Create an extractor with very short timeout
        fast_extractor = ContentExtractor(timeout=0.001, max_length=2000)

-        def slow_fetch(url):
+        def slow_parse(*args, **kwargs):
            time.sleep(1)  # Sleep synchronously (this runs in thread pool)
-            return "<html></html>"
+            return _doc("late content")

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.fetch_url = slow_fetch
+        with patch.object(
+            fast_extractor, "_fetch", AsyncMock(return_value=TEST_HTML)
+        ), patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction = slow_parse

            result = await fast_extractor.extract(test_url)

@@ -139,11 +230,14 @@ class TestContentExtractor:
        """Test extraction from raw HTML."""
        test_html = "<html><body><article>Article content here.</article></body></html>"

-        with patch('src.clients.content_extractor.trafilatura') as mock_traf:
-            mock_traf.extract.return_value = "Article content here."
-            mock_traf.bare_extraction.return_value = {"title": "Test"}
+        with patch("src.clients.content_extractor.trafilatura") as mock_traf:
+            mock_traf.bare_extraction.return_value = _doc(
+                "Article content here.", title="Test"
+            )

-            result = await content_extractor.extract_from_html(test_html, url="https://example.com")
+            result = await content_extractor.extract_from_html(
+                test_html, url="https://example.com"
+            )

            assert result.success is True
            assert result.content == "Article content here."