rabbit fixes

docsgpt/parser/remote/base.py

@@ -4,7 +4,7 @@ import os
 import re
 from abc import abstractmethod
 from typing import Any, Dict, List
-from urllib.parse import parse_qsl, urldefrag, urlencode, urlparse
+from urllib.parse import parse_qsl, urlencode, urlparse, urlsplit, urlunsplit
 
 from docsgpt.parser.schema.base import Document
 from docsgpt.vectorstore.document_class import Document as VectorDocument
@@ -99,13 +99,44 @@ def url_to_virtual_path(url: str, include_host: bool = False) -> str:
 def spans_multiple_hosts(urls: List[str]) -> bool:
     """Return whether ``urls`` point at more than one host.
 
+    A URL ``urlparse`` rejects (an unclosed IPv6 bracket, say) is left out:
+    it fails validation and is never fetched, so it names no host here.
+
     Args:
         urls: Page URLs about to be ingested together.
 
     Returns:
         True when paths alone could collide and need a host prefix.
     """
-    return len({urlparse(u).netloc for u in urls}) > 1
+    hosts = set()
+    for url in urls:
+        try:
+            hosts.add(urlparse(url).netloc)
+        except ValueError:
+            continue
+    return len(hosts) > 1
+
+
+def normalize_page_url(url: str) -> str:
+    """Return the identity of the page ``url`` names, for deduplication.
+
+    The fragment is dropped (it names a spot on the same page) and the query
+    parameters are sorted, as ``_query_segment`` sorts them, so
+    ``/p?a=1&b=2`` and ``/p?b=2&a=1#top`` are one page. The result is a key
+    only; fetch the URL as it was written.
+
+    Args:
+        url: Page URL.
+
+    Returns:
+        The normalized URL.
+
+    Raises:
+        ValueError: If ``urlsplit`` cannot parse ``url``.
+    """
+    parts = urlsplit(url)
+    query = urlencode(sorted(parse_qsl(parts.query, keep_blank_values=True)))
+    return urlunsplit(parts._replace(query=query, fragment=""))
 
 
 def dedupe_virtual_paths(documents: List[Document]) -> List[Document]:
@@ -114,11 +145,13 @@ def dedupe_virtual_paths(documents: List[Document]) -> List[Document]:
     ``url_to_virtual_path`` folds some different URLs onto one path (``/a`` and
     ``/a.html`` are both ``a.md``), and the worker would then merge those pages
     into one tree entry. Within a path, the pages are ordered by URL (fragment
-    dropped): the first keeps the path and each later one gets ``-2``, ``-3``
-    and so on before ``.md``, skipping any path another page already has.
+    dropped, query parameters sorted; see ``normalize_page_url``): the first
+    keeps the path and each later one gets ``-2``, ``-3`` and so on before
+    ``.md``, skipping any path another page already has.
     Ordering by URL rather than by fetch order keeps the names stable across
     re-syncs of the same pages. Documents for the same URL (one page reached
-    via two fragments) are one page and keep one path.
+    via two fragments, or with its query parameters in another order) are one
+    page and keep one path.
 
     Args:
         documents: Loaded documents; each ``extra_info`` carries ``source``
@@ -133,7 +166,7 @@ def dedupe_virtual_paths(documents: List[Document]) -> List[Document]:
         path = info.get("file_path")
         if not path:
             continue
-        page = urldefrag(str(info.get("source") or path))[0]
+        page = normalize_page_url(str(info.get("source") or path))
         pages_by_path.setdefault(path, {}).setdefault(page, []).append(doc)
 
     taken = set(pages_by_path)

docsgpt/parser/remote/crawler_loader.py

@@ -2,7 +2,7 @@ import logging
 from bs4 import BeautifulSoup
 from urllib.parse import urljoin, urlparse
 
-from docsgpt.parser.remote.base import BaseRemote, dedupe_virtual_paths, url_to_virtual_path
+from docsgpt.parser.remote.base import BaseRemote, dedupe_virtual_paths, normalize_page_url, url_to_virtual_path
 from docsgpt.parser.schema.base import Document
 from docsgpt.core.url_validation import validate_url, SSRFError
 from docsgpt.security.safe_url import pinned_request
@@ -24,6 +24,8 @@ class CrawlerLoader(BaseRemote):
             logging.error(f"URL validation failed: {e}")
             return []
 
