From b8609522ba9ec745a087c84f52a7ff04c0769002 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:16:33 +0200 Subject: [PATCH 1/2] fix(agents): let the root read a shipped plot result whole PR #12114 added `version` to PlotResult so the chat UI can address the artifact and theme routes by the server's number. The tool-safety allowlist for the plot_pipeline result still listed the old fields, so every shipped plot reached the root as {"status": "error", "code": "invalid_result"} and the root's closing reply told the user that the run had failed while the plot was already on screen. Seen in the first local run through the /debug/agent gate on 2026-10-11; the eval harness never checks the root's text, so it did not notice. The allowlist is now derived from PlotResult.model_fields plus the tool error shape, and a plugin test passes a shipped result with version 1 through the callback and asserts the schema's fields stay allowed. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M --- agents/anyplot/plugins/tool_safety.py | 8 ++++---- changelog.d/agents-plot-result-version.md | 8 ++++++++ tests/unit/agents/runtime/test_plugins.py | 18 ++++++++++++++++-- 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 changelog.d/agents-plot-result-version.md diff --git a/agents/anyplot/plugins/tool_safety.py b/agents/anyplot/plugins/tool_safety.py index 86ede927a6..c365b73e01 100644 --- a/agents/anyplot/plugins/tool_safety.py +++ b/agents/anyplot/plugins/tool_safety.py @@ -26,7 +26,7 @@ from pydantic import BaseModel, ConfigDict, Field, ValidationError from ..policy import DATA_PREAMBLE, fence -from ..schemas import MAX_COLUMNS, Binding, PipelineArgs +from ..schemas import MAX_COLUMNS, Binding, PipelineArgs, PlotResult from .ledger import argument_hash, attribution, ledger_for @@ -79,9 +79,9 @@ class _BindingArgs(BaseModel): "get_spec_brief": frozenset({"status", "code", "brief"}), "get_current_code": frozenset({"status", "code", "version"}), "set_bindings": frozenset({"status", "reason", "code", "complete", "missing_roles", "errors"}), - PIPELINE_TOOL: frozenset( - {"status", "reason", "attempts", "artifacts", "changes", "residual_defects", "code", "result", "error"} - ), + # Every PlotResult field, so a field added to the schema (as `version` was) can never turn a + # shipped plot into `invalid_result` for the root, plus the tool-error shape. + PIPELINE_TOOL: frozenset(PlotResult.model_fields) | frozenset({"code", "result", "error"}), } diff --git a/changelog.d/agents-plot-result-version.md b/changelog.d/agents-plot-result-version.md new file mode 100644 index 0000000000..9c8387a08b --- /dev/null +++ b/changelog.d/agents-plot-result-version.md @@ -0,0 +1,8 @@ +### Fixed + +- **The root agent no longer reports a shipped plot as failed.** The chat UI + added `version` to the plot result, but the tool-safety allowlist for the + `plot_pipeline` result did not know the field, so every shipped plot reached + the root as `invalid_result` and its closing reply told the user that the + run had failed while the plot was already on screen. The allowlist is now + derived from the `PlotResult` schema, and a test holds the two together. diff --git a/tests/unit/agents/runtime/test_plugins.py b/tests/unit/agents/runtime/test_plugins.py index 26002a3ea9..d4cae16c3f 100644 --- a/tests/unit/agents/runtime/test_plugins.py +++ b/tests/unit/agents/runtime/test_plugins.py @@ -13,9 +13,9 @@ from agents.anyplot.plugins.budget import BudgetPlugin from agents.anyplot.plugins.ledger import CURRENT_LEDGER, RequestLedger, ledger_for from agents.anyplot.plugins.scope_guard import WITHHELD, ScopeGuardPlugin, judge_input -from agents.anyplot.plugins.tool_safety import ToolSafetyPlugin, has_url_or_path, result_limit, result_size +from agents.anyplot.plugins.tool_safety import RESULT_KEYS, ToolSafetyPlugin, has_url_or_path, result_limit, result_size from agents.anyplot.policy import DATA_PREAMBLE, fence -from agents.anyplot.schemas import ColumnProfile, DatasetProfile +from agents.anyplot.schemas import ColumnProfile, DatasetProfile, PlotResult from agents.anyplot.services import Services from agents.anyplot.settings import get_settings @@ -308,6 +308,20 @@ async def test_pipeline_notes_are_fenced_for_the_root(self, ledger: RequestLedge tool=FakeTool("plot_pipeline"), tool_args={}, tool_context=FakeContext(), result={"status": "failed"} ) == {"status": "failed"} + async def test_a_shipped_plot_result_reaches_the_root_whole(self, ledger: RequestLedger) -> None: + """Every PlotResult field is allowed: `version` (added with the chat UI) used to make the root read + `invalid_result` for every shipped plot and tell the user the run had failed.""" + shipped = PlotResult( + status="ok", attempts=1, artifacts=["plot-light.png"], changes=["Plotted df"], version=1 + ).model_dump(mode="json") + + result = await ToolSafetyPlugin().after_tool_callback( + tool=FakeTool("plot_pipeline"), tool_args={}, tool_context=FakeContext(), result=shipped + ) + + assert result is not None and result["status"] == "ok" and result["version"] == 1 + assert set(PlotResult.model_fields) <= RESULT_KEYS["plot_pipeline"] + async def test_errors_become_fixed_codes(self, ledger: RequestLedger) -> None: result = await ToolSafetyPlugin().on_tool_error_callback( tool=FakeTool("get_spec_brief"), From a972915e6a54c6a1aba6c918dc607ace2ab69d32 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:17:16 +0200 Subject: [PATCH 2/2] chore(changelog): reference #12123 in the fragment Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M --- changelog.d/agents-plot-result-version.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.d/agents-plot-result-version.md b/changelog.d/agents-plot-result-version.md index 9c8387a08b..0a98226b08 100644 --- a/changelog.d/agents-plot-result-version.md +++ b/changelog.d/agents-plot-result-version.md @@ -6,3 +6,4 @@ the root as `invalid_result` and its closing reply told the user that the run had failed while the plot was already on screen. The allowlist is now derived from the `PlotResult` schema, and a test holds the two together. + (#12123)