feat(web): content pipeline for web::fetch (static crawl parity, Phase 1) - #384
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 29 skipped (no docs/).
Four for four. Nicely done. |
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughIntroduces a new ChangesWeb content pipeline feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant FetchHandler as web::fetch
participant ContentPipeline as content::process
participant SelectFilter as select/filter modules
participant LinksMedia as links module
Client->>FetchHandler: fetch request (content_filter, target_elements, include_links, include_media)
FetchHandler->>FetchHandler: build_content_opts(payload, format)
FetchHandler->>ContentPipeline: process(html, base_url, opts, allow_transform)
ContentPipeline->>LinksMedia: extract_links / extract_media (if requested)
ContentPipeline->>SelectFilter: remove_excluded / scope_to_targets
SelectFilter-->>ContentPipeline: scoped html
ContentPipeline->>SelectFilter: filter::apply (pruning/bm25)
SelectFilter-->>ContentPipeline: filtered html
ContentPipeline->>ContentPipeline: render (Markdown/Text/HTML)
ContentPipeline-->>FetchHandler: PageContent (rendered, links, media, filtered)
FetchHandler-->>Client: response (body, transformed, links, media)
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
web/tests/content_pipeline.rs (2)
47-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a redirect case for URL resolution.
This only proves absolute URL joining on a direct fetch. The fetch-layer change here is threading the post-redirect
final_urlintoshape_response, so a/start -> /pfixture would actually verify thatlinks/mediaare resolved against the final URL rather than the originally requested one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/tests/content_pipeline.rs` around lines 47 - 70, The current test only covers absolute URL resolution on a direct fetch; update include_links_and_media_populate_envelope in web/tests/content_pipeline.rs to exercise a redirect flow and verify the post-redirect final_url is used by shape_response. Add a /start -> /p fixture in the existing serve-based setup, fetch /start, and assert links and media are resolved against the final URL rather than the originally requested URL.
132-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the backward-compat path exactly.
The PR objective says the no-new-fields path stays byte-identical, but this test only checks a few substrings and missing keys. An exact
body/envelope assertion would catch whitespace, extra-field, and transform-path regressions in the short-circuit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/tests/content_pipeline.rs` around lines 132 - 147, The backward-compat test in backward_compat_no_new_fields_unchanged only checks a few substrings and absent keys, so it can miss regressions in the short-circuit path. Strengthen the assertion in execute_fetch’s no-new-fields case by comparing the full returned body and envelope exactly, using the existing MockServer/serve setup and the same payload shape, so any whitespace, extra-field, or transform-path change is caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/docs/superpowers/specs/2026-06-30-web-static-crawl-parity-phase1-design.md`:
- Around line 84-92: The fenced block in this spec is missing a language tag,
which triggers markdownlint MD040. Update the fence in the docs snippet near the
content structure summary to explicitly use a plain-text language such as text
or plaintext, keeping the block content unchanged. Use the fenced block around
the src/content module overview as the target to locate it.
In `@web/README.md`:
- Around line 159-160: The README description for the `content_filter` behavior
overstates that `body` is always the filtered output; update the wording to
mention the fallback cases. In the `content_filter` docs section, clarify that
oversized or over-depth pages may keep raw content, and that if filtering yields
an empty result the selected unfiltered content is used instead. Keep the
guidance centered on the `content_filter`/`body` behavior so readers understand
when the filtered output is not returned.
In `@web/src/content/filter.rs`:
- Around line 143-175: The `link_density` calculation in the block-scoring logic
only counts anchors whose start offset falls inside the block, so ancestor links
wrapping a block are missed. Update the anchor attribution in the `dom.nodes()`
scan to also track anchor end offsets, and in the block loop around
`BLOCK_TAGS`/`tag.boundaries(parser)` treat any block fully enclosed by an
ancestor `<a>` as fully linked. Keep the existing prefix-sum approach in
`filter.rs`, but extend the `starts`/`prefix` lookup so `link_chars` includes
ancestor anchors before computing `link_density`.
- Around line 123-131: The current range-removal logic in the block filtering
path keeps any wrapper that overlaps a good descendant, which allows non-block
text and sibling content inside bad containers to leak through. Update the
implementation in the block selection/splicing flow around the block range
collection and the call to select::splice_out_ranges so the output is built from
merged good ranges only, rather than deleting just bad block spans. Use the
existing block start/end range data and the pruning logic in the filter code to
ensure only surviving block content remains for both prune and bm25.
In `@web/src/content/mod.rs`:
- Around line 97-109: The early return in the content pipeline is bypassing the
flat selection steps, so `select::remove_excluded` and
`select::scope_to_targets` in `mod.rs` are never applied when `allow_transform`
is false or `convert::max_tag_depth(html)` exceeds `convert::MAX_NESTING_DEPTH`.
Move the exclusion/scoping logic in the `PageContent` flow before the
depth/transform gate, then check nesting depth on the scoped `working` HTML and
only skip the recursive render/filter path after those flat `tl`-based helpers
have run.
In `@web/src/fetch.rs`:
- Around line 496-501: The truncation handling in the `rendered` response path
mutates HTML bodies by appending plain text, which breaks valid
`PageFormat::Html` output. Update the `fetch` logic around `pc.rendered` and
`resp.truncated` to preserve markup when `pf == PageFormat::Html`, using
`bytes_truncated` metadata or an HTML-safe comment instead of
`body.push_str(...)`. Keep the existing plain-text truncation message only for
non-HTML formats, and ensure the `transformed`/`format_label(pf)` flow still
reflects the rendered format correctly.
In `@web/src/schemas.rs`:
- Around line 154-174: The pruning threshold validation in `schemas.rs` is too
restrictive for `content_filter.threshold` when `FilterType::Pruning` is used.
Update the guard in the `content_filter` validation logic to either allow the
wider range accepted by crawl4ai (including negative values) or explicitly
document the narrower constraint if parity is intended. Keep the change
localized to the `FilterType::Pruning` branch so the `FilterType::Bm25`
validation remains unchanged.
---
Nitpick comments:
In `@web/tests/content_pipeline.rs`:
- Around line 47-70: The current test only covers absolute URL resolution on a
direct fetch; update include_links_and_media_populate_envelope in
web/tests/content_pipeline.rs to exercise a redirect flow and verify the
post-redirect final_url is used by shape_response. Add a /start -> /p fixture in
the existing serve-based setup, fetch /start, and assert links and media are
resolved against the final URL rather than the originally requested URL.
- Around line 132-147: The backward-compat test in
backward_compat_no_new_fields_unchanged only checks a few substrings and absent
keys, so it can miss regressions in the short-circuit path. Strengthen the
assertion in execute_fetch’s no-new-fields case by comparing the full returned
body and envelope exactly, using the existing MockServer/serve setup and the
same payload shape, so any whitespace, extra-field, or transform-path change is
caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18cfd970-868e-43d8-9ea6-939f78fcebb0
⛔ Files ignored due to path filters (1)
web/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
web/Cargo.tomlweb/README.mdweb/docs/superpowers/plans/2026-06-30-web-static-crawl-parity-phase1.mdweb/docs/superpowers/specs/2026-06-30-web-static-crawl-parity-phase1-design.mdweb/skills/index.mdweb/src/content/filter.rsweb/src/content/links.rsweb/src/content/meta.rsweb/src/content/mod.rsweb/src/content/select.rsweb/src/fetch.rsweb/src/lib.rsweb/src/schemas.rsweb/tests/content_pipeline.rs
…e 1) Enrich web::fetch with a single-page content pipeline. All new request fields are optional; with none set, output is byte-identical to before (short-circuit), so existing callers and the harness consumer are unaffected. - Content filters: pruning (boilerplate scoring) and BM25 (query relevance); filtered output replaces `body`. - Content selection: target_elements (restrict to CSS regions) and excluded_tags (drop subtrees) before rendering. - Link & media extraction: opt-in include_links / include_media with URL resolution and internal/external classification. - Page-metadata extraction feeds the BM25 query fallback. - Pipeline orchestrator (content::process) wired into fetch::shape_response, run under spawn_blocking; pure, synchronous, unit-tested modules.
08fdaaa to
18224d3
Compare
- collect_blocks: a block fully enclosed by an ancestor <a> (valid HTML5 <a><div>…</div></a> cards) is now scored as fully linked, so link tiles no longer survive pruning as prose. Adds end-offset tracking + prefix-max of anchor ends; descendant attribution unchanged. - shape_response: don't append the plain-text truncation note to format:"html" bodies (kept mixed HTML+text); bytes_truncated already signals truncation for every format. - schemas: document why the pruning threshold guard is [0,1] (scores are normalized to that range). - README: note body falls back to unfiltered content when a filter empties the page or it's too large/deep to transform.
Phase 1: static-crawl content pipeline for the
webworkerEnriches the existing
web::fetchbus function with a single-page content pipeline, the first phase of reaching full static-HTTP crawl parity. No browser, no JS — pure static HTTP + DOM.What's added
pruning(boilerplate scoring,threshold=0.48) andbm25(query relevance,threshold=1.0). Filtered output replacesbody(the JSON envelope is the LLM context, so we don't return both raw + fit).target_elements(restrict to CSS regions) andexcluded_tags(drop subtrees likenav/footer/aside) before rendering.include_links/include_media, with URL resolution.queryfallback when no explicit query is given.content/mod.rs::process) wired intofetch.rs::shape_response.Backward compatibility
Hard requirement, preserved: all new request fields are optional. When none are set,
web::fetchoutput is byte-identical to today via short-circuit. Existing callers and the harness consumer are unaffected.Design
Pure, synchronous, side-effect-free functions (matches existing
convert.rsstyle); only the existing fetch does I/O.Tests
Integration coverage in
web/tests/content_pipeline.rsplus per-module unit tests (pruning scoring/threshold, BM25 relevance, selection scoping, boilerplate hints).Roadmap (later, separate PRs):
web::extract(structured/table),web::crawl(deep crawl),web::seed(URL discovery).