From a5adee18ba54051d432490b85d5199cde2174a17 Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Wed, 18 Mar 2026 16:15:45 +0800 Subject: [PATCH 1/7] fix(kb): add traceable web ingest and owners-compatible metadata Fixes #162 by adding end-to-end trace_id logging, stricter start_url validation, fail-fast crawl errors, and optional JS-rendered crawling (Playwright) with clearer user guidance. Also adds compatibility for collections requiring an owners field in LanceDB metadata (aligned with #166). --- CHANGELOG.md | 24 ++ .../kb/knowledge-base-creation-dialog.tsx | 3 + .../components/kb/knowledge-base-detail.tsx | 20 ++ .../core/tools/core/RAG_tools/core/schemas.py | 24 ++ .../management/collection_manager.py | 1 + .../RAG_tools/pipelines/document_ingestion.py | 17 +- .../core/RAG_tools/pipelines/web_ingestion.py | 167 +++++++++-- .../core/RAG_tools/utils/migration_utils.py | 2 + .../RAG_tools/web_crawler/content_cleaner.py | 40 ++- .../core/RAG_tools/web_crawler/crawler.py | 272 ++++++++++++++++-- src/xagent/providers/vector_store/lancedb.py | 9 +- src/xagent/web/api/kb.py | 137 ++++++++- ...eb_ingestion_fail_fast_on_crawl_failure.py | 37 +++ tests/web/api/test_kb_ingest_separators.py | 5 + .../test_kb_ingest_web_invalid_start_url.py | 77 +++++ 15 files changed, 769 insertions(+), 66 deletions(-) create mode 100644 tests/core/tools/core/RAG_tools/pipelines/test_web_ingestion_fail_fast_on_crawl_failure.py create mode 100644 tests/web/api/test_kb_ingest_web_invalid_start_url.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a5535768c..95afa5df4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,3 +33,27 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - **LanceDB user_id migration hardening** Startup and migration logic now include cross-process file locking, legacy `-1` orphan marker remapping to reserved int64 sentinel values, zero-progress loop protection, and shared embeddings-table listing utilities to avoid API-compat drift. + +### Added + +- **Knowledge Base web ingestion: JS-rendered crawling (Playwright)** + Web ingestion now supports an optional **JS-rendered** mode for SPA/JavaScript-heavy websites (e.g. pages where the initial HTML contains mostly `` metadata and the real content is rendered by JavaScript). + + If you enable the **"Render JS (Playwright)"** option in the Knowledge Base web ingestion UI: + + - **Docker (recommended)**: the official backend image installs Chromium during build (see `docker/Dockerfile.backend`). If you see an error about missing Playwright browsers, you are likely using an older image tag. Fix by updating/rebuilding the backend image: + + ```bash + docker compose pull backend + docker compose up -d backend + ``` + + - **Local (non-Docker)**: install Playwright browsers once: + + ```bash + cd xagent + source .venv/bin/activate + playwright install chromium + ``` + + If browsers are missing, the backend returns an error like **"Playwright browsers are not installed"** with the command to run. diff --git a/frontend/src/components/kb/knowledge-base-creation-dialog.tsx b/frontend/src/components/kb/knowledge-base-creation-dialog.tsx index 2f67d384f..c4649c8a2 100644 --- a/frontend/src/components/kb/knowledge-base-creation-dialog.tsx +++ b/frontend/src/components/kb/knowledge-base-creation-dialog.tsx @@ -105,6 +105,7 @@ export function KnowledgeBaseCreationDialog({ open, onOpenChange, onSuccess }: K request_delay: 1.0, timeout: 30, respect_robots_txt: true, + render_js: false, }) // Cloud connect state @@ -298,6 +299,7 @@ export function KnowledgeBaseCreationDialog({ open, onOpenChange, onSuccess }: K request_delay: 1.0, timeout: 30, respect_robots_txt: true, + render_js: false, }) setCurrentStep(1) } @@ -425,6 +427,7 @@ export function KnowledgeBaseCreationDialog({ open, onOpenChange, onSuccess }: K formData.append("request_delay", webIngestionConfig.request_delay.toString()) formData.append("timeout", webIngestionConfig.timeout.toString()) formData.append("respect_robots_txt", webIngestionConfig.respect_robots_txt.toString()) + formData.append("render_js", webIngestionConfig.render_js.toString()) appendIngestionConfigToFormData(formData, ingestionConfig) diff --git a/frontend/src/components/kb/knowledge-base-detail.tsx b/frontend/src/components/kb/knowledge-base-detail.tsx index 047040baf..dd335def4 100644 --- a/frontend/src/components/kb/knowledge-base-detail.tsx +++ b/frontend/src/components/kb/knowledge-base-detail.tsx @@ -176,6 +176,7 @@ export function KnowledgeBaseDetailContent({ collectionName }: { collectionName: request_delay: 1.0, timeout: 30, respect_robots_txt: true, + render_js: false, }) // Embedding models state @@ -482,6 +483,7 @@ export function KnowledgeBaseDetailContent({ collectionName }: { collectionName: formData.append("request_delay", webIngestionConfig.request_delay.toString()) formData.append("timeout", webIngestionConfig.timeout.toString()) formData.append("respect_robots_txt", webIngestionConfig.respect_robots_txt.toString()) + formData.append("render_js", webIngestionConfig.render_js.toString()) // Add ingestion configuration appendIngestionConfigToFormData(formData, ingestionConfig) @@ -538,6 +540,7 @@ export function KnowledgeBaseDetailContent({ collectionName }: { collectionName: request_delay: 1.0, timeout: 30, respect_robots_txt: true, + render_js: false, }) } catch (err) { @@ -1296,6 +1299,23 @@ export function KnowledgeBaseDetailContent({ collectionName }: { collectionName: /> +
+ + setWebIngestionConfig((prev) => ({ + ...prev, + render_js: e.target.checked, + })) + } + className="w-4 h-4" + /> + +
diff --git a/src/xagent/core/tools/core/RAG_tools/core/schemas.py b/src/xagent/core/tools/core/RAG_tools/core/schemas.py index 259d9480f..5866e763a 100644 --- a/src/xagent/core/tools/core/RAG_tools/core/schemas.py +++ b/src/xagent/core/tools/core/RAG_tools/core/schemas.py @@ -1291,6 +1291,12 @@ class CollectionInfo(BaseModel): # Basic identifier name: str = Field(..., description="Collection identifier") + # 👥 Ownership / access control (stored as JSON string in LanceDB) + owners: List[str] = Field( + default_factory=list, + description="Collection owners. Stored as JSON string in LanceDB for compatibility.", + ) + # 🎯 Core binding: Embedding configuration (lazy initialization) embedding_model_id: Optional[str] = Field( default=None, # None indicates not initialized @@ -1769,6 +1775,24 @@ class WebCrawlConfig(BaseModel): description="Whether to respect robots.txt rules", ) + # JavaScript rendering (Playwright) + render_js: bool = Field( + default=False, + description=( + "Whether to render pages with a real browser (Playwright) to support SPA/JS-heavy sites. " + "When enabled, crawling fetches the post-rendered DOM instead of raw HTML." + ), + ) + render_wait_until: Literal["load", "domcontentloaded", "networkidle"] = Field( + default="networkidle", + description="Playwright navigation wait strategy when render_js=True", + ) + render_timeout_ms: int = Field( + default=30000, + ge=1, + description="Playwright navigation timeout in milliseconds when render_js=True", + ) + class CrawlResult(BaseModel): """Result of crawling a single page.""" diff --git a/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py b/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py index 5aee7ddaa..d16de0f0a 100644 --- a/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py +++ b/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py @@ -279,6 +279,7 @@ async def _ensure_metadata_table(self) -> None: schema = pa.schema( [ ("name", pa.string()), + ("owners", pa.string()), # JSON string ("schema_version", pa.string()), ("embedding_model_id", pa.string()), # Nullable ("embedding_dimension", pa.int32()), # Nullable diff --git a/src/xagent/core/tools/core/RAG_tools/pipelines/document_ingestion.py b/src/xagent/core/tools/core/RAG_tools/pipelines/document_ingestion.py index 61a8a574d..ed914dc49 100644 --- a/src/xagent/core/tools/core/RAG_tools/pipelines/document_ingestion.py +++ b/src/xagent/core/tools/core/RAG_tools/pipelines/document_ingestion.py @@ -191,6 +191,7 @@ def run_document_ingestion( user_id: Optional[int] = None, is_admin: Optional[bool] = None, file_id: Optional[str] = None, + trace_id: Optional[str] = None, ) -> IngestionResult: """Public entrypoint for LangGraph-compatible ingestion tooling. @@ -224,6 +225,7 @@ def run_document_ingestion( user_id=user_id, is_admin=is_admin, file_id=file_id, + trace_id=trace_id, ) @@ -445,10 +447,14 @@ def _handle_ingestion_error( vector_count: int, warnings: List[str], user_id: Optional[int] = None, + trace_id: Optional[str] = None, ) -> IngestionResult: """Unify error handling for the ingestion pipeline.""" logger.exception( - "Document ingestion pipeline failed at step '%s': %s", current_step, exc + "Document ingestion pipeline failed at step '%s': %s", + current_step, + exc, + extra={"trace_id": trace_id, "collection": collection, "doc_id": doc_id}, ) status = "partial" if completed_steps else "error" @@ -484,6 +490,7 @@ def process_document( user_id: Optional[int] = None, is_admin: bool = False, file_id: Optional[str] = None, + trace_id: Optional[str] = None, ) -> IngestionResult: """Execute the full ingestion pipeline for a document. @@ -558,7 +565,11 @@ def process_document( # Step 0: Initialize/validate collection embedding configuration logger.info( "Step initialize_collection started", - extra={"collection": collection, "source_path": source_path}, + extra={ + "trace_id": trace_id, + "collection": collection, + "source_path": source_path, + }, ) init_start = time.time() @@ -610,6 +621,7 @@ def process_document( logger.info( "Step initialize_collection completed", extra={ + "trace_id": trace_id, "collection": collection, "embedding_model_id": selected_model_id, "elapsed_ms": init_elapsed, @@ -1292,4 +1304,5 @@ def _update_embedding_progress(current: int, total: int) -> None: vector_count=vector_count, warnings=warnings, user_id=user_id, + trace_id=trace_id, ) diff --git a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py index a309632aa..fa697b362 100644 --- a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py +++ b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py @@ -76,6 +76,7 @@ async def run_web_ingestion( user_id: Optional[int] = None, is_admin: Optional[bool] = None, file_handler: Optional[Callable[[Path, str, str, str], FileHandlerResult]] = None, + trace_id: Optional[str] = None, ) -> WebIngestionResult: """Crawl a website and ingest all pages into the knowledge base. @@ -117,14 +118,42 @@ async def run_web_ingestion( ing_cfg = coerce_ingestion_config(ingestion_config) logger.info( - "Starting web ingestion: collection=%s, start_url=%s", - collection, - crawl_config.start_url, + "Starting web ingestion", + extra={ + "trace_id": trace_id, + "collection": collection, + "start_url": crawl_config.start_url, + "user_id": user_id, + "is_admin": is_admin, + "max_pages": crawl_config.max_pages, + "max_depth": crawl_config.max_depth, + "same_domain_only": crawl_config.same_domain_only, + "url_patterns": crawl_config.url_patterns, + "exclude_patterns": crawl_config.exclude_patterns, + "content_selector": crawl_config.content_selector, + "remove_selectors": crawl_config.remove_selectors, + "concurrent_requests": crawl_config.concurrent_requests, + "request_delay": crawl_config.request_delay, + "timeout": crawl_config.timeout, + "respect_robots_txt": crawl_config.respect_robots_txt, + "render_js": getattr(crawl_config, "render_js", None), + "render_wait_until": getattr(crawl_config, "render_wait_until", None), + "render_timeout_ms": getattr(crawl_config, "render_timeout_ms", None), + "ingestion_parse_method": str(ing_cfg.parse_method), + "ingestion_chunk_strategy": str(ing_cfg.chunk_strategy), + "ingestion_chunk_size": ing_cfg.chunk_size, + "ingestion_chunk_overlap": ing_cfg.chunk_overlap, + "ingestion_separators": ing_cfg.separators, + "embedding_model_id": ing_cfg.embedding_model_id, + "embedding_batch_size": ing_cfg.embedding_batch_size, + "embedding_use_async": ing_cfg.embedding_use_async, + "embedding_concurrent": ing_cfg.embedding_concurrent, + }, ) # Step 1: Crawl the website logger.info("Step 1: Crawling website") - crawler = WebCrawler(crawl_config, progress_callback) + crawler = WebCrawler(crawl_config, progress_callback, trace_id=trace_id) try: crawl_results: list[CrawlResult] = await crawler.crawl() @@ -144,7 +173,14 @@ async def run_web_ingestion( embeddings_created=0, crawled_urls=[], failed_urls={}, - message=f"Website crawling failed: {str(e)}", + message=( + "Website crawling failed: " + + ( + "Playwright browsers are not installed. Run: playwright install chromium" + if "Playwright browsers are not installed" in str(e) + else str(e) + ) + ), warnings=[], elapsed_time_ms=elapsed_ms, ) @@ -159,9 +195,53 @@ async def run_web_ingestion( pages_failed = len(failed_urls) logger.info( - "Crawling completed: %s successful, %s failed", pages_crawled, pages_failed + "Crawling completed", + extra={ + "trace_id": trace_id, + "collection": collection, + "start_url": crawl_config.start_url, + "successful_pages": pages_crawled, + "failed_pages": pages_failed, + "total_urls_found": crawler.total_urls_found, + "results_count": len(crawl_results), + }, ) + # Fail fast: crawling produced no successful pages but has concrete failure reasons. + # This avoids continuing into ingestion and makes debugging much easier. + if pages_crawled == 0 and pages_failed > 0: + elapsed_ms = int( + (datetime.now(timezone.utc) - start_time).total_seconds() * 1000 + ) + if failed_urls and all( + _looks_like_crawler_block(err) for err in failed_urls.values() + ): + fail_fast_message = _CRAWLER_BLOCK_MESSAGE + else: + reasons: list[str] = [] + for url, reason in list(failed_urls.items())[:3]: + reasons.append(f"{url}: {reason}") + reason_suffix = "; ".join(reasons) + fail_fast_message = ( + "Website crawling failed: no valid pages extracted." + + (f" Reasons: {reason_suffix}" if reason_suffix else "") + ) + return WebIngestionResult( + status="error", + collection=collection, + total_urls_found=crawler.total_urls_found, + pages_crawled=0, + pages_failed=pages_failed, + documents_created=0, + chunks_created=0, + embeddings_created=0, + crawled_urls=[], + failed_urls=failed_urls, + message=fail_fast_message, + warnings=warnings, + elapsed_time_ms=elapsed_ms, + ) + # Step 2: Ingest each crawled page logger.info("Step 2: Ingesting crawled pages") @@ -190,6 +270,19 @@ async def run_web_ingestion( ) try: + logger.debug( + "Ingesting crawled page", + extra={ + "trace_id": trace_id, + "collection": collection, + "url": crawl_result.url, + "title": crawl_result.title, + "depth": crawl_result.depth, + "content_length": crawl_result.content_length, + "index": i + 1, + "total": len(crawl_results), + }, + ) # Save crawled content to temporary markdown file filename = sanitize_for_doc_id(crawl_result.title or f"page_{i + 1}") temp_file = Path(temp_dir) / f"{filename}.md" @@ -239,7 +332,6 @@ async def run_web_ingestion( continue try: - # Ingest the file progress_manager = get_progress_manager() def _ingest_file() -> IngestionResult: @@ -253,47 +345,53 @@ def _ingest_file() -> IngestionResult: is_admin=is_admin, ) - # Run ingestion in thread pool while preserving ContextVar user scope - # NOTE: request_context was copied before the loop to avoid repeated copying. - # Modifications made in the thread pool won't propagate back to the main request context. - # This is acceptable for user scope (read-only) but observability systems should - # be aware that child span updates may be lost. with concurrent.futures.ThreadPoolExecutor() as executor: ingest_result: IngestionResult = await loop.run_in_executor( executor, lambda: request_context.run(_ingest_file) ) - # Track statistics if ingest_result.status == "success": documents_created += 1 total_chunks += ingest_result.chunk_count total_embeddings += ingest_result.embedding_count - logger.info( - "Ingested %s: %s chunks, %s embeddings", - crawl_result.url, - ingest_result.chunk_count, - ingest_result.embedding_count, + logger.debug( + "Ingested crawled page successfully", + extra={ + "trace_id": trace_id, + "collection": collection, + "url": crawl_result.url, + "doc_id": ingest_result.doc_id, + "parse_hash": ingest_result.parse_hash, + "chunk_count": ingest_result.chunk_count, + "embedding_count": ingest_result.embedding_count, + "vector_count": ingest_result.vector_count, + }, ) - # Only clear temp file reference on success copied_persistent_file = None else: - # Non-success ingestion (e.g., embedding failed) without exception. - # Keep file and DB record for potential retry scenarios. - # Note: This accumulates files on persistent failures. - # TODO: Add periodic cleanup for orphaned files from persistent failures. failed_urls[crawl_result.url] = ingest_result.message msg = ( f"Partial ingestion for {crawl_result.url}: " f"{ingest_result.message}" ) warnings.append(msg) + logger.debug( + "Ingested crawled page failed", + extra={ + "trace_id": trace_id, + "collection": collection, + "url": crawl_result.url, + "status": ingest_result.status, + "failed_step": ingest_result.failed_step, + "ingest_message": ingest_result.message, + }, + ) except Exception as e: logger.exception("Failed to ingest %s", crawl_result.url) failed_urls[crawl_result.url] = str(e) warnings.append(f"Failed to ingest {crawl_result.url}: {str(e)}") - # Clean up copied persistent file on ingestion failure if copied_persistent_file and copied_persistent_file.exists(): try: copied_persistent_file.unlink() @@ -381,6 +479,7 @@ def _ingest_file() -> IngestionResult: f"{total_chunks} chunks, {total_embeddings} embeddings" ) + result = WebIngestionResult( status=status, collection=collection, @@ -398,10 +497,22 @@ def _ingest_file() -> IngestionResult: ) logger.info( - "Web ingestion completed: %s, %s documents, %sms", - result.status, - documents_created, - elapsed_ms, + "Web ingestion completed", + extra={ + "trace_id": trace_id, + "collection": collection, + "start_url": crawl_config.start_url, + "status": result.status, + "total_urls_found": crawler.total_urls_found, + "pages_crawled": pages_crawled, + "pages_failed": pages_failed, + "documents_created": documents_created, + "chunks_created": total_chunks, + "embeddings_created": total_embeddings, + "failed_urls_count": len(failed_urls), + "warnings_count": len(warnings), + "elapsed_time_ms": elapsed_ms, + }, ) return result diff --git a/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py b/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py index ca4c05606..33122e091 100644 --- a/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py +++ b/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py @@ -109,6 +109,8 @@ def _migrate_0_0_0_to_1_0_0( "schema_version": "1.0.0", # Basic fields (preserve existing or set defaults) "name": collection_name, + # Owners (default empty for legacy data) + "owners": data.get("owners", []), # Embedding config (inferred from existing data or None for lazy init) "embedding_model_id": embedding_model_id, "embedding_dimension": embedding_dimension, diff --git a/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py b/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py index e20a78023..8cc795b8e 100644 --- a/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py +++ b/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py @@ -16,6 +16,8 @@ def __init__( self, content_selector: Optional[str] = None, remove_selectors: Optional[list[str]] = None, + *, + trace_id: Optional[str] = None, ): """Initialize content cleaner. @@ -25,6 +27,7 @@ def __init__( """ self.content_selector = content_selector self.remove_selectors = remove_selectors or [] + self.trace_id = trace_id def clean_and_convert(self, html: str, url: str) -> dict: """Clean HTML and convert to markdown. @@ -81,8 +84,21 @@ def clean_and_convert(self, html: str, url: str) -> dict: "content_length": len(markdown), } - except Exception as e: - logger.error("Failed to clean content from %s: %s", url, e) + except Exception: + html_snippet = (html or "").replace("\n", "\\n") + if len(html_snippet) > 400: + html_snippet = html_snippet[:400] + "...(truncated)" + logger.exception( + "Failed to clean content", + extra={ + "trace_id": self.trace_id, + "url": url, + "html_length": len(html or ""), + "html_snippet": html_snippet, + "content_selector": self.content_selector, + "remove_selectors": self.remove_selectors, + }, + ) return { "title": None, "content_markdown": "", @@ -207,8 +223,26 @@ def is_valid_content(self, content: str, min_length: int = 100) -> bool: True if valid, False otherwise """ if not content or len(content) < min_length: + logger.info( + "Content rejected by min_length", + extra={ + "trace_id": self.trace_id, + "content_length": len(content or ""), + "min_length": min_length, + }, + ) return False # Check if content has actual text (not just empty lines/spaces) lines = [line.strip() for line in content.split("\n") if line.strip()] - return len(lines) > 0 + if len(lines) == 0: + logger.info( + "Content rejected: no non-empty lines", + extra={ + "trace_id": self.trace_id, + "content_length": len(content), + "min_length": min_length, + }, + ) + return False + return True diff --git a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py index 810bcc4da..8f9fab4aa 100644 --- a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py +++ b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py @@ -153,6 +153,8 @@ def __init__( self, config: WebCrawlConfig, progress_callback: Optional[Callable[[str, int, int], None]] = None, + *, + trace_id: Optional[str] = None, ): """Initialize web crawler. @@ -163,6 +165,7 @@ def __init__( """ self.config = config self.progress_callback = progress_callback + self.trace_id = trace_id # Initialize components self.url_filter = URLFilter( @@ -175,6 +178,7 @@ def __init__( self.content_cleaner = ContentCleaner( content_selector=config.content_selector, remove_selectors=config.remove_selectors, + trace_id=trace_id, ) self.link_extractor = LinkExtractor(config.start_url) @@ -200,13 +204,6 @@ def __init__( self._tls_chain = (config.tls_impersonate,) # Effective User-Agent for policy decisions (robots.txt, etc.). - # When tls_impersonate is None we send config.user_agent on the - # wire, so policy must reason about that string. When tls_impersonate - # is set, curl_cffi controls the UA based on the impersonate spec -- - # config.user_agent is intentionally ignored in that path to keep - # TLS fingerprint and HTTP UA consistent. For policy we use the - # mapped UA of the *first* fingerprint in the chain, falling back - # to "*" if the spec is unknown to us. if config.tls_impersonate is None: self._policy_user_agent: str = config.user_agent or _DEFAULT_USER_AGENT else: @@ -215,6 +212,11 @@ def __init__( _IMPERSONATE_TO_UA.get(first_fp, "*") if first_fp else "*" ) + # Playwright runtime (optional, used when render_js=True) + self._playwright: Any | None = None + self._browser: Any | None = None + self._browser_context: Any | None = None + async def crawl(self) -> List[CrawlResult]: """Start crawling from the configured start URL. @@ -227,15 +229,47 @@ async def crawl(self) -> List[CrawlResult]: start_url_normalized = self.url_filter.normalize_url(self.config.start_url) if start_url_normalized: self.pending_urls.append((start_url_normalized, 0)) # (url, depth) + else: + self.failed_urls[self.config.start_url] = ( + "Invalid start_url: must start with http:// or https://" + ) + logger.error( + "Start URL normalization failed; crawl will not run", + extra={ + "trace_id": self.trace_id, + "start_url": self.config.start_url, + "base_domain": getattr(self.url_filter, "base_domain", None), + "same_domain_only": self.config.same_domain_only, + }, + ) + # Fail fast: without a valid start URL, the crawl loop would do nothing. + # We record the failure in failed_urls so upstream can surface an error. + self.start_time = self.start_time or time.time() + return [] - logger.info("Starting crawl from %s", self.config.start_url) + logger.info( + "Starting crawl", + extra={ + "trace_id": self.trace_id, + "start_url": self.config.start_url, + "start_url_normalized": start_url_normalized, + "max_pages": self.config.max_pages, + "max_depth": self.config.max_depth, + "same_domain_only": self.config.same_domain_only, + "url_patterns": self.config.url_patterns, + "exclude_patterns": self.config.exclude_patterns, + "respect_robots_txt": self.config.respect_robots_txt, + "content_selector": self.config.content_selector, + "remove_selectors": self.config.remove_selectors, + "concurrent_requests": self.config.concurrent_requests, + "request_delay": self.config.request_delay, + "timeout": self.config.timeout, + "user_agent": self.config.user_agent, + "tls_impersonate": self.config.tls_impersonate, + "render_js": self.config.render_js, + }, + ) - # When tls_impersonate is None we use plain httpx; in that path we - # need to manually compose a browser-like header set so the request - # doesn't look obviously bot-shaped. When tls_impersonate is set, - # curl_cffi owns headers (matching its TLS impersonation), and we - # MUST NOT inject our own UA/headers -- doing so creates a TLS-vs- - # HTTP-fingerprint mismatch which is exactly what WAFs catch. httpx_headers: Optional[Dict[str, str]] = None if None in self._tls_chain: user_agent = self.config.user_agent or _DEFAULT_USER_AGENT @@ -255,15 +289,30 @@ async def crawl(self) -> List[CrawlResult]: "DNT": "1", } - async with self._open_sessions(httpx_headers) as sessions: - await self._crawl_loop(sessions) + try: + if getattr(self.config, "render_js", False): + ua = self.config.user_agent or _DEFAULT_USER_AGENT + await self._start_playwright(user_agent=ua) + + async with self._open_sessions(httpx_headers) as sessions: + await self._crawl_loop(sessions) + finally: + if getattr(self.config, "render_js", False): + await self._stop_playwright() elapsed = time.time() - self.start_time logger.info( - "Crawl completed: %s pages, %s failed, %.2fs", - len(self.crawl_results), - len(self.failed_urls), - elapsed, + "Crawl completed", + extra={ + "trace_id": self.trace_id, + "start_url": self.config.start_url, + "pages": len(self.crawl_results), + "failed": len(self.failed_urls), + "visited_urls": len(self.visited_urls), + "pending_urls": len(self.pending_urls), + "total_urls_found": self.total_urls_found, + "elapsed_sec": round(elapsed, 3), + }, ) return self.crawl_results @@ -536,9 +585,6 @@ async def _crawl_page( self.failed_urls[url] = error_msg return None, set() - # Explicit status check is library-agnostic (works for both - # httpx and curl_cffi response objects, which raise different - # exception types from raise_for_status()). if not 200 <= response.status_code < 300: error_msg = f"HTTP {response.status_code}" logger.error("Failed to crawl %s: %s", url, error_msg) @@ -553,7 +599,22 @@ async def _crawl_page( # Validate content content = cleaned["content_markdown"] if not self.content_cleaner.is_valid_content(content, min_length=10): - logger.warning("Insufficient content at %s", url) + snippet = (content or "").replace("\n", "\\n") + if len(snippet) > 400: + snippet = snippet[:400] + "...(truncated)" + logger.warning( + "Insufficient content at %s (content_length=%s, title=%s)", + url, + cleaned.get("content_length"), + cleaned.get("title"), + extra={"trace_id": self.trace_id}, + ) + logger.debug( + "Insufficient content snippet at %s (snippet=%s)", + url, + snippet, + extra={"trace_id": self.trace_id}, + ) self.failed_urls[url] = "Insufficient content" return None, set() @@ -621,6 +682,169 @@ async def _process_links(self, links: Set[str], current_depth: int) -> None: if link not in self.visited_urls and link not in pending_urls_set: self.pending_urls.append((link, next_depth)) + async def _fetch_html( + self, client: httpx.AsyncClient, url: str + ) -> Tuple[str, Dict[str, object]]: + """Fetch HTML either via httpx or Playwright (rendered).""" + if self.config.render_js: + return await self._fetch_html_rendered(url) + return await self._fetch_html_static(client, url) + + async def _fetch_html_static( + self, client: httpx.AsyncClient, url: str + ) -> Tuple[str, Dict[str, object]]: + response = await client.get(url, follow_redirects=True) + response.raise_for_status() + html = response.text or "" + self._log_fetched_page( + requested_url=url, + final_url=str(response.url), + status_code=response.status_code, + content_type=response.headers.get("content-type"), + html=html, + mode="static", + ) + return html, { + "mode": "static", + "final_url": str(response.url), + "status_code": response.status_code, + } + + async def _fetch_html_rendered(self, url: str) -> Tuple[str, Dict[str, object]]: + # Lazy import to keep Playwright optional at runtime. + try: + from playwright.async_api import Error as PlaywrightError + from playwright.async_api import TimeoutError as PlaywrightTimeoutError + except Exception as exc: # noqa: BLE001 + raise RuntimeError( + "Playwright is not available. Install with: pip install playwright " + "and then download browsers: playwright install chromium" + ) from exc + + if self._browser_context is None: + raise RuntimeError("Playwright browser context is not initialized") + + context = self._browser_context + page = await context.new_page() + try: + resp = await page.goto( + url, + wait_until=self.config.render_wait_until, + timeout=self.config.render_timeout_ms, + ) + status_code: object = None + if resp is not None: + status_attr = getattr(resp, "status", None) + # Playwright Response.status is an int property (not a method). + status_code = status_attr() if callable(status_attr) else status_attr + content_type = None + try: + headers = getattr(resp, "headers", None) + if isinstance(headers, dict): + content_type = headers.get("content-type") + except Exception: # noqa: BLE001 + content_type = None + + html = await page.content() + self._log_fetched_page( + requested_url=url, + final_url=page.url, + status_code=status_code, + content_type=content_type, + html=html or "", + mode="rendered", + ) + return html or "", { + "mode": "rendered", + "final_url": getattr(page, "url", url), + "status_code": status_code, + } + except (PlaywrightTimeoutError, PlaywrightError) as exc: + raise RuntimeError(f"Playwright navigation failed: {exc}") from exc + finally: + await page.close() + + def _log_fetched_page( + self, + *, + requested_url: str, + final_url: str, + status_code: object, + content_type: object, + html: str, + mode: str, + ) -> None: + logger.info( + "Fetched page [%s] (requested_url=%s, final_url=%s, status_code=%s, content_type=%s, html_length=%s)", + mode, + requested_url, + final_url, + status_code, + content_type, + len(html or ""), + extra={"trace_id": self.trace_id}, + ) + html_snippet = (html or "").replace("\n", "\\n") + if len(html_snippet) > 400: + html_snippet = html_snippet[:400] + "...(truncated)" + logger.debug( + "Fetched page snippet [%s] (requested_url=%s, html_snippet=%s)", + mode, + requested_url, + html_snippet, + extra={"trace_id": self.trace_id}, + ) + + async def _start_playwright(self, *, user_agent: str) -> None: + try: + from playwright.async_api import Error as PlaywrightError + from playwright.async_api import async_playwright + except Exception as exc: # noqa: BLE001 + raise RuntimeError( + "Playwright is not installed. Install with: pip install playwright. " + "Then download browsers: playwright install chromium" + ) from exc + + self._playwright = await async_playwright().start() + chromium = getattr(self._playwright, "chromium", None) + if chromium is None: + raise RuntimeError("Playwright chromium is unavailable") + try: + self._browser = await chromium.launch(headless=True) + self._browser_context = await self._browser.new_context( + user_agent=user_agent + ) + except PlaywrightError as exc: + msg = str(exc) + # Common setup issue: playwright python package installed but browsers not downloaded. + if "Executable doesn't exist" in msg or "playwright install" in msg: + raise RuntimeError( + "Playwright browsers are not installed. " + "If running locally, run: playwright install chromium. " + "If running in Docker, update/rebuild the backend image (or run: docker compose exec backend playwright install chromium)." + ) from exc + raise RuntimeError(f"Playwright launch failed: {msg}") from exc + + async def _stop_playwright(self) -> None: + try: + if self._browser_context is not None: + await self._browser_context.close() + except Exception: # noqa: BLE001 + pass + try: + if self._browser is not None: + await self._browser.close() + except Exception: # noqa: BLE001 + pass + try: + if self._playwright is not None: + await self._playwright.stop() + except Exception: # noqa: BLE001 + pass + self._browser_context = None + self._browser = None + self._playwright = None + def get_statistics(self) -> dict: """Get crawl statistics. diff --git a/src/xagent/providers/vector_store/lancedb.py b/src/xagent/providers/vector_store/lancedb.py index 506aa138b..2dacc1f07 100644 --- a/src/xagent/providers/vector_store/lancedb.py +++ b/src/xagent/providers/vector_store/lancedb.py @@ -200,18 +200,25 @@ def get_connection_from_env(self, env_var: str = "LANCEDB_DIR") -> DBConnection: KeyError: If environment variable (other than LANCEDB_DIR) is not set """ db_dir = os.getenv(env_var) + source = "env" if db_dir is None: if env_var == "LANCEDB_DIR": # Use default path only for the standard LANCEDB_DIR environment variable db_dir = self.get_default_lancedb_dir() - logger.info(f"Using default LanceDB directory: {db_dir}") + source = "default" else: # For other environment variables, raise KeyError as before raise KeyError(f"Environment variable {env_var} is not set") elif db_dir.strip() == "": raise ValueError(f"Environment variable {env_var} is empty") + logger.info( + "Using LanceDB directory: %s (source=%s, env_var=%s)", + db_dir, + source, + env_var, + ) return self.get_connection(db_dir) diff --git a/src/xagent/web/api/kb.py b/src/xagent/web/api/kb.py index 3cc04eda9..515953701 100644 --- a/src/xagent/web/api/kb.py +++ b/src/xagent/web/api/kb.py @@ -13,6 +13,8 @@ import uuid from pathlib import Path from typing import Any, Callable, Dict, List, Optional, TypedDict, TypeVar, cast +from urllib.parse import urlparse +from uuid import uuid4 from fastapi import ( APIRouter, @@ -1207,6 +1209,7 @@ async def ingest( max_retries: Maximum retry attempts for failures. retry_delay: Delay between retry attempts in seconds. """ + trace_id = uuid4().hex if not file.filename or not file.filename.strip(): raise HTTPException(status_code=422, detail="No filename provided") @@ -1228,6 +1231,18 @@ async def ingest( collection = Path(safe_filename).stem logger.info("Using file name as collection: %s", collection) + logger.info( + "KB ingest request received", + extra={ + "trace_id": trace_id, + "collection": collection, + "filename": safe_filename, + "content_type": getattr(file, "content_type", None), + "user_id": getattr(_user, "id", None), + "is_admin": bool(getattr(_user, "is_admin", False)), + }, + ) + try: # SECURITY: Validate collection name at API boundary safe_collection = sanitize_path_component(collection, "collection") @@ -1373,6 +1388,23 @@ async def ingest( else 1.0, ) + logger.info( + "KB ingest ingestion_config built", + extra={ + "trace_id": trace_id, + "collection": collection, + "parse_method": str(config.parse_method), + "chunk_strategy": str(config.chunk_strategy), + "chunk_size": config.chunk_size, + "chunk_overlap": config.chunk_overlap, + "separators": config.separators, + "embedding_model_id": config.embedding_model_id, + "embedding_batch_size": config.embedding_batch_size, + "max_retries": config.max_retries, + "retry_delay": config.retry_delay, + }, + ) + progress_manager = get_progress_manager() try: @@ -1449,6 +1481,21 @@ def _run_ingestion() -> IngestionResult: except OSError: logger.warning("Failed to remove ingest backup %s", file_backup_path) + logger.info( + "KB ingest result returned", + extra={ + "trace_id": trace_id, + "collection": collection, + "doc_id": result.doc_id, + "parse_hash": result.parse_hash, + "status": result.status, + "failed_step": result.failed_step, + "chunk_count": result.chunk_count, + "embedding_count": result.embedding_count, + "vector_count": result.vector_count, + }, + ) + return JSONResponse( status_code=200, content={**result.model_dump(), "file_id": file_record.file_id}, @@ -1510,7 +1557,6 @@ async def ingest_cloud( results = [] - # Common configuration setup final_chunk_size = ( request.chunk_size if request.chunk_size and request.chunk_size > 0 else 1000 ) @@ -1556,7 +1602,7 @@ async def ingest_cloud( except Exception as e: logger.warning("Failed to save collection config during ingest_cloud: %s", e) - # Concurrency limit for cloud ingestion to avoid overloading + semaphore = asyncio.Semaphore(5) async def process_file(file_info: CloudFile) -> IngestionResult: @@ -1585,7 +1631,6 @@ async def process_file(file_info: CloudFile) -> IngestionResult: ) try: if file_info.provider == "google-drive": - # Get credentials (run in thread to avoid blocking) try: creds = await asyncio.to_thread( get_google_credentials, int(_user.id), db @@ -1597,12 +1642,10 @@ async def process_file(file_info: CloudFile) -> IngestionResult: doc_id=file_info.fileName, ) - # Build service (blocking) service = await asyncio.to_thread( build, "drive", "v3", credentials=creds, cache_discovery=False ) - # Save to local path had_existing_file = file_path.exists() if had_existing_file: file_backup_path = file_path.with_name( @@ -1610,7 +1653,6 @@ async def process_file(file_info: CloudFile) -> IngestionResult: ) shutil.copy2(file_path, file_backup_path) - # Download file directly to disk try: def _download_file() -> None: @@ -1666,7 +1708,6 @@ def _download_file() -> None: file_size=int(file_path.stat().st_size), ) - # Run ingestion (blocking) try: normalized_parse_method = _normalize_parse_method_for_filename( request.parse_method, @@ -1776,7 +1817,6 @@ def _download_file() -> None: doc_id=file_info.fileName, ) - # Run all file processings concurrently results = await asyncio.gather(*[process_file(f) for f in request.files]) if not collection_existed_before and not any( @@ -2243,6 +2283,19 @@ async def ingest_web( True, description="Respect robots.txt (default: True)", ), + render_js: Optional[bool] = Form( + False, + description="Render pages with a real browser (Playwright) for JS-heavy sites (default: False)", + ), + render_wait_until: Optional[str] = Form( + "networkidle", + description="Playwright wait_until: load|domcontentloaded|networkidle (default: networkidle)", + ), + render_timeout_ms: Optional[int] = Form( + 30000, + ge=1, + description="Playwright navigation timeout in ms (default: 30000)", + ), # IngestionConfig parameters parse_method: Optional[ParseMethod] = Form( None, @@ -2316,6 +2369,54 @@ async def ingest_web( max_retries: Maximum retry attempts retry_delay: Delay between retries """ + trace_id = uuid4().hex + start_url = start_url.strip() + parsed_start = urlparse(start_url) + if parsed_start.scheme not in ("http", "https") or not parsed_start.netloc: + raise HTTPException( + status_code=422, + detail=( + "Invalid start_url. It must be an absolute URL starting with " + "'http://' or 'https://', e.g. 'https://example.com'." + ), + ) + + logger.info( + "KB web ingest request received", + extra={ + "trace_id": trace_id, + "collection": collection, + "start_url": start_url, + "user_id": int(_user.id), + "is_admin": bool(_user.is_admin), + "max_pages": max_pages, + "max_depth": max_depth, + "same_domain_only": same_domain_only, + "url_patterns": url_patterns, + "exclude_patterns": exclude_patterns, + "content_selector": content_selector, + "remove_selectors": remove_selectors, + "concurrent_requests": concurrent_requests, + "request_delay": request_delay, + "timeout": timeout, + "respect_robots_txt": respect_robots_txt, + "render_js": render_js, + "render_wait_until": render_wait_until, + "render_timeout_ms": render_timeout_ms, + "parse_method": str(parse_method) if parse_method is not None else None, + "chunk_strategy": str(chunk_strategy) + if chunk_strategy is not None + else None, + "chunk_size": chunk_size, + "chunk_overlap": chunk_overlap, + "separators": separators, + "embedding_model_id": embedding_model_id, + "embedding_batch_size": embedding_batch_size, + "max_retries": max_retries, + "retry_delay": retry_delay, + }, + ) + try: try: safe_collection = sanitize_path_component(collection, "collection") @@ -2642,10 +2743,30 @@ def _file_handler_with_db( user_id=int(_user.id), is_admin=bool(_user.is_admin), file_handler=_file_handler_with_db, + trace_id=trace_id, ) ), ) + logger.info( + "KB web ingest result returned", + extra={ + "trace_id": trace_id, + "collection": collection, + "start_url": start_url, + "status": getattr(result, "status", None), + "total_urls_found": getattr(result, "total_urls_found", None), + "pages_crawled": getattr(result, "pages_crawled", None), + "pages_failed": getattr(result, "pages_failed", None), + "documents_created": getattr(result, "documents_created", None), + "chunks_created": getattr(result, "chunks_created", None), + "embeddings_created": getattr(result, "embeddings_created", None), + "failed_urls_count": len(getattr(result, "failed_urls", {}) or {}), + "warnings_count": len(getattr(result, "warnings", []) or []), + "elapsed_time_ms": getattr(result, "elapsed_time_ms", None), + }, + ) + if result.status == "error": if not collection_existed_before: await _cleanup_failed_new_collection_metadata( diff --git a/tests/core/tools/core/RAG_tools/pipelines/test_web_ingestion_fail_fast_on_crawl_failure.py b/tests/core/tools/core/RAG_tools/pipelines/test_web_ingestion_fail_fast_on_crawl_failure.py new file mode 100644 index 000000000..c2a7172f8 --- /dev/null +++ b/tests/core/tools/core/RAG_tools/pipelines/test_web_ingestion_fail_fast_on_crawl_failure.py @@ -0,0 +1,37 @@ +import pytest + +from xagent.core.tools.core.RAG_tools.core.schemas import WebCrawlConfig +from xagent.core.tools.core.RAG_tools.pipelines import web_ingestion + + +@pytest.mark.asyncio +async def test_run_web_ingestion_fail_fast_when_no_pages_crawled( + monkeypatch: pytest.MonkeyPatch, +) -> None: + class DummyCrawler: + def __init__(self, *_args, **_kwargs) -> None: + self.failed_urls = {"https://example.com": "Insufficient content"} + self.total_urls_found = 1 + + async def crawl(self): + return [] + + monkeypatch.setattr(web_ingestion, "WebCrawler", DummyCrawler) + + res = await web_ingestion.run_web_ingestion( + collection="c", + crawl_config=WebCrawlConfig( + start_url="https://example.com", max_pages=1, max_depth=1 + ), + ingestion_config=None, + user_id=1, + is_admin=False, + trace_id="t", + ) + + assert res.status == "error" + assert res.pages_crawled == 0 + assert res.pages_failed == 1 + assert res.documents_created == 0 + assert "Website crawling failed" in res.message + assert "Insufficient content" in res.message diff --git a/tests/web/api/test_kb_ingest_separators.py b/tests/web/api/test_kb_ingest_separators.py index 5700c2831..afb1e576f 100644 --- a/tests/web/api/test_kb_ingest_separators.py +++ b/tests/web/api/test_kb_ingest_separators.py @@ -115,6 +115,7 @@ def capture_ingestion( user_id, progress_manager=None, is_admin=False, + **_: object, ): captured_config.append(ingestion_config) return IngestionResult( @@ -319,6 +320,7 @@ def capture_ingestion( user_id, progress_manager=None, is_admin=False, + **_: object, ): captured_config.append(ingestion_config) return IngestionResult( @@ -389,6 +391,7 @@ def capture_ingestion( user_id, progress_manager=None, is_admin=False, + **_: object, ): captured_config.append(ingestion_config) return IngestionResult( @@ -441,6 +444,7 @@ def capture_ingestion( user_id, progress_manager=None, is_admin=False, + **_: object, ): captured_config.append(ingestion_config) return IngestionResult( @@ -508,6 +512,7 @@ async def _fake_run_web_ingestion( user_id, is_admin=False, file_handler=None, + **_: object, ): """Async fake that captures ingestion_config and returns WebIngestionResult.""" captured_config: list = _fake_run_web_ingestion.captured # type: ignore[attr-defined] diff --git a/tests/web/api/test_kb_ingest_web_invalid_start_url.py b/tests/web/api/test_kb_ingest_web_invalid_start_url.py new file mode 100644 index 000000000..0577c20d0 --- /dev/null +++ b/tests/web/api/test_kb_ingest_web_invalid_start_url.py @@ -0,0 +1,77 @@ +"""Tests for KB web ingestion input validation.""" + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from xagent.web.api.kb import kb_router + + +@pytest.fixture +def mock_user(): + """Minimal user-like object for ingest dependency.""" + + return type("User", (), {"id": 1, "is_admin": False})() + + +@pytest.fixture +def app_with_kb(mock_user): + """FastAPI app with kb_router and mocked auth.""" + + from xagent.web.api.kb import get_current_user + + def override_get_current_user(): + return mock_user + + app = FastAPI() + app.include_router(kb_router) + app.dependency_overrides[get_current_user] = override_get_current_user + return app + + +@pytest.mark.parametrize( + "start_url", + [ + "xinference.cn", + " www.xinference.cn ", + "ftp://xinference.cn", + "https://", + "http://", + "://xinference.cn", + "", + " ", + ], +) +def test_ingest_web_rejects_invalid_start_url(app_with_kb, start_url: str) -> None: + client = TestClient(app_with_kb) + + resp = client.post( + "/api/kb/ingest-web", + data={ + "collection": "test_coll", + "start_url": start_url, + }, + ) + + assert resp.status_code == 422 + body = resp.json() + assert "Invalid start_url" in body.get("detail", "") + + +def test_ingest_web_accepts_stripped_url(app_with_kb) -> None: + """Whitespace/newline around a valid URL should be accepted after strip().""" + + client = TestClient(app_with_kb) + resp = client.post( + "/api/kb/ingest-web", + data={ + "collection": "test_coll", + "start_url": " https://xinference.cn\n", + "max_pages": "1", + "max_depth": "1", + "respect_robots_txt": "false", + }, + ) + + # We don't assert success here (depends on network), only that validation passed. + assert resp.status_code != 422 From 301e2977df10cf199c28ab117a0ce707bf131aaf Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Wed, 18 Mar 2026 16:35:48 +0800 Subject: [PATCH 2/7] test(kb): relax blank start_url validation assertion Some FastAPI/Starlette versions treat empty form fields as missing ("Field required"), while others pass an empty string through to our custom start_url validation. This test now accepts either 422 detail shape for blank inputs. Made-with: Cursor --- .../api/test_kb_ingest_web_invalid_start_url.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/web/api/test_kb_ingest_web_invalid_start_url.py b/tests/web/api/test_kb_ingest_web_invalid_start_url.py index 0577c20d0..b7fd2f175 100644 --- a/tests/web/api/test_kb_ingest_web_invalid_start_url.py +++ b/tests/web/api/test_kb_ingest_web_invalid_start_url.py @@ -55,7 +55,21 @@ def test_ingest_web_rejects_invalid_start_url(app_with_kb, start_url: str) -> No assert resp.status_code == 422 body = resp.json() - assert "Invalid start_url" in body.get("detail", "") + detail = body.get("detail", "") + + # Different FastAPI/Starlette versions may treat empty form fields as missing + # ("Field required") or pass through as empty string (our custom validator). + # Accept either behavior for blank inputs. + if not start_url.strip(): + if isinstance(detail, list): + assert any( + (isinstance(item, dict) and item.get("msg") == "Field required") + for item in detail + ) + else: + assert "Invalid start_url" in str(detail) + else: + assert "Invalid start_url" in str(detail) def test_ingest_web_accepts_stripped_url(app_with_kb) -> None: From 08400dc7a1b6a74a6d57d83391772998fb38e84c Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Mon, 11 May 2026 16:35:46 +0800 Subject: [PATCH 3/7] fix: resolve post-rebase test failures (mypy + 13 test fixes) - Remove duplicate `owners: List[str]` field in CollectionInfo (mypy no-redef) - Fix datetime naive/aware mismatch in web_ingestion fail-fast path - Add get_db override in test_kb_ingest_web_invalid_start_url fixture - Accept extra kwargs in test_kb_web_ingestion_uploaded_file stubs (trace_id) - Update test assertion to match main's "Website crawling failed:" message format --- src/xagent/core/tools/core/RAG_tools/core/schemas.py | 6 ------ .../core/tools/core/RAG_tools/pipelines/web_ingestion.py | 1 - tests/web/api/test_kb_ingest_web_invalid_start_url.py | 7 +++++++ tests/web/api/test_kb_web_ingestion_uploaded_file.py | 2 ++ 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/xagent/core/tools/core/RAG_tools/core/schemas.py b/src/xagent/core/tools/core/RAG_tools/core/schemas.py index 5866e763a..4a3d41ce3 100644 --- a/src/xagent/core/tools/core/RAG_tools/core/schemas.py +++ b/src/xagent/core/tools/core/RAG_tools/core/schemas.py @@ -1291,12 +1291,6 @@ class CollectionInfo(BaseModel): # Basic identifier name: str = Field(..., description="Collection identifier") - # 👥 Ownership / access control (stored as JSON string in LanceDB) - owners: List[str] = Field( - default_factory=list, - description="Collection owners. Stored as JSON string in LanceDB for compatibility.", - ) - # 🎯 Core binding: Embedding configuration (lazy initialization) embedding_model_id: Optional[str] = Field( default=None, # None indicates not initialized diff --git a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py index fa697b362..19df5acf3 100644 --- a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py +++ b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py @@ -479,7 +479,6 @@ def _ingest_file() -> IngestionResult: f"{total_chunks} chunks, {total_embeddings} embeddings" ) - result = WebIngestionResult( status=status, collection=collection, diff --git a/tests/web/api/test_kb_ingest_web_invalid_start_url.py b/tests/web/api/test_kb_ingest_web_invalid_start_url.py index b7fd2f175..082c5ba9e 100644 --- a/tests/web/api/test_kb_ingest_web_invalid_start_url.py +++ b/tests/web/api/test_kb_ingest_web_invalid_start_url.py @@ -18,14 +18,21 @@ def mock_user(): def app_with_kb(mock_user): """FastAPI app with kb_router and mocked auth.""" + from unittest.mock import MagicMock + from xagent.web.api.kb import get_current_user + from xagent.web.models.database import get_db def override_get_current_user(): return mock_user + def override_get_db(): + yield MagicMock() + app = FastAPI() app.include_router(kb_router) app.dependency_overrides[get_current_user] = override_get_current_user + app.dependency_overrides[get_db] = override_get_db return app diff --git a/tests/web/api/test_kb_web_ingestion_uploaded_file.py b/tests/web/api/test_kb_web_ingestion_uploaded_file.py index e94dddb38..4b042871a 100644 --- a/tests/web/api/test_kb_web_ingestion_uploaded_file.py +++ b/tests/web/api/test_kb_web_ingestion_uploaded_file.py @@ -535,6 +535,7 @@ async def stub_run_web_ingestion( user_id: int, is_admin: bool, file_handler, + **_: object, ): temp_md = tmp_path / "temp.md" temp_md.write_text("# Title\n\nBody", encoding="utf-8") @@ -633,6 +634,7 @@ async def stub_run_web_ingestion( user_id: int, is_admin: bool, file_handler, + **_: object, ): temp_md = tmp_path / "temp.md" temp_md.write_text("# Title\n\nBody", encoding="utf-8") From 0bda870f4d41dd4feb11260109c77304927bd66a Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Mon, 11 May 2026 16:53:21 +0800 Subject: [PATCH 4/7] fix(web-crawler): wire Playwright rendering path and address review findings Critical fixes: - Pass render_js/render_wait_until/render_timeout_ms to WebCrawlConfig in kb.py - Route _crawl_page through _fetch_html_rendered when render_js=True - Remove duplicate owners column from PyArrow metadata schema - Remove owners migration leftover (conflicts with derived-only design) Major fixes: - Pass trace_id to run_document_ingestion in web_ingestion pipeline - Clean up Playwright process on _start_playwright failure - Replace defensive getattr(config, "render_js", False) with direct access Performance: - Guard html_snippet construction with logger.isEnabledFor(DEBUG) - Downgrade is_valid_content rejection logs from INFO to DEBUG --- .../management/collection_manager.py | 1 - .../core/RAG_tools/pipelines/web_ingestion.py | 7 +- .../core/RAG_tools/utils/migration_utils.py | 2 - .../RAG_tools/web_crawler/content_cleaner.py | 4 +- .../core/RAG_tools/web_crawler/crawler.py | 103 ++++++++++-------- src/xagent/web/api/kb.py | 3 + 6 files changed, 67 insertions(+), 53 deletions(-) diff --git a/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py b/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py index d16de0f0a..5aee7ddaa 100644 --- a/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py +++ b/src/xagent/core/tools/core/RAG_tools/management/collection_manager.py @@ -279,7 +279,6 @@ async def _ensure_metadata_table(self) -> None: schema = pa.schema( [ ("name", pa.string()), - ("owners", pa.string()), # JSON string ("schema_version", pa.string()), ("embedding_model_id", pa.string()), # Nullable ("embedding_dimension", pa.int32()), # Nullable diff --git a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py index 19df5acf3..8420a70d5 100644 --- a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py +++ b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py @@ -136,9 +136,9 @@ async def run_web_ingestion( "request_delay": crawl_config.request_delay, "timeout": crawl_config.timeout, "respect_robots_txt": crawl_config.respect_robots_txt, - "render_js": getattr(crawl_config, "render_js", None), - "render_wait_until": getattr(crawl_config, "render_wait_until", None), - "render_timeout_ms": getattr(crawl_config, "render_timeout_ms", None), + "render_js": crawl_config.render_js, + "render_wait_until": crawl_config.render_wait_until, + "render_timeout_ms": crawl_config.render_timeout_ms, "ingestion_parse_method": str(ing_cfg.parse_method), "ingestion_chunk_strategy": str(ing_cfg.chunk_strategy), "ingestion_chunk_size": ing_cfg.chunk_size, @@ -343,6 +343,7 @@ def _ingest_file() -> IngestionResult: progress_manager=progress_manager, user_id=user_id, is_admin=is_admin, + trace_id=trace_id, ) with concurrent.futures.ThreadPoolExecutor() as executor: diff --git a/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py b/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py index 33122e091..ca4c05606 100644 --- a/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py +++ b/src/xagent/core/tools/core/RAG_tools/utils/migration_utils.py @@ -109,8 +109,6 @@ def _migrate_0_0_0_to_1_0_0( "schema_version": "1.0.0", # Basic fields (preserve existing or set defaults) "name": collection_name, - # Owners (default empty for legacy data) - "owners": data.get("owners", []), # Embedding config (inferred from existing data or None for lazy init) "embedding_model_id": embedding_model_id, "embedding_dimension": embedding_dimension, diff --git a/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py b/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py index 8cc795b8e..86f89b278 100644 --- a/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py +++ b/src/xagent/core/tools/core/RAG_tools/web_crawler/content_cleaner.py @@ -223,7 +223,7 @@ def is_valid_content(self, content: str, min_length: int = 100) -> bool: True if valid, False otherwise """ if not content or len(content) < min_length: - logger.info( + logger.debug( "Content rejected by min_length", extra={ "trace_id": self.trace_id, @@ -236,7 +236,7 @@ def is_valid_content(self, content: str, min_length: int = 100) -> bool: # Check if content has actual text (not just empty lines/spaces) lines = [line.strip() for line in content.split("\n") if line.strip()] if len(lines) == 0: - logger.info( + logger.debug( "Content rejected: no non-empty lines", extra={ "trace_id": self.trace_id, diff --git a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py index 8f9fab4aa..6c499e059 100644 --- a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py +++ b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py @@ -290,14 +290,14 @@ async def crawl(self) -> List[CrawlResult]: } try: - if getattr(self.config, "render_js", False): + if self.config.render_js: ua = self.config.user_agent or _DEFAULT_USER_AGENT await self._start_playwright(user_agent=ua) async with self._open_sessions(httpx_headers) as sessions: await self._crawl_loop(sessions) finally: - if getattr(self.config, "render_js", False): + if self.config.render_js: await self._stop_playwright() elapsed = time.time() - self.start_time @@ -569,29 +569,38 @@ async def _crawl_page( try: logger.debug("Crawling %s (depth: %s)", url, depth) - # Fetch page (with TLS fingerprint fallback chain) - response, fingerprint_used = await self._fetch_with_fallback( - sessions, url - ) - if response is None: - error_msg = "All TLS fingerprints raised exceptions" - logger.error("Failed to crawl %s: %s", url, error_msg) - self.failed_urls[url] = error_msg - return None, set() - - if fingerprint_used is None and 200 <= response.status_code < 300: - error_msg = "TLS fallback exhausted with challenge page" - logger.error("Failed to crawl %s: %s", url, error_msg) - self.failed_urls[url] = error_msg - return None, set() - - if not 200 <= response.status_code < 300: - error_msg = f"HTTP {response.status_code}" - logger.error("Failed to crawl %s: %s", url, error_msg) - self.failed_urls[url] = error_msg - return None, set() - - html = response.text + # Fetch page — Playwright (JS rendered) or TLS fallback chain + if self.config.render_js: + try: + html, _meta = await self._fetch_html_rendered(url) + except RuntimeError as e: + error_msg = str(e) + logger.error("Failed to crawl %s: %s", url, error_msg) + self.failed_urls[url] = error_msg + return None, set() + else: + response, fingerprint_used = await self._fetch_with_fallback( + sessions, url + ) + if response is None: + error_msg = "All TLS fingerprints raised exceptions" + logger.error("Failed to crawl %s: %s", url, error_msg) + self.failed_urls[url] = error_msg + return None, set() + + if fingerprint_used is None and 200 <= response.status_code < 300: + error_msg = "TLS fallback exhausted with challenge page" + logger.error("Failed to crawl %s: %s", url, error_msg) + self.failed_urls[url] = error_msg + return None, set() + + if not 200 <= response.status_code < 300: + error_msg = f"HTTP {response.status_code}" + logger.error("Failed to crawl %s: %s", url, error_msg) + self.failed_urls[url] = error_msg + return None, set() + + html = response.text # Clean and convert content cleaned = self.content_cleaner.clean_and_convert(html, url) @@ -599,9 +608,6 @@ async def _crawl_page( # Validate content content = cleaned["content_markdown"] if not self.content_cleaner.is_valid_content(content, min_length=10): - snippet = (content or "").replace("\n", "\\n") - if len(snippet) > 400: - snippet = snippet[:400] + "...(truncated)" logger.warning( "Insufficient content at %s (content_length=%s, title=%s)", url, @@ -609,12 +615,16 @@ async def _crawl_page( cleaned.get("title"), extra={"trace_id": self.trace_id}, ) - logger.debug( - "Insufficient content snippet at %s (snippet=%s)", - url, - snippet, - extra={"trace_id": self.trace_id}, - ) + if logger.isEnabledFor(logging.DEBUG): + snippet = (content or "").replace("\n", "\\n") + if len(snippet) > 400: + snippet = snippet[:400] + "...(truncated)" + logger.debug( + "Insufficient content snippet at %s (snippet=%s)", + url, + snippet, + extra={"trace_id": self.trace_id}, + ) self.failed_urls[url] = "Insufficient content" return None, set() @@ -784,16 +794,17 @@ def _log_fetched_page( len(html or ""), extra={"trace_id": self.trace_id}, ) - html_snippet = (html or "").replace("\n", "\\n") - if len(html_snippet) > 400: - html_snippet = html_snippet[:400] + "...(truncated)" - logger.debug( - "Fetched page snippet [%s] (requested_url=%s, html_snippet=%s)", - mode, - requested_url, - html_snippet, - extra={"trace_id": self.trace_id}, - ) + if logger.isEnabledFor(logging.DEBUG): + html_snippet = (html or "").replace("\n", "\\n") + if len(html_snippet) > 400: + html_snippet = html_snippet[:400] + "...(truncated)" + logger.debug( + "Fetched page snippet [%s] (requested_url=%s, html_snippet=%s)", + mode, + requested_url, + html_snippet, + extra={"trace_id": self.trace_id}, + ) async def _start_playwright(self, *, user_agent: str) -> None: try: @@ -808,6 +819,8 @@ async def _start_playwright(self, *, user_agent: str) -> None: self._playwright = await async_playwright().start() chromium = getattr(self._playwright, "chromium", None) if chromium is None: + await self._playwright.stop() + self._playwright = None raise RuntimeError("Playwright chromium is unavailable") try: self._browser = await chromium.launch(headless=True) @@ -815,8 +828,8 @@ async def _start_playwright(self, *, user_agent: str) -> None: user_agent=user_agent ) except PlaywrightError as exc: + await self._stop_playwright() msg = str(exc) - # Common setup issue: playwright python package installed but browsers not downloaded. if "Executable doesn't exist" in msg or "playwright install" in msg: raise RuntimeError( "Playwright browsers are not installed. " diff --git a/src/xagent/web/api/kb.py b/src/xagent/web/api/kb.py index 515953701..b63463e27 100644 --- a/src/xagent/web/api/kb.py +++ b/src/xagent/web/api/kb.py @@ -2459,6 +2459,9 @@ async def ingest_web( respect_robots_txt=( respect_robots_txt if respect_robots_txt is not None else True ), + render_js=bool(render_js), + render_wait_until=render_wait_until or "networkidle", + render_timeout_ms=render_timeout_ms or 30000, ) final_chunk_size = ( From 198a0b99a487b4cde64bd700443050d8655d3747 Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Tue, 12 May 2026 16:34:30 +0800 Subject: [PATCH 5/7] fix(kb): address PR #179 review comments - Validate HTTP status code in Playwright rendered path to reject non-2xx pages before content cleaning (data correctness bug) - Mock run_web_ingestion in test_ingest_web_accepts_stripped_url to avoid network/storage dependency and assert stripped URL - Add render_js checkbox to knowledge-base-creation-dialog for feature parity with the detail-page add-source flow - Replace hardcoded Chinese label with i18n key in detail page --- .../kb/knowledge-base-creation-dialog.tsx | 10 ++++ .../components/kb/knowledge-base-detail.tsx | 2 +- frontend/src/i18n/locales/en.ts | 1 + frontend/src/i18n/locales/zh.ts | 1 + .../core/RAG_tools/web_crawler/crawler.py | 8 +++ src/xagent/web/api/kb.py | 1 - .../test_kb_ingest_web_invalid_start_url.py | 49 +++++++++++++------ 7 files changed, 56 insertions(+), 16 deletions(-) diff --git a/frontend/src/components/kb/knowledge-base-creation-dialog.tsx b/frontend/src/components/kb/knowledge-base-creation-dialog.tsx index c4649c8a2..f08567c0b 100644 --- a/frontend/src/components/kb/knowledge-base-creation-dialog.tsx +++ b/frontend/src/components/kb/knowledge-base-creation-dialog.tsx @@ -844,6 +844,16 @@ export function KnowledgeBaseCreationDialog({ open, onOpenChange, onSuccess }: K /> +
+ setWebIngestionConfig(prev => ({ ...prev, render_js: e.target.checked }))} + className="w-4 h-4" + /> + +
)} diff --git a/frontend/src/components/kb/knowledge-base-detail.tsx b/frontend/src/components/kb/knowledge-base-detail.tsx index dd335def4..9b1f2e364 100644 --- a/frontend/src/components/kb/knowledge-base-detail.tsx +++ b/frontend/src/components/kb/knowledge-base-detail.tsx @@ -1313,7 +1313,7 @@ export function KnowledgeBaseDetailContent({ collectionName }: { collectionName: className="w-4 h-4" /> diff --git a/frontend/src/i18n/locales/en.ts b/frontend/src/i18n/locales/en.ts index 2a893ea06..be4991a56 100644 --- a/frontend/src/i18n/locales/en.ts +++ b/frontend/src/i18n/locales/en.ts @@ -1709,6 +1709,7 @@ Build when you need.` timeoutSeconds: "Request Timeout (seconds)", sameDomainOnly: "Same Domain Only", respectRobotsTxt: "Respect robots.txt", + renderJs: "Enable JS rendering (Playwright)", hintMultiple: "Separate multiple patterns with commas", hintContentSelector: "CSS selector for main content", }, diff --git a/frontend/src/i18n/locales/zh.ts b/frontend/src/i18n/locales/zh.ts index a24f15f1f..bad537288 100644 --- a/frontend/src/i18n/locales/zh.ts +++ b/frontend/src/i18n/locales/zh.ts @@ -1709,6 +1709,7 @@ Build when you need.` timeoutSeconds: "超时时间（秒）", sameDomainOnly: "仅同域名", respectRobotsTxt: "遵守 robots.txt", + renderJs: "启用 JS 渲染（Playwright）", hintMultiple: "多个模式用逗号分隔", hintContentSelector: "CSS 选择器，用于提取主内容", }, diff --git a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py index 6c499e059..a9f79faf5 100644 --- a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py +++ b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py @@ -578,6 +578,14 @@ async def _crawl_page( logger.error("Failed to crawl %s: %s", url, error_msg) self.failed_urls[url] = error_msg return None, set() + + raw_status = _meta.get("status_code") + if raw_status is not None and isinstance(raw_status, int): + if not (200 <= raw_status < 300): + error_msg = f"HTTP {raw_status}" + logger.error("Failed to crawl %s: %s", url, error_msg) + self.failed_urls[url] = error_msg + return None, set() else: response, fingerprint_used = await self._fetch_with_fallback( sessions, url diff --git a/src/xagent/web/api/kb.py b/src/xagent/web/api/kb.py index b63463e27..6cb3a80f9 100644 --- a/src/xagent/web/api/kb.py +++ b/src/xagent/web/api/kb.py @@ -1602,7 +1602,6 @@ async def ingest_cloud( except Exception as e: logger.warning("Failed to save collection config during ingest_cloud: %s", e) - semaphore = asyncio.Semaphore(5) async def process_file(file_info: CloudFile) -> IngestionResult: diff --git a/tests/web/api/test_kb_ingest_web_invalid_start_url.py b/tests/web/api/test_kb_ingest_web_invalid_start_url.py index 082c5ba9e..266561024 100644 --- a/tests/web/api/test_kb_ingest_web_invalid_start_url.py +++ b/tests/web/api/test_kb_ingest_web_invalid_start_url.py @@ -82,17 +82,38 @@ def test_ingest_web_rejects_invalid_start_url(app_with_kb, start_url: str) -> No def test_ingest_web_accepts_stripped_url(app_with_kb) -> None: """Whitespace/newline around a valid URL should be accepted after strip().""" - client = TestClient(app_with_kb) - resp = client.post( - "/api/kb/ingest-web", - data={ - "collection": "test_coll", - "start_url": " https://xinference.cn\n", - "max_pages": "1", - "max_depth": "1", - "respect_robots_txt": "false", - }, - ) - - # We don't assert success here (depends on network), only that validation passed. - assert resp.status_code != 422 + from unittest.mock import AsyncMock, patch + + mock_result = { + "status": "success", + "collection": "test_coll", + "pages_crawled": 1, + "pages_failed": 0, + "chunks_stored": 5, + "message": "OK", + } + + with patch( + "xagent.web.api.kb.run_web_ingestion", + new_callable=AsyncMock, + return_value=mock_result, + ) as mock_run: + client = TestClient(app_with_kb) + resp = client.post( + "/api/kb/ingest-web", + data={ + "collection": "test_coll", + "start_url": " https://xinference.cn\n", + "max_pages": "1", + "max_depth": "1", + "respect_robots_txt": "false", + }, + ) + + assert resp.status_code != 422 + + if mock_run.called: + call_kwargs = mock_run.call_args[1] + crawl_config = call_kwargs.get("crawl_config") + assert crawl_config is not None + assert crawl_config.start_url == "https://xinference.cn" From c43242bec29a356a97ae33cccfc2252f2f0b0dae Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Tue, 19 May 2026 11:58:44 +0800 Subject: [PATCH 6/7] chore(kb): address PR #179 non-blocking cleanup items - Remove unused _fetch_html/_fetch_html_static dead code in web crawler - Skip HTTP session creation when render_js uses Playwright only - Validate render_wait_until at API boundary with 422 on invalid values --- .../core/RAG_tools/web_crawler/crawler.py | 73 ++++++------------- src/xagent/web/api/kb.py | 12 ++- .../test_kb_ingest_web_invalid_start_url.py | 17 +++++ 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py index a9f79faf5..fde4d770b 100644 --- a/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py +++ b/src/xagent/core/tools/core/RAG_tools/web_crawler/crawler.py @@ -270,32 +270,33 @@ async def crawl(self) -> List[CrawlResult]: }, ) - httpx_headers: Optional[Dict[str, str]] = None - if None in self._tls_chain: - user_agent = self.config.user_agent or _DEFAULT_USER_AGENT - httpx_headers = { - "User-Agent": user_agent, - "Accept": ( - "text/html,application/xhtml+xml,application/xml;q=0.9," - "image/avif,image/webp,*/*;q=0.8" - ), - "Accept-Language": "en-US,en;q=0.9", - "Accept-Encoding": "gzip, deflate, br", - "Sec-Fetch-Dest": "document", - "Sec-Fetch-Mode": "navigate", - "Sec-Fetch-Site": "none", - "Sec-Fetch-User": "?1", - "Upgrade-Insecure-Requests": "1", - "DNT": "1", - } - try: if self.config.render_js: ua = self.config.user_agent or _DEFAULT_USER_AGENT await self._start_playwright(user_agent=ua) - - async with self._open_sessions(httpx_headers) as sessions: - await self._crawl_loop(sessions) + await self._crawl_loop({}) + else: + httpx_headers: Optional[Dict[str, str]] = None + if None in self._tls_chain: + user_agent = self.config.user_agent or _DEFAULT_USER_AGENT + httpx_headers = { + "User-Agent": user_agent, + "Accept": ( + "text/html,application/xhtml+xml,application/xml;q=0.9," + "image/avif,image/webp,*/*;q=0.8" + ), + "Accept-Language": "en-US,en;q=0.9", + "Accept-Encoding": "gzip, deflate, br", + "Sec-Fetch-Dest": "document", + "Sec-Fetch-Mode": "navigate", + "Sec-Fetch-Site": "none", + "Sec-Fetch-User": "?1", + "Upgrade-Insecure-Requests": "1", + "DNT": "1", + } + + async with self._open_sessions(httpx_headers) as sessions: + await self._crawl_loop(sessions) finally: if self.config.render_js: await self._stop_playwright() @@ -700,34 +701,6 @@ async def _process_links(self, links: Set[str], current_depth: int) -> None: if link not in self.visited_urls and link not in pending_urls_set: self.pending_urls.append((link, next_depth)) - async def _fetch_html( - self, client: httpx.AsyncClient, url: str - ) -> Tuple[str, Dict[str, object]]: - """Fetch HTML either via httpx or Playwright (rendered).""" - if self.config.render_js: - return await self._fetch_html_rendered(url) - return await self._fetch_html_static(client, url) - - async def _fetch_html_static( - self, client: httpx.AsyncClient, url: str - ) -> Tuple[str, Dict[str, object]]: - response = await client.get(url, follow_redirects=True) - response.raise_for_status() - html = response.text or "" - self._log_fetched_page( - requested_url=url, - final_url=str(response.url), - status_code=response.status_code, - content_type=response.headers.get("content-type"), - html=html, - mode="static", - ) - return html, { - "mode": "static", - "final_url": str(response.url), - "status_code": response.status_code, - } - async def _fetch_html_rendered(self, url: str) -> Tuple[str, Dict[str, object]]: # Lazy import to keep Playwright optional at runtime. try: diff --git a/src/xagent/web/api/kb.py b/src/xagent/web/api/kb.py index 6cb3a80f9..e70060eec 100644 --- a/src/xagent/web/api/kb.py +++ b/src/xagent/web/api/kb.py @@ -2380,6 +2380,16 @@ async def ingest_web( ), ) + effective_render_wait_until = render_wait_until or "networkidle" + if effective_render_wait_until not in ("load", "domcontentloaded", "networkidle"): + raise HTTPException( + status_code=422, + detail=( + "Invalid render_wait_until. Must be one of: " + "load, domcontentloaded, networkidle" + ), + ) + logger.info( "KB web ingest request received", extra={ @@ -2459,7 +2469,7 @@ async def ingest_web( respect_robots_txt if respect_robots_txt is not None else True ), render_js=bool(render_js), - render_wait_until=render_wait_until or "networkidle", + render_wait_until=effective_render_wait_until, render_timeout_ms=render_timeout_ms or 30000, ) diff --git a/tests/web/api/test_kb_ingest_web_invalid_start_url.py b/tests/web/api/test_kb_ingest_web_invalid_start_url.py index 266561024..a61316733 100644 --- a/tests/web/api/test_kb_ingest_web_invalid_start_url.py +++ b/tests/web/api/test_kb_ingest_web_invalid_start_url.py @@ -79,6 +79,23 @@ def test_ingest_web_rejects_invalid_start_url(app_with_kb, start_url: str) -> No assert "Invalid start_url" in str(detail) +def test_ingest_web_rejects_invalid_render_wait_until(app_with_kb) -> None: + """Invalid render_wait_until should return 422 at the API boundary.""" + + client = TestClient(app_with_kb) + resp = client.post( + "/api/kb/ingest-web", + data={ + "collection": "test_coll", + "start_url": "https://example.com", + "render_wait_until": "invalid", + }, + ) + + assert resp.status_code == 422 + assert "Invalid render_wait_until" in str(resp.json().get("detail", "")) + + def test_ingest_web_accepts_stripped_url(app_with_kb) -> None: """Whitespace/newline around a valid URL should be accepted after strip().""" From 9d30031a5ee8d6aa8c5b9ec8dd73852db43cf782 Mon Sep 17 00:00:00 2001 From: sqhyz55 Date: Tue, 19 May 2026 12:08:44 +0800 Subject: [PATCH 7/7] fix(kb): align fail-fast crawler block detection with main Use any blocking failure (not all) to return the friendly anti-bot message during fail-fast, matching post-rebase message behavior. --- .../core/RAG_tools/pipelines/web_ingestion.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py index 8420a70d5..7d3ee3601 100644 --- a/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py +++ b/src/xagent/core/tools/core/RAG_tools/pipelines/web_ingestion.py @@ -213,18 +213,23 @@ async def run_web_ingestion( elapsed_ms = int( (datetime.now(timezone.utc) - start_time).total_seconds() * 1000 ) - if failed_urls and all( - _looks_like_crawler_block(err) for err in failed_urls.values() - ): + blocking_entry = next( + ( + (url, err) + for url, err in failed_urls.items() + if _looks_like_crawler_block(err) + ), + None, + ) + if blocking_entry is not None: fail_fast_message = _CRAWLER_BLOCK_MESSAGE else: reasons: list[str] = [] for url, reason in list(failed_urls.items())[:3]: reasons.append(f"{url}: {reason}") reason_suffix = "; ".join(reasons) - fail_fast_message = ( - "Website crawling failed: no valid pages extracted." - + (f" Reasons: {reason_suffix}" if reason_suffix else "") + fail_fast_message = "Website crawling failed: no valid pages extracted." + ( + f" Reasons: {reason_suffix}" if reason_suffix else "" ) return WebIngestionResult( status="error",