feat(pipeline): skip extraction when the source has not changed (#105) #126
Merged
mathias
merged 4 commits from 2026-09-23 05:09:30 +00:00
fix/105-source-drift-on-main into main
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  mathiasandClaude Sonnet 5
|
43f43e4621 |
fix(wiki): parseFrontmatterBlock must not treat comments or indented lines as keys
Third review round on PR #126. Two real bugs in the rewrite from the previous commit: 1. The comment guard the sibling api package's frontmatter parser has (strings.HasPrefix(trimmed, "#")) was dropped in this reimplementation. A frontmatter comment containing a colon got cut at the colon and re-rendered with collapsed internal whitespace. 2. Worse: an indented YAML block-list item containing a colon (e.g. a title with a subtitle inside an aliases: list) was misparsed as a top-level key/value pair -- strings.Cut doesn't know a line is a continuation just because TrimSpace(key) happens to be non-empty. Re-rendering then stripped the line's indentation, de-indenting it out of the list it belonged to. refreshOwned re-renders this block on essentially every non-skipped ingest, so this was the normal path, not an edge case. Checked whether the sibling api/frontmatter.go has the same list-item gap: it does (same strings.Cut-then-TrimSpace shape, no indentation check) -- not a regression introduced here, a pre-existing latent defect shared with the package this was modeled on. Filed separately rather than fixed here (out of scope for this PR, needs its own review). Fixed by checking indentation BEFORE calling Cut at all -- a line starting with a space or tab is a continuation line, full stop, regardless of what it contains -- and restoring the comment guard verbatim from the sibling. Flip-tested both guards independently: removing either one turns its corresponding new test red in isolation; the other stays green, confirming they're independent fixes, not one masking the other. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
mathiasandClaude Sonnet 5
|
874206be9d |
fix(wiki): rewrite refreshOwned on a parsed line list, not fence-index tracking
Second review round on PR #126 found two more problems with the first append fix: 1. When a has no frontmatter fence at all, the fallback spliced raw, unfenced "key: value" text straight into the body -- worse than the original bug, since the next parseSections call wouldn't recognize it as frontmatter and would absorb it as preamble text, duplicating on every subsequent merge. 2. The regression test only asserted substrings (Contains), which doesn't prove structural position. Two independent mutations (closeIdx locking onto the wrong fence, a reversed sort) both survived the full suite undetected. Rewritten from string-index surgery (tracking a fence's position across two passes over split lines) to a small parsed representation: fmLine (key/value or raw), parseFrontmatterBlock, setLine (update in place or append -- the one behavior the original code got wrong), and renderFrontmatterBlock. Same shape as the sibling api package's own frontmatter type, reproduced locally rather than imported (it's unexported there, and this package's needs are smaller) because that set()-appends-if-absent behavior was already proven correct by that package's existing test suite -- this bug class shouldn't have existed here in the first place. A page with no fence gets one created to hold the new fields, instead of corrupting the body. Tests now assert exact frontmatter block output via assert.Equal, not Contains, and a new fixture (mixed replace+append in one call) covers both code paths together. Flip-tested: reversing the sort turns two of the new exact-match tests red (the Contains-based version of the same test missed this on the prior commit). The "wrong fence" bug class is now structurally impossible rather than merely untested -- there's no fence-position index left to get wrong. Also fixed a byte-stability regression the new render introduced and caught via the full pipeline suite: a key with no inline value (the YAML block-list shape, "aliases:" followed by indented "- item" lines) rendered as "aliases: " (trailing space) instead of "aliases:". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
mathiasandClaude Sonnet 5
|
41a9112860 |
fix(wiki): append generator-owned frontmatter fields a page never had, not just replace
Review of PR #126 found refreshOwned only replaced a generator-owned line that already existed in a's frontmatter -- it never appended one a didn't have. Every page ingested before this feature shipped has no source_hash line at all, so on its first re-ingest after deploy, the fresh hash was silently dropped: storedSourceHash reads "" forever for that page, the hash never matches, and the skip-on-unchanged feature this whole PR/issue exists to build never engages for the entire pre-existing corpus, permanently, with nothing telling anyone. Fixed by tracking the closing frontmatter fence's position while scanning a's lines, then inserting any generator-owned fields b supplied that a didn't have just before that fence -- sorted for a deterministic, byte-stable result, since map iteration order isn't. Flip-tested: forcing the old drop-silently return turns the new test red; restoring the fix turns it green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
mathias
|
a5a328fea5 |
feat(pipeline): skip extraction when the source has not changed (#105)
Ingest now stamps source_hash — sha256 of the ingested body, frontmatter excluded — onto the source page, and compares it before doing any work. An unchanged source returns Skipped with no LLM call at all. This is cost and signal, not correctness: #103 already made a re-ingest byte-identical, so a repeated run was harmless but not free — a full extraction over every chunk to produce a result that merges to nothing. Building this first would have put idempotency in the guard rather than in the write, which is the wrong dependency direction. Body-only hashing on purpose. A re-crawl that stamps a new fetch date has not changed the source, and treating that as drift would re-extract the whole corpus every time a crawler ran. A missing or unreadable hash always means "extract": a missed skip costs one extraction, a wrong skip loses an update silently, and silence is the failure mode this repo keeps paying for. The skip is reported (Result.Skipped plus a warning naming the hash), not silent — a run that does nothing must say so, or "the pipeline is fine" and "the pipeline stopped working" look identical (#107). Storing the hash required settling who owns which frontmatter field. #103 made the on-disk block win wholesale, which protects human-set metadata from a regenerating writer but froze fields the generator legitimately owns. wiki.Merge now refreshes an explicit generatorOwned allowlist — last_updated and source_hash — and keeps everything else on disk. An allowlist rather than a carve-out, so a field nobody has thought about stays human-owned by default. That also retires the last_updated limitation #103 shipped with. Refs #105, #103, #107, brain#21. |