fix: detect and drop error-boilerplate text in fit_markdown pruning#110
fix: detect and drop error-boilerplate text in fit_markdown pruning#110LuminDev wants to merge 4 commits into
Conversation
Error banners (e.g. GitHub's "You can't perform that action at this time", "Uh oh! There was an error while loading") could survive fit_markdown pruning because their moderate text density and low link density outscored real content despite having no negative class-name patterns. Three-layer defense: 1. Error boilerplate text detection — elements whose visible text matches known error strings are dropped outright during prune_node(). 2. Warning class patterns — "blankslate", "flash", "toast", "alert-banner" receive a -0.3 scoring penalty per match via class_id_score(). 3. Navigate fallback — resolve_content() now checks for error boilerplate in the pruned output and falls back to plain text content when detected. Includes unit tests for all three layers. Closes #108
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44682f0646
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if has_error_boilerplate_text(element) { | ||
| return None; |
There was a problem hiding this comment.
When a page wraps both a transient error banner and valid content in one short root element (common for SPA <div id="app">... pages, especially sparse issue/login/status pages), this check returns None for the whole container before recursion can prune only the banner. Because the 500-byte guard still allows these small containers, prune_html_with_profile can discard all real content; navigate(..., format="fit_markdown") then either falls back to raw unpruned text or returns empty instead of structured markdown.
Useful? React with 👍 / 👎.
| "an error occurred", | ||
| "this page is taking too long", | ||
| "there was an error loading", | ||
| "please try again", |
There was a problem hiding this comment.
The standalone "please try again" matcher is too generic for an unconditional drop: any short legitimate article, form, or help text such as “If checkout fails, please try again” matches has_error_boilerplate_text and is removed whenever its containing element is under 500 bytes. This causes valid page content to disappear simply because it includes a common retry instruction, not because it is an error banner.
Useful? React with 👍 / 👎.
| ERROR_BOILERPLATE_PATTERNS | ||
| .iter() | ||
| .any(|pattern| lower.contains(pattern)) |
There was a problem hiding this comment.
If the same boilerplate is split by normal template whitespace, such as You can't perform that action at this\n time, this exact contains check misses it because element.text().collect() is not whitespace-normalized before comparing with the single-space patterns. In that case the new pruning and navigate fallback both fail to recognize the short error banner, so the GitHub-style boilerplate this change is meant to remove can still be returned as fit_markdown.
Useful? React with 👍 / 👎.
| if (md.trim().is_empty() || looks_like_error_boilerplate(&md)) | ||
| && !text.trim().is_empty() | ||
| { | ||
| cap_content(text, max_chars) |
There was a problem hiding this comment.
When pruning successfully removes an error-only main section, md is empty, but this fallback returns text, which is derived from the same fetched HTML. For a page or extracted main area that only contains Something went wrong/GitHub error boilerplate, navigate(..., format="fit_markdown") still surfaces the boilerplate instead of the empty pruned result, so the safety net reintroduces the exact content the new pruning removed.
Useful? React with 👍 / 👎.
…ers, generic retry text, whitespace normalization - Use direct text nodes (not descendant text) in has_error_boilerplate_text to prevent dropping containers that wrap both an error banner and valid content. Only drop elements without element children. - Remove 'please try again' from ERROR_BOILERPLATE_PATTERNS (too generic, caused false positives on legitimate content containing retry text). - Normalize whitespace before boilerplate pattern matching so multi-line text like 'you can't perform that action at this time' matches single-space patterns.
…ning already removed it When fit_markdown pruning strips error banners from the main HTML section and md is empty (or looks_like_error_boilerplate), the old fallback returned the raw text — which is derived from the same HTML and still contains the same error boilerplate. The safety net reintroduced the exact content the pruning removed. Now the fallback also checks !looks_like_error_boilerplate(text), so error pages that produce nothing but boilerplate return the (empty) pruned md instead of surfacing the raw error text. Closes: review comment on PR #110 (comment 3530817196)
…hitespace normalization - Use direct text nodes (not descendant text) in has_error_boilerplate_text to prevent dropping containers that wrap both an error banner and valid content. Only drop elements without element children. - Remove 'please try again' from ERROR_BOILERPLATE_PATTERNS (too generic, caused false positives on legitimate content containing retry text). - Normalize whitespace before boilerplate pattern matching so multi-line text like 'you can't perform that action at this time' matches single-space patterns.
…ning already removed it When fit_markdown pruning strips error banners from the main HTML section and md is empty (or looks_like_error_boilerplate), the old fallback returned the raw text — which is derived from the same HTML and still contains the same error boilerplate. The safety net reintroduced the exact content the pruning removed. Now the fallback also checks !looks_like_error_boilerplate(text), so error pages that produce nothing but boilerplate return the (empty) pruned md instead of surfacing the raw error text. Closes: review comment on PR #110 (comment 3530817196)
e69f1df to
c4ad133
Compare
2 participants
Summary
Fixes #108:
navigate(..., format="fit_markdown")returned error boilerplate text (e.g. "You can't perform that action at this time") instead of actual page content on GitHub and some other SPAs.Root Cause
The
prune_html_with_profile()scoring algorithm uses text density, link density, tag weights, and class-name negative patterns. GitHub's error banners (with classes likeflash-error,js-flash-alert) have moderate text density and low link density — they outscore real content despite containing only boilerplate, and their class names didn't match the existingNEGATIVE_PATTERNSarray (nav,footer,sidebar, etc.).Fix (three-layer defense)
Error boilerplate text detection (
prune.rs): Newhas_error_boilerplate_text()function checks for known error strings ("You can't perform that action at this time", "Uh oh! There was an error while loading", etc.) and drops matching elements outright duringprune_node().Warning class patterns (
prune.rs): NewWARNING_CLASS_PATTERNS(blankslate,flash,toast,alert-banner) contribute a -0.3 scoring penalty per match viaclass_id_score(), deprioritizing error banners without full elimination.Navigate fallback (
navigate.rs):resolve_content()now checks pruned fit_markdown output for error boilerplate. If detected and output is short (<500 chars), falls back to plain text content as a safety net.Changes
crates/browser/src/prune.rs: +4 constants, +1 helper function, updated scoringcrates/agent/src/tools/navigate.rs: +1 constant, +1 helper, +2 modified branches, +3 testsTest Plan
error_boilerplate_text_pruned,error_class_patterns_penalized,real_content_survives_with_error_bannerfit_markdown_falls_back_on_error_text,looks_like_error_boilerplate_detects_known_strings,looks_like_error_boilerplate_ignores_long_contentCloses #108