SurfSense/plans/backend/03a-crawler-core.md
2026-06-24 21:07:08 -07:00

123 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Phase 3a — WebURL Crawler core (Scrapling-only) + success semantics
> Part of **Phase 3 — WebURL Crawler & Crawl Billing**. See `00-umbrella-plan.md`.
> Sibling subplans: `03b-proxy-expansion.md`, `03c-crawl-billing.md`, `03d-captcha-solving.md` (deferred).
> **Implementation note (applies to all Phase-3 plans).** Phase 3 lands **after** Phases 12, which rename `SearchSpace`→`Workspace` and `search_space_id`→`workspace_id` everywhere. Citations below use **today's** names so they stay greppable against current code; when implementing, the live code will already say `workspace_*` — map accordingly. Apply every edit by **symbol/grep** (e.g. `firecrawl_api_key`, `crawl_url`, `FIRECRAWL_API_KEY`), **not** by the absolute line numbers cited here — Phase 2's rename (and `03a`'s own Firecrawl removal) shift line numbers.
## Objective
Make the Universal WebURL Crawler a **single-framework (Scrapling) component** with deterministic per-URL outcome semantics and a clean, explicit **"successful crawl" signal** that Phase 3c bills on.
Two hard requirements from the decisions log:
1. **Remove Firecrawl entirely.** No other scraping framework now or planned. Scrapling's `StealthyFetcher` can bypass Cloudflare Turnstile when invoked with `solve_cloudflare=True` (see tier design below); captcha-tools (`03d`, deferred) covers the rest.
2. **One billable unit = one URL that yields usable extracted content**, regardless of how many internal fallback tiers ran. `03a` must expose that signal; `03c` meters it.
This subplan does NOT touch proxy rotation (→ `03b`), credit metering (→ `03c`), or captcha (→ `03d`).
## Current state (cited)
### The crawler
`app/connectors/webcrawler_connector.py``WebCrawlerConnector.crawl_url()` is a 4-tier fallback ladder:
1. **Firecrawl** (premium, if `firecrawl_api_key` set) — `_crawl_with_firecrawl()` (lines 89100, 223272), imports `from firecrawl import AsyncFirecrawlApp` (line 22).
2. Scrapling `AsyncFetcher` (static HTTP, curl_cffi) — `_crawl_with_async_fetcher()` (lines 274310).
3. Scrapling `DynamicFetcher` (browser, run in a thread) — `_crawl_with_dynamic()` (lines 312339).
4. Scrapling `StealthyFetcher` (Camoufox anti-bot, run in a thread) — `_crawl_with_stealthy()` (lines 341369).
Extraction is Trafilatura HTML→markdown in `_build_result()` (lines 371469). Every Scrapling tier passes `proxy=get_proxy_url()` (lines 287, 329, 359). `crawl_url()` returns a tuple `(result_dict | None, error | None)`; `result_dict` has `content` / `metadata` / `crawler_type`.
### The two call sites
- **Type-1 indexer (billable path):** `app/tasks/connector_indexers/webcrawler_indexer.py` reads `FIRECRAWL_API_KEY` from connector config (line 115), builds `WebCrawlerConnector(firecrawl_api_key=api_key)` (line 139), and calls `crawler.crawl_url(url)` per URL (line 297). It already tracks `documents_indexed` / `documents_updated` / `documents_skipped` / `documents_failed` / `duplicate_content_count` (lines 167171), but **none of these is a clean "crawl succeeded" count** — duplicates and unchanged docs are bucketed as skipped even though a successful fetch happened.
- **Chat scrape tool (ad-hoc):** `main_agent/tools/scrape_webpage.py` (line 232) and `subagents/builtins/research/tools/scrape_webpage.py` (line 226) build `WebCrawlerConnector(firecrawl_api_key=...)` and call `crawl_url(url, formats=["markdown"])`. The `formats` arg is Firecrawl-only (declared at `crawl_url` signature line 59; docstring line 72: "only for Firecrawl").
### Firecrawl threading (the cross-cutting surface)
Firecrawl's API key is plumbed end-to-end and must be removed everywhere:
| Layer | File:line | What to remove |
|-------|-----------|----------------|
| Crawler | `app/connectors/webcrawler_connector.py` | `firecrawl` import (22), `firecrawl_api_key`/`use_firecrawl` ctor (3646), `set_api_key()` (4856), tier-1 block (89100), `_crawl_with_firecrawl()` (223272), `formats` param + Firecrawl mentions in docstrings |
| Dependency | `pyproject.toml:45` | `"firecrawl-py>=4.9.0"` (+ regenerate `uv.lock`). Scrapling stays (`pyproject.toml:91``"scrapling[fetchers]>=0.4.9"`) |
| Connector config | `app/utils/validators.py` | `FIRECRAWL_API_KEY` from `WEBCRAWLER_CONNECTOR.optional`/`validators` (573580); delete `validate_firecrawl_api_key_format()` (472478) |
| Indexer | `app/tasks/connector_indexers/webcrawler_indexer.py` | `api_key = connector.config.get("FIRECRAWL_API_KEY")` (115), `use_firecrawl` log field (135), pass-through to ctor (139) |
| Chat setup | `app/tasks/chat/streaming/flows/shared/pre_stream_setup.py` | `setup_connector_and_firecrawl()` returns `firecrawl_api_key` (130) — collapse to connector-service-only |
| Chat orchestrators | `new_chat/orchestrator.py` (86, 378, 412, 665), `resume_chat/orchestrator.py` (65, 317, 347, 483) | `firecrawl_api_key` threading |
| Automations | `automations/actions/builtin/agent_task/dependencies.py` (19, 34, 85, 103), `.../invoke.py` (174) | `firecrawl_api_key` dep field |
| Main agent | `main_agent/runtime/factory.py` (71, 142), `main_agent/tools/registry.py` (36) | `firecrawl_api_key` param/wiring |
| Scrape tools | `main_agent/tools/scrape_webpage.py` (170, 232), `research/tools/scrape_webpage.py` (164, 226), `research/tools/index.py` (28) | `firecrawl_api_key` factory arg + ctor arg |
| Tests | `tests/unit/automations/actions/builtin/agent_task/test_dependencies.py` (44, 81) | `firecrawl_api_key == "fc-key"` assertion + fake |
> **Not a global env var.** `FIRECRAWL_API_KEY` is **not** in `.env.example` and there is no `Config.FIRECRAWL_API_KEY`. It lives only inside each WebCrawler connector's `config` JSON — the validator reads it from that dict (`validators.py:474`). So removal is code-only; **no env/docs change**. Existing `WEBCRAWLER_CONNECTOR` rows may still carry a now-dead `FIRECRAWL_API_KEY` key in `config` — harmless (it's simply ignored), optionally scrubbed by a tiny data migration if we want clean rows.
>
> **Signature change, not just deletions.** Removing `firecrawl_api_key` mutates the `RuntimeDeps`-style dataclass (`agent_task/dependencies.py:34`) and the agent runtime factory/tool factories — a coordinated signature change across the chat + automations call graph, so land it atomically.
### Runtime/deps already in place
- `Dockerfile:112115` runs `RUN scrapling install` to fetch patchright Chromium + Camoufox; the `scrapling[fetchers]` extra pulls playwright/patchright. **No new install step needed** once Firecrawl is gone.
- Proxy is read via `app/utils/proxy/get_proxy_url()` (`__init__.py:13`), backed by the `PROXY_PROVIDER` registry (`config/__init__.py:983`). `03a` leaves this single-URL model untouched (rotation is `03b`).
## Target design
### Tier ladder (Scrapling-only)
`crawl_url()` becomes a 3-tier ladder, preserving the existing thread-offload + `NotImplementedError` handling for the browser tiers (Windows `SelectorEventLoop` cannot spawn subprocesses — lines 134141, 161168):
1. `AsyncFetcher.get(...)` — fast static HTTP.
2. `DynamicFetcher.fetch(...)` — full browser (via `asyncio.to_thread`).
3. `StealthyFetcher.fetch(...)` — Camoufox anti-bot, last resort. Enable Cloudflare solving here by passing **`solve_cloudflare=True`** — a documented `StealthyFetcher.fetch` kwarg ("Solves all types of the Cloudflare's Turnstile/Interstitial challenges before returning the response", `references/Scrapling/scrapling/fetchers/stealth_chrome.py:38`; it's a `StealthSession` TypedDict key passed via `**kwargs`). The current stealthy call (connector lines 354360) passes `headless`/`network_idle`/`block_ads`/`proxy` but **not** `solve_cloudflare`, so this is a real behavior add. (Note: `solve_cloudflare` runs the full browser challenge loop, so it's correctly scoped to the last-resort tier only.)
Trafilatura extraction (`_build_result`) and `format_to_structured_document()` are unchanged.
### Explicit outcome model
Replace the implicit `(dict|None, str|None)` contract with an explicit outcome so callers (indexer, chat tool, and `03c` metering) agree on what "success" means. Proposed:
```python
class CrawlOutcomeStatus(str, Enum):
SUCCESS = "success" # a tier returned usable extracted content
EMPTY = "empty" # fetched, but no usable content after ALL tiers
FAILED = "failed" # invalid URL or every tier errored
```
`crawl_url()` returns a small dataclass `CrawlOutcome(status, result, error, tier)` (or, to minimize churn, keep the tuple and add a third element). Either way the **billable success predicate is single-sourced**: `status == SUCCESS`.
| Outcome | When | Billable (`03c`)? | Document status (indexer) |
|---------|------|-------------------|---------------------------|
| `SUCCESS` | a tier extracted usable content (`_build_result` returned a dict) | **Yes — 1 unit** | `ready` (or unchanged/duplicate, see note) |
| `EMPTY` | every tier was reached but none produced usable extracted content (the static tier also treats HTTP ≥ 400 as a miss and falls through — connector lines 292301; the browser tiers attempt extraction regardless of status) | No | `failed("No content extracted")` |
| `FAILED` | invalid URL, or all tiers raised | No | `failed(<error>)` |
**Billing-policy note for `03c`:** success is the *crawl* succeeding (we fetched + extracted), independent of downstream KB dedupe. The indexer currently marks unchanged content as `skipped` (`webcrawler_indexer.py:341347`) and cross-connector duplicates as `failed` (`:350369`) — those still represent a **successful crawl** and should bill. `03c` must count `CrawlOutcomeStatus.SUCCESS`, not `documents_indexed`. Flagging here; final call lives in `03c`.
### Success counter for the indexer
Add an explicit `crawls_succeeded` counter in `index_crawled_urls()` incremented whenever `crawl_url` returns `SUCCESS` (right after line 297's call, before the dedupe/unchanged branches), and surface it in the task-success metadata (lines 455466) and return value. This is the hand-off point `03c` meters against.
### Chat scrape tool
Drop the Firecrawl-only `formats=["markdown"]` arg (markdown is already the Trafilatura default). The tool keeps returning its asset dict; map the new outcome onto the existing `error` / `content` shape (lines 235269) with no behavioral change for the agent.
## Work items
1. **Rip out Firecrawl** across the surface table above (crawler, dep, validators, indexer, chat plumbing, automations, tests) — code-only, no env/docs change.
2. **Refactor `crawl_url`** to the 3-tier Scrapling ladder + explicit `CrawlOutcome`; enable `solve_cloudflare` on the stealthy tier.
3. **Add `crawls_succeeded`** counting in `webcrawler_indexer.py` + expose in result/metadata.
4. **Update both `scrape_webpage` tools** to drop `firecrawl_api_key` + `formats`.
5. **Tests:** unit tests for `crawl_url` outcomes (mock Scrapling fetchers → SUCCESS/EMPTY/FAILED); update `test_dependencies.py`; assert the indexer's `crawls_succeeded` count.
## Risks / trade-offs
- **Loss of a managed fallback.** Firecrawl was a hosted last resort for hostile anti-bot sites. Mitigation: `StealthyFetcher` + Cloudflare solving now, `03b` proxy rotation, `03d` captcha solving. Acceptable per the decisions log (single-framework intent).
- **Browser tiers on dev/Windows.** `DynamicFetcher`/`StealthyFetcher` need subprocess support; the existing `to_thread` + `NotImplementedError` guards (lines 134141, 161168) are preserved so static-only crawling still works in `uvicorn --reload`.
- **`uv.lock` churn.** Removing `firecrawl-py` requires a lockfile regen + image rebuild; no new runtime deps are added.
## Out of scope (hand-offs)
- Proxy provider expansion + rotation → `03b`.
- Crawl credit metering on `CrawlOutcomeStatus.SUCCESS``03c`.
- reCAPTCHA/hCaptcha solving via captcha-tools → `03d` (deferred). Cloudflare Turnstile stays in-framework (Scrapling).
- Whether ad-hoc **chat** scrapes are billed (vs only pipeline crawls) → decided in `03c`.