mathias/hyperguild
1
0
Fork 0
Code Issues 30 Pull Requests 3 Actions Packages Projects Releases Wiki Activity

feat(pipeline): page-aware extraction — feed the current page into the revision prompt #130

Closed
mathias wants to merge 3 commits from feat/106-page-aware-extraction into main
pull from: feat/106-page-aware-extraction
merge into: :main
Conversation 1 Commits 3 Files Changed 5
+247 -13
mathias commented 2026-09-23 05:28:48 +00:00
Owner
Copy Link
Copy Source

What

Closes #106, the second half of compile-on-ingest. #103 (merged) made the merge asymmetric — on-disk wins, extraction-only sections get added, re-ingest of unchanged content is byte-identical. What it didn't do: the extraction itself was page-blind, writing every page as if fresh. This PR feeds the current page into the prompt and asks for a revision instead.

Two commits:

  1. feat(pipeline) — BuildPrompt now takes the source page's current on-disk content (when one exists) and asks the model to revise it: add what's missing, strengthen what the source now supports, leave the rest alone and reuse existing wording verbatim so unchanged claims are recognized as unchanged. Scoped to the source page only — wiki/sources/<sourceSlug>.md is the one page whose path is deterministic before extraction runs (a concept/entity page's path only exists once the model has produced a title, so it can't be fed forward the same way within the current chunked-extraction architecture).
  2. fix(wiki) — normalises heading casing/whitespace before Merge decides "is this the same section," closing the drift the issue names explicitly ("Key Claims" vs "Key claims" forking into two sections).

Heading-normalization rule (exact)

normalizeHeading(h) = strings.ToLower(strings.TrimSpace(h)), used only as the comparison key in Merge's section-matching (idx map) and in the bullet/append strategy lookup (bulletSectionsNorm / appendSectionsNorm, derived from the existing maps via the same function). The on-disk section's original heading text (secsA[i].heading) is never touched — it's what gets written back out. So a human's heading casing is never silently rewritten by a merge; normalization only decides sameness, never display text.

I deliberately did not attempt pluralization stemming ("Claims" vs "Claim") — the issue's own motivating example ("Key Claims" vs "Key claims") is a pure casing difference, not a plural/singular one, and naive stemming (e.g. strip trailing "s") is fragile against real headings like "Related Entities" → "Related Entitie". Flagging as a judgment call: if plural drift turns out to be a real observed failure mode, it needs its own targeted rule, not a blanket stemmer.

wiki/merge.go — why I touched a file under today's review scrutiny

Per the task brief, wiki/merge.go had six real bugs found across five review rounds earlier today and was flagged as "don't touch unless you find an actual bug or the issue genuinely needs it, and explain why." Heading normalization is #106's own explicit checklist item ("normalise headings before matching") and there is nowhere else to hook it in — it has to live in Merge's section-matching logic. I kept the diff to exactly the two lookup sites (idx construction/lookup, and the bullet/append strategy switch) plus the new normalizeHeading/normalizeSet helpers. No other behavior in the file changed; all 17 pre-existing merge_test.go tests pass unmodified.

Deterministic merge stays the backstop

Unchanged from #103: wiki.Merge still keeps on-disk content for any section the model doesn't return, and non-bullet/non-append sections still keep the on-disk version even when the model does return a same-named one. So a model that ignores the "revise" instruction (or hallucinates a rewrite) still cannot destroy human content — page-aware extraction changes what the model is asked to do, not what the merge is permitted to do.

Byte-identical-on-unchanged-revision test

TestRun_RevisionEchoingExistingPageStaysByteIdentical (pipeline_test.go) — the issue's explicit ask: "a model given the current page will sometimes echo it back verbatim" must not duplicate or perturb the page. It seeds an on-disk source page, runs Run() with different (hash-changing) source content but a Complete stub that echoes the seeded page's sections back unchanged, and asserts the resulting file via exact assert.Equal against the expected byte content (including the generator-owned last_updated/source_hash frontmatter fields, which are expected to update). Passes.

