From 25bdee61c53ab7344292ee06fc17379825d3b077 Mon Sep 17 00:00:00 2001 From: adarshh347 Date: Thu, 29 Oct 2026 18:16:50 +0530 Subject: [PATCH] fix(daily-paper): retry Codex JSON failures and stabilize parser tests --- .../src/reme_daily_paper/analyze.py | 5 +- .../tests/test_structured_recovery.py | 52 ++++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/plugins/daily_paper/src/reme_daily_paper/analyze.py b/plugins/daily_paper/src/reme_daily_paper/analyze.py index 2d08362d..c1f84001 100644 --- a/plugins/daily_paper/src/reme_daily_paper/analyze.py +++ b/plugins/daily_paper/src/reme_daily_paper/analyze.py @@ -4,7 +4,6 @@ import asyncio import json from pathlib import Path -from pydantic import ValidationError from .arxiv import ArxivPdfClient from .base import ( @@ -109,8 +108,8 @@ class DailyPaperAnalyzeStep(DailyPaperStep): truncated=str(truncated).lower(), pdf_text=pdf_text, ) - # AgentScope is optional: other wrappers can report Pydantic validation failures. - recoverable = (ValidationError,) + # AgentScope is optional: Codex JSON parsing and Pydantic validation both raise ValueError. + recoverable = (ValueError,) try: from agentscope.exception import StructuredOutputError except ImportError: diff --git a/plugins/daily_paper/tests/test_structured_recovery.py b/plugins/daily_paper/tests/test_structured_recovery.py index 139ba205..bbbdc1d6 100644 --- a/plugins/daily_paper/tests/test_structured_recovery.py +++ b/plugins/daily_paper/tests/test_structured_recovery.py @@ -34,16 +34,23 @@ class _TruncatedModel(ChatModelBase): super().__init__(credential=None, model="offline", parameters=_Parameters(), stream=False) self.formatter = OpenAIChatFormatter() self.recover = recover + self.retry_instruction = "" self.analysis_calls = 0 self.structured_calls = 0 async def _call_api(self, *_args, **kwargs): """Replace provider I/O while retaining real agent and parser behavior.""" + messages = kwargs.get("messages") or [] + for message in messages: + if message.role == "user": + message.id = "forced-user-id-800" tools = kwargs.get("tools") or [] structured = any(item.get("function", {}).get("name") == "generate_structured_output" for item in tools) if structured: self.structured_calls += 1 - shortened = "800" in str(kwargs.get("messages")) + shortened = any( + message.role == "user" and self.retry_instruction in message.get_text_content() for message in messages + ) raw = json.dumps({"title": "论文摘要", "desc": "summary", "body": "Supported result"}) if not (self.recover and shortened): raw = '{"body":"' + "unfinished note " * 2000 @@ -67,6 +74,7 @@ async def test_shorter_retry_uses_real_structured_parser(tmp_path, monkeypatch, agent = AsAgentWrapper(app_context=app, as_llm="") agent.as_llm = SimpleNamespace(model=model) step = DailyPaperAnalyzeStep(app_context=app, agent_wrapper=agent) + model.retry_instruction = step.prompt_format("analyze_short_retry") step.context = RuntimeContext(daily_paper_run_date="2026-07-21") monkeypatch.setattr(step, "_extract_pdf_text_sync", lambda *_: ("Supported result [p. 1]", 1, False)) downloader = SimpleNamespace(download=AsyncMock()) @@ -84,3 +92,45 @@ async def test_shorter_retry_uses_real_structured_parser(tmp_path, monkeypatch, assert model.analysis_calls == 2 assert model.structured_calls >= 2 assert downloader.download.await_count == 1 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("recover", [True, False]) +async def test_codex_json_parse_failure_gets_one_shorter_retry(tmp_path, monkeypatch, recover): + """Use the real Codex reply parser with only SDK/lifecycle boundaries replaced.""" + from reme.components.agent_wrapper import CodexAgentWrapper + + app = ApplicationContext(workspace_dir=str(tmp_path)) + wrapper = CodexAgentWrapper(app_context=app) + valid = json.dumps({"title": "论文摘要", "desc": "summary", "body": "Supported result"}) + thread = SimpleNamespace( + id="offline-thread", + run=AsyncMock( + side_effect=[ + SimpleNamespace(final_response='{"body":"unfinished', usage=None), + SimpleNamespace(final_response=valid if recover else '{"body":"unfinished', usage=None), + ], + ), + ) + monkeypatch.setattr(wrapper, "start", AsyncMock()) + monkeypatch.setattr(wrapper, "_get_codex", AsyncMock(return_value=object())) + monkeypatch.setattr(wrapper, "_open_thread", AsyncMock(return_value=thread)) + monkeypatch.setattr(wrapper, "_release_thread", AsyncMock()) + monkeypatch.setattr(wrapper, "_serialize", lambda _: {}) + step = DailyPaperAnalyzeStep(app_context=app, agent_wrapper=wrapper) + step.context = RuntimeContext(daily_paper_run_date="2026-07-21") + monkeypatch.setattr(step, "_extract_pdf_text_sync", lambda *_: ("Evidence [p. 1]", 1, False)) + downloader = SimpleNamespace(download=AsyncMock()) + paper = PaperInfo(arxiv_id="2607.10001") + pick = PaperPick(arxiv_id=paper.arxiv_id, reasoning="test") + if recover: + result = await step._analyze_one(downloader, paper, pick, set()) + assert (tmp_path / result.note_path).is_file() + else: + with pytest.raises(ValueError, match="Codex returned invalid JSON"): + await step._analyze_one(downloader, paper, pick, set()) + assert thread.run.await_count == 2 + instruction = step.prompt_format("analyze_short_retry") + assert instruction not in thread.run.await_args_list[0].args[0] + assert instruction in thread.run.await_args_list[1].args[0] + downloader.download.assert_awaited_once()