Skip to content

Commit 7cfa30d

Browse files
committed
fix(review): harden frontmatter writes (title-case type, quote full_text, backfill order + dirty-check, lint isinstance)
1 parent 4dea511 commit 7cfa30d

7 files changed

Lines changed: 49 additions & 15 deletions

File tree

openkb/agent/compiler.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ def _write_summary(wiki_dir: Path, doc_name: str, summary: str,
783783
if description:
784784
fm_lines.append(_yaml_kv_line("description", description))
785785
fm_lines.append(f"doc_type: {doc_type}")
786-
fm_lines.append(f"full_text: sources/{doc_name}.{ext}")
786+
fm_lines.append(_yaml_kv_line("full_text", f"sources/{doc_name}.{ext}"))
787787
frontmatter = "---\n" + "\n".join(fm_lines) + "\n---\n\n"
788788
(summaries_dir / f"{doc_name}.md").write_text(frontmatter + summary, encoding="utf-8")
789789

@@ -917,7 +917,7 @@ def _write_entity(
917917

918918
def _build_frontmatter(sources: list[str]) -> str:
919919
fm_lines = [_yaml_list_line("sources", sources)]
920-
fm_lines.append(_yaml_kv_line("type", (type_ or "other").capitalize()))
920+
fm_lines.append(_yaml_kv_line("type", (type_ or "other").title()))
921921
if brief:
922922
fm_lines.append(_yaml_kv_line("description", brief))
923923
if aliases:
@@ -932,7 +932,7 @@ def _build_frontmatter(sources: list[str]) -> str:
932932
if end != -1:
933933
fm = existing[:end + 3]
934934
fm = _set_fm_line(fm, "description", brief) if brief else fm
935-
fm = _set_fm_line(fm, "type", type_.capitalize()) if type_ else fm
935+
fm = _set_fm_line(fm, "type", type_.title()) if type_ else fm
936936
# Drop any legacy ``brief:`` key (migrated to ``description:``),
937937
# mirroring _write_concept's update path.
938938
fm = re.sub(r"^brief:.*\n?", "", fm, flags=re.MULTILINE)
@@ -2055,17 +2055,21 @@ async def compile_long_doc(
20552055
schema_md = get_agents_md(wiki_dir)
20562056
summary_content = summary_path.read_text(encoding="utf-8")
20572057

2058-
# Backfill OKF fields on the indexer-written summary (idempotent).
2058+
# Backfill OKF fields on the indexer-written summary. Idempotent: set
2059+
# description before type so that when both keys are missing the prepends
2060+
# leave `type` first (canonical order); only rewrite when content changed.
20592061
if summary_content.startswith("---"):
20602062
fm_end = summary_content.find("---", 3)
20612063
if fm_end != -1:
20622064
fm = summary_content[:fm_end + 3]
20632065
body = summary_content[fm_end + 3:]
2064-
fm = _set_fm_line(fm, "type", "Summary")
20652066
if doc_description:
20662067
fm = _set_fm_line(fm, "description", doc_description)
2067-
summary_content = fm + body
2068-
summary_path.write_text(summary_content, encoding="utf-8")
2068+
fm = _set_fm_line(fm, "type", "Summary")
2069+
updated = fm + body
2070+
if updated != summary_content:
2071+
summary_content = updated
2072+
summary_path.write_text(summary_content, encoding="utf-8")
20692073

20702074
# Base context A. cache_control marker on the doc message creates a
20712075
# cache breakpoint covering (system + doc) for every concept call.

openkb/lint.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,9 +511,11 @@ def find_missing_okf_fields(wiki: Path) -> list[str]:
511511
fm = None
512512
fm = fm if isinstance(fm, dict) else {}
513513
rel = path.relative_to(wiki)
514-
if not str(fm.get("type") or "").strip():
514+
type_val = fm.get("type")
515+
if not (isinstance(type_val, str) and type_val.strip()):
515516
issues.append(f"{rel}: missing non-empty 'type'")
516-
if not str(fm.get("description") or "").strip():
517+
desc_val = fm.get("description")
518+
if not (isinstance(desc_val, str) and desc_val.strip()):
517519
issues.append(f"{rel}: missing non-empty 'description'")
518520
return issues
519521

openkb/tree_renderer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def _yaml_frontmatter(source_name: str, doc_id: str, description: str = "") -> s
1515
if description:
1616
lines.append(_yaml_kv_line("description", description))
1717
lines.append("doc_type: pageindex")
18-
lines.append(f"full_text: sources/{source_name}.json")
18+
lines.append(_yaml_kv_line("full_text", f"sources/{source_name}.json"))
1919
return "---\n" + "\n".join(lines) + "\n---\n"
2020

2121

tests/test_compiler.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ def test_writes_type_and_description(self, tmp_path):
209209
assert 'type: "Summary"' in text
210210
assert 'description: "A one-line summary."' in text
211211
assert "doc_type: short" in text
212-
assert "full_text: sources/my-doc.md" in text
212+
assert 'full_text: "sources/my-doc.md"' in text
213213
assert "# Summary" in text
214214

215215
def test_omits_description_when_empty(self, tmp_path):
@@ -773,6 +773,12 @@ def test_update_entity_strips_legacy_brief(self, tmp_path):
773773
assert "Old brief." not in text
774774
assert 'description: "New desc."' in text
775775

776+
def test_entity_type_multiword_title_cased(self, tmp_path):
777+
_write_entity(tmp_path, "acme", "# Acme\n\nx.", "summaries/a.md",
778+
is_update=False, brief="b", type_="real estate")
779+
text = (tmp_path / "entities" / "acme.md").read_text(encoding="utf-8")
780+
assert 'type: "Real Estate"' in text
781+
776782

777783
class TestBacklinkSummary:
778784
def test_adds_missing_concept_links(self, tmp_path):
@@ -1063,7 +1069,7 @@ async def test_full_pipeline(self, tmp_path):
10631069
summary_path = wiki / "summaries" / "test-doc.md"
10641070
assert summary_path.exists()
10651071
summary_text = summary_path.read_text()
1066-
assert "full_text: sources/test-doc.md" in summary_text
1072+
assert 'full_text: "sources/test-doc.md"' in summary_text
10671073
assert 'type: "Summary"' in summary_text
10681074
# Summary body comes from the rewrite step
10691075
assert "[[concepts/transformer]]" in summary_text
@@ -1741,7 +1747,7 @@ async def test_short_doc_briefs_in_index_and_frontmatter(self, tmp_path):
17411747
# Summary frontmatter has doc_type and full_text
17421748
summary_text = (wiki / "summaries" / "test-doc.md").read_text()
17431749
assert "doc_type: short" in summary_text
1744-
assert "full_text: sources/test-doc.md" in summary_text
1750+
assert 'full_text: "sources/test-doc.md"' in summary_text
17451751

17461752
# Concept frontmatter has type and description
17471753
concept_text = (wiki / "concepts" / "transformer.md").read_text()

tests/test_lint.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,3 +675,12 @@ def test_flags_null_type_as_missing(tmp_path):
675675
'---\ntype: null\ndescription: "x"\n---\n\n# Bad\n', encoding="utf-8")
676676
issues = find_missing_okf_fields(wiki)
677677
assert any("null_type.md" in i and "type" in i for i in issues)
678+
679+
680+
def test_flags_non_string_type_as_missing(tmp_path):
681+
wiki = tmp_path / "wiki"
682+
(wiki / "concepts").mkdir(parents=True)
683+
(wiki / "concepts" / "bool_type.md").write_text(
684+
'---\ntype: true\ndescription: "x"\n---\n\n# Bad\n', encoding="utf-8")
685+
issues = find_missing_okf_fields(wiki)
686+
assert any("bool_type.md" in i and "type" in i for i in issues)

tests/test_recompile.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,3 +343,5 @@ def test_compile_long_doc_backfills_summary_frontmatter(tmp_path):
343343
text = summary_path.read_text(encoding="utf-8")
344344
assert 'type: "Summary"' in text
345345
assert 'description: "A long report."' in text
346+
# canonical order: type before description
347+
assert text.index('type:') < text.index('description:')

tests/test_tree_renderer.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def test_has_yaml_frontmatter(self, sample_tree):
1515
output = render_summary_md(sample_tree, "Sample Document", "doc-abc")
1616
assert output.startswith("---\n")
1717
assert "doc_type: pageindex" in output
18-
assert "full_text: sources/Sample Document.json" in output
18+
assert 'full_text: "sources/Sample Document.json"' in output
1919

2020
def test_top_level_nodes_are_h1(self, sample_tree):
2121
output = render_summary_md(sample_tree, "Sample Document", "doc-abc")
@@ -47,4 +47,15 @@ def test_summary_md_has_type_and_description():
4747
assert 'type: "Summary"' in md
4848
assert 'description: "Quarterly report."' in md
4949
assert "doc_type: pageindex" in md
50-
assert "full_text: sources/my-doc.json" in md
50+
assert 'full_text: "sources/my-doc.json"' in md
51+
52+
53+
def test_summary_full_text_quoted_yaml_safe():
54+
import yaml
55+
tree = {"structure": []}
56+
md = render_summary_md(tree, "weird: name", "doc-1", description="d")
57+
# full_text is JSON-quoted, so a source name with a colon stays valid YAML
58+
assert 'full_text: "sources/weird: name.json"' in md
59+
fm = yaml.safe_load(md.split("---")[1])
60+
assert fm["full_text"] == "sources/weird: name.json"
61+
assert fm["type"] == "Summary"

0 commit comments

Comments
 (0)