mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-10-27 06:15:20 +00:00
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.
This commit is contained in:
parent
33d44b8132
commit
7c6ed1e2a0
2 changed files with 69 additions and 13 deletions
|
|
@ -25,15 +25,17 @@ def _render_value(value) -> str:
|
|||
"""Render one frontmatter key or value on a single answer line.
|
||||
|
||||
``frontmatter_read`` is served straight to the LLM, so the answer has
|
||||
to carry the frontmatter itself. Collections keep their JSON shape so
|
||||
list boundaries, empty collections, and nested structure stay visible;
|
||||
``default=str`` covers YAML-only types such as dates, and skipping key
|
||||
sort keeps mixed-type mapping keys loadable. Bare strings are the one
|
||||
exception — quoting every scalar would drown the answer in noise — but
|
||||
their line breaks are escaped so the one-key-per-line layout survives.
|
||||
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 (it never calls ``json.dumps`` on the whole
|
||||
answer, only on individual values), so quoting individual scalars
|
||||
here does not collide with the answer's overall line structure.
|
||||
"""
|
||||
if isinstance(value, str):
|
||||
return value.replace("\r", "\\r").replace("\n", "\\n")
|
||||
try:
|
||||
return json.dumps(value, ensure_ascii=False, default=str)
|
||||
except (TypeError, ValueError, RecursionError):
|
||||
|
|
|
|||
|
|
@ -208,7 +208,7 @@ async def test_read_answer_carries_the_frontmatter_itself():
|
|||
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.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()
|
||||
|
||||
|
|
@ -229,8 +229,8 @@ async def test_read_answer_preserves_value_structure_and_reports_empty():
|
|||
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"
|
||||
'- name: "Cobalt"\n'
|
||||
'- description: "line one line two"\n'
|
||||
'- tags: ["a", "b"]\n'
|
||||
'- nested: {"x": 1}'
|
||||
)
|
||||
|
|
@ -285,8 +285,8 @@ async def test_read_answer_tolerates_rich_yaml_types():
|
|||
'- schedule: {"1": "one", "label": "two"}\n'
|
||||
"- tags: []\n"
|
||||
'- aliases: ["a, b", "c"]\n'
|
||||
"- command: echo a b\n"
|
||||
"- description: line one\\nline two"
|
||||
'- command: "echo a b"\n'
|
||||
'- description: "line one\nline two"'
|
||||
)
|
||||
assert resp.metadata["frontmatter"] == {
|
||||
"created": date(2026, 10, 12),
|
||||
|
|
@ -297,3 +297,57 @@ async def test_read_answer_tolerates_rich_yaml_types():
|
|||
"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()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue