mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-11-05 07:39:47 +00:00
fix(frontmatter-read): put the frontmatter in the answer, not just a key count (#564)
* fix(frontmatter-read): put the frontmatter in the answer, not just a key count `frontmatter_read` answered `Read frontmatter from <path> (N key(s))` and left the frontmatter itself in `metadata["frontmatter"]`. MCP returns only `response.answer`, so the LLM that asked for a file's frontmatter learned how many keys exist and nothing else — the same shape of gap #379 describes for `list`, where the caller cannot act on the result. Render each key on its own line in the answer. Values are flattened to one line each so a multi-line description cannot break the line-per-key layout: lists join with commas and nested mappings render as compact JSON. An empty frontmatter says `(no keys)` rather than trailing off after the colon. The answer also names the probed path instead of the raw input, so a suffix-less request reports the `.md` file that was actually read and the caller can follow up with `read` without consulting metadata. Structured `metadata["frontmatter"]` is unchanged, so programmatic consumers see no difference. Towards #379. * docs(config): match the frontmatter_read description to the new answer The job description still advertised "Read a file's frontmatter as a dict", but the answer is now rendered `key: value` lines — the dict only survives in metadata. An LLM choosing between `frontmatter_read` and a whole-file `read` would expect a dict it can index into and get prose instead. This is the wording-alignment point raised on #397, applied here before review rather than after. * fix(frontmatter-read): never fail a valid YAML read on rendering Review on #564 found two rendering regressions plus boundary losses: - json.dumps(..., sort_keys=True) raised on datetime.date values (YAML timestamps) and on mixed-type mapping keys, turning a parseable frontmatter into a failed job with empty metadata - the base revision read those files fine. - Comma-joining lists lost item boundaries (["a, b", "c"] read back as one ambiguous string), [] was indistinguishable from an empty string, and the whitespace collapse also mangled spaces inside serialized values. _render_value is now a single tolerant renderer: bare strings keep their text but have line breaks escaped; everything else goes through json.dumps(ensure_ascii=False, default=str) (no key sort) so collections stay structured and YAML-only types serialize; anything JSON still cannot represent falls back to repr. Keys are rendered through the same path so a multiline key cannot break the one-key-per-line layout either. The structured metadata contract is untouched. Also aligns the benchmark.yaml job description with the textual answer (default.yaml already says 'one key: value per line') and the module docstring now distinguishes the textual answer from the structured payload in metadata. Regression coverage: nested dates, mixed mapping keys, empty lists, comma-containing items, significant whitespace, and a literal block scalar with a real newline. * fix(frontmatter-read): use json.dumps for all scalars to preserve string and mapping distinctions Follow-up to the @jinliyl review on #564 (10-15 round 2): - _render_value now sends every scalar through json.dumps rather than the str-only branch. The str-only branch collapsed a literal '\n' two-character sequence and an actual newline into the same rendered form ('\n') and lost the distinction between the string 'null' and YAML null. The JSON form preserves both. - Mixed mapping key types (int 1 vs string '1') now round-trip with their JSON-form distinction in the answer, matching the metadata. Callers that re-parse the answer body as JSON still discard one value (this is a YAML-side limitation, not a renderer one); pinned by a regression test. - Updated the three existing answer assertions to carry the new quoted scalar form, and added two regression tests: test_read_answer_distinguishes_newline_from_literal_backslash_n and test_read_answer_mixed_mapping_key_types_render_via_json. Note: the str-only branch's '\n' escape was the only reason the description: |- literal-block test in test_read_answer_tolerates_rich_yaml_types rendered as 'line one\\nline two'. With the change, the same description renders as '"line one\nline two"' (real newline, JSON- escaped), which is exactly the fix the review asked for. * fix(frontmatter-read): split key/value rendering so keys are not JSON-quoted The round-2 change sent every scalar (including string keys) through json.dumps, which added quotes around string keys. The rendered answer came out as '- "name": "Cobalt"' instead of '- name: "Cobalt"' on the round-2 head; the 5 new and updated tests in test_frontmatter_steps all failed for that reason on Unit Tests - py3.11 through py3.14. Split the renderer: - _render_key: pass-through str(key) so the answer reads as '- key:'. - _render_value: still json.dumps (with default=str and ensure_ascii=False) for the structural pinning the round-2 review asked for (literal '\n' vs actual newline, the string 'null' vs YAML null, mixed mapping key types). Caller formats the line as '- {key}: {value}'. Updated the three existing answer assertions to match the key-unchanged, value-quoted shape (e.g. '- name: "Cobalt"'). The two new tests added in the round-2 commit (newline, mixed keys) were already pinned on the right shape; the round-1 key-quote was the only bug. * fix(frontmatter-read): JSON-encode keys too so every answer entry stays on one line A multiline YAML key broke the one-key-per-line layout because keys were interpolated raw. Route keys through the same json.dumps renderer as values, update the expected answers to the quoted-key form, correct the renderer docstring and mixed-key test comments (JSON collapses int 1 and string "1" keys alike — the metadata keeps the distinction), and pin the rich-type answer to the quoted "created": "2026-10-12" rendering. * test(frontmatter-read): cover multiline keys, reseed YAML blocks as literal strings Add the multiline-key regression the review asked for, restate the mixed-mapping-key case as a documented presentation limitation (JSON stringifies both keys, so a re-parser keeps only the last value), and write the YAML seeds as triple-quoted literals so black stops joining the implicit concatenations and pylint's implicit-str-concat goes quiet. * refactor(frontmatter-read): simplify answer rendering --------- Co-authored-by: jinli.yl <jinli.yl@alibaba-inc.com>
This commit is contained in:
Ziyang Guo
GitHub
parent
b0dc50188a
commit
b0991db725
4 changed files with 206 additions and 6 deletions
|
|
@ -331,7 +331,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:
|
||||
|
|
|
|||
|
|
@ -583,7 +583,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:
|
||||
|
|
|
|||
|
|
@ -1,13 +1,15 @@
|
|||
"""``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 has one ``- "key": value`` entry per
|
||||
line; the parsed mapping 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 +20,19 @@ from ..base_step import BaseStep
|
|||
from ...components import R
|
||||
|
||||
|
||||
def _render_value(value) -> str:
|
||||
"""Render a key or value on one line using JSON with a ``repr`` fallback.
|
||||
|
||||
YAML-only values such as dates use ``str``. Unsupported mapping keys
|
||||
and recursive aliases fall back to ``repr``. JSON stringifies nested
|
||||
mapping keys; only metadata preserves their original types.
|
||||
"""
|
||||
try:
|
||||
return json.dumps(value, ensure_ascii=False, default=str)
|
||||
except (TypeError, ValueError, RecursionError):
|
||||
return repr(value)
|
||||
|
||||
|
||||
@R.register("frontmatter_read_step")
|
||||
class FrontmatterReadStep(BaseStep):
|
||||
"""Read a Markdown file's frontmatter (YAML metadata only)."""
|
||||
|
|
@ -71,6 +86,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))"
|
||||
body = "\n".join(f"- {_render_value(key)}: {_render_value(value)}" for key, value in meta.items())
|
||||
self.context.response.answer = f"{header}:\n{body}" if body 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)}")
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ probed path in not-found errors instead of the caller's raw input.
|
|||
|
||||
from contextlib import chdir as temp_chdir
|
||||
import tempfile
|
||||
from datetime import date
|
||||
from pathlib import Path
|
||||
|
||||
import frontmatter
|
||||
|
|
@ -246,3 +247,185 @@ 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 = """---
|
||||
created: 2026-10-12
|
||||
schedule:
|
||||
1: one
|
||||
label: two
|
||||
tags: []
|
||||
aliases:
|
||||
- "a, b"
|
||||
- c
|
||||
command: "echo a b"
|
||||
description: |-
|
||||
line one
|
||||
line two
|
||||
---
|
||||
body
|
||||
"""
|
||||
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
|
||||
expected_lines = [
|
||||
"Read frontmatter from notes/types.md (6 key(s)):",
|
||||
'- "created": "2026-10-12"',
|
||||
'- "schedule": {"1": "one", "label": "two"}',
|
||||
'- "tags": []',
|
||||
'- "aliases": ["a, b", "c"]',
|
||||
'- "command": "echo a b"',
|
||||
'- "description": "line one\\nline two"',
|
||||
]
|
||||
assert resp.answer == "\n".join(expected_lines)
|
||||
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 = r"""---
|
||||
real_newline: "line one\nline two"
|
||||
literal_seq: "a\\nb"
|
||||
---
|
||||
body
|
||||
"""
|
||||
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_escapes_multiline_keys():
|
||||
"""Keys go through the same JSON renderer as values, so a multiline
|
||||
YAML key stays a single ``- "key": value`` entry instead of breaking
|
||||
the one-key-per-line layout. Review finding: a key like
|
||||
``first\\n- forged`` used to render as two lines and look like a
|
||||
second entry. The metadata keeps the original key.
|
||||
"""
|
||||
body = r"""---
|
||||
"first\n- forged": "value"
|
||||
---
|
||||
body
|
||||
"""
|
||||
with tempfile.TemporaryDirectory() as tmp, temp_chdir(tmp):
|
||||
_seed(Path(tmp), "notes/keys.md", body)
|
||||
store = await _make_store()
|
||||
resp = await _run(FrontmatterReadStep, store, path="notes/keys.md")
|
||||
assert resp.success is True
|
||||
assert resp.answer == 'Read frontmatter from notes/keys.md (1 key(s)):\n- "first\\n- forged": "value"'
|
||||
assert resp.metadata["frontmatter"] == {"first\n- forged": "value"}
|
||||
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" stay distinct in YAML and in the
|
||||
metadata, but the JSON-rendered answer collapses both keys to the
|
||||
string "1": the fragment textually keeps both entries, while
|
||||
re-parsing it as JSON keeps only the latter value. Documented
|
||||
presentation limitation — the structured metadata is the source of
|
||||
truth for key types.
|
||||
"""
|
||||
body = """---
|
||||
mixed:
|
||||
1: "number"
|
||||
"1": "string"
|
||||
---
|
||||
body
|
||||
"""
|
||||
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
|
||||
# Both entries render textually; JSON's key coercion means a
|
||||
# re-parser sees only the last "1". The metadata keeps the types.
|
||||
assert '- "mixed": {"1": "number", "1": "string"}' in resp.answer
|
||||
assert resp.metadata["frontmatter"] == {
|
||||
"mixed": {1: "number", "1": "string"},
|
||||
}
|
||||
await store.close()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue