feat(graph): zola graph migrate (once) + refresh (local) CLI - #2
Conversation
Adds the `zola graph` subcommand (Tasks 1-5 of the graph migration plan):
bootstrap a topical knowledge graph from a live site once via Firecrawl,
then maintain it locally forever with `refresh` (no Firecrawl).
zola graph migrate --from <origin> [--max N] [--dry-run] [--force]
zola graph refresh [--max N] [--dry-run]
Modules (src/cmd/graph/):
- schema.rs GraphStore {pages,topics,relations,meta} load/save;
meta.schema_version=1; is_migrated_for() = once-guard.
- sitemap.rs parse_sitemap (pure, fixture-tested) + live collect_urls
that recurses sitemapindex; discover() tries index then plain.
- firecrawl.rs PageFetcher trait + FirecrawlFetcher (migrate-only).
MockFetcher gated under cfg(test).
- html_to_md.rs minimal HTML->markdown + extract_title (fallback only;
Firecrawl returns markdown natively).
- openrouter.rs TopicClient trait + OpenRouterTopicClient
(openai/gpt-4o-mini, ADR-003); parse_extract pure.
- topics.rs pure merge_page_topics (idempotent, case-insensitive label
dedup) + enrich_one wrapper shared by migrate & refresh.
- migrate.rs sitemap -> Firecrawl -> write content/<slug>/index.md
-> topics -> save. Bails on second crawl for same origin
unless --force (reads FIRECRAWL_API_KEY + OPENROUTER_API_KEY).
- refresh.rs local-only: walks default-lang markdown, re-topics stale
content_hash, stamps meta.last_refresh. Never imports
firecrawl. (reads OPENROUTER_API_KEY).
Wired in cli.rs (GraphCommand) / main.rs / cmd/mod.rs. Network lives only
in migrate/refresh; zola build stays offline. Artifacts committed under
data/graph/{pages,topics,relations,meta}.json.
All 57 bin tests pass (32 graph, incl. schema round-trip, sitemap fixtures,
mock fetcher, mock LLM merge, and the migrate-once -> refresh -> guard
integration test). Zero warnings.
Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe PR adds ChangesGraph command workflows
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant migrate
participant LiveSitemap
participant FirecrawlFetcher
participant OpenRouterTopicClient
participant GraphStore
CLI->>migrate: run migration options
migrate->>LiveSitemap: collect sitemap URLs
LiveSitemap-->>migrate: return page URLs
migrate->>FirecrawlFetcher: fetch each page
FirecrawlFetcher-->>migrate: return page content
migrate->>OpenRouterTopicClient: extract topics and relations
OpenRouterTopicClient-->>migrate: return topic extract
migrate->>GraphStore: save pages, topics, relations, and metadata
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: 14
🧹 Nitpick comments (11)
src/cmd/graph/mod.rs (2)
115-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
parse_pageaccepts a body-only+++sequence and silently splits on it.The loop latches
closedon the first+++line after line 1, which is correct. One gap remains: the function returns success when the frontmatter block is empty and the file has only the two delimiters. That case yields an emptyValuetable, sotitleanddescriptionresolve to empty strings downstream inrefresh.rs. The page is then enriched with no title context.Consider rejecting an empty frontmatter table, or logging a warning that identifies the file.
🤖 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/mod.rs` around lines 115 - 148, Update parse_page to reject pages whose frontmatter buffer is empty or parses to an empty table before returning success. Emit an error identifying path.display(), preserving the existing successful path for non-empty TOML frontmatter and preventing downstream refresh handling without title context.
85-100: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrevent unbounded recursion through directory symlinks
Path::is_dir()follows symlinks. A symlink insidecontent/that points to an ancestor directory can makewalk_mdrecurse until stack exhaustion. Usewalkdirwith symlink following disabled and add it to the root package dependencies inCargo.toml.🤖 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/mod.rs` around lines 85 - 100, Replace the recursive directory traversal in walk_md with walkdir configured not to follow symlinks, while preserving missing-directory handling, Markdown filtering, and error propagation. Add walkdir to the root package dependencies in Cargo.toml.src/cmd/graph/openrouter.rs (1)
79-86: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse one HTTP client across pages.
extractcreates a newreqwest::blocking::Clientfor every page. Store one client inOpenRouterTopicClientand construct it once in bothmigrate.rsandrefresh.rs. The workspace usesreqwest 0.13with theblockingfeature enabled.🤖 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/openrouter.rs` around lines 79 - 86, Update OpenRouterTopicClient to own a reusable reqwest::blocking::Client, construct it once when creating the client in both migrate.rs and refresh.rs, and use that stored client in extract instead of rebuilding it for each page. Preserve the existing timeout, authentication, headers, and request behavior.src/cmd/graph/schema.rs (2)
131-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd path context to save errors.
load_jsonreports the failing path.save_jsondoes not. A permission or disk-full failure produces a bare io error with no file name.♻️ Proposed context
fn save_json<T: Serialize>(path: &Path, value: &T) -> Result<()> { - let bytes = serde_json::to_vec_pretty(value)?; - fs::write(path, bytes)?; + let bytes = serde_json::to_vec_pretty(value) + .map_err(|e| anyhow!("{}: serialize JSON: {e}", path.display()))?; + fs::write(path, bytes).map_err(|e| anyhow!("{}: write: {e}", path.display()))?; Ok(()) }🤖 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/schema.rs` around lines 131 - 135, Update save_json to attach the target path to errors from fs::write, matching the path context provided by load_json while preserving serialization error propagation and the existing Result behavior.
105-112: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider atomic writes for the four-file save.
savewritespages.json,topics.json,relations.json, andmeta.jsonsequentially. If the process fails or is interrupted between writes, the four files describe different graph states.refreshthen loads a graph wherepagesandtopicsdisagree, andis_migrated_forcan report a stale origin.Write each file to a temporary path in the same directory, then rename it into place. That makes each file replacement atomic on the common platforms.
🤖 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/schema.rs` around lines 105 - 112, Update Graph::save to write pages.json, topics.json, relations.json, and meta.json through temporary files in the same directory, then atomically rename each temporary file into its final path only after serialization succeeds. Preserve the existing directory creation and error propagation, and ensure temporary paths are distinct and cleaned up or safely replaced on failure.src/cmd/graph/sitemap.rs (1)
89-100: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFilter collected URLs to the origin host.
discoverreturns every<loc>value the sitemap contains. A sitemap can list URLs on other hosts, for example a CDN, a partner domain, or a stale absolute URL from a previous domain.migrate_withfetches each returned URL through Firecrawl and writes it undercontent/, so off-origin pages enter the site content and the graph, and each one costs an API credit.Restrict the result to the requested origin's host.
🛡️ Proposed filter
pub fn discover(origin: &str, client: &Client) -> Result<Vec<String>> { let origin = origin.trim_end_matches('/'); + let host = reqwest::Url::parse(origin) + .map_err(|e| anyhow!("--from {origin}: not an absolute URL: {e}"))? + .host_str() + .map(str::to_string) + .ok_or_else(|| anyhow!("--from {origin}: no host"))?; for path in ["sitemap_index.xml", "sitemap.xml"] { let url = format!("{origin}/{path}"); match collect_urls(&url, client) { - Ok(urls) if !urls.is_empty() => return Ok(urls), + Ok(urls) => { + let kept: Vec<String> = urls + .into_iter() + .filter(|u| { + reqwest::Url::parse(u).ok().and_then(|p| p.host_str().map(str::to_string)) + == Some(host.clone()) + }) + .collect(); + if !kept.is_empty() { + return Ok(kept); + } + log::info!("sitemap: {url} returned no URLs on host {host}, trying next"); + } - Ok(_) => log::info!("sitemap: {url} returned no URLs, trying next"), Err(e) => log::info!("sitemap: {url} failed ({e}), trying next"), } }Adding an explicit URL parse also converts a non-absolute
--fromvalue into a clear error instead of a generic reqwest failure.🤖 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/sitemap.rs` around lines 89 - 100, Update discover to parse and validate the requested origin as an absolute URL, returning a clear error for invalid or non-absolute values, then filter collected sitemap URLs so only entries whose host matches the origin host are returned. Apply this filtering before deciding whether the collected result is non-empty, while preserving the existing sitemap fallback behavior.src/cmd/graph/html_to_md.rs (3)
46-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTrack the last emitted character, not the first.
block_separatortestslast == '\n'to decide between one and two newlines. Line 49 assigns the first character of the collapsed token. The intent is the last character.Today the observable output does not change, because
collapse_wsreplaces every whitespace run with a single space, so a collapsed token never contains'\n'. Both readings evaluate to false. The module doc names<pre>whitespace as a planned upgrade. Ifcollapse_wsever preserves newlines, this line breaks the separator logic silently.♻️ Proposed change
if !collapsed.is_empty() { out.push_str(&collapsed); - last = collapsed.chars().next().unwrap(); + last = collapsed.chars().next_back().unwrap(); }🤖 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/html_to_md.rs` around lines 46 - 50, Update the character tracking in the token-emission logic around collapse_ws and block_separator so last stores the final character of collapsed, not its first character. Preserve the existing empty-token guard and output behavior while ensuring newline detection reflects the actual last emitted character.
112-127: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the lowercase conversion out of the loop.
Line 117 allocates a lowercase copy of the entire remaining input on every iteration. Line 119 allocates a second copy. For a document with
kmatching blocks,strip_blocksperforms2kfull-length allocations and scans.html_to_markdowncallsstrip_blockstwice per page.ASCII lowercasing preserves byte length, so one lowercase copy can drive all the offset lookups.
⚡ Proposed change
fn strip_blocks(html: &str, name: &str) -> String { let open = format!("<{name}"); let close = format!("</{name}>"); let mut out = String::with_capacity(html.len()); - let mut rest = html; - while let Some(s) = rest.to_ascii_lowercase().find(&open) { - out.push_str(&rest[..s]); - let after = match rest[s..].to_ascii_lowercase().find(&close) { - Some(e) => s + e + close.len(), - None => rest.len(), // unterminated — drop to end - }; - rest = &rest[after..]; - } - out.push_str(rest); + let lower = html.to_ascii_lowercase(); + let mut pos = 0usize; + while let Some(s) = lower[pos..].find(&open) { + let s = pos + s; + out.push_str(&html[pos..s]); + pos = match lower[s..].find(&close) { + Some(e) => s + e + close.len(), + None => html.len(), // unterminated — drop to end + }; + } + out.push_str(&html[pos..]); out }🤖 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/html_to_md.rs` around lines 112 - 127, Update strip_blocks to create one lowercase copy of the full html input before the loop and use it for both opening- and closing-tag searches, while applying the resulting byte offsets to the original rest slice. Keep the existing block-removal behavior, including dropping the remainder for unterminated blocks, and avoid repeated to_ascii_lowercase allocations inside the loop.
129-132: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTwo regexes are compiled on every call instead of once. The file establishes the
OnceLockpattern at lines 14-17 forTAG_RE, but two later regexes are built inline. Regex compilation is far more expensive than the match it performs.
src/cmd/graph/html_to_md.rs#L129-L132: move the\s+regex into aOnceLockstatic.collapse_wsruns once per text token, so this is the hot site.src/cmd/graph/html_to_md.rs#L60-L61: move the\n{3,}regex into aOnceLockstatic so it is compiled once per process instead of once perhtml_to_markdowncall.🤖 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/html_to_md.rs` around lines 129 - 132, The inline regexes in collapse_ws and html_to_markdown are recompiled repeatedly; define OnceLock-backed static regexes for the \s+ and \n{3,} patterns, following the existing TAG_RE pattern, and update both functions to reuse them. Apply the changes at src/cmd/graph/html_to_md.rs lines 129-132 and 60-61.src/cmd/graph/firecrawl.rs (1)
55-66: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd retry and backoff for rate limits and transient failures.
fetchperforms a single request. Firecrawl enforces per-plan rate limits and returns HTTP 429.migrate_withcallsfetchin a tight loop over every sitemap URL, so a large crawl reliably hits the limit. Each 429 incrementsfailures, the page is skipped permanently for that run, andmigrate_withbails at the end.Retry on 429 and 5xx with exponential backoff, and honor the
Retry-Afterheader. Also consider a small delay between pages in the migrate loop.🤖 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/firecrawl.rs` around lines 55 - 66, Update the request flow in fetch to retry transient HTTP 429 and 5xx responses with bounded exponential backoff, using the Retry-After header when provided before falling back to the calculated delay; preserve immediate failure for other unsuccessful statuses and successful response handling. Add a small delay between page requests in migrate_with to avoid issuing sitemap fetches in a tight loop.src/cmd/graph/migrate.rs (1)
273-338: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the failure path and the front matter round trip.
The three tests cover the success path, dry-run, and the once-per-origin guard. Three gaps map directly to the issues raised above:
- No test exercises a failed fetch. Pass a
FixedSitemapcontaining a URL thatMockFetcherdoes not hold. Assert thatmigrate_withreturns an error containing "failure(s)", and assert whatdata/graph/pages.jsonholds afterwards. That pins the behavior discussed at lines 128-135.- No test parses the generated front matter.
integration_migrate_then_refresh_then_guardasserts substrings only. Deserializecontent/a/index.mdfront matter with thetomlcrate and assert thetitle,description, andextra.source_urlvalues. Use a title with a quote and a non-ASCII character.integration_migrate_then_refresh_then_guarddoes not assertafter.pages.len() == 1. A regression that appends a duplicate page instead of updating the existing one would still pass, because the assertions only readpages[0].Also applies to: 362-418
🤖 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/migrate.rs` around lines 273 - 338, Extend migrate tests with a failed-fetch case using a sitemap URL absent from MockFetcher, asserting the error contains “failure(s)” and verifying the resulting data/graph/pages.json contents. In integration_migrate_then_refresh_then_guard, deserialize content/a/index.md front matter with toml and assert quoted/non-ASCII title, description, and extra.source_url values. Also assert after.pages.len() == 1 to ensure refresh updates rather than duplicates the page.
🤖 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 `@src/cmd/graph/firecrawl.rs`:
- Around line 49-84: Update the payload in the Firecrawl request to ask for both
markdown and html formats. In the response handling around `md`, `title`, and
the `(markdown, html)` fallback, retain the returned HTML regardless of whether
markdown is present, use it for markdown conversion when needed, and run
`html_to_md::extract_title` whenever metadata.title is empty so title fallback
remains available.
In `@src/cmd/graph/migrate.rs`:
- Around line 128-135: Prevent a partially failed forced re-crawl from
unconditionally overwriting the persisted graph: in the migration flow that
initializes the fresh GraphStore and saves it after the crawl, skip saving when
--force is active and failures > 0, preserving the existing graph while still
reporting failures. Keep successful forced re-crawls’ clean replacement behavior
unchanged.
- Around line 118-124: Update the dry-run documentation near the migration
function and the log message in the dry_run branch to state that sitemap
discovery/network access occurs while no writes or enrichment are performed.
Keep the planned.len() reporting and early return unchanged.
- Around line 179-183: Update the TopicInput construction in the migrate flow to
populate description with the same summary value written to front matter,
reusing the existing summary binding (or page.summary if that binding is
unavailable), instead of String::new(). Keep title and body unchanged so migrate
and refresh_with provide identical input to TopicClient::extract.
- Around line 156-157: Normalize fetched markdown line endings and trailing
whitespace consistently before computing the hash and writing migrated pages in
the migration flow around content_hash and write_page. Ensure CRLF and
reconstructed LF content produce the same normalized body, then hash that exact
body so the first refresh does not trigger unnecessary re-enrichment.
- Around line 158-164: Update the migrated front-matter construction around `fm`
to serialize a TOML structure using `toml::Value::String` for the title,
summarized description, source URL, and content hash, then call
`toml::to_string`. Compute `summarize(&fetched.markdown)` once and preserve the
existing `[extra]` and field names without adding apostrophe-specific handling.
In `@src/cmd/graph/mod.rs`:
- Around line 177-185: Update the frontmatter writer in migrate.rs to serialize
its data structure with the toml serializer instead of embedding title and
description via Rust Debug formatting. Ensure control characters in summarize’s
output are encoded as valid TOML strings, while preserving the existing
frontmatter fields and values.
- Around line 55-59: Update content_hash to normalize CRLF line endings to LF
before hashing, ensuring hashes match regardless of whether callers provide
fetched or parsed text. Keep the existing SHA-256 and hexadecimal output
behavior unchanged.
In `@src/cmd/graph/refresh.rs`:
- Around line 116-141: Defer updating each page’s content_hash until its
corresponding enrich_one call succeeds. In the scan logic around the stale-page
branch, new-page branch, and todo queue, retain the pending hash without
committing it to store.pages; after successful enrichment in the enrichment
loop, apply that hash and preserve the existing topic updates. Ensure skipped
pages and pages whose enrichment fails remain eligible for enrichment on the
next run, while keeping successful pages’ hashes persisted.
In `@src/cmd/graph/schema.rs`:
- Around line 95-102: Update GraphStore::load to validate the loaded
meta.schema_version against the module’s pinned current schema version before
returning. Reject future or otherwise unsupported versions with an error, while
preserving successful loading for the supported version and preventing
GraphStore::save from rewriting incompatible data.
In `@src/cmd/graph/sitemap.rs`:
- Around line 45-51: Update the sitemap classification logic around the visible
`xml.contains("sitemapindex")` check to detect the opening `sitemapindex`
element rather than searching the entire XML document. Ensure URLs containing
“sitemapindex” remain classified as `Sitemap::UrlSet`, while actual sitemap
index documents still produce `Sitemap::Index`.
- Around line 74-84: Update the child traversal in recurse so failures from
fetch_text, parse_sitemap yielding Sitemap::Empty, or recursive child processing
are handled per child instead of propagated with ?. Continue processing all
siblings and retain successfully collected URLs, while recording or reporting
each failed child consistently with migrate_with’s per-page failure aggregation.
Preserve top-level sitemap errors and successful recursion behavior.
- Around line 19-25: Replace the regex-based loc_re parsing used by
parse_sitemap with namespace-aware XML parsing that matches only the sitemap
namespace’s loc elements, excluding image:loc and video:loc asset URLs. Preserve
extraction of valid page URLs and pass only those URLs to migrate.
In `@src/cmd/graph/topics.rs`:
- Around line 43-59: Update the topic-resolution flow around the loop over
extract.topics so spec.aliases are merged into the resolved Topic, including
topics found in store.topics and label_to_id, before or alongside page
attachment. Preserve existing aliases while adding only missing values, and add
a test that merges two extracts with different aliases for the same label.
---
Nitpick comments:
In `@src/cmd/graph/firecrawl.rs`:
- Around line 55-66: Update the request flow in fetch to retry transient HTTP
429 and 5xx responses with bounded exponential backoff, using the Retry-After
header when provided before falling back to the calculated delay; preserve
immediate failure for other unsuccessful statuses and successful response
handling. Add a small delay between page requests in migrate_with to avoid
issuing sitemap fetches in a tight loop.
In `@src/cmd/graph/html_to_md.rs`:
- Around line 46-50: Update the character tracking in the token-emission logic
around collapse_ws and block_separator so last stores the final character of
collapsed, not its first character. Preserve the existing empty-token guard and
output behavior while ensuring newline detection reflects the actual last
emitted character.
- Around line 112-127: Update strip_blocks to create one lowercase copy of the
full html input before the loop and use it for both opening- and closing-tag
searches, while applying the resulting byte offsets to the original rest slice.
Keep the existing block-removal behavior, including dropping the remainder for
unterminated blocks, and avoid repeated to_ascii_lowercase allocations inside
the loop.
- Around line 129-132: The inline regexes in collapse_ws and html_to_markdown
are recompiled repeatedly; define OnceLock-backed static regexes for the \s+ and
\n{3,} patterns, following the existing TAG_RE pattern, and update both
functions to reuse them. Apply the changes at src/cmd/graph/html_to_md.rs lines
129-132 and 60-61.
In `@src/cmd/graph/migrate.rs`:
- Around line 273-338: Extend migrate tests with a failed-fetch case using a
sitemap URL absent from MockFetcher, asserting the error contains “failure(s)”
and verifying the resulting data/graph/pages.json contents. In
integration_migrate_then_refresh_then_guard, deserialize content/a/index.md
front matter with toml and assert quoted/non-ASCII title, description, and
extra.source_url values. Also assert after.pages.len() == 1 to ensure refresh
updates rather than duplicates the page.
In `@src/cmd/graph/mod.rs`:
- Around line 115-148: Update parse_page to reject pages whose frontmatter
buffer is empty or parses to an empty table before returning success. Emit an
error identifying path.display(), preserving the existing successful path for
non-empty TOML frontmatter and preventing downstream refresh handling without
title context.
- Around line 85-100: Replace the recursive directory traversal in walk_md with
walkdir configured not to follow symlinks, while preserving missing-directory
handling, Markdown filtering, and error propagation. Add walkdir to the root
package dependencies in Cargo.toml.
In `@src/cmd/graph/openrouter.rs`:
- Around line 79-86: Update OpenRouterTopicClient to own a reusable
reqwest::blocking::Client, construct it once when creating the client in both
migrate.rs and refresh.rs, and use that stored client in extract instead of
rebuilding it for each page. Preserve the existing timeout, authentication,
headers, and request behavior.
In `@src/cmd/graph/schema.rs`:
- Around line 131-135: Update save_json to attach the target path to errors from
fs::write, matching the path context provided by load_json while preserving
serialization error propagation and the existing Result behavior.
- Around line 105-112: Update Graph::save to write pages.json, topics.json,
relations.json, and meta.json through temporary files in the same directory,
then atomically rename each temporary file into its final path only after
serialization succeeds. Preserve the existing directory creation and error
propagation, and ensure temporary paths are distinct and cleaned up or safely
replaced on failure.
In `@src/cmd/graph/sitemap.rs`:
- Around line 89-100: Update discover to parse and validate the requested origin
as an absolute URL, returning a clear error for invalid or non-absolute values,
then filter collected sitemap URLs so only entries whose host matches the origin
host are returned. Apply this filtering before deciding whether the collected
result is non-empty, while preserving the existing sitemap fallback behavior.
🪄 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: 25c0603f-c0d2-4844-9f70-0a9a3271922f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlsrc/cli.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/mod.rssrc/main.rs
| let payload = json!({ | ||
| "url": url, | ||
| "formats": ["markdown"], | ||
| "onlyMainContent": true, | ||
| }); | ||
| let body = serde_json::to_vec(&payload)?; | ||
| let resp = self | ||
| .client | ||
| .post(FIRECRAWL_URL) | ||
| .bearer_auth(&self.api_key) | ||
| .header(reqwest::header::CONTENT_TYPE, "application/json") | ||
| .body(body) | ||
| .send()?; | ||
| let status = resp.status(); | ||
| let text = resp.text()?; | ||
| if !status.is_success() { | ||
| bail!("Firecrawl HTTP {status}: {}", take200(&text)); | ||
| } | ||
| let data: Value = serde_json::from_str(&text) | ||
| .map_err(|e| anyhow!("Firecrawl non-JSON response: {e}"))?; | ||
| let inner = &data["data"]; | ||
| let md = inner["markdown"].as_str().unwrap_or("").to_string(); | ||
| let mut title = inner["metadata"]["title"] | ||
| .as_str() | ||
| .unwrap_or("") | ||
| .to_string(); | ||
| // markdown fallback: some sites return html only — convert then. | ||
| let (markdown, html) = if md.trim().is_empty() { | ||
| let html = inner["html"].as_str().unwrap_or("").to_string(); | ||
| (html_to_md::html_to_markdown(&html), html) | ||
| } else { | ||
| (md, String::new()) | ||
| }; | ||
| if title.is_empty() && !html.is_empty() { | ||
| title = html_to_md::extract_title(&html); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Firecrawl v1 scrape endpoint formats parameter markdown html response data fields
💡 Result:
In the Firecrawl v1 /scrape endpoint, the formats parameter is an array used to specify the desired output content types [1][2]. If not provided, it defaults to ["markdown"] [3]. When you request specific formats, the response object returns data in fields corresponding to those formats [2][4]. Below is a summary of the common format parameters and their associated response data fields: Supported Formats and Response Fields: - Markdown: Use markdown in the formats array; the result is returned in the markdown field [1][2][4]. - HTML: Use html in the formats array to receive cleaned HTML (scripts/styles removed) in the html field [2][4]. - Raw HTML: Use rawHtml in the formats array to receive the unmodified page source in the raw_html field [2][4]. - Links: Use links to receive a list of links found on the page in the links field [2][4]. - Screenshot: Use screenshot (or screenshot@fullPage) to receive a signed URL in the screenshot field [1][5][3]. - JSON: Use json (often with an object containing schema and prompt) to receive structured data in the json field [2][3][4]. - Summary: Use summary to receive an AI-generated summary in the summary field [2][3][4]. - Additional formats: Supported formats also include images, branding, product, audio, video, and changeTracking (which requires markdown to be enabled) [5][3][4]. Important Notes: - The response object also consistently includes metadata fields such as metadata, metadata_dict, and metadata_typed [4]. - Fields corresponding to formats not requested in the formats parameter will typically be returned as null [4]. - For object-based formats like json, screenshot, or changeTracking, you pass an object in the formats array containing the relevant configuration (e.g., schema, prompt, fullPage) instead of just a string [3].
Citations:
- 1: https://docs.firecrawl.dev/v1-welcome
- 2: https://docs.firecrawl.dev/features/scrape
- 3: https://docs.firecrawl.dev/advanced-scraping-guide
- 4: https://www.firecrawl.dev/blog/mastering-firecrawl-scrape-endpoint
- 5: https://github.com/firecrawl/firecrawl/blob/main/apps/api/src/controllers/v1/types.ts
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -t f 'firecrawl|graph|html_to_md' . | head -80
printf '%s\n' '--- firecrawl symbols and references ---'
rg -n -C 4 'FirecrawlFetcher|FIRECRAWL_URL|html_to_markdown|extract_title|empty body|migrate_with' . \
-g '*.rs' -g '*.toml' -g '*.lock' | head -240Repository: curriculo-tech/zola
Length of output: 13688
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- src/cmd/graph/firecrawl.rs ---'
cat -n src/cmd/graph/firecrawl.rs
printf '%s\n' '--- migration fetch/write path ---'
sed -n '80,230p' src/cmd/graph/migrate.rs
printf '%s\n' '--- html conversion behavior ---'
sed -n '1,70p' src/cmd/graph/html_to_md.rs
sed -n '145,185p' src/cmd/graph/html_to_md.rs
printf '%s\n' '--- format declarations and response-field assumptions ---'
rg -n -C 3 '"formats"|inner\["html"\]|inner\["markdown"\]|title' src/cmd/graph -g '*.rs'Repository: curriculo-tech/zola
Length of output: 30698
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
payload = {
"url": "https://example.test",
"formats": ["markdown"],
"onlyMainContent": True,
}
response = {"data": {"markdown": None, "metadata": {"title": None}}}
inner = response["data"]
md = inner.get("markdown") or ""
title = inner.get("metadata", {}).get("title") or ""
html = inner.get("html") or ""
markdown = html # html_to_markdown("") is tested to return ""
if not md.strip():
markdown = html
print("requested_formats:", payload["formats"])
print("html_field_present:", "html" in inner)
print("html_value:", repr(html))
print("markdown_after_fallback:", repr(markdown))
print("would_bail_empty_body:", not markdown.strip())
print("title_after_fallback:", repr(title if not title and not html else title))
PYRepository: curriculo-tech/zola
Length of output: 311
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
payload = {
"url": "https://example.test",
"formats": ["markdown"],
"onlyMainContent": True,
}
response = {"data": {"markdown": None, "metadata": {"title": None}}}
inner = response["data"]
md = inner.get("markdown") or ""
title = inner.get("metadata", {}).get("title") or ""
html = inner.get("html") or ""
markdown = html
if not md.strip():
markdown = html
print("requested_formats:", payload["formats"])
print("html_field_present:", "html" in inner)
print("html_value:", repr(html))
print("markdown_after_fallback:", repr(markdown))
print("would_bail_empty_body:", not markdown.strip())
print("title_after_fallback:", repr(title if not title and not html else title))
PYRepository: curriculo-tech/zola
Length of output: 311
Request both markdown and html formats, and use the HTML for title fallback.
With formats: ["markdown"], Firecrawl does not provide a usable data.html. When data.markdown is empty, the fallback produces an empty body and fetch returns Firecrawl: empty body. Also, extract_title is skipped whenever markdown is present because the HTML value is discarded. A missing metadata.title can therefore still write an empty title to front matter.
🤖 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/firecrawl.rs` around lines 49 - 84, Update the payload in the
Firecrawl request to ask for both markdown and html formats. In the response
handling around `md`, `title`, and the `(markdown, html)` fallback, retain the
returned HTML regardless of whether markdown is present, use it for markdown
conversion when needed, and run `html_to_md::extract_title` whenever
metadata.title is empty so title fallback remains available.
| if dry_run { | ||
| log::info!( | ||
| "migrate [dry-run]: would fetch + enrich {} pages; no network, no writes", | ||
| planned.len() | ||
| ); | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The dry-run log claims "no network" after network calls already ran.
Line 113 calls sitemap_src.urls(from) before the dry_run branch. With LiveSitemap that performs live HTTP sitemap discovery. The message at line 120 states "no network, no writes".
Fetching the sitemap is required to report the plan. Correct the message and the doc comment at line 82 so an operator knows a dry-run reaches the origin.
📝 Proposed wording
if dry_run {
log::info!(
- "migrate [dry-run]: would fetch + enrich {} pages; no network, no writes",
+ "migrate [dry-run]: would fetch + enrich {} pages; sitemap was read, no Firecrawl/OpenRouter calls, no writes",
planned.len()
);
return Ok(());
}🤖 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/migrate.rs` around lines 118 - 124, Update the dry-run
documentation near the migration function and the log message in the dry_run
branch to state that sitemap discovery/network access occurs while no writes or
enrichment are performed. Keep the planned.len() reporting and early return
unchanged.
| // Fresh bootstrap (force re-crawl discards the old graph). | ||
| let mut store = GraphStore::default(); | ||
| store.meta = Meta { | ||
| schema_version: super::schema::SCHEMA_VERSION, | ||
| source_origin: from.to_string(), | ||
| migrated_at: now_iso(), | ||
| last_refresh: String::new(), | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
A partially failed --force re-crawl persists a truncated graph.
Line 129 starts from GraphStore::default(), so the forced run rebuilds the graph from the pages it fetches. The loop skips any page whose fetch, path conversion, or write fails and only increments failures. Line 202 then saves the store unconditionally, before line 209 reports the failures.
The result: a Firecrawl outage or a shrunken sitemap during --force deletes the previously persisted pages, topics, and relations for every page that did not come back, while the markdown files stay on disk under content/. The graph and the content directory diverge, and the topic edges and summaries for those pages are lost.
The comment on line 128 shows the discard is intentional for a clean re-crawl. The gap is the interaction with partial failure. Choose one of these:
- Skip the save when
failures > 0and the run used--force, so the prior graph survives. - Seed the new store from
existingpages and replace entries per successful fetch, rather than starting empty. - Back up the previous
data/graph/contents before the forced overwrite.
🤖 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/migrate.rs` around lines 128 - 135, Prevent a partially failed
forced re-crawl from unconditionally overwriting the persisted graph: in the
migration flow that initializes the fresh GraphStore and saves it after the
crawl, skip saving when --force is active and failures > 0, preserving the
existing graph while still reporting failures. Keep successful forced re-crawls’
clean replacement behavior unchanged.
| let disk_path = root_dir.join(&rel); | ||
| let hash = content_hash(fetched.markdown.trim()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect write_page, content_hash, parse_page, and summarize for body transformations.
set -euo pipefail
fd -t f 'mod.rs' -p 'graph' -x ast-grep outline {} --items all \;
fd -t f 'mod.rs' -p 'graph' -x rg -n -C 12 'fn write_page|fn content_hash|fn summarize|fn url_to_content_path' {} \;
fd -t f 'refresh.rs' -p 'graph' -x rg -n -C 8 'fn parse_page' {} \;Repository: curriculo-tech/zola
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- graph files ---'
git ls-files | rg '(^|/)src/cmd/graph/|(^|/)graph/'
printf '%s\n' '--- candidate implementations ---'
rg -n -C 18 'fn (write_page|content_hash|summarize|url_to_content_path|parse_page)|write_page\(|content_hash\(' srcRepository: curriculo-tech/zola
Length of output: 27055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import hashlib
def rust_trim(s):
return s.strip()
def rust_trim_end(s):
return s.rstrip()
def write_page_body(body):
return rust_trim_end(body) + "\n"
def rust_lines(s):
# Model str::lines(): split at LF and strip the preceding CR for CRLF.
out = []
for part in s.splitlines(keepends=True):
if part.endswith("\r\n"):
out.append(part[:-2])
elif part.endswith("\n") or part.endswith("\r"):
out.append(part[:-1])
else:
out.append(part)
return out
def parse_body(written):
# The frontmatter is already known to be closed. Model the body loop.
body = written.split("+++\n", 2)[-1]
return "".join(line + "\n" for line in rust_lines(body))
def digest(s):
return hashlib.sha256(s.encode()).hexdigest()
cases = {
"plain LF": "alpha\nbeta\n",
"leading/trailing whitespace": " \n alpha\nbeta \n\n",
"internal CRLF": "alpha\r\nbeta\r\n",
"trailing CRLF": "alpha\r\nbeta\r\n\r\n",
"empty": "",
}
for name, source in cases.items():
written = "+++\ntitle = \"x\"\n+++\n\n" + write_page_body(source)
reparsed = parse_body(written)
migrate_hash = digest(rust_trim(source))
refresh_hash = digest(rust_trim(reparsed))
print(f"{name}: {'MATCH' if migrate_hash == refresh_hash else 'MISMATCH'}")
print(f" source={source.encode()!r}")
print(f" reparsed={reparsed.encode()!r}")
PY
printf '%s\n' '--- markdown producers ---'
rg -n -C 12 'markdown\s*:|markdown\s*=|struct Fetched|Fetched|fetch\(' src/cmd/graphRepository: curriculo-tech/zola
Length of output: 10029
Normalize line endings before hashing migrated pages. write_page stores fetched.markdown with trailing whitespace removed, while parse_page reconstructs lines with \n. A fetched body containing CRLF therefore gets a different hash on the first refresh, which triggers unnecessary re-enrichment. Hash and write the same normalized body.
🤖 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/migrate.rs` around lines 156 - 157, Normalize fetched markdown
line endings and trailing whitespace consistently before computing the hash and
writing migrated pages in the migration flow around content_hash and write_page.
Ensure CRLF and reconstructed LF content produce the same normalized body, then
hash that exact body so the first refresh does not trigger unnecessary
re-enrichment.
| let fm = format!( | ||
| "title = {t:?}\ndescription = {d:?}\n[extra]\nsource_url = {u:?}\ncontent_hash = {h:?}\n", | ||
| t = fetched.title, | ||
| d = summarize(&fetched.markdown), | ||
| u = fetched.url, | ||
| h = hash, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the toml crate is already available to the zola binary crate.
set -euo pipefail
fd -H -t f 'Cargo.toml' -x rg -n -C2 '^\s*toml\b|^\[dependencies\]' {} \;Repository: curriculo-tech/zola
Length of output: 2564
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- migrate.rs structure ---'
ast-grep outline src/cmd/graph/migrate.rs
printf '%s\n' '--- relevant migration code ---'
sed -n '120,190p' src/cmd/graph/migrate.rs
printf '%s\n' '--- migrate.rs imports and declarations ---'
sed -n '1,125p' src/cmd/graph/migrate.rs
printf '%s\n' '--- dependency declarations ---'
fd -H -t f 'Cargo.toml' -x sh -c 'printf "\n--- %s ---\n" "$1"; rg -n -C2 "^(toml|anyhow|serde)\s*=|\[dependencies\]" "$1"' sh {}
printf '%s\n' '--- relevant symbol usages ---'
rg -n 'migrate_with|content_hash|summarize\(' src/cmd/graph src | head -120Repository: curriculo-tech/zola
Length of output: 16412
🌐 Web query:
Rust str Debug formatting escape_debug apostrophe \u{ TOML basic string escape rules
💡 Result:
In Rust, the str::escape_debug method provides an iterator that returns the escaped version of a string, similar to how the Debug trait formats strings [1][2]. The internal implementation, char::escape_debug_ext, uses a specific logic to handle characters, including a toggle for apostrophes [3][2]. Regarding your specific query: * str::escape_debug formatting: It is designed to represent non-printable characters as Unicode escapes (e.g., \u{...}) while using backslash escapes for common control characters like \n, \t, \r, and \0 [3][2]. * Apostrophe handling: The standard escape_debug method typically does not escape apostrophes (') by default, as it defaults to EscapeDebugExtArgs::ESCAPE_ALL (which may vary by context) [3][2]. However, the underlying char::escape_debug_ext allows for conditional escaping of single quotes (') and double quotes (") via specific flags [3][2]. This was added to allow consistent formatting between string literals and character literals [4]. * TOML basic string escape rules: In TOML, basic strings are enclosed in double quotes (") [5][6]. Within these strings, you must escape the following characters using a backslash [5][6]: * Quotation mark (") as \" [6][7] * Backslash (\) as \\ [6][7] * Control characters (U+0000 to U+0008, U+000A to U+001F, U+007F), except for tab (U+0009) [5][6]. * Apostrophes (') do not require escaping in TOML basic strings [5][6]. Note that TOML also supports literal strings (enclosed in single quotes '), where no escaping is performed at all, meaning a single quote cannot be escaped inside a single-quoted literal string [8]. Multi-line literal strings (enclosed in ''') are typically used to include single quotes [8].
Citations:
- 1: https://doc.rust-lang.org/stable/std/str/struct.EscapeDebug.html
- 2: https://github.com/rust-lang/rust/blob/master/library/core/src/char/methods.rs
- 3: https://doc.rust-lang.org/src/core/char/methods.rs.html
- 4: Update char::escape_debug_ext to handle different escapes in strings and chars rust-lang/rust#83079
- 5: https://toml.io/en/v1.1.0
- 6: https://github.com/toml-lang/toml/blob/main/toml.md
- 7: https://github.com/toml-lang/toml/blob/main/toml.abnf
- 8: https://toml.io/en/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- tool availability ---'
command -v rustc || true
rustc --version 2>/dev/null || true
if command -v rustc >/dev/null 2>&1; then
cat >/tmp/check_debug_toml.rs <<'RS'
fn main() {
let samples = ["a\u{200d}b", "a\u{301}b", "a'b", "a\"b", "a\nb", "a\tb"];
for value in samples {
println!("{value:?}");
}
}
RS
rustc /tmp/check_debug_toml.rs -o /tmp/check_debug_toml
/tmp/check_debug_toml
fi
printf '%s\n' '--- front-matter writer and parser helpers ---'
rg -n -C5 'fn write_page|fn parse_page|from_str::<|toml::' src/cmd/graph src | head -200
printf '%s\n' '--- lockfile TOML version ---'
rg -n -A4 -B2 'name = "toml"$' Cargo.lock 2>/dev/null || trueRepository: curriculo-tech/zola
Length of output: 451
🌐 Web query:
Rust Debug implementation for str escape_debug escape_single_quote false Unicode format characters U+200D
💡 Result:
In Rust, the behavior of str::escape_debug (and by extension the Debug implementation for str) is determined by the standard library's internal logic for identifying characters that should be escaped for debug output [1][2][3]. There is no parameter for escape_debug to set escape_single_quote to false [1][4]. The method str::escape_debug returns an iterator (EscapeDebug) that yields escape sequences for non-printable characters [1][3]. It does not provide configuration options for how it handles specific characters like single quotes or Unicode characters [1][2]. Regarding the Unicode format character U+200D (Zero Width Joiner), Rust's escape_debug behavior depends on the context in which it appears: 1. Consistency: The Debug implementation for str aims to be human-readable while remaining unambiguous [5][6]. It typically does not escape all Unicode characters, but it does escape characters that are deemed non-printable, control characters, or potentially confusing (such as some non-spacing combining marks) [2][5][6]. 2. Unicode Handling: U+200D is a Unicode format character. When using str::escape_debug, the character is generally treated as printable in many contexts, but if it falls into a category that the implementation chooses to escape (often based on whether the character is considered a "non-spacing mark" or other similar Unicode properties), it will be represented as a Unicode escape sequence, such as \u{200D} [2][6]. 3. Inconsistencies: It has been noted in Rust's development discussions that the escaping behavior across char::escape_debug, str::escape_debug, and the standard Debug trait formatting can be inconsistent, particularly regarding how combining marks and specific Unicode categories are handled [2]. There is no unified "toggle" to control these behaviors [2]. If you need specific control over escaping (e.g., to keep single quotes unescaped or to selectively escape Unicode characters), you must implement custom logic to iterate over the characters of the string and manually construct the desired output, rather than relying on the standard library's escape_debug method [2].
Citations:
- 1: https://doc.rust-lang.org/stable/std/str/struct.EscapeDebug.html
- 2: Debug escaping is confusingly inconsistent rust-lang/rust#107035
- 3: https://doc.rust-lang.org/std/primitive.char.html
- 4: https://doc.rust-lang.org/std/char/struct.EscapeDebug.html
- 5: Escape fewer Unicode codepoints in
Debugimpl ofstrrust-lang/rust#34485 - 6: Escape combining characters in char::Debug rust-lang/rust#49283
🏁 Script executed:
#!/bin/bash
set +e
printf '%s\n' '--- configured Rust toolchains ---'
rustup toolchain list 2>/dev/null || true
find "${RUSTUP_HOME:-$HOME/.rustup}/toolchains" -maxdepth 2 -type f -name rustc 2>/dev/null | head
printf '%s\n' '--- workspace dependency declaration ---'
sed -n '90,125p' Cargo.toml
printf '%s\n' '--- migration helpers ---'
rg -n -C8 'fn write_page|fn parse_page|toml::from_str|toml::Table|FrontMatter' src/cmd/graph src | head -240
printf '%s\n' '--- lockfile TOML entries ---'
rg -n -A5 -B2 '^name = "toml"$' Cargo.lock 2>/dev/null || true
printf '%s\n' '--- Rust escape-debug implementation references ---'
curl -L --fail --silent \
https://raw.githubusercontent.com/rust-lang/rust/master/library/core/src/char/methods.rs |
rg -n -C12 'escape_debug_ext|is_printable|ESCAPE_SINGLE_QUOTE' | head -180Repository: curriculo-tech/zola
Length of output: 15312
🏁 Script executed:
#!/bin/bash
set +e
python3 - <<'PY'
try:
import tomllib
except Exception as exc:
print(f"tomllib unavailable: {exc}")
raise SystemExit(0)
samples = {
"zwj": 'title = "a\\u{200d}b"\n',
"apostrophe": 'title = "a\\\'b"\n',
"unicode": 'title = "a\\u200Db"\n',
"newline": 'title = "a\\nb"\n',
}
for name, text in samples.items():
try:
value = tomllib.loads(text)
print(f"{name}: accepted: {value!r}")
except Exception as exc:
print(f"{name}: rejected: {type(exc).__name__}: {exc}")
PYRepository: curriculo-tech/zola
Length of output: 204
🌐 Web query:
Rust Debug format string U+200D output escape_debug exact behavior
💡 Result:
The exact behavior of escape_debug in Rust regarding the Zero Width Joiner (U+200D) is subject to ongoing changes and historically inconsistent implementation details [1][2]. As of August 10, 2026, the behavior is primarily guided by the goal of aligning str::escape_debug with the Debug trait implementation for str [1][2]. 1. General Behavior: escape_debug is designed to provide a programmer-facing, debug-friendly representation of strings and characters [3][4]. It generally escapes characters that are considered non-printable or potentially confusing [5][6]. 2. U+200D (ZWJ): While escape_debug historically attempted to avoid over-escaping characters to keep output readable for international text [7], recent efforts (notably Pull Request #158303 in June 2026) have moved toward consistently escaping grapheme extenders, which includes the Zero Width Joiner (U+200D), in str::escape_debug to ensure it matches the output of Debug for str [2]. 3. Consistency Issues: Historically, str::escape_debug and char::escape_debug have exhibited inconsistencies regarding how they handle combining characters and grapheme extenders [1][8]. The Debug implementation for str and str::escape_debug have sometimes differed, leading to proposals to strictly align them by escaping grapheme extenders across the board [2]. Because this area of the standard library is under active refinement to improve internationalization support and output consistency, it is recommended to treat the specific set of escaped characters as potentially subject to change [7][5]. If you require stable, predictable escaping for non-printable characters or ZWJ, dedicated JSON or Unicode-aware serialization crates are often preferred over reliance on escape_debug [5].
Citations:
- 1: Debug escaping is confusingly inconsistent rust-lang/rust#107035
- 2: Always escape grapheme extenders in
str::escape_debugrust-lang/rust#158303 - 3: https://doc.rust-lang.org/stable/std/fmt/trait.Debug.html
- 4: https://doc.rust-lang.org/std/primitive.char.html
- 5: ACP: Document how escape_debug escapes characters. rust-lang/libs-team#735
- 6: Tracking issue for the functions for debug escaping
char_escape_debugrust-lang/rust#35068 - 7: RFI: Rebalancing unicode escaping in str::escape_debug for internationalization rust-lang/rust#159694
- 8: Escape combining characters in char::Debug rust-lang/rust#49283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Unicode escape branch ---'
curl -L --fail --silent \
https://raw.githubusercontent.com/rust-lang/rust/master/library/core/src/char/methods.rs |
sed -n '478,535p'
printf '%s\n' '--- string Debug implementation ---'
for url in \
https://raw.githubusercontent.com/rust-lang/rust/master/library/core/src/fmt/mod.rs \
https://raw.githubusercontent.com/rust-lang/rust/master/library/alloc/src/string.rs \
https://raw.githubusercontent.com/rust-lang/rust/master/library/core/src/str/mod.rs
do
echo "--- $url ---"
curl -L --fail --silent "$url" |
rg -n -C10 'EscapeDebugExtArgs|escape_debug_ext|impl.*Debug.*str|debug_string' |
head -160
done
printf '%s\n' '--- TOML crate source/API metadata in lockfile ---'
sed -n '5515,5548p' Cargo.lock
rg -n 'toml::to_string|toml::Value::Table|map::Map' src | head -100Repository: curriculo-tech/zola
Length of output: 5369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- available TOML parsers ---'
command -v ruby && ruby -e 'require "toml"; puts Toml' 2>/dev/null || true
command -v perl && perl -MTOML -e 1 2>/dev/null && echo perl-toml-available || true
command -v node && node -e 'try { console.log(require.resolve("`@iarna/toml`")) } catch (_) {}' 2>/dev/null || true
command -v jq && jq --version || true
printf '%s\n' '--- TOML specification escape rules ---'
curl -L --fail --silent https://raw.githubusercontent.com/toml-lang/toml/main/toml.abnf |
rg -n -C5 'basic-string|escape|unicode' | head -180Repository: curriculo-tech/zola
Length of output: 2337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'JS'
const toml = require('`@iarna/toml`');
const cases = {
rustUnicodeEscape: 'title = "a\\u{200d}b"\n',
tomlUnicodeEscape: 'title = "a\\u200Db"\n',
apostrophe: 'title = "a\'b"\n',
};
for (const [name, source] of Object.entries(cases)) {
try {
console.log(`${name}: accepted`, toml.parse(source));
} catch (error) {
console.log(`${name}: rejected`, error.message);
}
}
JSRepository: curriculo-tech/zola
Length of output: 394
Serialize migrated front matter as TOML. Rust Debug formatting emits \u{200d}, which TOML rejects. Fetched titles or summaries can therefore produce invalid front matter. Apostrophes are valid and do not require special handling. Use the existing toml dependency with toml::Value::String fields and toml::to_string. Compute summarize(&fetched.markdown) once.
🤖 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/migrate.rs` around lines 158 - 164, Update the migrated
front-matter construction around `fm` to serialize a TOML structure using
`toml::Value::String` for the title, summarized description, source URL, and
content hash, then call `toml::to_string`. Compute
`summarize(&fetched.markdown)` once and preserve the existing `[extra]` and
field names without adding apostrophe-specific handling.
| pub fn load(dir: &Path) -> Result<Self> { | ||
| Ok(GraphStore { | ||
| pages: load_json(&dir.join("pages.json"))?, | ||
| topics: load_json(&dir.join("topics.json"))?, | ||
| relations: load_json(&dir.join("relations.json"))?, | ||
| meta: load_json(&dir.join("meta.json"))?, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate meta.schema_version on load.
load accepts any schema_version. If a graph directory was written by a future schema version, the load succeeds, serde drops the unknown fields, and the next save rewrites all four files without them. That silently discards persisted data.
The module doc states the version is pinned and bumped when the on-disk shape changes. Enforce that contract at the load boundary.
🛡️ Proposed version gate
pub fn load(dir: &Path) -> Result<Self> {
- Ok(GraphStore {
- pages: load_json(&dir.join("pages.json"))?,
- topics: load_json(&dir.join("topics.json"))?,
- relations: load_json(&dir.join("relations.json"))?,
- meta: load_json(&dir.join("meta.json"))?,
- })
+ let meta: Meta = load_json(&dir.join("meta.json"))?;
+ if meta.schema_version > SCHEMA_VERSION {
+ return Err(anyhow!(
+ "{}: graph schema_version {} is newer than supported {SCHEMA_VERSION}; upgrade zola",
+ dir.display(),
+ meta.schema_version
+ ));
+ }
+ Ok(GraphStore {
+ pages: load_json(&dir.join("pages.json"))?,
+ topics: load_json(&dir.join("topics.json"))?,
+ relations: load_json(&dir.join("relations.json"))?,
+ meta,
+ })
}📝 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.
| pub fn load(dir: &Path) -> Result<Self> { | |
| Ok(GraphStore { | |
| pages: load_json(&dir.join("pages.json"))?, | |
| topics: load_json(&dir.join("topics.json"))?, | |
| relations: load_json(&dir.join("relations.json"))?, | |
| meta: load_json(&dir.join("meta.json"))?, | |
| }) | |
| } | |
| pub fn load(dir: &Path) -> Result<Self> { | |
| let meta: Meta = load_json(&dir.join("meta.json"))?; | |
| if meta.schema_version > SCHEMA_VERSION { | |
| return Err(anyhow!( | |
| "{}: graph schema_version {} is newer than supported {SCHEMA_VERSION}; upgrade zola", | |
| dir.display(), | |
| meta.schema_version | |
| )); | |
| } | |
| Ok(GraphStore { | |
| pages: load_json(&dir.join("pages.json"))?, | |
| topics: load_json(&dir.join("topics.json"))?, | |
| relations: load_json(&dir.join("relations.json"))?, | |
| meta, | |
| }) | |
| } |
🤖 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/schema.rs` around lines 95 - 102, Update GraphStore::load to
validate the loaded meta.schema_version against the module’s pinned current
schema version before returning. Reject future or otherwise unsupported versions
with an error, while preserving successful loading for the supported version and
preventing GraphStore::save from rewriting incompatible data.
| static LOC: OnceLock<Regex> = OnceLock::new(); | ||
| fn loc_re() -> &'static Regex { | ||
| LOC.get_or_init(|| { | ||
| // local-name agnostic: matches <...loc> (any namespace prefix) until </...loc>. | ||
| Regex::new(r"(?s)<\w*:?\w*?loc>(.*?)</\w*:?\w*?loc>").unwrap() | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the current pattern captures image/video extension loc elements.
set -euo pipefail
mkdir -p /tmp/locchk && cd /tmp/locchk
cat > Cargo.toml <<'EOF'
[package]
name = "locchk"
version = "0.0.0"
edition = "2021"
[dependencies]
regex = "1"
EOF
mkdir -p src
cat > src/main.rs <<'EOF'
fn main() {
let re = regex::Regex::new(r"(?s)<\w*:?\w*?loc>(.*?)</\w*:?\w*?loc>").unwrap();
let xml = r#"<urlset><url><loc>https://x/blog/a</loc>
<image:loc>https://x/img/hero.jpg</image:loc>
<video:loc>https://x/v/clip.mp4</video:loc></url></urlset>"#;
for c in re.captures_iter(xml) {
println!("captured: {}", c[1].trim());
}
}
EOF
cargo run --quiet 2>&1 | tail -20Repository: curriculo-tech/zola
Length of output: 404
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(sitemap\.rs|Cargo\.toml|README\.md)$' || true
printf '%s\n' '--- sitemap structure ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline src/cmd/graph/sitemap.rs
fi
printf '%s\n' '--- sitemap implementation ---'
cat -n src/cmd/graph/sitemap.rs | sed -n '1,180p'
printf '%s\n' '--- related symbols and calls ---'
rg -n -C 3 'parse_sitemap|loc_re|sitemapindex|migrate|Firecrawl|firecrawl' src Cargo.toml README.md 2>/dev/null || trueRepository: curriculo-tech/zola
Length of output: 41830
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
pattern = re.compile(r'(?s)<\w*:?\w*?loc>(.*?)</\w*:?\w*?loc>')
xml = """<urlset>
<url>
<loc>https://x/blog/a</loc>
<image:loc>https://x/img/hero.jpg</image:loc>
<video:loc>https://x/v/clip.mp4</video:loc>
<news:publication>Example</news:publication>
</url>
</urlset>"""
print("captures:")
for match in pattern.finditer(xml):
print(repr(match.group(1).strip()))
print("sitemapindex substring cases:")
for document in [
'<urlset><url><loc>https://x/blog/sitemapindex-explained</loc></url></urlset>',
'<urlset><url><loc>https://x/sitemapindex.xml</loc></url></urlset>',
'<sitemapindex><sitemap><loc>https://x/child.xml</loc></sitemap></sitemapindex>',
]:
print("contains=", "sitemapindex" in document, "document=", document)
PYRepository: curriculo-tech/zola
Length of output: 561
Use namespace-aware parsing for <loc> elements.
The pattern captures <image:loc> and <video:loc> as page URLs. parse_sitemap passes these asset URLs to migrate, which sends them to Firecrawl and can write incorrect pages. Use an XML parser that resolves namespaces instead of excluding prefixes with a regex.
🤖 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/sitemap.rs` around lines 19 - 25, Replace the regex-based
loc_re parsing used by parse_sitemap with namespace-aware XML parsing that
matches only the sitemap namespace’s loc elements, excluding image:loc and
video:loc asset URLs. Preserve extraction of valid page URLs and pass only those
URLs to migrate.
| if xml.contains("sitemapindex") { | ||
| Sitemap::Index(urls) | ||
| } else if urls.is_empty() { | ||
| Sitemap::Empty | ||
| } else { | ||
| Sitemap::UrlSet(urls) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Detect a sitemap index by its element, not by a substring of the document.
Line 45 tests xml.contains("sitemapindex") across the whole document, including the <loc> values. A urlset that lists a page whose URL contains that word, for example https://x/blog/sitemapindex-explained, is classified as Sitemap::Index. recurse then fetches every page URL as a sitemap, and each one either fails or bails with "sitemap parsed empty".
Match the opening element instead.
🐛 Proposed check
- if xml.contains("sitemapindex") {
+ let is_index = xml.contains("<sitemapindex") || xml.contains(":sitemapindex");
+ if is_index {
Sitemap::Index(urls)
} else if urls.is_empty() {📝 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.
| if xml.contains("sitemapindex") { | |
| Sitemap::Index(urls) | |
| } else if urls.is_empty() { | |
| Sitemap::Empty | |
| } else { | |
| Sitemap::UrlSet(urls) | |
| } | |
| let is_index = xml.contains("<sitemapindex") || xml.contains(":sitemapindex"); | |
| if is_index { | |
| Sitemap::Index(urls) | |
| } else if urls.is_empty() { | |
| Sitemap::Empty | |
| } else { | |
| Sitemap::UrlSet(urls) | |
| } |
🤖 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/sitemap.rs` around lines 45 - 51, Update the sitemap
classification logic around the visible `xml.contains("sitemapindex")` check to
detect the opening `sitemapindex` element rather than searching the entire XML
document. Ensure URLs containing “sitemapindex” remain classified as
`Sitemap::UrlSet`, while actual sitemap index documents still produce
`Sitemap::Index`.
| let body = fetch_text(url, client)?; | ||
| match parse_sitemap(&body) { | ||
| Sitemap::Index(children) => { | ||
| for child in children { | ||
| recurse(&child, client, out, seen)?; | ||
| } | ||
| } | ||
| Sitemap::UrlSet(urls) => out.extend(urls), | ||
| Sitemap::Empty => bail!("{url}: sitemap parsed empty (not a sitemapindex/urlset?)"), | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
One bad child sitemap aborts the whole collection.
recurse propagates both a fetch_text error and Sitemap::Empty with ? and bail!. A sitemap index that references a single stale child is common: the child returns an HTML error page with status 200, parse_sitemap yields Empty, and the entire URL collection fails. Every sibling sitemap that parsed correctly is discarded.
discover then treats the top-level error as "this candidate path failed", falls through to the next path, and finally reports "no usable sitemap". The operator sees no indication that only one child was bad.
migrate_with already aggregates per-page failures and continues. Apply the same policy to child sitemaps.
🛡️ Proposed tolerance
let body = fetch_text(url, client)?;
match parse_sitemap(&body) {
Sitemap::Index(children) => {
for child in children {
- recurse(&child, client, out, seen)?;
+ if let Err(e) = recurse(&child, client, out, seen) {
+ log::warn!("sitemap: child {child} skipped ({e})");
+ }
}
+ if out.is_empty() {
+ bail!("{url}: sitemapindex yielded no page URLs");
+ }
}
Sitemap::UrlSet(urls) => out.extend(urls),
Sitemap::Empty => bail!("{url}: sitemap parsed empty (not a sitemapindex/urlset?)"),
}📝 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.
| let body = fetch_text(url, client)?; | |
| match parse_sitemap(&body) { | |
| Sitemap::Index(children) => { | |
| for child in children { | |
| recurse(&child, client, out, seen)?; | |
| } | |
| } | |
| Sitemap::UrlSet(urls) => out.extend(urls), | |
| Sitemap::Empty => bail!("{url}: sitemap parsed empty (not a sitemapindex/urlset?)"), | |
| } | |
| Ok(()) | |
| let body = fetch_text(url, client)?; | |
| match parse_sitemap(&body) { | |
| Sitemap::Index(children) => { | |
| for child in children { | |
| if let Err(e) = recurse(&child, client, out, seen) { | |
| log::warn!("sitemap: child {child} skipped ({e})"); | |
| } | |
| } | |
| if out.is_empty() { | |
| bail!("{url}: sitemapindex yielded no page URLs"); | |
| } | |
| } | |
| Sitemap::UrlSet(urls) => out.extend(urls), | |
| Sitemap::Empty => bail!("{url}: sitemap parsed empty (not a sitemapindex/urlset?)"), | |
| } | |
| Ok(()) |
🤖 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/sitemap.rs` around lines 74 - 84, Update the child traversal in
recurse so failures from fetch_text, parse_sitemap yielding Sitemap::Empty, or
recursive child processing are handled per child instead of propagated with ?.
Continue processing all siblings and retain successfully collected URLs, while
recording or reporting each failed child consistently with migrate_with’s
per-page failure aggregation. Preserve top-level sitemap errors and successful
recursion behavior.
| for spec in &extract.topics { | ||
| let key = spec.label.to_ascii_lowercase(); | ||
| let id = if let Some(t) = store.topics.iter().find(|t| t.label.to_ascii_lowercase() == key) { | ||
| t.id.clone() | ||
| } else if let Some((_, id)) = label_to_id.iter().find(|(l, _)| *l == key) { | ||
| id.clone() | ||
| } else { | ||
| let id = unique_topic_id(&store.topics, &spec.label, &label_to_id); | ||
| label_to_id.push((key, id.clone())); | ||
| store.topics.push(Topic { | ||
| id: id.clone(), | ||
| label: spec.label.clone(), | ||
| aliases: spec.aliases.clone(), | ||
| page_ids: vec![page_url.to_string()], | ||
| }); | ||
| id | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Aliases are dropped when the topic already exists.
spec.aliases is only applied when the code creates a new Topic (line 55). If the topic already exists in store.topics, the branch at line 45-46 reuses the id and discards the new aliases. A later extract that returns additional synonyms for a known label never updates Topic.aliases. The doc comment at line 34-35 implies alias merging happens.
Merge aliases into the resolved topic where the page attachment already happens.
🐛 Proposed fix to union aliases on the resolved topic
// attach page to topic (dedup)
let topic = store.topics.iter_mut().find(|t| t.id == id).unwrap();
+ for alias in &spec.aliases {
+ if !topic
+ .aliases
+ .iter()
+ .any(|a| a.to_ascii_lowercase() == alias.to_ascii_lowercase())
+ {
+ topic.aliases.push(alias.clone());
+ }
+ }
if !topic.page_ids.iter().any(|u| u == page_url) {Add a test that merges two extracts with different aliases for the same label.
🤖 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/topics.rs` around lines 43 - 59, Update the topic-resolution
flow around the loop over extract.topics so spec.aliases are merged into the
resolved Topic, including topics found in store.topics and label_to_id, before
or alongside page attachment. Preserve existing aliases while adding only
missing values, and add a test that merges two extracts with different aliases
for the same label.
1 participant
Adds the
zola graphsubcommand (plan Tasks 1-5): bootstrap a topicalknowledge graph from a live site once via Firecrawl, then maintain it
locally forever with
refresh(no Firecrawl).Why
Per the graph migration design (
docs/superpowers/specs/2026-08-11-zola-graph-design.md),the landing site needs a committed topical KG (
pages ↔ topics ↔ relations)that bootstraps once from
curriculo.meand is then maintained from localmarkdown.
zola buildstays offline; network lives only in migrate/refresh.Modules (
src/cmd/graph/)schema.rsGraphStore {pages,topics,relations,meta}, load/save;meta.schema_version=1;is_migrated_for()= once-guardsitemap.rsparse_sitemap(fixture-tested) + livecollect_urlsrecursing sitemapindex;discover()tries index then plainfirecrawl.rsPageFetchertrait +FirecrawlFetcher(migrate-only);MockFetcherundercfg(test)html_to_md.rsextract_title(fallback; Firecrawl returns markdown natively)openrouter.rsTopicClienttrait +OpenRouterTopicClient(openai/gpt-4o-mini, ADR-003)topics.rsmerge_page_topics(idempotent, case-insensitive label dedup) +enrich_oneshared by migrate & refreshmigrate.rscontent/<slug>/index.md→ topics → save; bails on 2nd crawl unless--forcerefresh.rscontent_hash, stampsmeta.last_refresh; never imports firecrawlWired in
cli.rs(GraphCommand) /main.rs/cmd/mod.rs.Hard rules honored
migrate.rs+firecrawl.rs;refresh.rsdoes not import it.meta.source_originunless--force.zola buildstays offline (no network in build path).FIRECRAWL_API_KEY,OPENROUTER_API_KEY.translate.rstrait-injection pattern (*_withtestable cores); TDD.Artifacts (
data/graph/)Tests
All 57 bin tests pass (32 new graph tests), zero build warnings:
parse_extract+ topics merge (idempotent, case-insensitive, inter-topic)--forcefails →--forcere-migratesOut of scope (Task 6+, orchestrator)
Release tag, landing secret pin, GH Actions workflows (graph-migrate / master
refresh→build). This PR is the CLI only; no release tagged.
🤖 Generated with Claude Code
Summary by CodeRabbit
graph migrateto import site content from sitemaps, generate Markdown pages, and build topic relationships.graph refreshto detect changed pages and update their metadata and topics.