diff --git a/ingestion/internal/frontmatter/frontmatter.go b/ingestion/internal/frontmatter/frontmatter.go index 0b6f959..e18079a 100644 --- a/ingestion/internal/frontmatter/frontmatter.go +++ b/ingestion/internal/frontmatter/frontmatter.go @@ -101,21 +101,23 @@ func quoteScalars(block []byte) []byte { return []byte(strings.Join(lines, "\n")) } -// needsQuoting reports whether a scalar value must be quoted to parse as the -// string it plainly is. Only the colon case is handled: it is the one this -// package exists for, and every other special case risks rewriting a value -// that was deliberately structured (a nested mapping, an anchor, a number). +// needsQuoting reports whether a scalar value has to be quoted to load as the +// string it plainly is. +// +// The test is YAML's own parser rather than a pattern: ask whether the value +// stands alone as the right-hand side of a mapping. Heuristics get this wrong +// in both directions — `title: "Foo" — bar` opens with a quote but is not +// quoted, and a value may contain a colon inside quotes and be perfectly fine. +// +// Anything that already parses is left alone, whatever it parses into — a flow +// collection, a number, a bool. It is not broken, and quoting would change its +// type. func needsQuoting(value string) bool { - if strings.HasPrefix(value, "'") || strings.HasPrefix(value, `"`) { - return false - } if strings.HasPrefix(value, "|") || strings.HasPrefix(value, ">") { return false // block scalar; its content is not parsed as YAML } - if strings.HasPrefix(value, "{") || strings.HasPrefix(value, "[") { - return false // flow collection; quoting would silently make it a string - } - return strings.Contains(value, ": ") || strings.HasSuffix(value, ":") + var probe map[string]any + return yaml.Unmarshal([]byte("v: "+value+"\n"), &probe) != nil } // block returns the raw YAML between the opening and closing fences. diff --git a/ingestion/internal/frontmatter/frontmatter_test.go b/ingestion/internal/frontmatter/frontmatter_test.go index 7edebbf..8f0c8a4 100644 --- a/ingestion/internal/frontmatter/frontmatter_test.go +++ b/ingestion/internal/frontmatter/frontmatter_test.go @@ -90,6 +90,17 @@ func TestFix(t *testing.T) { wantKey: "title", wantValue: "Don't do this: it breaks", }, + { + // The value opens with a double quote and then keeps going after + // the closing one, so YAML reads it as a quoted scalar followed by + // junk. Treating "starts with a quote" as "already quoted" leaves + // this broken forever. + name: "quotes a value that only looks quoted", + doc: "---\ntitle: \"Evaluation Driven Generation\" — a TDD-style scaffold\ntags: [a, b]\n---\n\nbody\n", + wantChanged: true, + wantKey: "title", + wantValue: "\"Evaluation Driven Generation\" — a TDD-style scaffold", + }, { name: "leaves an already-valid document untouched", doc: "---\ntitle: 'Already quoted: fine'\nwing: homelab\n---\n\nbody\n", @@ -129,6 +140,25 @@ func TestFix(t *testing.T) { } } +func TestFix_LeavesStructuredValuesStructured(t *testing.T) { + // Only the broken scalar may be touched. A flow sequence on a neighbouring + // line still has to load as a sequence afterwards, not as a string. + doc := "---\ntitle: Broken: here\ntags: [prompting, llm]\ncount: 3\n---\n\nbody\n" + + got, changed, err := Fix([]byte(doc)) + require.NoError(t, err) + require.True(t, changed) + + b, ok := block(got) + require.True(t, ok) + var m map[string]any + require.NoError(t, yaml.Unmarshal(b, &m)) + + assert.Equal(t, "Broken: here", m["title"]) + assert.Equal(t, []any{"prompting", "llm"}, m["tags"]) + assert.Equal(t, 3, m["count"]) +} + func TestFix_PreservesBodyExactly(t *testing.T) { body := "\nThe body has: colons, 'quotes', and\n---\na horizontal rule.\n" doc := "---\ntitle: Broken: title\n---" + body