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>
This commit is contained in:
@@ -3,6 +3,7 @@ package wiki
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -84,6 +85,14 @@ var generatorOwned = map[string]bool{
|
|||||||
// refreshOwned returns a's frontmatter with the generator-owned fields taken
|
// refreshOwned returns a's frontmatter with the generator-owned fields taken
|
||||||
// from b where b supplies them. Fields absent from b keep a's value, so a
|
// from b where b supplies them. Fields absent from b keep a's value, so a
|
||||||
// generator that stops emitting a field cannot blank it.
|
// generator that stops emitting a field cannot blank it.
|
||||||
|
//
|
||||||
|
// A field b supplies that a's frontmatter doesn't have yet — every page
|
||||||
|
// ingested before this mechanism existed has no source_hash line at all —
|
||||||
|
// is appended, not dropped. Dropping it silently defeats the whole reason a
|
||||||
|
// field is generator-owned: it would never reach the page it's meant to
|
||||||
|
// live on (found in review, PR #126: source_hash then never populates for
|
||||||
|
// the pre-existing corpus, so the skip-on-unchanged feature it exists to
|
||||||
|
// support never engages, permanently, with nothing to say so).
|
||||||
func refreshOwned(a, b string) string {
|
func refreshOwned(a, b string) string {
|
||||||
bVals := make(map[string]string)
|
bVals := make(map[string]string)
|
||||||
for _, line := range strings.Split(b, "\n") {
|
for _, line := range strings.Split(b, "\n") {
|
||||||
@@ -97,8 +106,10 @@ func refreshOwned(a, b string) string {
|
|||||||
return a
|
return a
|
||||||
}
|
}
|
||||||
|
|
||||||
out := make([]string, 0, len(strings.Split(a, "\n")))
|
aLines := strings.Split(a, "\n")
|
||||||
for _, line := range strings.Split(a, "\n") {
|
out := make([]string, 0, len(aLines)+len(bVals))
|
||||||
|
closeIdx := -1
|
||||||
|
for _, line := range aLines {
|
||||||
key, _, found := strings.Cut(line, ":")
|
key, _, found := strings.Cut(line, ":")
|
||||||
k := strings.TrimSpace(key)
|
k := strings.TrimSpace(key)
|
||||||
if found && generatorOwned[k] {
|
if found && generatorOwned[k] {
|
||||||
@@ -108,11 +119,40 @@ func refreshOwned(a, b string) string {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if strings.TrimSpace(line) == "---" {
|
||||||
|
closeIdx = len(out) // the fence's position in the OUTPUT slice
|
||||||
|
}
|
||||||
out = append(out, line)
|
out = append(out, line)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if len(bVals) == 0 {
|
||||||
return strings.Join(out, "\n")
|
return strings.Join(out, "\n")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Sorted for a deterministic, byte-stable result — map order is not.
|
||||||
|
remaining := make([]string, 0, len(bVals))
|
||||||
|
for k := range bVals {
|
||||||
|
remaining = append(remaining, k)
|
||||||
|
}
|
||||||
|
sort.Strings(remaining)
|
||||||
|
insert := make([]string, len(remaining))
|
||||||
|
for i, k := range remaining {
|
||||||
|
insert[i] = k + ": " + bVals[k]
|
||||||
|
}
|
||||||
|
|
||||||
|
if closeIdx < 0 {
|
||||||
|
// No closing fence found — malformed input for this well-formed-only
|
||||||
|
// helper; append at the end rather than lose the fields.
|
||||||
|
out = append(out, insert...)
|
||||||
|
return strings.Join(out, "\n")
|
||||||
|
}
|
||||||
|
final := make([]string, 0, len(out)+len(insert))
|
||||||
|
final = append(final, out[:closeIdx]...)
|
||||||
|
final = append(final, insert...)
|
||||||
|
final = append(final, out[closeIdx:]...)
|
||||||
|
return strings.Join(final, "\n")
|
||||||
|
}
|
||||||
|
|
||||||
// parseSections splits markdown into its frontmatter, its preamble (content
|
// parseSections splits markdown into its frontmatter, its preamble (content
|
||||||
// before the first `## ` heading), and its `## `-headed sections.
|
// before the first `## ` heading), and its `## `-headed sections.
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -214,3 +214,39 @@ Rewritten by the model.
|
|||||||
assert.Contains(t, got, "Original.", "an existing section still wins over the model's rewrite")
|
assert.Contains(t, got, "Original.", "an existing section still wins over the model's rewrite")
|
||||||
assert.NotContains(t, got, "Renamed by the model")
|
assert.NotContains(t, got, "Renamed by the model")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestMerge_GeneratorOwnedFieldsAreAppendedWhenAPageNeverHadThemBefore(t *testing.T) {
|
||||||
|
// Found in review of PR #126: 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 and the skip-on-unchanged feature this
|
||||||
|
// PR exists to build would never engage for the entire pre-existing
|
||||||
|
// corpus, permanently, with nothing telling anyone.
|
||||||
|
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
||||||
|
title: 'X'
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Original.
|
||||||
|
`}
|
||||||
|
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
||||||
|
title: 'Renamed by the model'
|
||||||
|
last_updated: 2026-08-22
|
||||||
|
source_hash: bbbb
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Rewritten by the model.
|
||||||
|
`}
|
||||||
|
|
||||||
|
got := Merge(existing, fresh).Content
|
||||||
|
|
||||||
|
assert.Contains(t, got, "source_hash: bbbb", "a generator-owned field a page never had before must be appended, not dropped")
|
||||||
|
assert.Contains(t, got, "last_updated: 2026-08-22")
|
||||||
|
assert.Contains(t, got, "title: 'X'", "a human-set title must still survive regeneration")
|
||||||
|
assert.Contains(t, got, "Original.")
|
||||||
|
assert.NotContains(t, got, "Renamed by the model")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user