Big source revamp

tests/parser/remote/test_base.py

@@ -0,0 +1,133 @@
+"""Tests for the shared web-ingest path helpers in docsgpt/parser/remote/base.py."""
+
+import pytest
+
+from docsgpt.parser.remote.base import (
+    MAX_QUERY_SEGMENT_LENGTH,
+    dedupe_virtual_paths,
+    url_to_virtual_path,
+)
+from docsgpt.parser.schema.base import Document
+
+
+@pytest.mark.unit
+class TestUrlToVirtualPath:
+    @pytest.mark.parametrize(
+        "url, path",
+        [
+            # Paths without a query string are unchanged.
+            ("https://x.io/", "index.md"),
+            ("https://x.io", "index.md"),
+            ("https://x.io/guides/setup", "guides/setup.md"),
+            ("https://x.io/guides/setup/", "guides/setup.md"),
+            ("https://x.io/page.html", "page.md"),
+            ("https://x.io/readme.md", "readme.md"),
+            # The fragment names a spot on the same page.
+            ("https://x.io/p#install", "p.md"),
+            ("https://x.io/p?#install", "p.md"),
+        ],
+    )
+    def test_query_less_paths_are_unchanged(self, url, path):
+        assert url_to_virtual_path(url) == path
+
+    def test_distinct_queries_get_distinct_paths(self):
+        first = url_to_virtual_path("https://x.io/p?page=1")
+        second = url_to_virtual_path("https://x.io/p?page=2")
+
+        assert first == "p__page=1.md"
+        assert second == "p__page=2.md"
+        assert url_to_virtual_path("https://x.io/p") == "p.md"
+
+    def test_parameter_order_does_not_matter(self):
+        assert (
+            url_to_virtual_path("https://x.io/p?b=2&a=1")
+            == url_to_virtual_path("https://x.io/p?a=1&b=2")
+            == "p__a=1&b=2.md"
+        )
+
+    def test_query_on_the_root_and_on_page_extensions(self):
+        assert url_to_virtual_path("https://x.io/?page=2") == "index__page=2.md"
+        assert url_to_virtual_path("https://x.io/a.php?id=7#top") == "a__id=7.md"
+        assert url_to_virtual_path("https://x.io/doc.md?v=2") == "doc__v=2.md"
+
+    def test_query_is_sanitized_to_a_single_tree_segment(self):
+        path = url_to_virtual_path("https://x.io/p?q=a/b%20c&next=..%2Fetc&flag")
+
+        assert path == "p__flag&next=..-etc&q=a-b-c.md"
+        assert path.count("/") == 0
+        assert "?" not in path and "#" not in path and "%" not in path
+
+    def test_unicode_query_values_survive(self):
+        assert url_to_virtual_path("https://x.io/s?q=%D0%BA%D0%BE%D1%82") == "s__q=кот.md"
+
+    def test_long_queries_are_bounded_with_a_stable_hash(self):
+        long_a = "https://x.io/p?q=" + "a" * 500
+        long_b = "https://x.io/p?q=" + "a" * 499 + "b"
+
+        path_a = url_to_virtual_path(long_a)
+        path_b = url_to_virtual_path(long_b)
+
+        segment = path_a[len("p__"):-len(".md")]
+        assert len(segment) <= MAX_QUERY_SEGMENT_LENGTH
+        assert path_a != path_b
+        assert url_to_virtual_path(long_a) == path_a
+
+    def test_host_prefix_still_applies(self):
+        assert (
+            url_to_virtual_path("https://x.io/p?page=1", include_host=True)
+            == "x.io/p__page=1.md"
+        )
+
+
+def _doc(url, file_path=None):
+    return Document(
+        "text",
+        extra_info={"source": url, "file_path": file_path or url_to_virtual_path(url)},
+    )
+
+
+def _paths(docs):
+    return [d.extra_info["file_path"] for d in docs]
+
+
+@pytest.mark.unit
+class TestDedupeVirtualPaths:
+    def test_distinct_pages_are_left_alone(self):
+        docs = [_doc("https://x.io/"), _doc("https://x.io/a"), _doc("https://x.io/b")]
+
+        assert _paths(dedupe_virtual_paths(docs)) == ["index.md", "a.md", "b.md"]
+
+    def test_extension_collision_gets_a_numbered_suffix(self):
+        docs = [_doc("https://x.io/a.html"), _doc("https://x.io/a")]
+
+        assert _paths(dedupe_virtual_paths(docs)) == ["a-2.md", "a.md"]
+
+    def test_suffixes_do_not_depend_on_input_order(self):
+        urls = ["https://x.io/a", "https://x.io/a.htm", "https://x.io/a.html"]
+
+        forward = _paths(dedupe_virtual_paths([_doc(u) for u in urls]))
+        backward = _paths(dedupe_virtual_paths([_doc(u) for u in reversed(urls)]))
+
+        assert forward == ["a.md", "a-2.md", "a-3.md"]
+        assert backward == list(reversed(forward))
+
+    def test_suffix_skips_a_path_another_page_already_has(self):
+        docs = [
+            _doc("https://x.io/g/a-2"),
+            _doc("https://x.io/g/a"),
+            _doc("https://x.io/g/a.html"),
+        ]
+
+        assert _paths(dedupe_virtual_paths(docs)) == ["g/a-2.md", "g/a.md", "g/a-3.md"]
+
+    def test_the_same_page_reached_twice_keeps_one_path(self):
+        # A crawler that followed ``#section`` fetched the same page again.
+        docs = [_doc("https://x.io/a"), _doc("https://x.io/a#section")]
+
+        assert _paths(dedupe_virtual_paths(docs)) == ["a.md", "a.md"]
+
+    def test_documents_without_a_file_path_are_ignored(self):
+        doc = Document("text", extra_info={"source": "https://x.io/a"})
+
+        assert dedupe_virtual_paths([doc]) == [doc]
+        assert "file_path" not in doc.extra_info