+        # Keyed by normalize_page_url, so fragment and query-order variants of
+        # one page are fetched once.
         visited_urls = set()
         base_url = urlparse(url).scheme + "://" + urlparse(url).hostname
         urls_to_visit = [url]
@@ -31,7 +33,10 @@ class CrawlerLoader(BaseRemote):
 
         while urls_to_visit:
             current_url = urls_to_visit.pop(0)
-            visited_urls.add(current_url)
+            page_key = normalize_page_url(current_url)
+            if page_key in visited_urls:
+                continue
+            visited_urls.add(page_key)
 
             try:
                 response = pinned_request("GET", current_url, timeout=30)
@@ -57,15 +62,21 @@ class CrawlerLoader(BaseRemote):
                 logging.error(f"Error processing URL {current_url}: {e}", exc_info=True)
                 continue
 
-            # Parse the HTML content to extract all links
-            all_links = [
-                urljoin(current_url, a['href'])
-                for a in soup.find_all('a', href=True)
-                if base_url in urljoin(current_url, a['href'])
-            ]
+            # Parse the HTML content to extract all links. A malformed href
+            # (``http://[bad``) makes urljoin or the key parse raise; skip it
+            # rather than abort the crawl.
+            all_links = []
+            for a in soup.find_all('a', href=True):
+                try:
+                    link = urljoin(current_url, a['href'])
+                    link_key = normalize_page_url(link)
+                except ValueError:
+                    continue
+                if base_url in link and link_key not in visited_urls:
+                    all_links.append(link)
 
             # Add new links to the list of URLs to visit if they haven't been visited yet
-            urls_to_visit.extend([link for link in all_links if link not in visited_urls])
+            urls_to_visit.extend(all_links)
             urls_to_visit = list(set(urls_to_visit))
 
             # Stop crawling if the limit of pages to scrape is reached

docsgpt/parser/remote/crawler_markdown.py

@@ -1,6 +1,6 @@
 from urllib.parse import urlparse, urljoin
 from bs4 import BeautifulSoup
-from docsgpt.parser.remote.base import BaseRemote, dedupe_virtual_paths, url_to_virtual_path
+from docsgpt.parser.remote.base import BaseRemote, dedupe_virtual_paths, normalize_page_url, url_to_virtual_path
 from docsgpt.core.url_validation import validate_url, SSRFError
 from docsgpt.security.safe_url import UnsafeUserUrlError, pinned_request
 import re
@@ -37,7 +37,8 @@ class CrawlerLoader(BaseRemote):
             print(f"URL validation failed: {e}")
             return []
 
-        # Keep track of visited URLs to avoid revisiting the same page
+        # Keep track of visited pages to avoid revisiting the same page. Keyed
+        # by normalize_page_url, so fragment and query-order variants are one.
         visited_urls = set()
 
         # Determine the base domain for link filtering using tldextract
@@ -49,9 +50,10 @@ class CrawlerLoader(BaseRemote):
             current_url = urls_to_visit.pop()
 
             # Skip if already visited
-            if current_url in visited_urls:
+            page_key = normalize_page_url(current_url)
+            if page_key in visited_urls:
                 continue
-            visited_urls.add(current_url)
+            visited_urls.add(page_key)
 
             # Fetch the page content
             html_content = self._fetch_page(current_url)
@@ -84,7 +86,9 @@ class CrawlerLoader(BaseRemote):
             filtered_links = self._filter_links(new_links, base_domain)
 
             # Add any new, not-yet-visited links to the queue
-            urls_to_visit.update(link for link in filtered_links if link not in visited_urls)
+            urls_to_visit.update(
+                link for link in filtered_links if normalize_page_url(link) not in visited_urls
+            )
 
             # If we've reached the limit, stop crawling
             if self.limit is not None and len(visited_urls) >= self.limit:
@@ -123,7 +127,13 @@ class CrawlerLoader(BaseRemote):
         soup = BeautifulSoup(html_content, 'html.parser')
         links = []
         for a in soup.find_all('a', href=True):
-            full_url = urljoin(current_url, a['href'])
+            # A malformed href (``http://[bad``) makes urljoin or the later
+            # parse raise; skip that link rather than abort the crawl.
+            try:
+                full_url = urljoin(current_url, a['href'])
+                normalize_page_url(full_url)
+            except ValueError:
+                continue
             links.append((full_url, a.text.strip()))
         return links