From efb7890f3c2fa79c6465b38609d6421210def8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicholas=20Pi=C3=ABl?= Date: Thu, 2 Jul 2026 12:44:03 +0000 Subject: [PATCH] [argus] patch #41: coerce stringified write_todos arg (planner pipeline) glm-nw sometimes double-encodes the write_todos tool argument: args.todos arrives as the JSON string of a valid list. Pydantic rejects it, the agent gets an error ToolMessage, and state.todos never hydrates - caught by the first weekly eval (2026-07-02, pythia/planning on glm-planner). ArgusTodoMiddleware.after_model now parses a string arg in place (only when it json-parses to a list) before tool validation, so the trajectory records the normalized call and the eval graders stay strict. Unparseable strings keep the normal validation-error path. The sibling flake (plan.json written but write_todos never called) is model behavior the weekly baseline tracks, not patched. Also refreshes the module docstring's stale agent-selection note (qwen-local-coder -> the #18 uses_planner_pipeline gate). Tests: test_argus_todo_middleware.py 12 passed (+3 new). PATCHES.md #41 in the same commit (ledger hard rule). Co-Authored-By: Claude Fable 5 --- PATCHES.md | 30 +++++++- .../middlewares/argus_todo_middleware.py | 71 ++++++++++++++++++- backend/tests/test_argus_todo_middleware.py | 56 +++++++++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) diff --git a/PATCHES.md b/PATCHES.md index 94a435a1c47..556e07ec1d1 100644 --- a/PATCHES.md +++ b/PATCHES.md @@ -65,7 +65,8 @@ half is upstreamable, the Argus behavior lives in project config). | [#37](#patch-37) | retry blank final turn + web display guard | argus-edit | 4a19e4fe | | [#38](#patch-38) | per-thread debug-sandbox link | argus-additive | 1aad692a, cd03995e, 2df36c99 (merge) | | [#39](#patch-39) | checkpointer pool bounds | generic-upstreamable | f37b8292 (PR #4 squash-merge) | -| [#40](#patch-40) | Telegram send-path extraction to `_telegram_sender.py` | argus-edit (carry-negative refactor) | this PR | +| [#40](#patch-40) | Telegram send-path extraction to `_telegram_sender.py` | argus-edit (carry-negative refactor) | a8516b07 (PR #6 squash-merge) | +| [#41](#patch-41) | coerce stringified write_todos arg (planner pipeline) | argus-additive | this PR | Dropped / deferred / not-carried records are at the bottom, followed by the carry budget ledger. @@ -795,6 +796,33 @@ carry budget ledger. but is a real (if tiny) behavior delta - out of scope for a zero-behavior patch. +## Patch #41 + +**Patch #41 - coerce a stringified `write_todos` arg in the planner pipeline** + +- Class: argus-additive (edits only the argus-owned + `argus_todo_middleware.py`; zero upstream-file carry) +- Intent: glm-nw sometimes double-encodes the `write_todos` tool argument - + `args.todos` arrives as the JSON STRING of a valid list instead of a native + array. Pydantic rejects it (`todos: list[Todo]`), the agent gets an error + ToolMessage, and `state.todos[]` never hydrates; caught by the weekly eval + 2026-07-02 (pythia/planning on glm-planner: "write_todos was called but its + 'todos' arg is str, expected list"). `ArgusTodoMiddleware.after_model` now + parses a string arg in place (only when it json-parses to a list) BEFORE + tool validation, so the trajectory records the normalized call and eval + graders stay strict. Unparseable strings keep the normal validation-error + path. The sibling flake (plan.json written but write_todos never called) is + NOT patched - it is model behavior the weekly eval baseline tracks. +- Files: `backend/packages/harness/deerflow/agents/middlewares/argus_todo_middleware.py` + (argus-owned file; also refreshed its stale "qwen-local-coder" selection + note to the #18 `uses_planner_pipeline` gate) +- Tests: `backend/tests/test_argus_todo_middleware.py` (+3: coercion, + unparseable-left-alone, native-list/other-tools untouched) +- Delete-when: glm-nw (or a replacement planner model) reliably emits native + arrays, or upstream langchain's todo tool grows arg coercion. +- Upstream status: none (candidate: the coercion is generic enough for + upstream langchain's TodoListMiddleware). + --- ## Dropped / deferred / re-expressed (v2.0.0 rebase record - do not re-add blindly) diff --git a/backend/packages/harness/deerflow/agents/middlewares/argus_todo_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/argus_todo_middleware.py index 223e55b7fe0..e176a312681 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/argus_todo_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/argus_todo_middleware.py @@ -16,14 +16,58 @@ inherits from the parent unchanged. Selected by ``deerflow.agents.lead_agent.agent._create_todo_list_middleware`` -when ``agent_name == "qwen-local-coder"``. Other agents continue to receive -the upstream prompt. +for agents whose config sets ``uses_planner_pipeline: true`` (e.g. +glm-planner; historically keyed on ``agent_name == "qwen-local-coder"``). +Other agents continue to receive the upstream prompt. + +[argus patch #41] also normalizes a double-encoded ``write_todos`` arg (the +model passing ``todos`` as a JSON string instead of a list) in ``after_model`` +— see ``_coerce_stringified_todos``. """ from __future__ import annotations +import json +import logging +from typing import Any + +from langchain_core.messages import AIMessage + from .todo_middleware import TodoMiddleware +logger = logging.getLogger(__name__) + + +def _coerce_stringified_todos(message: AIMessage) -> int: + """[argus patch #41] Normalize a JSON-string ``todos`` arg to a list. + + glm-nw (and peers) sometimes double-encode the ``write_todos`` argument: + the tool call arrives with ``args.todos`` as the STRING + ``'[{"content": ..., "status": ...}, ...]'`` instead of a native array. + Pydantic then rejects the call (``todos: list[Todo]``), the agent gets an + error ToolMessage, and ``state.todos[]`` never hydrates — observed on the + weekly eval 2026-07-02 (pythia/planning, glm-planner). The payload is + valid; only the encoding is wrong, so parse it in place before tool + validation runs. Anything that does not parse to a list is left alone + for the normal validation error path. Returns the number of calls fixed. + """ + fixed = 0 + for tc in message.tool_calls or []: + if tc.get("name") != "write_todos": + continue + args = tc.get("args") or {} + todos = args.get("todos") + if not isinstance(todos, str): + continue + try: + parsed = json.loads(todos) + except ValueError: + continue + if isinstance(parsed, list): + args["todos"] = parsed + fixed += 1 + return fixed + _ARGUS_SYSTEM_PROMPT = """ @@ -88,3 +132,26 @@ def __init__( tool_description=tool_description if tool_description is not None else _ARGUS_TOOL_DESCRIPTION, **kwargs, ) + + def after_model( + self, + state: dict[str, Any], + runtime: Any, + ) -> dict[str, Any] | None: + """[argus patch #41] Coerce stringified todos, then run parent logic. + + after_model runs between the model response and tool execution, so + rewriting the tool-call args in place here means (a) pydantic sees a + valid list, (b) the trajectory records the normalized call — eval + graders stay strict — and (c) the parent's premature-exit and + parallel-call checks operate on the corrected message. + """ + messages = state.get("messages") or [] + if messages and isinstance(messages[-1], AIMessage): + fixed = _coerce_stringified_todos(messages[-1]) + if fixed: + logger.info( + "[argus patch #41] coerced stringified todos arg in %d write_todos call(s)", + fixed, + ) + return super().after_model(state, runtime) diff --git a/backend/tests/test_argus_todo_middleware.py b/backend/tests/test_argus_todo_middleware.py index daefd562e72..b841add85c1 100644 --- a/backend/tests/test_argus_todo_middleware.py +++ b/backend/tests/test_argus_todo_middleware.py @@ -145,3 +145,59 @@ def test_factory_returns_none_when_plan_mode_off(): cfg = AgentConfig(name="qwen-local-coder", uses_planner_pipeline=True) mw = _create_todo_list_middleware(is_plan_mode=False, agent_name="qwen-local-coder", agent_config=cfg) assert mw is None + + +# --------------------------------------------------------------------------- +# [argus patch #41] stringified-todos coercion +# --------------------------------------------------------------------------- + + +def _write_todos_call(todos): + return {"name": "write_todos", "args": {"todos": todos}, "id": "tc-1", "type": "tool_call"} + + +def test_after_model_coerces_stringified_todos(): + """glm-nw double-encodes the todos arg as a JSON string (weekly eval + 2026-07-02, pythia/planning): after_model must parse it in place so + pydantic validation and state.todos hydration succeed.""" + mw = ArgusTodoMiddleware() + payload = '[{"content": "S1: do the thing", "status": "in_progress"}, {"content": "S2: verify", "status": "pending"}]' + msg = AIMessage(content="", tool_calls=[_write_todos_call(payload)]) + state = {"todos": [], "messages": [msg]} + + mw.after_model(state, _make_runtime()) + + todos = msg.tool_calls[0]["args"]["todos"] + assert isinstance(todos, list) + assert todos[0] == {"content": "S1: do the thing", "status": "in_progress"} + assert todos[1]["status"] == "pending" + + +def test_after_model_leaves_unparseable_string_for_validation(): + """A todos string that is not JSON (or not a list) must be left alone so + the normal pydantic validation error path still fires.""" + mw = ArgusTodoMiddleware() + not_json = AIMessage(content="", tool_calls=[_write_todos_call("do the thing")]) + not_a_list = AIMessage(content="", tool_calls=[_write_todos_call('{"content": "x"}')]) + + mw.after_model({"todos": [], "messages": [not_json]}, _make_runtime()) + mw.after_model({"todos": [], "messages": [not_a_list]}, _make_runtime()) + + assert not_json.tool_calls[0]["args"]["todos"] == "do the thing" + assert not_a_list.tool_calls[0]["args"]["todos"] == '{"content": "x"}' + + +def test_after_model_ignores_native_list_and_other_tools(): + """Native list args and non-write_todos calls pass through untouched.""" + mw = ArgusTodoMiddleware() + native = [{"content": "S1", "status": "in_progress"}] + msg = AIMessage(content="", tool_calls=[ + _write_todos_call(list(native)), + {"name": "bash", "args": {"command": '["not", "todos"]'}, "id": "tc-2", "type": "tool_call"}, + ]) + state = {"todos": [], "messages": [msg]} + + mw.after_model(state, _make_runtime()) + + assert msg.tool_calls[0]["args"]["todos"] == native + assert msg.tool_calls[1]["args"]["command"] == '["not", "todos"]'