Two companion tests, also exact/targeted rather than Contains-only where it matters:

  • TestRun_ExistingSourcePageIsIncludedInTheExtractionPrompt — asserts a phrase that exists only on the on-disk page (not in the new source content) reaches the prompt, proving the page is actually threaded through rather than the assertion being trivially satisfiable.
  • TestRun_NewSourceHasNoExistingPageInThePrompt — a source with no page on disk yet gets no revision section injected at all.
  • TestMerge_HeadingCaseDriftMatchesSameSection / TestMerge_HeadingWhitespaceDriftMatchesSameSection (merge_test.go) — exact section-count assertions (assert.Equal(t, 1, strings.Count(...))) plus explicit checks that the on-disk casing survives and the fresh extraction's casing does not leak into the output.

Judgment calls / flags for review

  1. Scope of page-awareness = source page only, not every concept/entity page the extraction touches. Concept/entity page paths aren't knowable before the LLM call in the current per-chunk architecture (title comes back in the response), so true page-awareness for those would need a bigger restructure (e.g. a second pass, or pre-resolving titles). Flagging as a possible follow-up issue rather than in scope here.
  2. The prompt explicitly tells the model "it's fine to omit a section you aren't revising" — since Merge already keeps on-disk sections the fresh extraction doesn't return. This should reduce prompt/response size on a re-ingest (partially answering the issue's cost concern) but I haven't measured actual token savings against a real model.
  3. wiki/merge.go was edited — see the section above for exactly why and how narrowly.

Verification

  • cd ingestion && go build ./... && go test ./... — all packages pass, including the two new/changed test files.
  • task check from repo root (lint + test -race + vet + govulncheck + build + gitleaks, both modules) — exit 0, zero FAIL, verified via explicit $? and grep -c FAIL, not colored output.

Not merging this myself — per the task brief, an independent adversarial review happens first.

🤖 Generated with Claude Code

Created via git-mcp on behalf of @mathias

