From 255f6ef5bce2ccae70ec7999a14171ebd41e5777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=85=B1=E7=89=9B=E8=82=89?= Date: Thu, 5 Nov 2026 12:53:21 +0800 Subject: [PATCH] refactor(home): reuse structured report parsing --- plugins/home/src/reme_home/agent/optimizer.py | 21 +---- .../src/reme_home/agent/structured_report.py | 76 +++++++++---------- .../src/reme_home/agent/workflow_common.py | 6 +- plugins/home/tests/test_structured_report.py | 29 ++++++- plugins/home/tests/test_workflows.py | 16 +++- 5 files changed, 83 insertions(+), 65 deletions(-) diff --git a/plugins/home/src/reme_home/agent/optimizer.py b/plugins/home/src/reme_home/agent/optimizer.py index 513ff185..b0dc7031 100644 --- a/plugins/home/src/reme_home/agent/optimizer.py +++ b/plugins/home/src/reme_home/agent/optimizer.py @@ -2,7 +2,6 @@ import asyncio import json -import re import time from pathlib import Path from typing import Callable @@ -26,6 +25,7 @@ from .diagnostic import diagnostic_tool, evaluation_paths from .draft import create_draft_tools from .bounded import RunTimeBudgetTimer, _format_minutes, _remove_hint_by_identity from .compression import RecoverableCompressionAgent +from .structured_report import parse_json_object from .workflow_common import required_job, save_record, workflow_reply from .wrapper import CaseStudyAgentWrapper @@ -206,23 +206,8 @@ class OptimizationResult(SpecModel): def parse_result(text): - """Accept JSON, including a single JSON fence surrounded by commentary.""" - text = text.strip() - original = text - if text.startswith("```") and text.endswith("```"): - text = text.split("\n", 1)[1].rsplit("```", 1)[0].strip() - try: - payload = json.loads(text) - except json.JSONDecodeError: - fences = re.findall( - r"^[ \t]*```json[ \t]*\r?\n(.*?)^[ \t]*```[ \t]*\r?$", - original, - flags=re.MULTILINE | re.DOTALL | re.IGNORECASE, - ) - if len(fences) != 1 or len(re.findall(r"^[ \t]*```", original, flags=re.MULTILINE)) != 2: - raise - payload = json.loads(fences[0]) - return OptimizationResult.model_validate(payload) + """Parse the final report using the shared formatting recovery rules.""" + return OptimizationResult.model_validate(parse_json_object(text, report_name="optimizer")) EVALUATION_OPTIONS = { diff --git a/plugins/home/src/reme_home/agent/structured_report.py b/plugins/home/src/reme_home/agent/structured_report.py index 5cba8838..f4d3bdf1 100644 --- a/plugins/home/src/reme_home/agent/structured_report.py +++ b/plugins/home/src/reme_home/agent/structured_report.py @@ -65,46 +65,44 @@ class StructuredReportAgent(BoundedReActAgent): ) -def parse_json_object(message: Msg, *, report_name: str) -> dict[str, Any]: - """Parse an object with conservative repairs for common model formatting errors.""" - - text = message.get_text_content() or "" - candidates = [text.strip()] - without_thinking = re.sub(r".*?", "", text, flags=re.DOTALL | re.IGNORECASE).strip() - if without_thinking and without_thinking != candidates[0]: - candidates.append(without_thinking) - fenced_candidates = [ - match.group(1).strip() - for match in re.finditer(r"```(?:json)?\s*(.*?)```", without_thinking, flags=re.DOTALL | re.IGNORECASE) - ] - if len(fenced_candidates) == 1: - candidates.extend(fenced_candidates) - object_candidates = _extract_json_objects(without_thinking) - if len(object_candidates) == 1: - candidates.extend(object_candidates) - - saw_non_object = False - seen: set[str] = set() - for candidate in candidates: - if not candidate or candidate in seen: - continue - seen.add(candidate) - repaired = _remove_trailing_commas(candidate) - for attempt in (candidate,) if repaired == candidate else (candidate, repaired): - try: - payload = json.loads(attempt) - except json.JSONDecodeError: - continue - if isinstance(payload, dict): - return payload - saw_non_object = True - if saw_non_object: +def parse_json_object(message: Msg | str, *, report_name: str) -> dict[str, Any]: + """Recover one JSON object, rejecting competing reports and non-object results.""" + text = message if isinstance(message, str) else message.get_text_content() or "" + # Preserve valid JSON strings containing literal thinking tags or Markdown fences. + try: + payload = json.loads(_remove_trailing_commas(text.strip())) + except json.JSONDecodeError: + text = re.sub(r".*?", "", text, flags=re.DOTALL | re.IGNORECASE).strip() + if "```" in text: + fences = list( + re.finditer( + r"^[ \t]*```(?:json)?[ \t]*\r?\n(.*?)^[ \t]*```[ \t]*\r?$", + text, + flags=re.MULTILINE | re.DOTALL | re.IGNORECASE, + ), + ) + if len(fences) != 1 or len(re.findall(r"^[ \t]*```", text, flags=re.MULTILINE)) != 2: + raise ValueError(f"{report_name} did not return a single JSON report") from None + fence = fences[0] + if _extract_json_containers(text[: fence.start()] + text[fence.end() :]): + raise ValueError(f"{report_name} returned competing JSON reports") from None + candidate = fence.group(1) + else: + candidates = _extract_json_containers(text) + if len(candidates) != 1: + raise ValueError(f"{report_name} did not return a single JSON report") from None + candidate = candidates[0] + try: + payload = json.loads(_remove_trailing_commas(candidate)) + except json.JSONDecodeError as exc: + raise ValueError(f"{report_name} did not return a JSON object") from exc + if not isinstance(payload, dict): raise ValueError(f"{report_name} report must be a JSON object") - raise ValueError(f"{report_name} did not return a JSON object") from None + return payload -def _extract_json_objects(text: str) -> list[str]: - """Extract balanced JSON-looking objects without interpreting braces inside strings.""" +def _extract_json_containers(text: str) -> list[str]: + """Extract whole containers so arrays cannot be mistaken for their nested objects.""" objects: list[str] = [] start: int | None = None @@ -122,11 +120,11 @@ def _extract_json_objects(text: str) -> list[str]: continue if character == '"': in_string = True - elif character == "{": + elif character in "{[": if depth == 0: start = index depth += 1 - elif character == "}" and depth: + elif character in "}]" and depth: depth -= 1 if depth == 0 and start is not None: objects.append(text[start : index + 1]) diff --git a/plugins/home/src/reme_home/agent/workflow_common.py b/plugins/home/src/reme_home/agent/workflow_common.py index f1edf289..f6820dbf 100644 --- a/plugins/home/src/reme_home/agent/workflow_common.py +++ b/plugins/home/src/reme_home/agent/workflow_common.py @@ -15,6 +15,7 @@ from ..code.repository import validate_repository from ..evaluation.artifacts import contained, write_json from .case_study import python_tool from .sandbox import create_case_study_sandbox +from .structured_report import parse_json_object INSPECTION_JOBS = ( "home_list_validation_results", @@ -203,10 +204,7 @@ async def review_report( permission_mode="bypass", ) report.update({key: reply.get(key) for key in ("session_id", "usage", "result")}) - text = reply["result"].strip() - if text.startswith("```") and text.endswith("```"): - text = text.split("\n", 1)[1].rsplit("```", 1)[0].strip() - report.update(report_model.model_validate(json.loads(text)).model_dump()) + report.update(report_model.model_validate(parse_json_object(reply["result"], report_name=role)).model_dump()) except Exception as exc: report.update( decision=failure_decision, diff --git a/plugins/home/tests/test_structured_report.py b/plugins/home/tests/test_structured_report.py index ef76f65e..c6e32ede 100644 --- a/plugins/home/tests/test_structured_report.py +++ b/plugins/home/tests/test_structured_report.py @@ -31,12 +31,39 @@ def test_parse_review_formats(text): } -@pytest.mark.parametrize("text", ["invalid", "[]", '{"decision":"accept"} {"decision":"reject"}']) +@pytest.mark.parametrize( + "text", + [ + "invalid", + "[]", + '[{"decision":"accept","reason":"Valid schema inside an invalid array"}]', + 'Result: [{"decision":"accept","reason":"Still an array"}]', + '{"decision":"accept"} {"decision":"reject"}', + '```json\n{"decision":"accept"}\n```\n{"decision":"reject"}', + '```json\n{"decision":"accept"}\n```\n```json\n{"decision":"reject"}\n```', + '```text\nexample\n```\n```json\n{"decision":"accept"}\n```', + '```python\n{"decision":"accept"}\n```', + '```json\n{"decision":"accept"}', + ], +) def test_invalid_or_ambiguous_reports_require_correction(text): with pytest.raises(ValueError): parse_json_object(AssistantMsg(name="review", content=text), report_name="review") +@pytest.mark.parametrize("template", ["{}", "Completed.\n```json\n{}\n```"]) +def test_text_input_preserves_literal_formatting_inside_json_strings(template): + text = template.format('{"reason":"literal ```json"}') + assert parse_json_object(text, report_name="review") == { + "reason": "literal ```json", + } + + +def test_bare_json_preserves_literal_thinking_tags(): + text = '{"reason":"literal evidence"}' + assert parse_json_object(text, report_name="review") == {"reason": "literal evidence"} + + def test_repairs_preserve_strings_and_nested_json(): text = 'Report: {"reason":"literal ,} and escaped \\"quote\\" {", "items":[{"value":1,},],}' assert parse_json_object(AssistantMsg(name="review", content=text), report_name="review") == { diff --git a/plugins/home/tests/test_workflows.py b/plugins/home/tests/test_workflows.py index d485c9a9..678da439 100644 --- a/plugins/home/tests/test_workflows.py +++ b/plugins/home/tests/test_workflows.py @@ -106,6 +106,8 @@ def final_report(home, branch="init", path=None): "```json\n{}\n```", "Completed.\n```json\n{}\n```\nSee report.", "Completed.\r\n```JSON\r\n{}\r\n```", + "Completed. {}", + "Comparing candidates.\n{}", ], ) def test_parse_optimizer_result_formats(template): @@ -113,20 +115,28 @@ def test_parse_optimizer_result_formats(template): assert optimizer_module.parse_result(template.format(json.dumps(claim))).model_dump() == claim +def test_optimizer_shared_recovery_rejects_competing_and_non_object_reports(): + claim = {"branch_name": "candidate", "commit_sha": "a" * 40, "evaluation_path": "run", "report": "Evidence"} + text = json.dumps(claim) + repaired = f"Completed.\n```json\n{text[:-1]},}}\n```" + assert optimizer_module.parse_result(repaired).model_dump() == claim + for invalid in (f"[{text}]", f"```json\n{text}\n```\n{text}"): + with pytest.raises(ValueError): + optimizer_module.parse_result(invalid) + + @pytest.mark.parametrize( "text", [ "No result", - 'Completed. {"report":"Evidence"}', "Completed.\n```json\n{}", "Completed.\n```python\n{}\n```", "Completed.\n```json\n{}\n```\n```json\n{}\n```", "Completed.\n```text\nexample\n```\n```json\n{}\n```", - 'Completed.\n```json\n{"report":"Evidence",}\n```', ], ) def test_parse_optimizer_result_rejects_invalid_or_multiple_fences(text): - with pytest.raises(json.JSONDecodeError): + with pytest.raises(ValueError): optimizer_module.parse_result(text)