From 43f43e462128159de344f2fcb7ff0b8a7a6932de Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 23 Sep 2026 07:07:21 +0200 Subject: [PATCH] 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 --- ingestion/internal/wiki/merge.go | 11 ++++++++++- ingestion/internal/wiki/merge_test.go | 28 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/ingestion/internal/wiki/merge.go b/ingestion/internal/wiki/merge.go index 64fd226..20ca53f 100644 --- a/ingestion/internal/wiki/merge.go +++ b/ingestion/internal/wiki/merge.go @@ -108,9 +108,18 @@ func parseFrontmatterBlock(block string) (lines []fmLine, ok bool) { 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 == "" { + if !found || key == "" || strings.HasPrefix(strings.TrimSpace(line), "#") { lines = append(lines, fmLine{raw: line}) continue } diff --git a/ingestion/internal/wiki/merge_test.go b/ingestion/internal/wiki/merge_test.go index 8f730cf..5bd962d 100644 --- a/ingestion/internal/wiki/merge_test.go +++ b/ingestion/internal/wiki/merge_test.go @@ -304,3 +304,31 @@ Rewritten by the model. 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) +}