tests/parser/remote/test_crawler_loader.py

@@ -183,3 +183,28 @@ class TestCrawlerLoaderGaps:
 
                 result = loader.load_data("https://example.com")
                 assert result[0].text == "test"
+
+
+@patch("docsgpt.parser.remote.crawler_loader.validate_url", side_effect=_mock_validate_url)
+@patch("docsgpt.parser.remote.crawler_loader.pinned_request")
+def test_colliding_pages_get_distinct_file_paths(mock_pinned_request, mock_validate_url):
+    responses = {
+        "http://example.com": DummyResponse(
+            "<html><body><a href='/a'>A</a><a href='/a.html'>A2</a>"
+            "<a href='/l?page=2'>L</a></body></html>"
+        ),
+        "http://example.com/a": DummyResponse("<html><body>a</body></html>"),
+        "http://example.com/a.html": DummyResponse("<html><body>a2</body></html>"),
+        "http://example.com/l?page=2": DummyResponse("<html><body>l</body></html>"),
+    }
+    mock_pinned_request.side_effect = lambda _m, url, timeout=30: responses[url]
+
+    result = CrawlerLoader(limit=10).load_data("http://example.com")
+
+    paths = {doc.extra_info["source"]: doc.extra_info["file_path"] for doc in result}
+    assert paths == {
+        "http://example.com": "index.md",
+        "http://example.com/a": "a.md",
+        "http://example.com/a.html": "a-2.md",
+        "http://example.com/l?page=2": "l__page=2.md",
+    }

tests/parser/remote/test_crawler_markdown.py

@@ -179,3 +179,28 @@ def test_load_data_returns_empty_on_ssrf_validation_failure(monkeypatch):
 
     assert result == []
 
+
+
+def test_colliding_pages_get_distinct_file_paths(monkeypatch, _patch_markdownify):
+    root_html = (
+        "<html><head><title>Home</title></head><body>"
+        "<a href='/a'>A</a><a href='/a.html'>A2</a><a href='/l?page=2'>L</a>"
+        "</body></html>"
+    )
+    responses = {
+        "http://example.com": DummyResponse(root_html),
+        "http://example.com/a": DummyResponse("<html><body>a</body></html>"),
+        "http://example.com/a.html": DummyResponse("<html><body>a2</body></html>"),
+        "http://example.com/l?page=2": DummyResponse("<html><body>l</body></html>"),
+    }
+    _patch_pinned_request(monkeypatch, lambda url: responses[url])
+
+    docs = CrawlerLoader(limit=10).load_data("http://example.com")
+
+    paths = {doc.extra_info["source"]: doc.extra_info["file_path"] for doc in docs}
+    assert paths == {
+        "http://example.com": "index.md",
+        "http://example.com/a": "a.md",
+        "http://example.com/a.html": "a-2.md",
+        "http://example.com/l?page=2": "l__page=2.md",
+    }

tests/parser/remote/test_sitemap_loader.py

@@ -246,7 +246,10 @@ class TestSitemapLoaderLoadData:
             assert len(docs) == 1
             assert isinstance(docs[0], Document)
             assert docs[0].text == "Page body"
-            assert docs[0].extra_info == {"source": "https://example.com/page1"}
+            assert docs[0].extra_info == {
+                "source": "https://example.com/page1",
+                "file_path": "page1.md",
+            }
 
     @patch("docsgpt.parser.remote.sitemap_loader.validate_url")
     def test_load_data_no_urls(self, mock_validate):
@@ -330,3 +333,21 @@ class TestSitemapLoaderLoadData:
         with patch.object(loader, "_extract_urls", return_value=urls):
             docs = loader.load_data("https://example.com/sitemap.xml")
             assert len(docs) == 5
+
+
+@pytest.mark.unit
+class TestSitemapLoaderDistinctPaths:
+    @patch("docsgpt.parser.remote.sitemap_loader.validate_url", side_effect=lambda url: url)
+    @patch("docsgpt.parser.remote.sitemap_loader.pinned_request")
+    def test_colliding_pages_get_distinct_file_paths(self, mock_pinned_request, mock_validate):
+        loader = SitemapLoader(limit=None)
+        response = MagicMock()
+        response.text = "Page body"
+        response.raise_for_status.return_value = None
+        mock_pinned_request.return_value = response
+        urls = ["https://x.io/p?page=2", "https://x.io/p", "https://x.io/p.htm"]
+
+        with patch.object(loader, "_extract_urls", return_value=urls):
+            docs = loader.load_data("https://x.io/sitemap.xml")
+
+        assert [d.extra_info["file_path"] for d in docs] == ["p__page=2.md", "p.md", "p-2.md"]

