Unified scraper stack - #56
Conversation
New module ``king_context.scraper.url_utils`` exposes ``canonicalize_url``, lifted verbatim from ``audit._canonicalize``. ``audit.py`` re-exports the private name for compatibility with its own callsites. Pure refactor, no behaviour change. Sets up shared use from ``update.py`` in a follow-up commit where URL diff parity between corpus_urls and fresh_urls needs the same normalisation.
chunk_pages used md_file.stem as source_url, so every chunk produced by the scraper carried slug-form URLs like docs-example-com-quickstart instead of the real https://docs.example.com/quickstart. Pre-existing since the chunk module landed; the consequence surfaced in deandevz#51 as noisy added_urls / removed_urls diffs when update compared corpus_urls (slug-form, written by this path) against fresh_urls (real URLs, from the provider). Fix: read the real URL from the page sidecar fetch.py writes alongside each <slug>.md as <slug>.meta.json. When the sidecar is absent or unreadable (legacy _temp/ from pre-deandevz#46 scrapes), fall back to the slug so existing work directories continue to chunk. Backward compat at the corpus boundary: committed data/*.json files keep their slug-form URLs until the next refresh writes through the new path. The audit command's URL canonicalisation absorbs the transient mismatch in the meantime. Four new tests cover the sidecar-present, sidecar-missing, sidecar-malformed, and url-key-missing branches.
|
Hey Nelson, took the unified PR through the same e2e path What I verified
Code-level readThe commentary explaining the why across the diff is One small nit, not blocking
I will also file a small cosmetic cleanup issue from a few Approved. Ship it whenever CI is green on your side. |
|
Hey @deandevz folded the null |
|
Validated Comment in the code explains the why nicely (legacy guard Approved, ready to merge whenever you are. |
2 participants
Summary
Consolidates PRs #51, #52, #53, #54 into one stack per #55. Lands
king-scrape update <name>(ADR-0014) with the six safety fixes you flagged in the live e2e run, plus the upsert and enrich resilience pieces that the update flow depends on, plus the--no-fetch-cacheflag wired through every subcommand.Related issue
Closes #47
Closes #48
Closes #50
Refs #55
Supersedes #51, #52, #53, #54.
Type of change
How it was tested
pytest -q
816 passed, 1 skipped
The six fixes have direct regression tests in
tests/test_scraper/test_update.py: legacy_metarefuse, fetch failure threshold abort, discover-divergence guard, growth allowed counter case, cost prompt short-circuit,fetch_failedin the report.--no-fetch-cachecovered on all three subcommands (cli.main,audit_main,update_main) plus direct unit testsfor
_cache_modehelpers.Ran a senior review pass (two voltagent reviewers in parallel) on the staged diff before opening. They caught a regression of your earlier
4a50531audit signature fix that my patchreverted by accident — re-applied. Also tightened the env restore window in
cli.pyso aload_configfailure can't leakSCRAPE_CACHE_MODE=bypass.Checklist
pytestpasses locallydocs/CLI_GUIDE.mdupdated with the cache mode footgun note forupdateAdditional notes
Three design decisions you locked in are implemented as-is, with no opt-in flag for any of them:
_meta-absent corpora: hard refuse, no flag. Re-scrape from scratch.> 10%, fixed constant. 1-of-10 passes; 2-of-10 aborts.> 50%loss aborts; any amount of growth is fine.Trade offs I accepted rather than fixed:
updatedoes not auto-setSCRAPE_CACHE_MODE=bypass. A user runningking-scrape update foowithout remembering--no-fetch-cachemay still hit stale provider cache for unchanged URL but changed-content pages. Documented inCLI_GUIDE.mdnext to step 3. Auto bypass onupdateis one CLI guarantee I'd rather have you sign off on before changing._interleave_in_chunk_ordernow silently dedupes repeatedcontent_hashfromfresh_chunks(boilerplate sections that recur across pages). One section per unique content, fresh order preserved. Documented in the helper.Follow-up that I'd ship separately, not in this PR:
embeddings.npypruning on re-seed (you acknowledged this as pre existing). Happy to file the issue.Happy to split the stack if you'd rather review the pieces independently.