diff --git a/reme/config/benchmark.yaml b/reme/config/benchmark.yaml index 9212092c..0afcb0ed 100644 --- a/reme/config/benchmark.yaml +++ b/reme/config/benchmark.yaml @@ -327,7 +327,7 @@ jobs: frontmatter_read: backend: base - description: "Read a file's frontmatter as a dict." + description: "Read a file's frontmatter, one key: value per line." parameters: type: object properties: diff --git a/reme/config/default.yaml b/reme/config/default.yaml index dbab8ac2..08c25dde 100644 --- a/reme/config/default.yaml +++ b/reme/config/default.yaml @@ -556,7 +556,7 @@ jobs: frontmatter_read: backend: base - description: "Read a file's frontmatter as a dict." + description: "Read a file's frontmatter, one key: value per line." parameters: type: object properties: diff --git a/reme/steps/file_io/frontmatter_read.py b/reme/steps/file_io/frontmatter_read.py index fea6312d..21157696 100644 --- a/reme/steps/file_io/frontmatter_read.py +++ b/reme/steps/file_io/frontmatter_read.py @@ -1,13 +1,16 @@ -"""``frontmatter_read_step`` — return the frontmatter dict of a markdown file. +"""``frontmatter_read_step`` — surface a markdown file's frontmatter. Cheap structured read — frontmatter only, no body. Use ``body:read`` for the post-frontmatter content slice, or whole-file ``read`` when -you want both at once. Returns ``{exists: false}`` when the target -doesn't exist; otherwise ``{exists: true, frontmatter: {...}}``. +you want both at once. The answer is textual — ``- key: value`` per +line — because MCP callers only ever see ``answer``; the parsed +mapping itself is returned in ``metadata["frontmatter"]`` (and +``exists: false`` in metadata when the target doesn't exist). ``path`` is a path relative to the workspace. """ +import json from pathlib import Path import frontmatter @@ -18,6 +21,35 @@ from ..base_step import BaseStep from ...components import R +def _render_value(value) -> str: + """Render one frontmatter value on a single answer line. + + ``frontmatter_read`` is served straight to the LLM, so the answer + has to carry the frontmatter itself. ``json.dumps`` (with + ``default=str`` for YAML-only types such as dates, and + ``ensure_ascii=False`` for non-ASCII) preserves the structural + distinction between a literal ``"\\n"`` two-character sequence and + an actual newline, between the string ``"null"`` and YAML ``null``, + and between mixed mapping key types (an int ``1`` and a string + ``"1"`` round-trip with their JSON-form distinction). The + one-key-per-line layout is enforced upstream by the caller. + """ + try: + return json.dumps(value, ensure_ascii=False, default=str) + except (TypeError, ValueError, RecursionError): + return repr(value) + + +def _render_key(key) -> str: + """Render a frontmatter key (the caller formats ``key: value`` lines). + + Keys are always strings in YAML frontmatter, so they do not need + the json.dumps structural pinning. Return them as-is so the answer + reads as ``- key: value`` rather than ``- "key": value``. + """ + return str(key) + + @R.register("frontmatter_read_step") class FrontmatterReadStep(BaseStep): """Read a Markdown file's frontmatter (YAML metadata only).""" @@ -71,6 +103,8 @@ class FrontmatterReadStep(BaseStep): self.logger.info(f"[{self.name}] path={path} parse_error={exc!r}") return self.context.response.success = True - self.context.response.answer = f"Read frontmatter from {path} ({len(meta)} key(s))" + header = f"Read frontmatter from {probed} ({len(meta)} key(s))" + rendered = [f"- {_render_key(key)}: {_render_value(value)}" for key, value in meta.items()] + self.context.response.answer = f"{header}:\n" + "\n".join(rendered) if rendered else f"{header}: (no keys)" self.context.response.metadata.update({"path": path, "exists": True, "frontmatter": meta, **resolved}) self.logger.info(f"[{self.name}] path={path} keys={len(meta)}") diff --git a/tests/unit/test_frontmatter_steps.py b/tests/unit/test_frontmatter_steps.py index f95504d8..300cb128 100644 --- a/tests/unit/test_frontmatter_steps.py +++ b/tests/unit/test_frontmatter_steps.py @@ -12,6 +12,7 @@ probed path in not-found errors instead of the caller's raw input. import os import tempfile +from datetime import date from pathlib import Path import frontmatter @@ -262,3 +263,161 @@ async def test_explicit_md_path_has_no_resolved_path(): assert resp.success is True assert "resolved_path" not in resp.metadata await store.close() + + +@pytest.mark.asyncio +async def test_read_answer_carries_the_frontmatter_itself(): + """The answer must contain the frontmatter, not just a key count. + + ``frontmatter_read`` is served straight to the LLM and MCP returns only + ``response.answer``, so a count alone leaves the caller with nothing to + act on. The suffix-less substitution is disclosed in the answer too. + """ + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + _seed(Path(tmp), NOTE, BODY) + store = await _make_store() + resp = await _run(FrontmatterReadStep, store, path="notes/post") + assert resp.success is True + assert resp.answer == 'Read frontmatter from notes/post.md (2 key(s)):\n- name: "n"\n- tags: ["a"]' + assert resp.metadata["frontmatter"] == {"name": "n", "tags": ["a"]} + await store.close() + + +@pytest.mark.asyncio +async def test_read_answer_preserves_value_structure_and_reports_empty(): + """Multi-line values stay on one line each; lists keep their shape; empty says so.""" + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + _seed( + Path(tmp), + "notes/rich.md", + "---\nname: Cobalt\ndescription: line one\n line two\ntags:\n- a\n- b\nnested:\n x: 1\n---\nbody\n", + ) + _seed(Path(tmp), "notes/empty.md", "---\n---\nbody\n") + store = await _make_store() + + resp = await _run(FrontmatterReadStep, store, path="notes/rich.md") + assert resp.success is True + assert resp.answer == ( + "Read frontmatter from notes/rich.md (4 key(s)):\n" + '- name: "Cobalt"\n' + '- description: "line one line two"\n' + '- tags: ["a", "b"]\n' + '- nested: {"x": 1}' + ) + assert resp.metadata["frontmatter"] == { + "name": "Cobalt", + "description": "line one line two", + "tags": ["a", "b"], + "nested": {"x": 1}, + } + + resp = await _run(FrontmatterReadStep, store, path="notes/empty.md") + assert resp.success is True + assert resp.answer == "Read frontmatter from notes/empty.md (0 key(s)): (no keys)" + assert resp.metadata["frontmatter"] == {} + await store.close() + + +@pytest.mark.asyncio +async def test_read_answer_tolerates_rich_yaml_types(): + """Rendering must never turn a parseable frontmatter into a failed job. + + Regression test for the review findings: YAML dates (``datetime.date``) + crashed ``json.dumps``, mixed mapping keys crashed ``sort_keys``, and + comma-joined lists / collapsed whitespace lost value boundaries. All of + these stay successful reads with the structured ``metadata`` untouched. + """ + rich = ( + "---\n" + "created: 2026-10-12\n" + "schedule:\n" + " 1: one\n" + " label: two\n" + "tags: []\n" + "aliases:\n" + ' - "a, b"\n' + " - c\n" + 'command: "echo a b"\n' + "description: |-\n" + " line one\n" + " line two\n" + "---\n" + "body\n" + ) + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + _seed(Path(tmp), "notes/types.md", rich) + store = await _make_store() + resp = await _run(FrontmatterReadStep, store, path="notes/types.md") + assert resp.success is True + assert resp.answer == ( + "Read frontmatter from notes/types.md (6 key(s)):\n" + '- created: 2026-10-12\n' + '- schedule: {"1": "one", "label": "two"}\n' + "- tags: []\n" + '- aliases: ["a, b", "c"]\n' + '- command: "echo a b"\n' + '- description: "line one\\nline two"' + ) + assert resp.metadata["frontmatter"] == { + "created": date(2026, 10, 12), + "schedule": {1: "one", "label": "two"}, + "tags": [], + "aliases": ["a, b", "c"], + "command": "echo a b", + "description": "line one\nline two", + } + await store.close() + + +@pytest.mark.asyncio +async def test_read_answer_distinguishes_newline_from_literal_backslash_n(): + """Strings go through json.dumps, so an actual newline and a + two-character ``"\\n"`` sequence render as different escape + sequences. Review follow-up: pin the answer so this distinction + survives future refactors. + """ + body = ( + "---\n" + 'real_newline: "line one\\nline two"\n' + 'literal_seq: "a\\\\nb"\n' + "---\n" + "body\n" + ) + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + _seed(Path(tmp), "notes/newline.md", body) + store = await _make_store() + resp = await _run(FrontmatterReadStep, store, path="notes/newline.md") + assert resp.success is True + assert "- real_newline: \"line one\\nline two\"" in resp.answer + # Literal two-character \n renders as a JSON-escaped backslash-n + # (the JSON string contains a backslash + an n). + assert '- literal_seq: "a\\\\nb"' in resp.answer + await store.close() + + +@pytest.mark.asyncio +async def test_read_answer_mixed_mapping_key_types_render_via_json(): + """An int 1 and a string "1" must render with their JSON-form + distinction in the answer (the metadata already preserves it). + Review follow-up: document the round-trip caveat for callers that + re-parse the answer body as JSON. + """ + body = ( + "---\n" + 'mixed:\n' + ' 1: "number"\n' + ' "1": "string"\n' + "---\n" + "body\n" + ) + with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp): + _seed(Path(tmp), "notes/mixed_keys.md", body) + store = await _make_store() + resp = await _run(FrontmatterReadStep, store, path="notes/mixed_keys.md") + assert resp.success is True + # json.dumps preserves int vs string in the rendered answer + assert '- mixed: {"1": "number", "1": "string"}' in resp.answer + assert resp.metadata["frontmatter"] == { + "mixed": {1: "number", "1": "string"} + } + await store.close()