tests/parser/remote/test_web_loader.py

@@ -60,6 +60,20 @@ class TestWebLoaderHeaders:
 class TestWebLoaderLoadData:
     """Test WebLoader load_data method."""
 
+    @patch("docsgpt.parser.remote.web_loader.validate_url", side_effect=_mock_validate_url)
+    @patch("docsgpt.parser.remote.web_loader.pinned_request")
+    def test_same_host_urls_get_path_only_file_paths(self, mock_pinned_request, mock_validate, web_loader):
+        # The tree and the chunk path filter both key off file_path, so each
+        # page needs a stable, distinct one (not its title).
+        mock_pinned_request.side_effect = [
+            _fake_response("<html><head><title>Home</title></head><body>a</body></html>"),
+            _fake_response("<html><head><title>Setup</title></head><body>b</body></html>"),
+        ]
+
+        result = web_loader.load_data(["https://docs.x.io/", "https://docs.x.io/guides/setup.html"])
+
+        assert [d.extra_info["file_path"] for d in result] == ["index.md", "guides/setup.md"]
+
     @patch("docsgpt.parser.remote.web_loader.validate_url", side_effect=_mock_validate_url)
     @patch("docsgpt.parser.remote.web_loader.pinned_request")
     def test_load_data_single_url_string(self, mock_pinned_request, mock_validate, web_loader):
@@ -77,6 +91,7 @@ class TestWebLoaderLoadData:
             "source": "https://example.com",
             "title": "Test Page",
             "language": "en",
+            "file_path": "index.md",
         }
         mock_pinned_request.assert_called_once_with(
             "GET", "https://example.com", headers=headers, timeout=30
@@ -97,8 +112,9 @@ class TestWebLoaderLoadData:
         assert all(isinstance(doc, Document) for doc in result)
         assert result[0].text == "Content from site 1"
         assert result[1].text == "Content from site 2"
-        assert result[0].extra_info == {"source": "https://site1.com"}
-        assert result[1].extra_info == {"source": "https://site2.com"}
+        # Two hosts would both map to index.md, so the host prefixes the path.
+        assert result[0].extra_info == {"source": "https://site1.com", "file_path": "site1.com/index.md"}
+        assert result[1].extra_info == {"source": "https://site2.com", "file_path": "site2.com/index.md"}
 
         assert mock_pinned_request.call_count == 2
         mock_pinned_request.assert_any_call(
@@ -146,7 +162,7 @@ class TestWebLoaderLoadData:
 
         assert len(result) == 1
         assert result[0].text == "Bare body"
-        assert result[0].extra_info == {"source": "https://example.com"}
+        assert result[0].extra_info == {"source": "https://example.com", "file_path": "index.md"}
 
     @patch("docsgpt.parser.remote.web_loader.validate_url", side_effect=_mock_validate_url)
     @patch("docsgpt.parser.remote.web_loader.pinned_request")
@@ -203,7 +219,7 @@ class TestWebLoaderErrorHandling:
 
         assert len(result) == 1
         assert result[0].text == "Success content"
-        assert result[0].extra_info == {"source": "https://good-url.com"}
+        assert result[0].extra_info == {"source": "https://good-url.com", "file_path": "good-url.com/index.md"}
 
         mock_logging.error.assert_called_once()
         error_call = mock_logging.error.call_args
@@ -276,7 +292,7 @@ class TestWebLoaderEdgeCases:
 
         assert len(result) == 1
         assert result[0].text == ""
-        assert result[0].extra_info == {"source": "https://empty-page.com"}
+        assert result[0].extra_info == {"source": "https://empty-page.com", "file_path": "index.md"}
 
     def test_url_scheme_detection(self):
         """Test URL scheme detection logic."""
@@ -314,3 +330,24 @@ class TestWebLoaderIntegration:
         assert "Test web page content" in result[0].page_content
         assert result[0].metadata["source"] == "https://example.com"
         assert result[0].metadata["title"] == "Test Page"
+
+
+@pytest.mark.unit
+class TestWebLoaderDistinctPaths:
+    @patch("docsgpt.parser.remote.web_loader.validate_url", side_effect=_mock_validate_url)
+    @patch("docsgpt.parser.remote.web_loader.pinned_request")
+    def test_colliding_pages_get_distinct_file_paths(self, mock_pinned_request, mock_validate, web_loader):
+        mock_pinned_request.side_effect = lambda *a, **k: _fake_response("<html><body>x</body></html>")
+
+        result = web_loader.load_data(
+            [
+                "https://x.io/list?page=1",
+                "https://x.io/list?page=2",
+                "https://x.io/a.html",
+                "https://x.io/a",
+            ]
+        )
+
+        assert [d.extra_info["file_path"] for d in result] == [
+            "list__page=1.md", "list__page=2.md", "a-2.md", "a.md"
+        ]