## What Closes #106, the second half of compile-on-ingest. #103 (merged) made the *merge* asymmetric — on-disk wins, extraction-only sections get added, re-ingest of unchanged content is byte-identical. What it didn't do: the extraction itself was page-blind, writing every page as if fresh. This PR feeds the current page into the prompt and asks for a revision instead. Two commits: 1. `feat(pipeline)` — `BuildPrompt` now takes the source page's current on-disk content (when one exists) and asks the model to revise it: add what's missing, strengthen what the source now supports, leave the rest alone and reuse existing wording verbatim so unchanged claims are recognized as unchanged. Scoped to the **source page only** — `wiki/sources/<sourceSlug>.md` is the one page whose path is deterministic *before* extraction runs (a concept/entity page's path only exists once the model has produced a title, so it can't be fed forward the same way within the current chunked-extraction architecture). 2. `fix(wiki)` — normalises heading casing/whitespace before `Merge` decides "is this the same section," closing the drift the issue names explicitly ("Key Claims" vs "Key claims" forking into two sections). ## Heading-normalization rule (exact) `normalizeHeading(h) = strings.ToLower(strings.TrimSpace(h))`, used **only** as the comparison key in `Merge`'s section-matching (`idx` map) and in the bullet/append strategy lookup (`bulletSectionsNorm` / `appendSectionsNorm`, derived from the existing maps via the same function). The on-disk section's original heading text (`secsA[i].heading`) is never touched — it's what gets written back out. So a human's heading casing is never silently rewritten by a merge; normalization only decides *sameness*, never *display text*. I deliberately did **not** attempt pluralization stemming ("Claims" vs "Claim") — the issue's own motivating example ("Key Claims" vs "Key claims") is a pure casing difference, not a plural/singular one, and naive stemming (e.g. strip trailing "s") is fragile against real headings like "Related Entities" → "Related Entitie". Flagging as a judgment call: if plural drift turns out to be a real observed failure mode, it needs its own targeted rule, not a blanket stemmer. ## wiki/merge.go — why I touched a file under today's review scrutiny Per the task brief, `wiki/merge.go` had six real bugs found across five review rounds earlier today and was flagged as "don't touch unless you find an actual bug or the issue genuinely needs it, and explain why." Heading normalization is #106's own explicit checklist item ("normalise headings before matching") and there is nowhere else to hook it in — it has to live in `Merge`'s section-matching logic. I kept the diff to exactly the two lookup sites (`idx` construction/lookup, and the bullet/append strategy switch) plus the new `normalizeHeading`/`normalizeSet` helpers. No other behavior in the file changed; all 17 pre-existing `merge_test.go` tests pass unmodified. ## Deterministic merge stays the backstop Unchanged from #103: `wiki.Merge` still keeps on-disk content for any section the model doesn't return, and non-bullet/non-append sections still keep the on-disk version even when the model *does* return a same-named one. So a model that ignores the "revise" instruction (or hallucinates a rewrite) still cannot destroy human content — page-aware extraction changes what the model is *asked* to do, not what the merge is *permitted* to do. ## Byte-identical-on-unchanged-revision test `TestRun_RevisionEchoingExistingPageStaysByteIdentical` (pipeline_test.go) — the issue's explicit ask: "a model given the current page will sometimes echo it back verbatim" must not duplicate or perturb the page. It seeds an on-disk source page, runs `Run()` with different (hash-changing) source content but a `Complete` stub that echoes the seeded page's sections back unchanged, and asserts the resulting file via **exact `assert.Equal`** against the expected byte content (including the generator-owned `last_updated`/`source_hash` frontmatter fields, which *are* expected to update). Passes. Two companion tests, also exact/targeted rather than `Contains`-only where it matters: - `TestRun_ExistingSourcePageIsIncludedInTheExtractionPrompt` — asserts a phrase that exists *only* on the on-disk page (not in the new source content) reaches the prompt, proving the page is actually threaded through rather than the assertion being trivially satisfiable. - `TestRun_NewSourceHasNoExistingPageInThePrompt` — a source with no page on disk yet gets no revision section injected at all. - `TestMerge_HeadingCaseDriftMatchesSameSection` / `TestMerge_HeadingWhitespaceDriftMatchesSameSection` (merge_test.go) — exact section-count assertions (`assert.Equal(t, 1, strings.Count(...))`) plus explicit checks that the on-disk casing survives and the fresh extraction's casing does *not* leak into the output. ## Judgment calls / flags for review 1. **Scope of page-awareness = source page only**, not every concept/entity page the extraction touches. Concept/entity page paths aren't knowable before the LLM call in the current per-chunk architecture (title comes back *in* the response), so true page-awareness for those would need a bigger restructure (e.g. a second pass, or pre-resolving titles). Flagging as a possible follow-up issue rather than in scope here. 2. The prompt explicitly tells the model "it's fine to omit a section you aren't revising" — since `Merge` already keeps on-disk sections the fresh extraction doesn't return. This should reduce prompt/response size on a re-ingest (partially answering the issue's cost concern) but I haven't measured actual token savings against a real model. 3. `wiki/merge.go` was edited — see the section above for exactly why and how narrowly. ## Verification - `cd ingestion && go build ./... && go test ./...` — all packages pass, including the two new/changed test files. - `task check` from repo root (lint + test -race + vet + govulncheck + build + gitleaks, both modules) — exit 0, zero FAIL, verified via explicit `$?` and `grep -c FAIL`, not colored output. **Not merging this myself** — per the task brief, an independent adversarial review happens first. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Created via git-mcp on behalf of @mathias_
mathias added 2 commits 2026-09-23 05:28:48 +00:00
feat(pipeline): feed the current source page into the extraction prompt 9fd9a0b920 
#103 made ingest merge into the page already on disk instead of
clobbering it, but the extraction itself stayed page-blind: the model
wrote every page as if fresh, and Merge reconciled the two texts
afterwards by heading. It could restate a claim but never refine it,
and had no way to know what already existed.

BuildPrompt now accepts the source page's current on-disk content
(when one exists) and asks the model to revise it explicitly: add
what's missing, strengthen what the source now supports, leave the
rest alone and reuse its wording verbatim so unchanged claims are
recognized as unchanged. Scoped to the source page specifically —
its path (wiki/sources/<sourceSlug>.md) is deterministic before
extraction runs, unlike a concept/entity page's path, which only
exists once the model has produced a title.

The deterministic merge in wiki.Merge is unchanged and stays the
backstop: an on-disk section the model doesn't touch survives
untouched regardless of what the prompt asks for, so a model that
ignores the instruction still cannot destroy human content.

