Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9345a0a867 | ||
|
|
a5a328fea5 |
@@ -3,7 +3,6 @@ package wiki
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"sort"
|
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -84,7 +83,7 @@ func MergeReport(a, b Page) (Page, []string) {
|
|||||||
secsA[i].content = appendNew(sA.content, sB.content)
|
secsA[i].content = appendNew(sA.content, sB.content)
|
||||||
default:
|
default:
|
||||||
// a wins — but say that b disagreed, rather than dropping it mute.
|
// a wins — but say that b disagreed, rather than dropping it mute.
|
||||||
if canonicalBlock(sA.content) != canonicalBlock(sB.content) {
|
if strings.TrimSpace(sA.content) != strings.TrimSpace(sB.content) {
|
||||||
conflicts = append(conflicts, sB.heading)
|
conflicts = append(conflicts, sB.heading)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -110,131 +109,36 @@ var generatorOwned = map[string]bool{
|
|||||||
"source_hash": true,
|
"source_hash": true,
|
||||||
}
|
}
|
||||||
|
|
||||||
// fmLine is one line of a frontmatter block: a parsed key/value pair, or a
|
|
||||||
// raw line (blank, comment, or anything else that isn't a simple "key:
|
|
||||||
// value") kept verbatim. Same shape as api.frontmatter's fmLine — this
|
|
||||||
// package doesn't import that one (it's unexported there, and a's/b's
|
|
||||||
// frontmatter here always arrives as a string, not a parsed struct), so the
|
|
||||||
// small, already-proven set()-appends-if-absent behavior is reproduced
|
|
||||||
// locally rather than string-index surgery reinvented per call site.
|
|
||||||
type fmLine struct {
|
|
||||||
key, value, raw string
|
|
||||||
}
|
|
||||||
|
|
||||||
// parseFrontmatterBlock parses a "---\n...\n---\n" fenced block (the shape
|
|
||||||
// parseSections produces) into an ordered line list. A block with no
|
|
||||||
// opening/closing fence — including the empty string, a page with no
|
|
||||||
// frontmatter at all — returns nil, ok=false: there is nothing to update in
|
|
||||||
// place, only something to potentially create.
|
|
||||||
func parseFrontmatterBlock(block string) (lines []fmLine, ok bool) {
|
|
||||||
if !strings.HasPrefix(block, "---\n") {
|
|
||||||
return nil, false
|
|
||||||
}
|
|
||||||
rest := block[len("---\n"):]
|
|
||||||
end := strings.Index(rest, "\n---\n")
|
|
||||||
if end < 0 {
|
|
||||||
return nil, false
|
|
||||||
}
|
|
||||||
for _, line := range strings.Split(rest[:end], "\n") {
|
|
||||||
// An indented line is a continuation (a YAML block-list item, a
|
|
||||||
// wrapped value) never a top-level key, regardless of whether it
|
|
||||||
// happens to contain a colon — " - 'Chapter 3: The Reckoning'"
|
|
||||||
// is not a key named "- 'Chapter 3". Checked before Cut so a
|
|
||||||
// colon inside one never reaches the key/value path at all.
|
|
||||||
if line != "" && (line[0] == ' ' || line[0] == '\t') {
|
|
||||||
lines = append(lines, fmLine{raw: line})
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
key, val, found := strings.Cut(line, ":")
|
|
||||||
key = strings.TrimSpace(key)
|
|
||||||
if !found || key == "" || strings.HasPrefix(strings.TrimSpace(line), "#") {
|
|
||||||
lines = append(lines, fmLine{raw: line})
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
lines = append(lines, fmLine{key: key, value: strings.TrimSpace(val)})
|
|
||||||
}
|
|
||||||
return lines, true
|
|
||||||
}
|
|
||||||
|
|
||||||
// setLine updates key's value in place if lines already has it, or appends
|
|
||||||
// a new entry if it doesn't — the one behavior the original refreshOwned
|
|
||||||
// got wrong (PR #126 review, round 2: it only ever updated in place).
|
|
||||||
func setLine(lines []fmLine, key, value string) []fmLine {
|
|
||||||
for i := range lines {
|
|
||||||
if lines[i].key == key {
|
|
||||||
lines[i].value = value
|
|
||||||
return lines
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return append(lines, fmLine{key: key, value: value})
|
|
||||||
}
|
|
||||||
|
|
||||||
func renderFrontmatterBlock(lines []fmLine) string {
|
|
||||||
if len(lines) == 0 {
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
var sb strings.Builder
|
|
||||||
sb.WriteString("---\n")
|
|
||||||
for _, l := range lines {
|
|
||||||
switch {
|
|
||||||
case l.key == "":
|
|
||||||
sb.WriteString(l.raw)
|
|
||||||
case l.value == "":
|
|
||||||
// A key with nothing after the colon (the YAML block-list shape:
|
|
||||||
// "aliases:" followed by indented "- item" lines on their own
|
|
||||||
// raw lines) must stay bare — "key: " with a trailing space
|
|
||||||
// is a different, if invisible, byte sequence, and this file's
|
|
||||||
// whole purpose is byte-stability across an unrelated merge.
|
|
||||||
sb.WriteString(l.key)
|
|
||||||
sb.WriteByte(':')
|
|
||||||
default:
|
|
||||||
sb.WriteString(l.key)
|
|
||||||
sb.WriteString(": ")
|
|
||||||
sb.WriteString(l.value)
|
|
||||||
}
|
|
||||||
sb.WriteByte('\n')
|
|
||||||
}
|
|
||||||
sb.WriteString("---\n")
|
|
||||||
return sb.String()
|
|
||||||
}
|
|
||||||
|
|
||||||
// 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 doesn't have yet — every page ingested before
|
|
||||||
// this mechanism existed has no source_hash line at all — is appended, not
|
|
||||||
// dropped (round 1 of review: dropping it silently meant source_hash never
|
|
||||||
// populated for the pre-existing corpus). If a has no frontmatter fence at
|
|
||||||
// all, one is created to hold the new fields rather than splicing raw
|
|
||||||
// "key: value" text into the body (round 2: that corrupted the page and the
|
|
||||||
// fields would duplicate on every subsequent merge, since unfenced text
|
|
||||||
// isn't recognized as frontmatter by parseSections on the next pass).
|
|
||||||
func refreshOwned(a, b string) string {
|
func refreshOwned(a, b string) string {
|
||||||
bLines, _ := parseFrontmatterBlock(b)
|
|
||||||
bVals := make(map[string]string)
|
bVals := make(map[string]string)
|
||||||
for _, l := range bLines {
|
for _, line := range strings.Split(b, "\n") {
|
||||||
if l.key != "" && generatorOwned[l.key] {
|
key, value, found := strings.Cut(line, ":")
|
||||||
bVals[l.key] = l.value
|
if !found || !generatorOwned[strings.TrimSpace(key)] {
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
bVals[strings.TrimSpace(key)] = strings.TrimSpace(value)
|
||||||
}
|
}
|
||||||
if len(bVals) == 0 {
|
if len(bVals) == 0 {
|
||||||
return a
|
return a
|
||||||
}
|
}
|
||||||
|
|
||||||
aLines, _ := parseFrontmatterBlock(a) // ok==false (no fence) -> aLines nil, same as "start fresh"
|
out := make([]string, 0, len(strings.Split(a, "\n")))
|
||||||
|
for _, line := range strings.Split(a, "\n") {
|
||||||
// Sorted for a deterministic, byte-stable result when more than one
|
key, _, found := strings.Cut(line, ":")
|
||||||
// field is appended in the same call — map iteration order isn't.
|
k := strings.TrimSpace(key)
|
||||||
keys := make([]string, 0, len(bVals))
|
if found && generatorOwned[k] {
|
||||||
for k := range bVals {
|
if v, ok := bVals[k]; ok {
|
||||||
keys = append(keys, k)
|
out = append(out, k+": "+v)
|
||||||
|
delete(bVals, k)
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
sort.Strings(keys)
|
|
||||||
for _, k := range keys {
|
|
||||||
aLines = setLine(aLines, k, bVals[k])
|
|
||||||
}
|
}
|
||||||
return renderFrontmatterBlock(aLines)
|
out = append(out, line)
|
||||||
|
}
|
||||||
|
return strings.Join(out, "\n")
|
||||||
}
|
}
|
||||||
|
|
||||||
// parseSections splits markdown into its frontmatter, its preamble (content
|
// parseSections splits markdown into its frontmatter, its preamble (content
|
||||||
@@ -329,21 +233,6 @@ func canonicalLine(line string) string {
|
|||||||
return strings.Join(strings.Fields(line), " ")
|
return strings.Join(strings.Fields(line), " ")
|
||||||
}
|
}
|
||||||
|
|
||||||
// canonicalBlock applies canonicalLine per line of a multi-line section body
|
|
||||||
// — the same whitespace-insensitivity appendNew/mergeBullets already give a
|
|
||||||
// single line, extended to a whole section for MergeReport's disagreement
|
|
||||||
// check (found in review, PR #129: comparing raw strings.TrimSpace only
|
|
||||||
// tolerated leading/trailing whitespace, not a re-extraction's mid-line
|
|
||||||
// spacing, which every other comparison in this file already tolerates).
|
|
||||||
func canonicalBlock(block string) string {
|
|
||||||
lines := strings.Split(block, "\n")
|
|
||||||
out := make([]string, len(lines))
|
|
||||||
for i, l := range lines {
|
|
||||||
out[i] = canonicalLine(l)
|
|
||||||
}
|
|
||||||
return strings.Join(out, "\n")
|
|
||||||
}
|
|
||||||
|
|
||||||
// appendNew concatenates the lines of b that a does not already carry.
|
// appendNew concatenates the lines of b that a does not already carry.
|
||||||
//
|
//
|
||||||
// Append sections used to concatenate wholesale, which is correct exactly once.
|
// Append sections used to concatenate wholesale, which is correct exactly once.
|
||||||
|
|||||||
@@ -215,124 +215,6 @@ Rewritten by the model.
|
|||||||
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
|
|
||||||
|
|
||||||
// Exact frontmatter block, not Contains: the second review round found
|
|
||||||
// that a substring check doesn't prove the fields landed inside a valid
|
|
||||||
// fence, in the right order, or that the fence itself wasn't corrupted —
|
|
||||||
// two independent mutations (closeIdx targeting the wrong fence, reversed
|
|
||||||
// sort) both survived a Contains-only version of this test.
|
|
||||||
want := "---\ntitle: 'X'\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
|
|
||||||
assert.Equal(t, want, got)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMerge_GeneratorOwnedFieldsCreateAFrontmatterBlockWhenThePageHadNoneAtAll(t *testing.T) {
|
|
||||||
// Second review round on PR #126: when a has no frontmatter fence at all,
|
|
||||||
// the append fallback spliced raw, unfenced "key: value" lines straight
|
|
||||||
// into the body -- worse than the original bug, because the next
|
|
||||||
// parseSections call wouldn't recognize them as frontmatter (they don't
|
|
||||||
// start with `---`), so they'd be absorbed as ordinary preamble text and
|
|
||||||
// re-duplicate on every subsequent merge instead of updating in place.
|
|
||||||
existing := Page{Path: "wiki/sources/x.md", Content: "## Summary\n\nOriginal.\n"}
|
|
||||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
|
||||||
last_updated: 2026-08-22
|
|
||||||
source_hash: bbbb
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
Rewritten by the model.
|
|
||||||
`}
|
|
||||||
|
|
||||||
got := Merge(existing, fresh).Content
|
|
||||||
|
|
||||||
want := "---\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
|
|
||||||
assert.Equal(t, want, got)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMerge_GeneratorOwnedFieldsMixedReplaceAndAppend(t *testing.T) {
|
|
||||||
// a already has last_updated (replace in place) but not source_hash
|
|
||||||
// (append) -- the two code paths in the same call, together.
|
|
||||||
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
|
||||||
title: 'X'
|
|
||||||
last_updated: 2026-01-01
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
Original.
|
|
||||||
`}
|
|
||||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
|
||||||
last_updated: 2026-08-22
|
|
||||||
source_hash: bbbb
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
Rewritten by the model.
|
|
||||||
`}
|
|
||||||
|
|
||||||
got := Merge(existing, fresh).Content
|
|
||||||
|
|
||||||
want := "---\ntitle: 'X'\nlast_updated: 2026-08-22\nsource_hash: bbbb\n---\n\n## Summary\n\nOriginal.\n"
|
|
||||||
assert.Equal(t, want, got)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestRefreshOwned_CommentLinesSurviveUnmangled(t *testing.T) {
|
|
||||||
// Third review round on PR #126: parseFrontmatterBlock dropped the
|
|
||||||
// sibling api package's comment guard (strings.HasPrefix(trimmed, "#")).
|
|
||||||
// Without it, a comment line containing a colon gets cut at the colon
|
|
||||||
// and re-rendered with collapsed whitespace instead of surviving
|
|
||||||
// verbatim.
|
|
||||||
a := "---\n#see also: https://x/y\ntitle: 'X'\n---\n"
|
|
||||||
b := "---\nsource_hash: bbbb\n---\n"
|
|
||||||
|
|
||||||
got := refreshOwned(a, b)
|
|
||||||
assert.Equal(t, "---\n#see also: https://x/y\ntitle: 'X'\nsource_hash: bbbb\n---\n", got)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestRefreshOwned_IndentedListItemsWithColonsAreNotMisparsedAsKeys(t *testing.T) {
|
|
||||||
// Same round: an indented YAML block-list item that happens to contain a
|
|
||||||
// colon (e.g. a title with a subtitle) is a continuation line, never a
|
|
||||||
// top-level key, regardless of what strings.Cut finds in it. Misparsing
|
|
||||||
// it as a key/value pair and re-rendering strips its indentation,
|
|
||||||
// de-indenting it out of the list it belongs to -- and refreshOwned
|
|
||||||
// re-renders this block on essentially every non-skipped ingest, so this
|
|
||||||
// is the normal path, not an edge case.
|
|
||||||
a := "---\naliases:\n - 'Chapter 3: The Reckoning'\ntitle: 'X'\n---\n"
|
|
||||||
b := "---\nsource_hash: bbbb\n---\n"
|
|
||||||
|
|
||||||
got := refreshOwned(a, b)
|
|
||||||
assert.Equal(t, "---\naliases:\n - 'Chapter 3: The Reckoning'\ntitle: 'X'\nsource_hash: bbbb\n---\n", got)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestMerge_ReportsSectionsWhereTheFreshVersionWasDiscarded(t *testing.T) {
|
func TestMerge_ReportsSectionsWhereTheFreshVersionWasDiscarded(t *testing.T) {
|
||||||
// The asymmetry that makes ingest safe (on-disk wins) also throws away a
|
// The asymmetry that makes ingest safe (on-disk wins) also throws away a
|
||||||
// real signal: the source now says something different about a section we
|
// real signal: the source now says something different about a section we
|
||||||
@@ -385,32 +267,3 @@ Appetite is fixed, scope is variable.
|
|||||||
_, conflicts := MergeReport(page, page)
|
_, conflicts := MergeReport(page, page)
|
||||||
assert.Empty(t, conflicts, "identical content is not a disagreement")
|
assert.Empty(t, conflicts, "identical content is not a disagreement")
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestMerge_ReportsNothingForInternalWhitespaceVariantsOnly(t *testing.T) {
|
|
||||||
// Found in review of PR #129: the conflict comparison used raw
|
|
||||||
// strings.TrimSpace, which only tolerates LEADING/TRAILING whitespace —
|
|
||||||
// unlike appendNew/mergeBullets, which already canonicalize internal
|
|
||||||
// whitespace runs too (#103: a re-extraction's spacing isn't guaranteed
|
|
||||||
// byte-stable). A mid-line double-space from re-extraction would
|
|
||||||
// false-positive as a "disagreement" here even though every other
|
|
||||||
// whitespace-sensitive comparison in this file already tolerates it.
|
|
||||||
existing := Page{Path: "wiki/sources/x.md", Content: `---
|
|
||||||
title: 'X'
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
Appetite is fixed, scope is variable.
|
|
||||||
`}
|
|
||||||
fresh := Page{Path: "wiki/sources/x.md", Content: `---
|
|
||||||
title: 'X'
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
Appetite is fixed, scope is variable.
|
|
||||||
`}
|
|
||||||
|
|
||||||
_, conflicts := MergeReport(existing, fresh)
|
|
||||||
assert.Empty(t, conflicts, "internal whitespace differences alone are not a disagreement")
|
|
||||||
}
|
|
||||||
|
|||||||
Reference in New Issue
Block a user