fix(graph): clean migrate content extraction (description + boilerplate + asset filter) - #3
Merged
Merged
Conversation
New clean.rs exposes strip_boilerplate(md): deterministic backstop to Firecrawl's onlyMainContent/excludeTags ask. Three passes: 1. drop wp-content/litespeed/avatar author lines anywhere; 2. drop leading chrome (search widget, link-only nav runs, logo+brand mash, bare breadcrumb words) until the first heading/prose line; 3. truncate at the first trailing footer marker (_Next Post_, You May Also Like heading, [Close Menu], cookie banner/RejectAccept, or a mashed-title link list item). Conservatism is the hard rule: never eat real prose. 16 tests pin every rule, including a realistic polluted fixture (condensed from a real curriculo.me page) where chrome is removed and all article prose + featured image + disclosure survive.
…T1, T4) T1 — front-matter description from source metadata, not the body: - FetchedPage gains pub description, populated from the Firecrawl response (first non-empty of metadata.description, og:description, ogDescription), whitespace-collapsed to a single line so TOML stays valid. - migrate uses fetched.description for the front-matter description and TopicInput.description; falls back to summarize(cleaned body) only when the site exposes no meta description. - scrape_payload() extracted as a testable seam: alongside onlyMainContent it now sends excludeTags for structural chrome + common WordPress theme selectors (clean.rs is still the deterministic backstop). T4 — cleaned body used everywhere: - migrate cleans once (clean::strip_boilerplate) then uses that value for the written body, content_hash, Page.summary, and TopicInput.body. No call site sees raw fetched markdown. MockFetcher.with gains a description param; existing tests updated.
The live run hard-failed on https://ats.curriculo.me/favicon.ico (Firecrawl SCRAPE_UNSUPPORTED_FILE_ERROR → HTTP 500), which counted as a failure and tripped the run's fail-through for an otherwise fine crawl. sitemap::is_asset_url(url) flags static-asset extensions (.ico .png .jpg .jpeg .gif .webp .svg .avif .css .js .json .xml .pdf .zip .mp4 .webm .woff .woff2 .ttf), case-insensitive, ignoring query string + fragment. migrate applies it to the whole sitemap BEFORE the --max cap (so the cap yields N real pages) and logs the skipped count; skipped assets are not counted as failures.
Drives a faithful polluted curriculo.me body (search widget, nav/category link runs, wp-content/litespeed avatar line, _Next Post_ / You May Also Like / mashed-title toolkit / Close Menu / cookie banner / RejectAccept) through the migrate write path with MockFetcher + a clean Yoast metadata description, then asserts the written index.md: - front-matter description == the metadata description (not 'Hit enter to search…' from the polluted body); - none of the chrome markers survive (Hit enter to search, Close Search, Close Menu, RejectAccept, wp-content/litespeed, _Next Post_, You May Also Like, We use cookies); - real article prose + featured image + disclosure survive the clean. Offline (MockFetcher), no network.
Mechanical rustfmt pass. No logic changes. The graph/ translate modules landed unformatted (the release CI builds but does not gate on fmt); this brings the whole crate to cargo fmt --check clean under the runner's toolchain (nixpkgs cargo/rustfmt 1.95) so the migrate-extraction PR can meet its fmt-clean gate.
WalkthroughThe graph migration now excludes static sitemap assets, retrieves normalized Firecrawl descriptions, removes recognized WordPress theme boilerplate, and uses cleaned Markdown for summaries, hashes, topics, and written pages. Unit, integration, and regression tests cover these changes. ChangesMigration quality
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@GLM-BRIEF-MIGRATE-QUALITY.md`:
- Around line 19-22: Add the text language identifier to the fenced example
containing the description lines, changing its opening fence to use text while
preserving the example contents.
In `@src/cmd/graph/clean.rs`:
- Around line 117-127: Restrict the breadcrumb predicate used by
strip_boilerplate to a finite set of verified labels or require adjacent
navigation evidence, instead of accepting arbitrary one-token text such as
“Abstract” or “Introduction.” Preserve ordinary article headings, and add a
regression test confirming a one-token heading without “#” reaches the page,
hash, summary, or topic inputs unchanged.
- Around line 169-178: Update is_mashed_title_list_item and the truncation logic
around it so one matching list item cannot trigger footer removal; require
multiple adjacent mashed-title links or an explicit footer marker before
truncating content. Add a regression test covering a legitimate camel-case
reference link such as “Configure iPhoneApp integration” and verify the article
remains intact.
🪄 Autofix
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 Plus
Run ID: 5b1d15e9-1e46-4e1e-b384-a42fdb64d3ed
📒 Files selected for processing (12)
GLM-BRIEF-MIGRATE-QUALITY.mdsrc/cmd/graph/clean.rssrc/cmd/graph/firecrawl.rssrc/cmd/graph/html_to_md.rssrc/cmd/graph/migrate.rssrc/cmd/graph/mod.rssrc/cmd/graph/openrouter.rssrc/cmd/graph/refresh.rssrc/cmd/graph/schema.rssrc/cmd/graph/sitemap.rssrc/cmd/graph/topics.rssrc/cmd/translate.rs
Comment on lines
+19
to
+22
| ``` | ||
| description = "Hit enter to search or ESC to closeSearch [Close Search](https://…/#) [Resume Tips](https://…)" | ||
| description = "CurriculoATS [Features](https://curriculo.me/features/) [AI Screening](h" | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced example.
markdownlint-cli2 reports MD040 because Line 19 opens a fence without an info string. Use text to keep the documented lint gate clean.
Proposed fix
-```
+```text
description = "Hit enter to search or ESC to closeSearch [Close Search](https://…/#) [Resume Tips](https://…)"
description = "CurriculoATS [Features](https://…)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| description = "Hit enter to search or ESC to closeSearch [Close Search](https://…/#) [Resume Tips](https://…)" | |
| description = "CurriculoATS [Features](https://curriculo.me/features/) [AI Screening](h" | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@GLM-BRIEF-MIGRATE-QUALITY.md` around lines 19 - 22, Add the text language
identifier to the fenced example containing the description lines, changing its
opening fence to use text while preserving the example contents.
Source: Linters/SAST tools
Comment on lines
+117
to
+127
| t.len() <= 12 | ||
| && !t.contains(' ') | ||
| && !t.contains('\t') | ||
| && !t.contains('[') | ||
| && !t.contains('!') | ||
| && !t.contains('#') | ||
| && !t.contains('.') | ||
| && !t.contains(',') | ||
| && !t.contains(':') | ||
| && !t.chars().next().map(|c| c.is_ascii_digit()).unwrap_or(false) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not remove arbitrary one-token leading content.
Line 117 classifies valid leading content such as Abstract or Introduction as a breadcrumb. strip_boilerplate then deletes that content before it reaches the written page, hash, summary, or topic input.
Match a finite set of verified breadcrumb labels, or require adjacent navigation evidence. Add a regression test where a one-token article heading without # survives.
🤖 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 `@src/cmd/graph/clean.rs` around lines 117 - 127, Restrict the breadcrumb
predicate used by strip_boilerplate to a finite set of verified labels or
require adjacent navigation evidence, instead of accepting arbitrary one-token
text such as “Abstract” or “Introduction.” Preserve ordinary article headings,
and add a regression test confirming a one-token heading without “#” reaches the
page, hash, summary, or topic inputs unchanged.
Comment on lines
+169
to
+178
| fn is_mashed_title_list_item(t: &str) -> bool { | ||
| if !t.starts_with("- [") { | ||
| return false; | ||
| } | ||
| let Some(caps) = single_link_re().captures(t) else { | ||
| return false; | ||
| }; | ||
| let text = caps.get(1).map(|m| m.as_str()).unwrap_or(""); | ||
| mash_re().is_match(text) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require a footer list run before truncating content.
A single normal reference item such as - [Configure iPhoneApp integration](...) matches the lowercase-to-uppercase rule. Line 153 then treats it as a footer marker and removes the rest of the article.
The requirement specifies a run of mashed-title links. Detect multiple adjacent matching items, or require an explicit footer marker before truncation. Add a preservation test for a legitimate camel-case reference link.
🤖 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 `@src/cmd/graph/clean.rs` around lines 169 - 178, Update
is_mashed_title_list_item and the truncation logic around it so one matching
list item cannot trigger footer removal; require multiple adjacent mashed-title
links or an explicit footer marker before truncating content. Add a regression
test covering a legitimate camel-case reference link such as “Configure
iPhoneApp integration” and verify the article remains intact.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
zola graph migrateran for real againsthttps://curriculo.meand committed 29 pages whose markdown (and 19/29 front-matterdescriptions) were polluted with WordPress theme chrome — nav runs, the search widget, avatar/author lines, the cookie banner, and a mashed-title "toolkit" link list. The same dirty text also flowed into the knowledge graph (content_hash,Page.summary, andtopics::enrich_oneinput), so topics were extracted from nav/footer text. The run also hard-failed onhttps://ats.curriculo.me/favicon.ico(FirecrawlSCRAPE_UNSUPPORTED_FILE_ERROR→ HTTP 500), which tripped the failure threshold for an otherwise fine crawl.Root cause:
onlyMainContent: truewas already set, but this theme lacks clean<main>/<article>semantics, so Firecrawl returns essentially the whole body. The migrate driver then took itsdescriptionfromsummarize(raw_body)— i.e. the body's first junk line.This PR fixes extraction in five TDD'd changes. Scope is this repo only — no landing-website content is touched.
Before / after
Front-matter
description(content/ai-resume-builder/blogs/how-ats-works-2026/index.md)Before (the bug — description came from the body's first line):
After (description comes from source metadata — Yoast
description, thenog:description, thenogDescription, whitespace-collapsed to one line):Body excerpt (same page)
Before — opened with chrome and closed with the footer:
After — opens at the heading, closes at the last real section, all chrome gone:
(The in-content "Ready to build your resume?" CTA is kept on purpose — conservatism is the hard requirement: never truncate real article prose. Everything from
_Next Post_onward is dropped.)Changes per task
src/cmd/graph/clean.rs(new):strip_boilerplate(md)— the deterministic backstop to Firecrawl'sexcludeTags. Three passes: (1) dropwp-content/litespeed/avatarauthor lines anywhere; (2) drop leading chrome (search-widget strings, link-only nav runs, logo+brand mash, bare breadcrumb words) until the first heading/prose line; (3) truncate at the first trailing footer marker (_Next Post_,### You May Also Like,[Close Menu], cookie banner /RejectAccept, or a mashed-title link list item). 16 unit tests pin every rule, including a realistic polluted fixture where all article prose + the featured image + the disclosure survive.FetchedPagegainspub description, populated from the Firecrawl response (first non-empty ofmetadata.description,og:description,ogDescription), whitespace-collapsed to one line.migrateuses it for the front-matterdescriptionandTopicInput.description, falling back tosummarize(cleaned body)only when the site exposes no meta description.scrape_payload(url)is extracted as a network-free, testable seam.migratecleans once then uses that value for the written body,content_hash,Page.summary, andTopicInput.body. No call site sees the raw fetched markdown.sitemap::is_asset_urlflags.ico .png .jpg .jpeg .gif .webp .svg .avif .css .js .json .xml .pdf .zip .mp4 .webm .woff .woff2 .ttf(case-insensitive, query/fragment ignored).migratefilters them out before the--maxcap (so the cap yields N real pages) and logs the skipped count; skipped assets are not failures.migratewrite path withMockFetcherasserts the writtenindex.mdcontains none ofHit enter to search,Close Search,Close Menu,RejectAccept,wp-content/litespeed, and thatdescriptionequals the metadata description.Defense in depth: Firecrawl is now also sent
excludeTagsfor structural chrome + common WordPress theme selectors, butclean.rsis the deterministic guarantee — neither layer is trusted alone (see the newscrape_payloadtest).Constraints honored
refresh.rshard rule untouched — Firecrawl is still migrate-only;refresh.rsdoes not import the firecrawl module.SCHEMA_VERSIONand schema are unchanged.MockFetcherextended with adescriptionparam; no network).Test gate
cargo test --workspace— all green (81 bin tests incl. 56graph::, plus all component crates).cargo fmt --check— clean (the priorfeat(graph)/feat(translate)commits landed unformatted and the release CI doesn't gate on fmt; commitstyle: cargo fmtis a purely mechanical rustfmt 1.95 pass, no logic change).cargo clippy— no new warnings (the 5 remaining are pre-existing intopics.rs/translate.rs/components/*).This PR only fixes extraction. Before the re-migrate of
curriculo.me, the orchestrator must:curriculo-tech/zolarelease tag off this branch.ZOLA_VERSIONandZOLA_BIN_URLinlanding-websiteto that tag.zola graph migrate --from https://curriculo.me --force(the--forcere-crawl discards the polluted graph) and retire the duplicate curated pages.Commits are per-task (T2, T1+T4, T3, T5, style) for easy review.
Summary by CodeRabbit
Bug Fixes
Tests