Closes hyperguild#106 (first half; heading normalization follows).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(wiki): normalise heading casing/whitespace before matching in Merge
CI / Lint / Test / Vet (pull_request) Successful in 36s
Details
CI / Mirror to GitHub (pull_request) Skipped
Details
3564498a1b 
Page-aware extraction (previous commit) asks the model to revise the
page it's given, but nothing guarantees it reproduces a heading's
exact on-disk text. Before this, Merge matched sections by exact
string equality, so "Key Claims" vs "Key claims" (or trailing
whitespace) silently created a parallel section instead of updating
the original — the drift #106 calls out by name.

Merge's idx map and the bulletSections/appendSections strategy lookup
now key on normalizeHeading (trim + lowercase), so casing/whitespace
drift matches the same section. The on-disk heading's original text
(secsA[i].heading) is untouched and is what gets written out — the
normalized form is used only to decide "is this the same section,"
never to rewrite a human's heading casing.

Touches wiki/merge.go, which is explicitly out of casual-edit scope
per today's review history on this file: justified because heading
normalization is #106's own checklist item and has to hook into
exactly this matching logic — there's nowhere else to put it. Scope
kept minimal: the two lookup sites only, no other behavior in this
file changed. bulletSectionsNorm/appendSectionsNorm exist because
matching a section by its normalized key but then classifying its
merge strategy by exact-string lookup would silently fall through to
"keep a's version" for a case-drifted bullet/append heading — dropping
b's content instead of unioning or appending it.

Closes hyperguild#106.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mathias added 1 commit 2026-09-23 05:34:53 +00:00
fix(wiki): first-occurrence wins when heading normalization collides pre-existing duplicates
CI / Lint / Test / Vet (pull_request) Successful in 37s
Details
CI / Mirror to GitHub (pull_request) Skipped
Details
28d9f40733 
Review of PR #130 found a real bug in the new normalizeHeading matching:
idx was built with idx[normalizeHeading(s.heading)] = i, plain map
assignment while iterating secsA — last write wins. A page that already
carries two on-disk sections differing only by case or whitespace (real,
pre-existing corpus drift, the exact pattern #106 itself cites) never
collided in idx before this normalization existed, since it keyed on the
exact string. After normalization they collide, and every future update
silently, deterministically routes to whichever duplicate happens to
sort last in the file — the first one is frozen forever, never
receiving an update again, with nothing to say so. Same bug class
parseSections's own doc comment already warns about (two keys sharing
one map slot means one loses lookup visibility), reintroduced here.

Fixed by keeping only the first occurrence's index when a collision
happens while building idx — deterministic, and matches "earliest is
canonical," the same reasoning behind every other on-disk-wins default
in this file.

New test reproduces the exact review finding with a positional
assertion (the fresh bullet must land adjacent to the first
occurrence's own content, not the second) — a Contains-only version of
this test would have passed even with the bug present, since nothing is
literally deleted, a bullet just silently routes to the wrong of two
still-present sections.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mathias commented 2026-09-23 05:38:59 +00:00
Author
Owner
Copy Link
Copy Source

Superseded by #132: same 3 commits, rebased onto current main so the collision fix lands in wiki.MergeReport (the function production actually calls after #104's split) instead of the now-vestigial Merge wrapper this branch predates.

Created via git-mcp on behalf of @mathias

Superseded by #132: same 3 commits, rebased onto current main so the collision fix lands in `wiki.MergeReport` (the function production actually calls after #104's split) instead of the now-vestigial `Merge` wrapper this branch predates. --- _Created via git-mcp on behalf of @mathias_
mathias closed this pull request 2026-09-23 05:39:01 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
agent/ready
Meets the agent-ready contract; authorizes unattended execution
 blocked/client
Waiting on the client
 blocked/supplier
Waiting on a supplier
 blocked/upstream
Waiting on an upstream project or vendor
 risk/high
Irreversible, security-relevant, or control plane
 risk/low
Reversible, local, test-covered
 risk/medium
Touches shared state, deploys or data
 size/l
Too big - split it
 size/m
Under two days
 size/s
Under half a day
 size/xs
Under an hour
 status:dispatch
status:dispatched
status:review
status:reviewing
type/chore
Maintenance, no behaviour change
 type/decision
Human decision required; never agent/ready
 type/docs
Documentation only
 type/feat
New capability
 type/fix
Corrects broken behaviour
 type/ops
Infrastructure, deploy, runtime
 type/research
Investigate; output is a finding, not code
 type/test
Test coverage only
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate-bot mathias
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mathias/hyperguild#130