fix(compiler): YAML-safe brief in concept frontmatter (closes #67) (#76)

* fix(compiler): YAML-safe brief in concept frontmatter (closes #67)

LLM-authored brief values often contain colons, quotes, or hashes; the
existing f-string interpolation in _write_concept produced invalid
frontmatter (e.g. "brief: Why X: Y" → mapping value not allowed).
OpenKB's own reader uses string slicing so it never noticed, but
external YAML-aware tools (VS Code preview, Obsidian, doc generators)
reject the page.

- New _yaml_kv_line helper routes the value through yaml.safe_dump so
  PyYAML auto-quotes when needed.
- Both write sites in _write_concept (re.sub update path + create
  path) use the helper. re.sub now takes a lambda replacement to
  bypass backref interpretation when brief contains literal \1 / \g<…>.
- New find_invalid_frontmatter check in lint.py runs yaml.safe_load
  on every wiki page's frontmatter and is wired into the
  run_structural_lint report; future regressions surface immediately
  instead of silently writing bad YAML.

Repro and root-cause analysis by @invu557 (issue #67).

* fix(compiler): close 4 review findings on YAML-safe frontmatter

- _yaml_kv_line: switch from yaml.safe_dump+split to json.dumps. The
  old form silently truncated briefs with embedded newlines (PyYAML
  promoted them to multi-line single-quoted scalars; taking the first
  line dropped the rest AND left an unterminated quote). json.dumps
  always produces single-line, correctly-escaped output that round-
  trips through any YAML loader.
- _read_concept_briefs: parse frontmatter via yaml.safe_load instead
  of string-slicing the brief line. Without this, every brief that
  PyYAML auto-quoted (any colon-bearing value) leaked its surrounding
  quotes back into the LLM planning prompt and the index.md display.
- New _yaml_list_line / _parse_yaml_list_value helpers harden the
  sources field the same way; _write_concept, _prepend_source_to_
  frontmatter, and _remove_source_from_frontmatter all route through
  them. Without this, the PR's own new lint flagged compiler-emitted
  pages as invalid whenever a source filename contained a comma,
  bracket, or other YAML-significant character.
- find_invalid_frontmatter now skips wiki/sources/ and wiki/reports/,
  matching the convention in find_broken_links / find_orphans —
  otherwise user-uploaded source markdown with malformed YAML
  produced non-actionable lint noise.

Tests: 520 pass. Updated 9 assertion strings to match the new quoted
output format; one assertion (TestAddRelatedLink) kept the old
unquoted form because that test exercises an existing-file path
where the brief line is preserved verbatim.

Author: KylinMountain

openkb/agent/compiler.py

@@ -27,6 +27,7 @@
 from pathlib import Path
 
 import litellm
+import yaml
 
 from openkb.lint import list_existing_wiki_targets, strip_ghost_wikilinks
 from openkb.schema import get_agents_md
@@ -405,12 +406,14 @@ def _read_concept_briefs(wiki_dir: Path) -> str:
         if text.startswith("---"):
             end = text.find("---", 3)
             if end != -1:
-                fm = text[:end + 3]
+                fm_text = text[3:end].strip("\n")
                 body = text[end + 3:]
-                for line in fm.split("\n"):
-                    if line.startswith("brief:"):
-                        brief = line[len("brief:"):].strip()
-                        break
+                try:
+                    fm = yaml.safe_load(fm_text)
+                except yaml.YAMLError:
+                    fm = None
+                if isinstance(fm, dict) and isinstance(fm.get("brief"), str):
+                    brief = fm["brief"].strip()
         if not brief:
             brief = body.strip().replace("\n", " ")[:150]
         if brief:
@@ -564,6 +567,39 @@ def _sanitize_concept_name(name: str) -> str:
     return sanitized or "unnamed-concept"
 
 
+def _yaml_kv_line(key: str, value: str) -> str:
+    """Render a single ``key: value`` line that round-trips through any YAML loader.
+
+    Uses ``json.dumps`` for the value — JSON strings are a strict subset of
+    YAML, always single-line, always correctly escaped (newlines, quotes,
+    control chars), and never auto-promoted to multi-line block scalars.
+    """
+    return f"{key}: {json.dumps(value, ensure_ascii=False)}"
+
+
+def _yaml_list_line(key: str, items: list[str]) -> str:
+    """Render ``key: [a, b, c]`` as JSON-style YAML (always single-line, always valid)."""
+    return f"{key}: {json.dumps(list(items), ensure_ascii=False)}"
+
+
+def _parse_yaml_list_value(line: str) -> list[str] | None:
+    """Parse the right-hand side of ``key: [...]`` into a list of strings.
+
+    Returns ``None`` when the value cannot be interpreted as a list — callers
+    treat that as "leave the frontmatter alone".
+    """
+    colon = line.find(":")
+    if colon == -1:
+        return None
+    try:
+        parsed = yaml.safe_load(line[colon + 1:])
+    except yaml.YAMLError:
+        return None
+    if not isinstance(parsed, list):
+        return None
+    return [str(x) for x in parsed]
+
+
 def _write_concept(wiki_dir: Path, name: str, content: str, source_file: str, is_update: bool, brief: str = "") -> None:
     """Write or update a concept page, managing the sources frontmatter."""
     concepts_dir = wiki_dir / "concepts"
@@ -598,20 +634,23 @@ def _write_concept(wiki_dir: Path, name: str, content: str, source_file: str, is
             if end != -1:
                 fm = existing[:end + 3]
                 body = existing[end + 3:]
+                brief_line = _yaml_kv_line("brief", brief)
                 if "brief:" in fm:
-                    fm = re.sub(r"brief:.*", f"brief: {brief}", fm)
+                    # Lambda to bypass re.sub backref interpretation in the
+                    # replacement string (brief may contain \1, \g<…>, etc.).
+                    fm = re.sub(r"brief:.*", lambda _m: brief_line, fm)
                 else:
-                    fm = fm.replace("---\n", f"---\nbrief: {brief}\n", 1)
+                    fm = fm.replace("---\n", f"---\n{brief_line}\n", 1)
                 existing = fm + body
         path.write_text(existing, encoding="utf-8")
     else:
         if content.startswith("---"):
             end = content.find("---", 3)
             if end != -1:
                 content = content[end + 3:].lstrip("\n")
-        fm_lines = [f"sources: [{source_file}]"]
+        fm_lines = [_yaml_list_line("sources", [source_file])]
         if brief:
-            fm_lines.append(f"brief: {brief}")
+            fm_lines.append(_yaml_kv_line("brief", brief))
         frontmatter = "---\n" + "\n".join(fm_lines) + "\n---\n\n"
         path.write_text(frontmatter + content, encoding="utf-8")
 
@@ -624,7 +663,7 @@ def _prepend_source_to_frontmatter(text: str, source_file: str) -> str:
     the frontmatter is malformed (no closing ``---``).
     """
     if not text.startswith("---"):
-        return f"---\nsources: [{source_file}]\n---\n\n" + text
+        return f"---\n{_yaml_list_line('sources', [source_file])}\n---\n\n" + text
 
     fm_end = text.find("---", 3)
     if fm_end == -1:
@@ -637,18 +676,16 @@ def _prepend_source_to_frontmatter(text: str, source_file: str) -> str:
     for i, line in enumerate(fm_lines):
         if not line.lstrip().startswith("sources:"):
             continue
-        lb = line.find("[")
-        rb = line.rfind("]")
-        if lb == -1 or rb == -1 or rb < lb:
+        items = _parse_yaml_list_value(line)
+        if items is None:
             return text
-        items = [s.strip() for s in line[lb + 1:rb].split(",") if s.strip()]
         if source_file in items:
             return text
         items.insert(0, source_file)
-        fm_lines[i] = f"sources: [{', '.join(items)}]"
+        fm_lines[i] = _yaml_list_line("sources", items)
         return "\n".join(fm_lines) + body
 
-    fm_lines.insert(1, f"sources: [{source_file}]")
+    fm_lines.insert(1, _yaml_list_line("sources", [source_file]))
     return "\n".join(fm_lines) + body
 
 
@@ -676,15 +713,13 @@ def _remove_source_from_frontmatter(text: str, source_file: str) -> tuple[str, b
     for i, line in enumerate(fm_lines):
         if not line.lstrip().startswith("sources:"):
             continue
-        lb = line.find("[")
-        rb = line.rfind("]")
-        if lb == -1 or rb == -1 or rb < lb:
+        items = _parse_yaml_list_value(line)
+        if items is None:
             return text, False
-        items = [s.strip() for s in line[lb + 1:rb].split(",") if s.strip()]
         if source_file not in items:
             return text, False
         items.remove(source_file)
-        fm_lines[i] = f"sources: [{', '.join(items)}]"
+        fm_lines[i] = _yaml_list_line("sources", items)
         return "\n".join(fm_lines) + body, len(items) == 0
 
     return text, False

openkb/lint.py

@@ -5,13 +5,16 @@
 - Orphaned pages — pages with no incoming or outgoing links
 - Missing wiki entries — raw files without corresponding sources/summaries
 - Index sync — index.md links vs actual files on disk
+- Invalid frontmatter — YAML that won't round-trip through safe_load
 """
 from __future__ import annotations
 
 import re
 import unicodedata
 from pathlib import Path
 
+import yaml
+
 # Matches [[wikilink]] or [[subdir/link]]
 _WIKILINK_RE = re.compile(r"\[\[([^\]]+)\]\]")
 
@@ -402,6 +405,42 @@ def check_index_sync(wiki: Path) -> list[str]:
     return sorted(issues)
 
 
+def find_invalid_frontmatter(wiki: Path) -> list[str]:
+    """Return wiki pages whose YAML frontmatter fails ``yaml.safe_load``.
+
+    Catches the silent-write class of bug where an LLM-authored field
+    (e.g. ``brief:``) ships unquoted and turns a colon-bearing value
+    into invalid YAML that OpenKB itself reads with string slicing but
+    external YAML-aware tools (VS Code, Obsidian, doc generators) reject.
+    """
+    issues: list[str] = []
+    if not wiki.exists():
+        return issues
+    for path in sorted(wiki.rglob("*.md")):
+        if path.name in _EXCLUDED_FILES:
+            continue
+        # Skip reports/ and sources/ — auto-generated / user-uploaded
+        # content, not wiki pages we manage. Matches the convention in
+        # find_broken_links / find_orphans.
+        rel_parts = path.relative_to(wiki).parts
+        if rel_parts and rel_parts[0] in ("reports", "sources"):
+            continue
+        text = _read_md(path)
+        if not text.startswith("---"):
+            continue
+        end = text.find("\n---", 3)
+        if end == -1:
+            continue
+        fm = text[3:end].strip("\n")
+        try:
+            yaml.safe_load(fm)
+        except yaml.YAMLError as exc:
+            rel = path.relative_to(wiki)
+            msg = str(exc).splitlines()[0]
+            issues.append(f"{rel}: {msg}")
+    return issues
+
+
 def run_structural_lint(kb_dir: Path) -> str:
     """Run all structural lint checks and return a formatted Markdown report.
 
@@ -418,6 +457,7 @@ def run_structural_lint(kb_dir: Path) -> str:
     orphans = find_orphans(wiki)
     missing = find_missing_entries(raw, wiki)
     sync_issues = check_index_sync(wiki)
+    bad_frontmatter = find_invalid_frontmatter(wiki)
 
     lines = ["## Structural Lint Report\n"]
 
@@ -455,5 +495,14 @@ def run_structural_lint(kb_dir: Path) -> str:
             lines.append(f"- {issue}")
     else:
         lines.append("Index is in sync.")
+    lines.append("")
+
+    # Invalid frontmatter
+    lines.append(f"### Invalid Frontmatter ({len(bad_frontmatter)})")
+    if bad_frontmatter:
+        for issue in bad_frontmatter:
+            lines.append(f"- {issue}")
+    else:
+        lines.append("All frontmatter parses as valid YAML.")
 
     return "\n".join(lines)

tests/test_compiler.py

@@ -137,8 +137,8 @@ def test_new_concept_with_brief(self, tmp_path):
         path = wiki / "concepts" / "attention.md"
         assert path.exists()
         text = path.read_text()
-        assert "sources: [paper.pdf]" in text
-        assert "brief: Mechanism for selective focus" in text
+        assert 'sources: ["paper.pdf"]' in text
+        assert 'brief: "Mechanism for selective focus"' in text
         assert "# Attention" in text
 
     def test_new_concept_without_brief(self, tmp_path):
@@ -148,7 +148,7 @@ def test_new_concept_without_brief(self, tmp_path):
         path = wiki / "concepts" / "attention.md"
         assert path.exists()
         text = path.read_text()
-        assert "sources: [paper.pdf]" in text
+        assert 'sources: ["paper.pdf"]' in text
         assert "brief:" not in text
 
     def test_update_concept_updates_brief(self, tmp_path):
@@ -163,7 +163,7 @@ def test_update_concept_updates_brief(self, tmp_path):
         text = (concepts / "attention.md").read_text()
         assert "paper2.pdf" in text
         assert "paper1.pdf" in text
-        assert "brief: Updated brief" in text
+        assert 'brief: "Updated brief"' in text
         assert "Old brief" not in text
 
     def test_update_concept_appends_source(self, tmp_path):
@@ -633,7 +633,8 @@ def test_frontmatter_without_sources_line_gets_one_inserted(self, tmp_path):
         )
         _add_related_link(wiki, "attention", "new-doc", "paper.pdf")
         text = (concepts / "attention.md").read_text()
-        assert "sources: [paper.pdf]" in text
+        assert 'sources: ["paper.pdf"]' in text
+        # Brief was not touched (existing line preserved); only sources was inserted.
         assert "brief: Focus mechanism" in text
         assert "[[summaries/new-doc]]" in text
 
@@ -732,7 +733,7 @@ async def test_full_pipeline(self, tmp_path):
         # Verify concept written
         concept_path = wiki / "concepts" / "transformer.md"
         assert concept_path.exists()
-        assert "sources: [summaries/test-doc.md]" in concept_path.read_text()
+        assert 'sources: ["summaries/test-doc.md"]' in concept_path.read_text()
 
         # Verify index updated
         index_text = (wiki / "index.md").read_text()
@@ -1205,7 +1206,7 @@ async def ordered_acompletion(*args, **kwargs):
         fa_path = wiki / "concepts" / "flash-attention.md"
         assert fa_path.exists()
         fa_text = fa_path.read_text()
-        assert "sources: [summaries/test-doc.md]" in fa_text
+        assert 'sources: ["summaries/test-doc.md"]' in fa_text
         assert "Flash Attention" in fa_text
 
         # Verify attention updated (is_update=True path in _write_concept)
@@ -1287,7 +1288,7 @@ async def test_fallback_list_format(self, tmp_path):
         att_path = wiki / "concepts" / "attention.md"
         assert att_path.exists()
         att_text = att_path.read_text()
-        assert "sources: [summaries/test-doc.md]" in att_text
+        assert 'sources: ["summaries/test-doc.md"]' in att_text
         assert "Attention" in att_text
 
 
@@ -1338,7 +1339,7 @@ async def test_short_doc_briefs_in_index_and_frontmatter(self, tmp_path):
 
         # Concept frontmatter has brief
         concept_text = (wiki / "concepts" / "transformer.md").read_text()
-        assert "brief: NN architecture using self-attention" in concept_text
+        assert 'brief: "NN architecture using self-attention"' in concept_text
 
         # Index has briefs
         index_text = (wiki / "index.md").read_text()