From 7544d22c5994316ea8d4f0f21b26a85d9c9dc334 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 15 Oct 2026 09:27:10 +0200 Subject: [PATCH 1/3] fix(agents): keep a first plot's expected_texts with its kept note A first plot that stopped on data_unconfirmed kept its change_request but not the texts the user asked to see, so the next .create_plot() checked the render for none of them. The texts now travel in anyplot.pending_expected_texts with the same lifecycle as the note: kept until a run ships, dropped by any binding write, new data version or library switch (one PENDING_CLEARED delta for both keys). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M --- agents/anyplot/dataset_versions.py | 4 +- agents/anyplot/opening.py | 4 +- agents/anyplot/pipeline.py | 52 +++++++-- agents/anyplot/session_state.py | 10 +- changelog.d/agent-pending-expected-texts.md | 8 ++ docs/concepts/agent-network.md | 2 +- .../agents/runtime/test_data_questions.py | 109 ++++++++++++++++-- 7 files changed, 164 insertions(+), 25 deletions(-) create mode 100644 changelog.d/agent-pending-expected-texts.md diff --git a/agents/anyplot/dataset_versions.py b/agents/anyplot/dataset_versions.py index db5168811d6..95144b03c01 100644 --- a/agents/anyplot/dataset_versions.py +++ b/agents/anyplot/dataset_versions.py @@ -38,7 +38,7 @@ DATASET_HISTORY, DATASET_ID, DATASET_VERSION, - PENDING_REQUEST, + PENDING_CLEARED, CatalogueSnapshot, SessionView, ) @@ -162,5 +162,5 @@ def _delta(dataset_id: str, bindings: list[Binding], history: list[str], version BINDINGS: [binding.model_dump() for binding in bindings], DATASET_HISTORY: history, DATASET_VERSION: version, - PENDING_REQUEST: None, + **PENDING_CLEARED, } diff --git a/agents/anyplot/opening.py b/agents/anyplot/opening.py index c4fb564aae1..70074f10da7 100644 --- a/agents/anyplot/opening.py +++ b/agents/anyplot/opening.py @@ -43,7 +43,7 @@ DATASET_HISTORY, DATASET_ID, DATASET_VERSION, - PENDING_REQUEST, + PENDING_CLEARED, CatalogueSnapshot, initial_state, ) @@ -112,7 +112,7 @@ def state_delta(self) -> dict[str, Any]: BINDINGS: [binding.model_dump() for binding in self.bindings], DATASET_HISTORY: [], DATASET_VERSION: 1, - PENDING_REQUEST: None, + **PENDING_CLEARED, } diff --git a/agents/anyplot/pipeline.py b/agents/anyplot/pipeline.py index ce09d0579bf..a448f0aa527 100644 --- a/agents/anyplot/pipeline.py +++ b/agents/anyplot/pipeline.py @@ -139,6 +139,8 @@ MAX_CHANGE_CHARS, MAX_CHANGE_REQUEST_CHARS, MAX_CHANGES, + MAX_EXPECTED_TEXT_CHARS, + MAX_EXPECTED_TEXTS, MAX_FEEDBACK, MAX_LINE_CHARS, MAX_NOTE_CHARS, @@ -166,7 +168,7 @@ ) from .scoring import ScoreAnswer, ScoreError, mend_answer, parse_answer from .services import PADDED_REASON, CodeVersion, Services, ThemeRender, get_services -from .session_state import PENDING_REQUEST, SessionView, read_session +from .session_state import PENDING_EXPECTED_TEXTS, PENDING_REQUEST, SessionView, read_session from .settings import AgentSettings, get_settings from .sub_agents.adapter import ADAPTERS, adapter_name from .sub_agents.reviewer import REVIEWER_NAME, use_reviewer_mode @@ -568,8 +570,27 @@ def hold_request(state: Any, args: PipelineArgs) -> None: Also the path of a run that passed the check but got no slot in the run queue (`capacity`, or a cancelled wait): its note waits for the next `.create_plot()` like a blocked one's. """ - if args.change_request and args.base != "previous": - state[PENDING_REQUEST] = merged_request(state.get(PENDING_REQUEST), args.change_request) + if (args.change_request or args.expected_texts) and args.base != "previous": + state[PENDING_REQUEST] = merged_request(state.get(PENDING_REQUEST), args.change_request) or None + state[PENDING_EXPECTED_TEXTS] = merged_texts(state.get(PENDING_EXPECTED_TEXTS), args.expected_texts) or None + + +def merged_texts(pending: object, own: Sequence[str]) -> list[str]: + """Kept `expected_texts` and a run's own as one list of at most `MAX_EXPECTED_TEXTS`, without repeats. + + The texts the user asked to see travel with the note they came with (`PENDING_EXPECTED_TEXTS`), + so the next `.create_plot()` checks the render for them as the stopped run would have. The + run's own come first when the cap cuts, as the user's latest word; a kept value that is no + list of texts within `MAX_EXPECTED_TEXT_CHARS` is ignored, item by item. + """ + kept = [ + text + for text in (pending if isinstance(pending, list) else []) + if isinstance(text, str) and 0 < len(text) <= MAX_EXPECTED_TEXT_CHARS + ] + texts = list(dict.fromkeys([*own, *kept]))[:MAX_EXPECTED_TEXTS] + order = {text: index for index, text in enumerate(dict.fromkeys([*kept, *own]))} + return sorted(texts, key=order.__getitem__) MIN_KEPT_NOTE_CHARS = 40 @@ -608,19 +629,28 @@ def take_pending_request(state: Any, args: PipelineArgs) -> PipelineArgs: (`settle_pending_request`), so a run that fails loses no note (review of 2026-10-15). """ pending = state.get(PENDING_REQUEST) - if pending is None or args.base == "previous": + texts = state.get(PENDING_EXPECTED_TEXTS) + if (pending is None and texts is None) or args.base == "previous": return args - return args.model_copy(update={"change_request": merged_request(pending, args.change_request)}) + return args.model_copy( + update={ + "change_request": merged_request(pending, args.change_request), + "expected_texts": merged_texts(texts, args.expected_texts), + } + ) def settle_pending_request(state: Any, args: PipelineArgs, shipped: bool) -> None: - """After a run: a shipped plot used the kept request, so it goes; a run that shipped nothing keeps its request - (the kept one with the run's own, as `take_pending_request` joined them) for the next first plot.""" + """After a run: a shipped plot used the kept request and texts, so they go; a run that shipped nothing keeps its + request and texts (the kept ones with the run's own, as `take_pending_request` joined them) for the next first + plot.""" if shipped: - if state.get(PENDING_REQUEST) is not None: - state[PENDING_REQUEST] = None - elif args.change_request and args.base != "previous": - state[PENDING_REQUEST] = args.change_request + for key in (PENDING_REQUEST, PENDING_EXPECTED_TEXTS): + if state.get(key) is not None: + state[key] = None + elif (args.change_request or args.expected_texts) and args.base != "previous": + state[PENDING_REQUEST] = args.change_request or None + state[PENDING_EXPECTED_TEXTS] = list(args.expected_texts) or None def render_adapt_request(request: AdaptRequest, view: SessionView) -> str: diff --git a/agents/anyplot/session_state.py b/agents/anyplot/session_state.py index 58ecb4797e1..8a366b52c8b 100644 --- a/agents/anyplot/session_state.py +++ b/agents/anyplot/session_state.py @@ -68,6 +68,12 @@ `prepare_data`, `generate_data`, `run_data_code`, `.to_long()`, undo) or library switch drops it: it was written for the data and bindings before (owner decision of 2026-10-15). The pipeline reads it defensively: a value that is no text is ignored, and text is cut to `MAX_CHANGE_REQUEST_CHARS` (`pipeline.merged_request`).""" +PENDING_EXPECTED_TEXTS = PREFIX + "pending_expected_texts" +"""The `expected_texts` that came with `PENDING_REQUEST` (the title or number format the user asked to see), kept +and dropped with it, so the next `.create_plot()` checks the render for the same texts (`pipeline.merged_texts`).""" +PENDING_CLEARED: dict[str, Any] = {PENDING_REQUEST: None, PENDING_EXPECTED_TEXTS: None} +"""The state delta that drops a kept first-plot note and its texts: every binding write, new data version and +library switch carries it.""" LAST_STATUS = PREFIX + "last_status" VERSIONS = PREFIX + "versions" @@ -204,7 +210,7 @@ def initial_state( NORMALISED: normalised, HINTS: list(readiness.get("hints", [])), READINESS: readiness, - PENDING_REQUEST: None, # a note kept for another library's first plot does not carry over + **PENDING_CLEARED, # a note kept for another library's first plot does not carry over } @@ -255,4 +261,4 @@ def apply_bindings( if check.errors: return check, {} # A kept plot note (`PENDING_REQUEST`) was written for the bindings before; any binding write drops it. - return check, {BINDINGS: [binding.model_dump() for binding in bindings], PENDING_REQUEST: None} + return check, {BINDINGS: [binding.model_dump() for binding in bindings], **PENDING_CLEARED} diff --git a/changelog.d/agent-pending-expected-texts.md b/changelog.d/agent-pending-expected-texts.md new file mode 100644 index 00000000000..ea90b605758 --- /dev/null +++ b/changelog.d/agent-pending-expected-texts.md @@ -0,0 +1,8 @@ +### Fixed + +- **A kept plot note keeps its requested texts.** When a first plot stops + before the confirmation, the texts and number formats the user asked to see + (`expected_texts`) are now kept with the note, so the next `.create_plot()` + checks the render for the same title or format. They follow the note's + lifecycle: kept until a plot ships, dropped by any change to the bindings, + the data or the library. diff --git a/docs/concepts/agent-network.md b/docs/concepts/agent-network.md index 956614a5c1a..ea737e0d841 100644 --- a/docs/concepts/agent-network.md +++ b/docs/concepts/agent-network.md @@ -206,7 +206,7 @@ Principles: 3. One truth on the screen: the preview table is always the current version. Every model step shows what changed and needs the user's confirmation. Step 2 stays locked until the data is confirmed, on the server: the pipeline answers `not_ready` with `data_unconfirmed` (`load_inputs` in `agents/anyplot/pipeline.py`), so neither the model nor a stale page can start a plot on data the user has not looked at. 4. The most common real preparation is aggregation (single bookings into sums per month or category), so it belongs to the first set of operations. -Built since 2026-10-11 (the gate): `POST /v1/sessions/{sid}/dataset/confirm` stores a fingerprint of the dataset id and the bindings as the user saw them (`anyplot.confirmed`, `confirmation_fingerprint`); the pipeline runs only while the fingerprint of the current state equals it. A new dataset gets a new id, so a paste or a transcription's `.use()` drops the confirmation by itself, and so does any binding change through the `PUT` route or the root's `set_bindings` tool, also one that lands between the confirmation's read and its write, because the stored fingerprint then names bindings the session no longer holds; a library switch keeps the data, the bindings and the confirmation. Since 2026-10-15 (live test round 2) there is one exception for the root's `set_bindings`: bindings that only leave out members of a confirmed number family (`y2`, `y3`, …), every other binding exactly as confirmed, keep the confirmation (`session_state.narrows` and `narrowed_confirmation`, `AGENT_NARROWED_BINDINGS=keep`; `reset` restores the old rule; owner decision of 2026-10-15: "e.g. drops the 2nd line"), so a step-2 note such as "only co2_per_capita, title …" plots in the same turn, and the tool result says `data_confirmed`. Leaving out a grouping, text or boolean role such as `series` (which merged two countries into one line in the review) or any single role, or renumbering a member so that `y1` plots another column, asks for `.confirm()` again. A first plot's `change_request` that stops on `data_unconfirmed` or `incomplete_bindings`, or gets no queue slot, is kept in `anyplot.pending_request` (`pipeline.keep_pending_request`, `hold_request`): the next first-plot run joins it to its own request (`merged_request`, at most 600 characters, the older note cut with `…` rather than dropped), and it stays until a run ships a plot (`settle_pending_request`), so a failed run loses nothing. Any binding write (the root's or the panel's), any new data version (a paste, `prepare_data`, `generate_data`, `run_data_code`, `.to_long()`, undo) and a library switch drop it, since it was written for the data and bindings before. The session block tells the root whether the data is confirmed, and the root answers `data_unconfirmed` by asking the user to confirm in the data panel, which it cannot do itself. The page shows a step header `1 data · 2 plot`, `.confirm()` after the bindings, and `.create_plot()`, which starts a run only after the confirmation (before it, the button explains the step instead of calling the server); the eval harness and the development fixture confirm on the user's behalf, because their data is the developer's own. +Built since 2026-10-11 (the gate): `POST /v1/sessions/{sid}/dataset/confirm` stores a fingerprint of the dataset id and the bindings as the user saw them (`anyplot.confirmed`, `confirmation_fingerprint`); the pipeline runs only while the fingerprint of the current state equals it. A new dataset gets a new id, so a paste or a transcription's `.use()` drops the confirmation by itself, and so does any binding change through the `PUT` route or the root's `set_bindings` tool, also one that lands between the confirmation's read and its write, because the stored fingerprint then names bindings the session no longer holds; a library switch keeps the data, the bindings and the confirmation. Since 2026-10-15 (live test round 2) there is one exception for the root's `set_bindings`: bindings that only leave out members of a confirmed number family (`y2`, `y3`, …), every other binding exactly as confirmed, keep the confirmation (`session_state.narrows` and `narrowed_confirmation`, `AGENT_NARROWED_BINDINGS=keep`; `reset` restores the old rule; owner decision of 2026-10-15: "e.g. drops the 2nd line"), so a step-2 note such as "only co2_per_capita, title …" plots in the same turn, and the tool result says `data_confirmed`. Leaving out a grouping, text or boolean role such as `series` (which merged two countries into one line in the review) or any single role, or renumbering a member so that `y1` plots another column, asks for `.confirm()` again. A first plot's `change_request` that stops on `data_unconfirmed` or `incomplete_bindings`, or gets no queue slot, is kept in `anyplot.pending_request`, its `expected_texts` with it in `anyplot.pending_expected_texts` (`pipeline.keep_pending_request`, `hold_request`, `merged_texts`: at most five, the run's own first when the cap cuts, so the next `.create_plot()` checks the render for the same texts): the next first-plot run joins it to its own request (`merged_request`, at most 600 characters, the older note cut with `…` rather than dropped), and it stays until a run ships a plot (`settle_pending_request`), so a failed run loses nothing. Any binding write (the root's or the panel's), any new data version (a paste, `prepare_data`, `generate_data`, `run_data_code`, `.to_long()`, undo) and a library switch drop it, since it was written for the data and bindings before. The session block tells the root whether the data is confirmed, and the root answers `data_unconfirmed` by asking the user to confirm in the data panel, which it cannot do itself. The page shows a step header `1 data · 2 plot`, `.confirm()` after the bindings, and `.create_plot()`, which starts a run only after the confirmation (before it, the button explains the step instead of calling the server); the eval harness and the development fixture confirm on the user's behalf, because their data is the developer's own. Built since 2026-10-11 (the operations): the root tool `prepare_data(operations)` with at most five operations per call, each a Pydantic model of the fixed set in `agents/anyplot/data/prepare.py`: `rename` (column, name), `drop_columns` (1 to 10 columns), `drop_rows` (`where` conditions on one column each, all of which must hold: `eq`, `ne`, `contains`, `is_missing`, `gt`, `lt`, at most three; a value is compared, never evaluated), `set_type` (number, datetime or text, with the date format and the decimal separator; to number is for the columns the profile still shows as text, because a column whose every cell carries the same unit is already numbers after the paste, the unit in its name such as `Anteil (%)`: one unit such as `%` or `EUR` is removed and the cells are read as the parser reads a number, the other mark a thousands separator between three-digit groups only, and when more cells read with the other decimal mark the whole column is read that way and the change line says so; a cell that cannot be read becomes missing, more than 10 % of them is a `type_mismatch`; to text the column stays text through the re-parse of every later version, stored with the version as `text_columns`; `MM/DD/YYYY` on a column that is already dates swaps day and month only for a slash column the parser flagged as ambiguous), `aggregate` (by one or two columns, a value column, `sum`, `mean`, `count`, `min` or `max`, and a `month`, `quarter` or `year` period for a datetime key), `sort`, `pivot_longer` (2 to 20 columns into a name/value pair; a long format over the 2 MB cap of computed data, `MAX_COMPUTED_BYTES`, is an `invalid_operation`; 200 KB until 2026-10-14) and `pivot_wider` (the distinct values of a column become at most 50 columns; duplicate pairs are refused, never aggregated silently), and, since 2026-10-12, `classify` (a new text column whose cells are labels: each row gets the label of the first of 1 to 10 rules whose `drop_rows`-style conditions all hold, else `otherwise` or a missing cell; labels are at most 40 characters, never a token the parser reads as a missing cell such as `NA` or `-`, exempt from ToolSafety's link rule like a rename's name, and the column stays text through every re-parse). `classify` came from the owner's case of 2026-10-12: a generated sine table on `bar-feature-importance` had no text column for the `feature` role, and "positive is class 1, negative class 2" had no operation, so the root had to refuse. Since 2026-10-14, `derive` adds a number column from number columns by one formula of a closed set: `ratio` (`a / b`) and `difference` (`a - b`) of two columns, `sum` of 2 to 10 columns in the same row, `share` of one column in the total of its group (`within` 0 to 2 key columns), and three formulas that walk each group in the order of `order`, a number or datetime column: `percent_change` from the previous row, `cumsum` (the running total) and `index` (the group's first row = 100; the last two by owner decision of 2026-10-14). A text order column is a `type_mismatch` with a hint to `set_type` it, because it would sort alphabetically (`April` before `Januar`, `10` before `9`); repeated order values within a group are refused (aggregate first), and a row without an order value keeps an empty cell; `percent` multiplies a ratio or a share by 100, and a division by zero or a missing input leaves the cell empty, which the change line counts. It came from the owner's live test of 2026-10-14: "which year has the largest share of foreigners?" on a long table (year, origin, count), where the root could only ask the user to compute the column; now it derives the share, drops the other rows, sorts, and answers from the new profile. No expression language (an injection surface), no joins, no model-guessed fills: the operations only move, type, aggregate, drop, label or compute from cells that are there, and `ToolSafety` validates the list against the same models before the tool runs. The server applies them with pandas to the stored canonical CSV, re-parses the result with `parse_dataset` (the dataset judge does not run again: the only new text is a rename's name or a `classify` label, which the model writes from a user message the scope judge already checked) and stores it as the next dataset version (`agents/anyplot/dataset_versions.py`): the previous version joins `anyplot.dataset_history`, which keeps the last five (older ones leave the store; the idle sweep takes a session's history only together with its current version), `anyplot.dataset_version` counts up from the paste's 1, the bindings are carried over (`carry_bindings`: the ones still valid stay, the roles left without one get the defaults), and the confirmation drops by itself because its fingerprint names the dataset id. The tool answers the change lines (one English line per operation with its counts, fenced as ``) and the new profile (fenced as ``), or a `PrepareError` code (`unknown_column`, `type_mismatch`, `empty_result`, `too_wide`, `invalid_operation`) with a message that names columns at most. The page learns about a changed dataset from the `done` event: after a turn that changed the dataset id it carries `dataset: {preview, profile, bindings, warnings, roles, changes, version, context_lines}`, the dataset route's shape plus the turn's change lines (`roles` since the formulas below), and `data_confirmed: false`; the BFF relays a `done` of up to 512 KiB whole (every other event stops at 64 KiB), which the largest dataset the parser accepts fits (`agents/stream.py`, `MAX_DONE_EVENT_CHARS`). `POST /v1/sessions/{sid}/dataset/undo` (the user's `.undo()` button; the model has no undo) restores the newest history entry as the current dataset, bindings carried over, unconfirmed again, and answers like the dataset route (`422 nothing_to_undo` for a paste that was never changed). The step-1 summary is the action `{"action": "summarize_data"}` on the messages route (judge skipped, like `create_plot`; since 2026-10-14 the turn also carries the notes pasted with the table, already judged at the paste, after a preamble and fenced as ``, and `get_dataset_profile` returns them under `user_notes` for later turns, with a fixed session-block line saying they exist, so the root uses them as context for the whole chat and acts on a clear request in them at once, the plot itself still waiting for the user's confirmation): the root reads the profile, which now carries `mean` and `median` for numeric columns, and answers in two to four sentences (rows, columns with types and units, missing values, values that look like percentages, the proposed bindings, one question when something is ambiguous), never claiming the data is right: the user confirms. The live eval `agents/evals/prep.py` scores the root's choices on `agents/evals/fixtures/prep/` by exact operation match, order-insensitive, with an 80 % gate (see [Run the prep eval](../../agents/README.md#run-the-prep-eval)). The page asks for the summary right after every parse that stores a dataset (a paste, or a transcription's `.use()`; never after an undo or a library switch), so it is the first turn in the thread, and the composer takes data requests in step 1. A `done` with `dataset` replaces the preview, the profile and the bindings, takes the page back to step 1, and shows a `# changed` block above the preview with the change lines and `.undo()`; after an undo the block names the restored version and keeps `.undo()` while the version is above 1, until the service answers `nothing_to_undo`. diff --git a/tests/unit/agents/runtime/test_data_questions.py b/tests/unit/agents/runtime/test_data_questions.py index fb0693e4907..b3fb24d7fa7 100644 --- a/tests/unit/agents/runtime/test_data_questions.py +++ b/tests/unit/agents/runtime/test_data_questions.py @@ -21,11 +21,26 @@ from agents.anyplot.data.query import MAX_QUERY_COLUMNS, MAX_QUERY_ROWS, run_query from agents.anyplot.data.roles import DataRole from agents.anyplot.dev_fixture import snapshot_from_repo -from agents.anyplot.pipeline import keep_pending_request, merged_request, settle_pending_request, take_pending_request +from agents.anyplot.pipeline import ( + EXPECTED_HEADING, + keep_pending_request, + merged_request, + merged_texts, + settle_pending_request, + take_pending_request, +) from agents.anyplot.plugins.tool_safety import ToolSafetyPlugin, result_limit, result_size from agents.anyplot.schemas import MAX_CHANGE_REQUEST_CHARS, Binding, PipelineArgs from agents.anyplot.services import Services, get_services -from agents.anyplot.session_state import CONFIRMED, DATASET_ID, DATASET_VERSION, PENDING_REQUEST, narrows, read_session +from agents.anyplot.session_state import ( + CONFIRMED, + DATASET_ID, + DATASET_VERSION, + PENDING_EXPECTED_TEXTS, + PENDING_REQUEST, + narrows, + read_session, +) from agents.anyplot.settings import get_settings from agents.anyplot.tools.session import query_data, query_result, set_bindings from agents.main import Runtime, app, get_runtime @@ -421,26 +436,106 @@ async def test_a_binding_change_new_data_or_a_library_switch_drops_the_note( ) -> None: sid = await open_session(client) runtime = app.dependency_overrides[get_runtime]() - await runtime.write_state(await runtime.session(USER, sid), {PENDING_REQUEST: "Title: A"}) + kept = {PENDING_REQUEST: "Title: A", PENDING_EXPECTED_TEXTS: ["A"]} + await runtime.write_state(await runtime.session(USER, sid), kept) method, path, body = change response = await client.request(method.upper(), f"/v1/sessions/{sid}/{path}", headers=HEADERS, json=body) assert response.status_code == 200, response.text - assert (await _session_state(client, sid)).get(PENDING_REQUEST) is None + state = await _session_state(client, sid) + assert state.get(PENDING_REQUEST) is None and state.get(PENDING_EXPECTED_TEXTS) is None def test_every_state_write_of_new_bindings_or_data_drops_the_note(self) -> None: from agents.anyplot.dataset_versions import _delta from agents.anyplot.session_state import apply_bindings, initial_state - assert _delta("d", [], [], 2)[PENDING_REQUEST] is None _, delta = apply_bindings([], roles(), parse_dataset(OWID).profile) - assert delta[PENDING_REQUEST] is None snapshot = snapshot_from_repo("line-multi", "matplotlib") state = initial_state( spec_id="line-multi", library="seaborn", locale="de", snapshot=snapshot, normalised="", readiness={} ) - assert state[PENDING_REQUEST] is None + for written in (_delta("d", [], [], 2), delta, state): + assert written[PENDING_REQUEST] is None and written[PENDING_EXPECTED_TEXTS] is None + + def test_the_texts_travel_with_the_note(self) -> None: + state: dict[str, Any] = {} + args = PipelineArgs(change_request="Title: CO2 pro Kopf", expected_texts=["CO2 pro Kopf"]) + keep_pending_request(state, args, "data_unconfirmed") + keep_pending_request( + state, PipelineArgs(change_request="Format 6,5 t", expected_texts=["6,5 t"]), "data_unconfirmed" + ) + + assert state[PENDING_EXPECTED_TEXTS] == ["CO2 pro Kopf", "6,5 t"] + taken = take_pending_request(state, PipelineArgs()) + assert taken.expected_texts == ["CO2 pro Kopf", "6,5 t"] + assert taken.change_request == "Title: CO2 pro Kopf\nFormat 6,5 t" + settle_pending_request(state, taken, shipped=False) # a failed run keeps both + assert state[PENDING_EXPECTED_TEXTS] == ["CO2 pro Kopf", "6,5 t"] + settle_pending_request(state, taken, shipped=True) + assert state[PENDING_REQUEST] is None and state[PENDING_EXPECTED_TEXTS] is None + + def test_texts_alone_are_kept_and_a_change_of_a_version_takes_none(self) -> None: + state: dict[str, Any] = {} + keep_pending_request(state, PipelineArgs(expected_texts=["12 %"]), "incomplete_bindings") + + assert state[PENDING_EXPECTED_TEXTS] == ["12 %"] and state[PENDING_REQUEST] is None + assert take_pending_request(state, PipelineArgs(base="previous")).expected_texts == [] + + @pytest.mark.parametrize( + ("pending", "own", "expected"), + [ + (["a", "b"], ["b", "c"], ["a", "b", "c"]), + (["k1", "k2", "k3", "k4"], ["n1", "n2"], ["k1", "k2", "k3", "n1", "n2"]), # the run's own survive the cap + (["ok", 7, "", "x" * 121], [], ["ok"]), # a stored value is read item by item + ("not a list", ["n"], ["n"]), + (None, [], []), + ], + ) + def test_merged_texts(self, pending: object, own: list[str], expected: list[str]) -> None: + assert merged_texts(pending, own) == expected + + async def test_a_kept_text_reaches_the_next_create_plot( + self, client: httpx.AsyncClient, swap_models, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("AGENT_EXPECT_CHECK", "off") # the fixture render draws none of the texts + get_settings.cache_clear() + script = { + "root": [ + { + "call": "set_bindings", + "args": { + "bindings": [{"role": "x", "column": "Exam Score"}, {"role": "y", "column": "Study Hours"}] + }, + }, + { + "call": "plot_pipeline", + "args": {"change_request": "Title: Lernzeit", "expected_texts": ["Lernzeit"]}, + }, + {"text": "Please confirm the new bindings."}, + {"call": "plot_pipeline", "args": {}}, + {"text": "Here is your plot."}, + ], + "adapter": [{"json": SCATTER_PLAN}], + "reviewer": [{"json": VERDICT_OK}], + } + fake = swap_models("gemini", script) + sid = await open_session(client) + + await client.post( + f"/v1/sessions/{sid}/messages", headers=HEADERS, json={"text": "swap the axes, title Lernzeit"} + ) + assert (await _session_state(client, sid))[PENDING_EXPECTED_TEXTS] == ["Lernzeit"] + await client.post(f"/v1/sessions/{sid}/dataset/confirm", headers=HEADERS) + events = await create_plot(client, sid) + + assert next(data for name, data in events if name == "plot")["status"] == "ok" + assert isinstance(fake, ScriptedLlm) + adapter = [r for r in fake.requests if (r.config.labels or {})["agent_kind"] == "adapter"] + text = "".join(part.text or "" for part in adapter[0].contents[-1].parts or []) + assert EXPECTED_HEADING in text and "Lernzeit" in text + assert (await _session_state(client, sid))[PENDING_EXPECTED_TEXTS] is None + get_settings.cache_clear() async def test_a_note_survives_a_binding_change_that_asks_for_confirmation( self, client: httpx.AsyncClient, swap_models From 08617518cd3fd30a41c7f460912130193896da2e Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 15 Oct 2026 09:27:24 +0200 Subject: [PATCH 2/3] docs(changelog): add the PR reference to the pending-texts fragment Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M --- changelog.d/agent-pending-expected-texts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/agent-pending-expected-texts.md b/changelog.d/agent-pending-expected-texts.md index ea90b605758..41cde34d7bc 100644 --- a/changelog.d/agent-pending-expected-texts.md +++ b/changelog.d/agent-pending-expected-texts.md @@ -5,4 +5,4 @@ (`expected_texts`) are now kept with the note, so the next `.create_plot()` checks the render for the same title or format. They follow the note's lifecycle: kept until a plot ships, dropped by any change to the bindings, - the data or the library. + the data or the library. (#12186) From fe583d0596c518a22a232f54cefa7c6895a063cd Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Thu, 15 Oct 2026 09:46:43 +0200 Subject: [PATCH 3/3] fix(agents): review fixes on the kept note's expected texts - A run that brings its own request or texts checks only its own; kept texts fill in for a bare .create_plot() only (a withdrawn title is never a miss). The latest blocked turn decides the texts, never the oldest. - A binding write that keeps every binding of the moment the note was kept and only fills open roles keeps the note (pending_bindings), so a note held on incomplete_bindings reaches its plot. - Stored texts go through the PipelineArgs validator on read; ExpectedText refuses whitespace-only and control characters; take validates the joined arguments. - settle only touches the note for a first-plot run, as take does; a change of a version only cleans corrupt stored texts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GdMSqLR5ww4ji74EUSmk9M --- agents/anyplot/pipeline.py | 92 ++++++--- agents/anyplot/schemas.py | 29 ++- agents/anyplot/session_state.py | 35 +++- agents/anyplot/tools/session.py | 11 +- agents/main.py | 3 +- changelog.d/agent-pending-expected-texts.md | 11 +- docs/concepts/agent-network.md | 2 +- .../agents/runtime/test_data_questions.py | 181 +++++++++++++++--- 8 files changed, 295 insertions(+), 69 deletions(-) diff --git a/agents/anyplot/pipeline.py b/agents/anyplot/pipeline.py index a448f0aa527..0448b13ec03 100644 --- a/agents/anyplot/pipeline.py +++ b/agents/anyplot/pipeline.py @@ -136,10 +136,10 @@ from .render.gates import data_rows, evaluate from .render.runtimes.python import PythonRuntime from .schemas import ( + EXPECTED_TEXT, MAX_CHANGE_CHARS, MAX_CHANGE_REQUEST_CHARS, MAX_CHANGES, - MAX_EXPECTED_TEXT_CHARS, MAX_EXPECTED_TEXTS, MAX_FEEDBACK, MAX_LINE_CHARS, @@ -168,7 +168,14 @@ ) from .scoring import ScoreAnswer, ScoreError, mend_answer, parse_answer from .services import PADDED_REASON, CodeVersion, Services, ThemeRender, get_services -from .session_state import PENDING_EXPECTED_TEXTS, PENDING_REQUEST, SessionView, read_session +from .session_state import ( + BINDINGS, + PENDING_BINDINGS, + PENDING_EXPECTED_TEXTS, + PENDING_REQUEST, + SessionView, + read_session, +) from .settings import AgentSettings, get_settings from .sub_agents.adapter import ADAPTERS, adapter_name from .sub_agents.reviewer import REVIEWER_NAME, use_reviewer_mode @@ -572,25 +579,37 @@ def hold_request(state: Any, args: PipelineArgs) -> None: """ if (args.change_request or args.expected_texts) and args.base != "previous": state[PENDING_REQUEST] = merged_request(state.get(PENDING_REQUEST), args.change_request) or None - state[PENDING_EXPECTED_TEXTS] = merged_texts(state.get(PENDING_EXPECTED_TEXTS), args.expected_texts) or None + state[PENDING_EXPECTED_TEXTS] = chosen_texts(state.get(PENDING_EXPECTED_TEXTS), args) or None + # The bindings the note was written for: a later binding change that only fills roles open then keeps it. + bindings = state.get(BINDINGS) + state[PENDING_BINDINGS] = list(bindings) if isinstance(bindings, list) else [] + +def kept_texts(value: object) -> list[str]: + """The stored `expected_texts` that pass the same validator as `PipelineArgs.expected_texts`, item by item, + without repeats and at most `MAX_EXPECTED_TEXTS`; anything else stored there is dropped.""" + texts: list[str] = [] + for item in value if isinstance(value, list) else []: + try: + text = EXPECTED_TEXT.validate_python(item, strict=True) + except ValidationError: + continue + if text not in texts: + texts.append(text) + return texts[:MAX_EXPECTED_TEXTS] -def merged_texts(pending: object, own: Sequence[str]) -> list[str]: - """Kept `expected_texts` and a run's own as one list of at most `MAX_EXPECTED_TEXTS`, without repeats. - The texts the user asked to see travel with the note they came with (`PENDING_EXPECTED_TEXTS`), - so the next `.create_plot()` checks the render for them as the stopped run would have. The - run's own come first when the cap cuts, as the user's latest word; a kept value that is no - list of texts within `MAX_EXPECTED_TEXT_CHARS` is ignored, item by item. +def chosen_texts(pending: object, args: PipelineArgs) -> list[str]: + """The texts a run checks: its own when it brings a request or texts of its own, else the kept ones. + + Review of 2026-10-15: the root never sees the kept texts, so joining them to a run's own kept + a text the user had since withdrawn or replaced ("call it 'Study time'" after "Lernzeit"), + which then counted as a miss and started a repair. A run that brings its own word replaces + them; only a bare run (`.create_plot()`) fills in the kept ones. """ - kept = [ - text - for text in (pending if isinstance(pending, list) else []) - if isinstance(text, str) and 0 < len(text) <= MAX_EXPECTED_TEXT_CHARS - ] - texts = list(dict.fromkeys([*own, *kept]))[:MAX_EXPECTED_TEXTS] - order = {text: index for index, text in enumerate(dict.fromkeys([*kept, *own]))} - return sorted(texts, key=order.__getitem__) + if args.change_request or args.expected_texts: + return list(args.expected_texts) + return kept_texts(pending) MIN_KEPT_NOTE_CHARS = 40 @@ -632,23 +651,40 @@ def take_pending_request(state: Any, args: PipelineArgs) -> PipelineArgs: texts = state.get(PENDING_EXPECTED_TEXTS) if (pending is None and texts is None) or args.base == "previous": return args - return args.model_copy( - update={ - "change_request": merged_request(pending, args.change_request), - "expected_texts": merged_texts(texts, args.expected_texts), - } - ) + # Validated like the root's own arguments: a stored value never reaches the run unchecked. + joined = { + **args.model_dump(), + "change_request": merged_request(pending, args.change_request), + "expected_texts": chosen_texts(texts, args), + } + try: + return PipelineArgs.model_validate(joined) + except ValidationError: + return args + + +PENDING_KEYS = (PENDING_REQUEST, PENDING_EXPECTED_TEXTS, PENDING_BINDINGS) def settle_pending_request(state: Any, args: PipelineArgs, shipped: bool) -> None: - """After a run: a shipped plot used the kept request and texts, so they go; a run that shipped nothing keeps its - request and texts (the kept ones with the run's own, as `take_pending_request` joined them) for the next first - plot.""" + """After a run, the kept note as the run leaves it; only a first-plot run touches it, as only it took it. + + A shipped first plot used the kept request and texts, so they go. A first-plot run that + shipped nothing keeps what it ran with (`take_pending_request` joined the kept and its own) + for the next one. A change of an existing version (`base="previous"`) took nothing, so it + leaves the note as it is (review of 2026-10-15), except that stored texts that no longer + pass the validator are cleaned (`kept_texts`). + """ + if args.base == "previous": + stored = state.get(PENDING_EXPECTED_TEXTS) + if stored is not None and kept_texts(stored) != stored: + state[PENDING_EXPECTED_TEXTS] = kept_texts(stored) or None + return if shipped: - for key in (PENDING_REQUEST, PENDING_EXPECTED_TEXTS): + for key in PENDING_KEYS: if state.get(key) is not None: state[key] = None - elif (args.change_request or args.expected_texts) and args.base != "previous": + elif args.change_request or args.expected_texts: state[PENDING_REQUEST] = args.change_request or None state[PENDING_EXPECTED_TEXTS] = list(args.expected_texts) or None diff --git a/agents/anyplot/schemas.py b/agents/anyplot/schemas.py index 30d48e5409f..177039cc67e 100644 --- a/agents/anyplot/schemas.py +++ b/agents/anyplot/schemas.py @@ -27,10 +27,21 @@ import json import re +import unicodedata from collections.abc import Iterable from typing import Annotated, Any, Literal, Self, get_args -from pydantic import BaseModel, ConfigDict, Field, NonNegativeInt, ValidationInfo, field_validator, model_validator +from pydantic import ( + AfterValidator, + BaseModel, + ConfigDict, + Field, + NonNegativeInt, + TypeAdapter, + ValidationInfo, + field_validator, + model_validator, +) from .language import DEFAULT_LANGUAGE, LANGUAGE_PATTERN @@ -79,7 +90,21 @@ DefectText = Annotated[str, Field(min_length=1, max_length=MAX_DEFECT_TEXT_CHARS)] MAX_EXPECTED_TEXTS = 5 MAX_EXPECTED_TEXT_CHARS = 120 -ExpectedText = Annotated[str, Field(min_length=1, max_length=MAX_EXPECTED_TEXT_CHARS)] + + +def _visible_text(text: str) -> str: + """An expected text holds something to find in the render: not only whitespace, no control character.""" + if not text.strip(): + raise ValueError("an expected text is not only whitespace") + if any(unicodedata.category(char) == "Cc" for char in text): + raise ValueError("an expected text has no control character") + return text + + +ExpectedText = Annotated[str, Field(min_length=1, max_length=MAX_EXPECTED_TEXT_CHARS), AfterValidator(_visible_text)] +EXPECTED_TEXTS: TypeAdapter[list[ExpectedText]] = TypeAdapter(list[ExpectedText]) +"""Validates `expected_texts` read back from session state the way `PipelineArgs` validates them from the root.""" +EXPECTED_TEXT: TypeAdapter[ExpectedText] = TypeAdapter(ExpectedText) MAX_DATASET_CONTEXT_CHARS = 1_600 """The dataset context block (`dataset_context.MAX_CONTEXT_CHARS`, 1,500) with room to spare.""" MAX_SHOWN_TEXT_CHARS = 80 diff --git a/agents/anyplot/session_state.py b/agents/anyplot/session_state.py index 8a366b52c8b..27e1eec5a43 100644 --- a/agents/anyplot/session_state.py +++ b/agents/anyplot/session_state.py @@ -70,8 +70,14 @@ no text is ignored, and text is cut to `MAX_CHANGE_REQUEST_CHARS` (`pipeline.merged_request`).""" PENDING_EXPECTED_TEXTS = PREFIX + "pending_expected_texts" """The `expected_texts` that came with `PENDING_REQUEST` (the title or number format the user asked to see), kept -and dropped with it, so the next `.create_plot()` checks the render for the same texts (`pipeline.merged_texts`).""" -PENDING_CLEARED: dict[str, Any] = {PENDING_REQUEST: None, PENDING_EXPECTED_TEXTS: None} +and dropped with it, so a bare `.create_plot()` checks the render for the same texts; a run that brings a request or +texts of its own checks only its own (`pipeline.chosen_texts`). Read through the `PipelineArgs` validator +(`pipeline.kept_texts`).""" +PENDING_BINDINGS = PREFIX + "pending_bindings" +"""The bindings as they were when the note was kept (`pipeline.hold_request`): a binding write that keeps every one +of them and only fills roles open then (the step `incomplete_bindings` waits for) keeps the note (`apply_bindings`, +review of 2026-10-15).""" +PENDING_CLEARED: dict[str, Any] = {PENDING_REQUEST: None, PENDING_EXPECTED_TEXTS: None, PENDING_BINDINGS: None} """The state delta that drops a kept first-plot note and its texts: every binding write, new data version and library switch carries it.""" LAST_STATUS = PREFIX + "last_status" @@ -249,16 +255,35 @@ def read_session(state: Any) -> SessionView | None: def apply_bindings( - bindings: list[Binding], roles: list[DataRole], profile: DatasetProfile + bindings: list[Binding], roles: list[DataRole], profile: DatasetProfile, pending_bindings: object = None ) -> tuple[BindingCheck, dict[str, Any]]: """`bindings.apply`: validate bindings against the spec roles and the profile. Returns the check and, when it has no errors, the state delta that stores the bindings (incomplete bindings are stored too; the pipeline answers `not_ready`). An empty delta means nothing may be written. + + A kept plot note (`PENDING_REQUEST`, its texts) was written for the bindings of the moment + it was kept (`pending_bindings`, the state's `PENDING_BINDINGS`), so a binding write drops + it, unless it keeps every one of those bindings and only fills roles that were open then + (`fills_open_roles`): that is the step a note held on `incomplete_bindings` waits for. """ check = check_bindings(bindings, roles, profile) if check.errors: return check, {} - # A kept plot note (`PENDING_REQUEST`) was written for the bindings before; any binding write drops it. - return check, {BINDINGS: [binding.model_dump() for binding in bindings], **PENDING_CLEARED} + delta: dict[str, Any] = {BINDINGS: [binding.model_dump() for binding in bindings]} + if not fills_open_roles(pending_bindings, bindings): + delta.update(PENDING_CLEARED) + return check, delta + + +def fills_open_roles(held: object, bindings: list[Binding]) -> bool: + """Whether `bindings` keep every binding of `held` (stored bindings) and only add roles `held` left open.""" + if not isinstance(held, list): + return False + try: + before = {(binding.role, binding.column) for binding in (Binding.model_validate(item) for item in held)} + except ValueError: + return False + after = {(binding.role, binding.column) for binding in bindings} + return before <= after diff --git a/agents/anyplot/tools/session.py b/agents/anyplot/tools/session.py index e6a1beef7a2..5916760c4d3 100644 --- a/agents/anyplot/tools/session.py +++ b/agents/anyplot/tools/session.py @@ -96,7 +96,14 @@ from ..render.serial import RenderBusy from ..schemas import MAX_COLUMNS, MAX_ROWS, Binding, DatasetProfile, PipelineArgs from ..services import get_services -from ..session_state import SessionView, apply_bindings, language_delta, narrowed_confirmation, read_session +from ..session_state import ( + PENDING_BINDINGS, + SessionView, + apply_bindings, + language_delta, + narrowed_confirmation, + read_session, +) from ..settings import get_settings @@ -211,7 +218,7 @@ async def set_bindings(bindings: list[dict[str, str]], tool_context: ToolContext except ValueError: return _error("invalid_bindings") roles = view.snapshot.roles() - check, delta = apply_bindings(parsed, roles, profile) + check, delta = apply_bindings(parsed, roles, profile, tool_context.state.get(PENDING_BINDINGS)) if not delta: # The errors quote column names from the user's dataset. errors = DATA_PREAMBLE + "\n" + fence("user_data", json.dumps(check.errors[:10], ensure_ascii=False)) diff --git a/agents/main.py b/agents/main.py index b5b4ea1f4a0..530014fd3d8 100644 --- a/agents/main.py +++ b/agents/main.py @@ -177,6 +177,7 @@ from agents.anyplot.session_state import ( BINDINGS, CONFIRMED, + PENDING_BINDINGS, CatalogueSnapshot, SessionView, apply_bindings, @@ -1051,7 +1052,7 @@ async def put_bindings( parsed = [Binding(role=item.role, column=item.column) for item in bindings if item.column] except ValueError: raise AgentsError(422, "invalid") from None - check, delta = apply_bindings(parsed, view.snapshot.roles(), stored.profile) + check, delta = apply_bindings(parsed, view.snapshot.roles(), stored.profile, session.state.get(PENDING_BINDINGS)) if not delta: return JSONResponse( status_code=422, content={"detail": "invalid", "errors": check.errors[:20]}, headers=_NO_STORE diff --git a/changelog.d/agent-pending-expected-texts.md b/changelog.d/agent-pending-expected-texts.md index 41cde34d7bc..c6af8f348f8 100644 --- a/changelog.d/agent-pending-expected-texts.md +++ b/changelog.d/agent-pending-expected-texts.md @@ -2,7 +2,10 @@ - **A kept plot note keeps its requested texts.** When a first plot stops before the confirmation, the texts and number formats the user asked to see - (`expected_texts`) are now kept with the note, so the next `.create_plot()` - checks the render for the same title or format. They follow the note's - lifecycle: kept until a plot ships, dropped by any change to the bindings, - the data or the library. (#12186) + (`expected_texts`) are now kept with the note, so the next bare + `.create_plot()` checks the render for the same title or format; a run that + brings its own request or texts checks only its own, so a withdrawn title is + never reported missing. The note follows one lifecycle: kept until a plot + ships, kept through a binding change that only fills roles that were still + open, dropped by any other change to the bindings, the data or the library. + (#12186) diff --git a/docs/concepts/agent-network.md b/docs/concepts/agent-network.md index ea737e0d841..f7b6ccb4f2b 100644 --- a/docs/concepts/agent-network.md +++ b/docs/concepts/agent-network.md @@ -206,7 +206,7 @@ Principles: 3. One truth on the screen: the preview table is always the current version. Every model step shows what changed and needs the user's confirmation. Step 2 stays locked until the data is confirmed, on the server: the pipeline answers `not_ready` with `data_unconfirmed` (`load_inputs` in `agents/anyplot/pipeline.py`), so neither the model nor a stale page can start a plot on data the user has not looked at. 4. The most common real preparation is aggregation (single bookings into sums per month or category), so it belongs to the first set of operations. -Built since 2026-10-11 (the gate): `POST /v1/sessions/{sid}/dataset/confirm` stores a fingerprint of the dataset id and the bindings as the user saw them (`anyplot.confirmed`, `confirmation_fingerprint`); the pipeline runs only while the fingerprint of the current state equals it. A new dataset gets a new id, so a paste or a transcription's `.use()` drops the confirmation by itself, and so does any binding change through the `PUT` route or the root's `set_bindings` tool, also one that lands between the confirmation's read and its write, because the stored fingerprint then names bindings the session no longer holds; a library switch keeps the data, the bindings and the confirmation. Since 2026-10-15 (live test round 2) there is one exception for the root's `set_bindings`: bindings that only leave out members of a confirmed number family (`y2`, `y3`, …), every other binding exactly as confirmed, keep the confirmation (`session_state.narrows` and `narrowed_confirmation`, `AGENT_NARROWED_BINDINGS=keep`; `reset` restores the old rule; owner decision of 2026-10-15: "e.g. drops the 2nd line"), so a step-2 note such as "only co2_per_capita, title …" plots in the same turn, and the tool result says `data_confirmed`. Leaving out a grouping, text or boolean role such as `series` (which merged two countries into one line in the review) or any single role, or renumbering a member so that `y1` plots another column, asks for `.confirm()` again. A first plot's `change_request` that stops on `data_unconfirmed` or `incomplete_bindings`, or gets no queue slot, is kept in `anyplot.pending_request`, its `expected_texts` with it in `anyplot.pending_expected_texts` (`pipeline.keep_pending_request`, `hold_request`, `merged_texts`: at most five, the run's own first when the cap cuts, so the next `.create_plot()` checks the render for the same texts): the next first-plot run joins it to its own request (`merged_request`, at most 600 characters, the older note cut with `…` rather than dropped), and it stays until a run ships a plot (`settle_pending_request`), so a failed run loses nothing. Any binding write (the root's or the panel's), any new data version (a paste, `prepare_data`, `generate_data`, `run_data_code`, `.to_long()`, undo) and a library switch drop it, since it was written for the data and bindings before. The session block tells the root whether the data is confirmed, and the root answers `data_unconfirmed` by asking the user to confirm in the data panel, which it cannot do itself. The page shows a step header `1 data · 2 plot`, `.confirm()` after the bindings, and `.create_plot()`, which starts a run only after the confirmation (before it, the button explains the step instead of calling the server); the eval harness and the development fixture confirm on the user's behalf, because their data is the developer's own. +Built since 2026-10-11 (the gate): `POST /v1/sessions/{sid}/dataset/confirm` stores a fingerprint of the dataset id and the bindings as the user saw them (`anyplot.confirmed`, `confirmation_fingerprint`); the pipeline runs only while the fingerprint of the current state equals it. A new dataset gets a new id, so a paste or a transcription's `.use()` drops the confirmation by itself, and so does any binding change through the `PUT` route or the root's `set_bindings` tool, also one that lands between the confirmation's read and its write, because the stored fingerprint then names bindings the session no longer holds; a library switch keeps the data, the bindings and the confirmation. Since 2026-10-15 (live test round 2) there is one exception for the root's `set_bindings`: bindings that only leave out members of a confirmed number family (`y2`, `y3`, …), every other binding exactly as confirmed, keep the confirmation (`session_state.narrows` and `narrowed_confirmation`, `AGENT_NARROWED_BINDINGS=keep`; `reset` restores the old rule; owner decision of 2026-10-15: "e.g. drops the 2nd line"), so a step-2 note such as "only co2_per_capita, title …" plots in the same turn, and the tool result says `data_confirmed`. Leaving out a grouping, text or boolean role such as `series` (which merged two countries into one line in the review) or any single role, or renumbering a member so that `y1` plots another column, asks for `.confirm()` again. A first plot's `change_request` that stops on `data_unconfirmed` or `incomplete_bindings`, or gets no queue slot, is kept in `anyplot.pending_request`, its `expected_texts` with it in `anyplot.pending_expected_texts`, and the bindings of that moment in `anyplot.pending_bindings` (`pipeline.keep_pending_request`, `hold_request`). A bare `.create_plot()` checks the render for the kept texts; a run that brings a request or texts of its own checks only its own, since the root never sees the kept ones and a withdrawn title must not count as a miss (`chosen_texts`); stored texts are read through the `PipelineArgs` validator (`kept_texts`). A binding write that keeps every binding of that moment and only fills roles open then (the step `incomplete_bindings` waits for) keeps the note (`session_state.fills_open_roles`); a change of an existing version neither takes nor clears it: the next first-plot run joins it to its own request (`merged_request`, at most 600 characters, the older note cut with `…` rather than dropped), and it stays until a run ships a plot (`settle_pending_request`), so a failed run loses nothing. Any binding write (the root's or the panel's), any new data version (a paste, `prepare_data`, `generate_data`, `run_data_code`, `.to_long()`, undo) and a library switch drop it, since it was written for the data and bindings before. The session block tells the root whether the data is confirmed, and the root answers `data_unconfirmed` by asking the user to confirm in the data panel, which it cannot do itself. The page shows a step header `1 data · 2 plot`, `.confirm()` after the bindings, and `.create_plot()`, which starts a run only after the confirmation (before it, the button explains the step instead of calling the server); the eval harness and the development fixture confirm on the user's behalf, because their data is the developer's own. Built since 2026-10-11 (the operations): the root tool `prepare_data(operations)` with at most five operations per call, each a Pydantic model of the fixed set in `agents/anyplot/data/prepare.py`: `rename` (column, name), `drop_columns` (1 to 10 columns), `drop_rows` (`where` conditions on one column each, all of which must hold: `eq`, `ne`, `contains`, `is_missing`, `gt`, `lt`, at most three; a value is compared, never evaluated), `set_type` (number, datetime or text, with the date format and the decimal separator; to number is for the columns the profile still shows as text, because a column whose every cell carries the same unit is already numbers after the paste, the unit in its name such as `Anteil (%)`: one unit such as `%` or `EUR` is removed and the cells are read as the parser reads a number, the other mark a thousands separator between three-digit groups only, and when more cells read with the other decimal mark the whole column is read that way and the change line says so; a cell that cannot be read becomes missing, more than 10 % of them is a `type_mismatch`; to text the column stays text through the re-parse of every later version, stored with the version as `text_columns`; `MM/DD/YYYY` on a column that is already dates swaps day and month only for a slash column the parser flagged as ambiguous), `aggregate` (by one or two columns, a value column, `sum`, `mean`, `count`, `min` or `max`, and a `month`, `quarter` or `year` period for a datetime key), `sort`, `pivot_longer` (2 to 20 columns into a name/value pair; a long format over the 2 MB cap of computed data, `MAX_COMPUTED_BYTES`, is an `invalid_operation`; 200 KB until 2026-10-14) and `pivot_wider` (the distinct values of a column become at most 50 columns; duplicate pairs are refused, never aggregated silently), and, since 2026-10-12, `classify` (a new text column whose cells are labels: each row gets the label of the first of 1 to 10 rules whose `drop_rows`-style conditions all hold, else `otherwise` or a missing cell; labels are at most 40 characters, never a token the parser reads as a missing cell such as `NA` or `-`, exempt from ToolSafety's link rule like a rename's name, and the column stays text through every re-parse). `classify` came from the owner's case of 2026-10-12: a generated sine table on `bar-feature-importance` had no text column for the `feature` role, and "positive is class 1, negative class 2" had no operation, so the root had to refuse. Since 2026-10-14, `derive` adds a number column from number columns by one formula of a closed set: `ratio` (`a / b`) and `difference` (`a - b`) of two columns, `sum` of 2 to 10 columns in the same row, `share` of one column in the total of its group (`within` 0 to 2 key columns), and three formulas that walk each group in the order of `order`, a number or datetime column: `percent_change` from the previous row, `cumsum` (the running total) and `index` (the group's first row = 100; the last two by owner decision of 2026-10-14). A text order column is a `type_mismatch` with a hint to `set_type` it, because it would sort alphabetically (`April` before `Januar`, `10` before `9`); repeated order values within a group are refused (aggregate first), and a row without an order value keeps an empty cell; `percent` multiplies a ratio or a share by 100, and a division by zero or a missing input leaves the cell empty, which the change line counts. It came from the owner's live test of 2026-10-14: "which year has the largest share of foreigners?" on a long table (year, origin, count), where the root could only ask the user to compute the column; now it derives the share, drops the other rows, sorts, and answers from the new profile. No expression language (an injection surface), no joins, no model-guessed fills: the operations only move, type, aggregate, drop, label or compute from cells that are there, and `ToolSafety` validates the list against the same models before the tool runs. The server applies them with pandas to the stored canonical CSV, re-parses the result with `parse_dataset` (the dataset judge does not run again: the only new text is a rename's name or a `classify` label, which the model writes from a user message the scope judge already checked) and stores it as the next dataset version (`agents/anyplot/dataset_versions.py`): the previous version joins `anyplot.dataset_history`, which keeps the last five (older ones leave the store; the idle sweep takes a session's history only together with its current version), `anyplot.dataset_version` counts up from the paste's 1, the bindings are carried over (`carry_bindings`: the ones still valid stay, the roles left without one get the defaults), and the confirmation drops by itself because its fingerprint names the dataset id. The tool answers the change lines (one English line per operation with its counts, fenced as ``) and the new profile (fenced as ``), or a `PrepareError` code (`unknown_column`, `type_mismatch`, `empty_result`, `too_wide`, `invalid_operation`) with a message that names columns at most. The page learns about a changed dataset from the `done` event: after a turn that changed the dataset id it carries `dataset: {preview, profile, bindings, warnings, roles, changes, version, context_lines}`, the dataset route's shape plus the turn's change lines (`roles` since the formulas below), and `data_confirmed: false`; the BFF relays a `done` of up to 512 KiB whole (every other event stops at 64 KiB), which the largest dataset the parser accepts fits (`agents/stream.py`, `MAX_DONE_EVENT_CHARS`). `POST /v1/sessions/{sid}/dataset/undo` (the user's `.undo()` button; the model has no undo) restores the newest history entry as the current dataset, bindings carried over, unconfirmed again, and answers like the dataset route (`422 nothing_to_undo` for a paste that was never changed). The step-1 summary is the action `{"action": "summarize_data"}` on the messages route (judge skipped, like `create_plot`; since 2026-10-14 the turn also carries the notes pasted with the table, already judged at the paste, after a preamble and fenced as ``, and `get_dataset_profile` returns them under `user_notes` for later turns, with a fixed session-block line saying they exist, so the root uses them as context for the whole chat and acts on a clear request in them at once, the plot itself still waiting for the user's confirmation): the root reads the profile, which now carries `mean` and `median` for numeric columns, and answers in two to four sentences (rows, columns with types and units, missing values, values that look like percentages, the proposed bindings, one question when something is ambiguous), never claiming the data is right: the user confirms. The live eval `agents/evals/prep.py` scores the root's choices on `agents/evals/fixtures/prep/` by exact operation match, order-insensitive, with an 80 % gate (see [Run the prep eval](../../agents/README.md#run-the-prep-eval)). The page asks for the summary right after every parse that stores a dataset (a paste, or a transcription's `.use()`; never after an undo or a library switch), so it is the first turn in the thread, and the composer takes data requests in step 1. A `done` with `dataset` replaces the preview, the profile and the bindings, takes the page back to step 1, and shows a `# changed` block above the preview with the change lines and `.undo()`; after an undo the block names the restored version and keeps `.undo()` while the version is above 1, until the service answers `nothing_to_undo`. diff --git a/tests/unit/agents/runtime/test_data_questions.py b/tests/unit/agents/runtime/test_data_questions.py index b3fb24d7fa7..c40da1cf1f2 100644 --- a/tests/unit/agents/runtime/test_data_questions.py +++ b/tests/unit/agents/runtime/test_data_questions.py @@ -24,18 +24,21 @@ from agents.anyplot.pipeline import ( EXPECTED_HEADING, keep_pending_request, + kept_texts, merged_request, - merged_texts, settle_pending_request, take_pending_request, ) from agents.anyplot.plugins.tool_safety import ToolSafetyPlugin, result_limit, result_size +from agents.anyplot.render.backends.fake import FakeBackend, FakeOutcome from agents.anyplot.schemas import MAX_CHANGE_REQUEST_CHARS, Binding, PipelineArgs from agents.anyplot.services import Services, get_services from agents.anyplot.session_state import ( + BINDINGS, CONFIRMED, DATASET_ID, DATASET_VERSION, + PENDING_BINDINGS, PENDING_EXPECTED_TEXTS, PENDING_REQUEST, narrows, @@ -415,6 +418,7 @@ def test_a_long_kept_note_is_cut_not_dropped(self) -> None: (True, PipelineArgs(change_request="Title: A\nlog y"), None), (False, PipelineArgs(change_request="Title: A\nlog y"), "Title: A\nlog y"), # a failed run loses nothing (False, PipelineArgs(change_request="bigger", base="previous"), "Title: A"), + (True, PipelineArgs(change_request="bigger", base="previous"), "Title: A"), # took nothing, clears nothing (False, PipelineArgs(), "Title: A"), ], ) @@ -423,6 +427,11 @@ def test_settle(self, shipped: bool, args: PipelineArgs, kept: str | None) -> No settle_pending_request(state, args, shipped) assert state[PENDING_REQUEST] == kept + def test_settle_after_a_change_cleans_corrupt_stored_texts(self) -> None: + state: dict[str, Any] = {PENDING_REQUEST: "Title: A", PENDING_EXPECTED_TEXTS: ["A", " ", "a\x07b", 7]} + settle_pending_request(state, PipelineArgs(change_request="bigger", base="previous"), shipped=True) + assert state == {PENDING_REQUEST: "Title: A", PENDING_EXPECTED_TEXTS: ["A"]} + @pytest.mark.parametrize( "change", [ @@ -459,47 +468,162 @@ def test_every_state_write_of_new_bindings_or_data_drops_the_note(self) -> None: assert written[PENDING_REQUEST] is None and written[PENDING_EXPECTED_TEXTS] is None def test_the_texts_travel_with_the_note(self) -> None: - state: dict[str, Any] = {} + state: dict[str, Any] = {BINDINGS: []} args = PipelineArgs(change_request="Title: CO2 pro Kopf", expected_texts=["CO2 pro Kopf"]) keep_pending_request(state, args, "data_unconfirmed") - keep_pending_request( - state, PipelineArgs(change_request="Format 6,5 t", expected_texts=["6,5 t"]), "data_unconfirmed" - ) - assert state[PENDING_EXPECTED_TEXTS] == ["CO2 pro Kopf", "6,5 t"] - taken = take_pending_request(state, PipelineArgs()) - assert taken.expected_texts == ["CO2 pro Kopf", "6,5 t"] - assert taken.change_request == "Title: CO2 pro Kopf\nFormat 6,5 t" + assert state[PENDING_EXPECTED_TEXTS] == ["CO2 pro Kopf"] + taken = take_pending_request(state, PipelineArgs()) # a bare `.create_plot()` fills in the kept texts + assert (taken.change_request, taken.expected_texts) == ("Title: CO2 pro Kopf", ["CO2 pro Kopf"]) settle_pending_request(state, taken, shipped=False) # a failed run keeps both - assert state[PENDING_EXPECTED_TEXTS] == ["CO2 pro Kopf", "6,5 t"] + assert state[PENDING_EXPECTED_TEXTS] == ["CO2 pro Kopf"] settle_pending_request(state, taken, shipped=True) assert state[PENDING_REQUEST] is None and state[PENDING_EXPECTED_TEXTS] is None - def test_texts_alone_are_kept_and_a_change_of_a_version_takes_none(self) -> None: - state: dict[str, Any] = {} + @pytest.mark.parametrize( + ("own", "expected"), + [ + (PipelineArgs(change_request="call it Study time", expected_texts=["Study time"]), ["Study time"]), + (PipelineArgs(change_request="log y"), []), # the user's latest word brings no text: none is checked + (PipelineArgs(expected_texts=["12 %"]), ["12 %"]), + (PipelineArgs(), ["Lernzeit"]), + ], + ) + def test_a_run_with_its_own_word_replaces_the_kept_texts(self, own: PipelineArgs, expected: list[str]) -> None: + # Review of 2026-10-15: the root never sees the kept texts, so a withdrawn one must not be checked. + state: dict[str, Any] = {PENDING_REQUEST: "Title: Lernzeit", PENDING_EXPECTED_TEXTS: ["Lernzeit"]} + assert take_pending_request(state, own).expected_texts == expected + + def test_the_latest_blocked_turn_decides_the_texts(self) -> None: + state: dict[str, Any] = {BINDINGS: []} + for turn in range(1, 4): + texts = [f"t{turn}.{index}" for index in range(4)] + keep_pending_request( + state, PipelineArgs(change_request=f"note {turn}", expected_texts=texts), "data_unconfirmed" + ) + + assert state[PENDING_EXPECTED_TEXTS] == ["t3.0", "t3.1", "t3.2", "t3.3"] # never the oldest + assert state[PENDING_REQUEST] == "note 1\nnote 2\nnote 3" + + def test_texts_alone_are_kept_and_reach_a_bare_first_plot(self) -> None: + state: dict[str, Any] = {BINDINGS: []} keep_pending_request(state, PipelineArgs(expected_texts=["12 %"]), "incomplete_bindings") assert state[PENDING_EXPECTED_TEXTS] == ["12 %"] and state[PENDING_REQUEST] is None + taken = take_pending_request(state, PipelineArgs()) + assert (taken.change_request, taken.expected_texts) == ("", ["12 %"]) assert take_pending_request(state, PipelineArgs(base="previous")).expected_texts == [] @pytest.mark.parametrize( - ("pending", "own", "expected"), + ("stored", "expected"), [ - (["a", "b"], ["b", "c"], ["a", "b", "c"]), - (["k1", "k2", "k3", "k4"], ["n1", "n2"], ["k1", "k2", "k3", "n1", "n2"]), # the run's own survive the cap - (["ok", 7, "", "x" * 121], [], ["ok"]), # a stored value is read item by item - ("not a list", ["n"], ["n"]), - (None, [], []), + (["a", "b", "a"], ["a", "b"]), + (["ok", 7, "", " ", "x" * 121, "a\x07b", None], ["ok"]), # read through the PipelineArgs validator + (["1", "2", "3", "4", "5", "6"], ["1", "2", "3", "4", "5"]), + ("not a list", []), + (None, []), ], ) - def test_merged_texts(self, pending: object, own: list[str], expected: list[str]) -> None: - assert merged_texts(pending, own) == expected + def test_kept_texts(self, stored: object, expected: list[str]) -> None: + assert kept_texts(stored) == expected + + def test_a_whitespace_or_control_text_is_refused_from_the_root_too(self) -> None: + for bad in (" ", "a\x07b"): + with pytest.raises(ValueError): + PipelineArgs(expected_texts=[bad]) + + def test_filling_the_roles_open_when_the_note_was_kept_keeps_it(self) -> None: + from agents.anyplot.session_state import apply_bindings + + held = [{"role": "x", "column": "year"}] + profile = parse_dataset(OWID).profile + filled = bind(("x", "year"), ("y1", "co2_per_capita")) + changed = bind(("x", "co2_per_capita"), ("y1", "year")) + + assert PENDING_REQUEST not in apply_bindings(filled, roles(), profile, held)[1] + assert apply_bindings(changed, roles(), profile, held)[1][PENDING_REQUEST] is None + assert apply_bindings(filled, roles(), profile, None)[1][PENDING_REQUEST] is None + + async def test_a_note_held_on_incomplete_bindings_survives_the_binding_that_completes_them( + self, client: httpx.AsyncClient + ) -> None: + sid, _ = await open_line_multi(client) + runtime = app.dependency_overrides[get_runtime]() + partial = [{"role": "x", "column": "year"}] + await client.put(f"/v1/sessions/{sid}/bindings", headers=HEADERS, json=partial) + state: dict[str, Any] = dict(await _session_state(client, sid)) + keep_pending_request( + state, PipelineArgs(change_request="Title: A", expected_texts=["A"]), "incomplete_bindings" + ) + kept = {key: state[key] for key in (PENDING_REQUEST, PENDING_EXPECTED_TEXTS, PENDING_BINDINGS)} + await runtime.write_state(await runtime.session(USER, sid), kept) + + filled = [*partial, {"role": "y1", "column": "co2_per_capita"}] + response = await client.put(f"/v1/sessions/{sid}/bindings", headers=HEADERS, json=filled) + + assert response.status_code == 200, response.text + after = await _session_state(client, sid) + assert (after[PENDING_REQUEST], after[PENDING_EXPECTED_TEXTS]) == ("Title: A", ["A"]) + changed = [{"role": "x", "column": "co2_per_capita"}, {"role": "y1", "column": "year"}] + await client.put(f"/v1/sessions/{sid}/bindings", headers=HEADERS, json=changed) + assert (await _session_state(client, sid))[PENDING_REQUEST] is None + + async def test_a_replaced_text_is_not_checked( + self, client: httpx.AsyncClient, swap_models, monkeypatch: pytest.MonkeyPatch, backend: FakeBackend + ) -> None: + """`AGENT_EXPECT_CHECK=repair`: the render draws the new title only, so a check of the kept one would miss.""" + monkeypatch.setenv("AGENT_EXPECT_CHECK", "repair") + get_settings.cache_clear() + probe = {"texts": [], "labels": [{"kind": "title", "text": "Study time"}]} + backend.script = lambda job, theme: FakeOutcome(probe=probe) + swap_axes = { + "call": "set_bindings", + "args": {"bindings": [{"role": "x", "column": "Exam Score"}, {"role": "y", "column": "Study Hours"}]}, + } + script = { + "root": [ + swap_axes, + { + "call": "plot_pipeline", + "args": {"change_request": "Title: Lernzeit", "expected_texts": ["Lernzeit"]}, + }, + {"text": "Please confirm the new bindings."}, + # After the confirmation the user replaces the title in words. + { + "call": "plot_pipeline", + "args": {"change_request": "Title: Study time", "expected_texts": ["Study time"]}, + }, + {"text": "Here is your plot."}, + ], + "adapter": [{"json": SCATTER_PLAN}] * 3, + "reviewer": [{"json": VERDICT_OK}] * 3, + } + swap_models("gemini", script) + sid = await open_session(client) + await client.post(f"/v1/sessions/{sid}/messages", headers=HEADERS, json={"text": "swap axes, title Lernzeit"}) + await client.post(f"/v1/sessions/{sid}/dataset/confirm", headers=HEADERS) + + response = await client.post( + f"/v1/sessions/{sid}/messages", headers=HEADERS, json={"text": "call it 'Study time'"} + ) + + plot = next(data for name, data in parse_sse(response.text) if name == "plot") + assert plot["status"] in ("ok", "needs_attention"), plot + shows = get_services().versions.all(sid)[-1].result.shows # the plot event leaves `shows` to the root + assert shows is not None and shows.texts is not None # the texts were measured + assert shows.requested_not_found == [] # the withdrawn "Lernzeit" was never checked + assert plot["attempts"] == 1 # so no repair round ran for it + assert (await _session_state(client, sid))[PENDING_EXPECTED_TEXTS] is None + get_settings.cache_clear() async def test_a_kept_text_reaches_the_next_create_plot( - self, client: httpx.AsyncClient, swap_models, monkeypatch: pytest.MonkeyPatch + self, client: httpx.AsyncClient, swap_models, monkeypatch: pytest.MonkeyPatch, backend: FakeBackend ) -> None: - monkeypatch.setenv("AGENT_EXPECT_CHECK", "off") # the fixture render draws none of the texts + """`AGENT_EXPECT_CHECK=repair`: the render never draws the kept title, so the kept text shows as checked.""" + monkeypatch.setenv("AGENT_EXPECT_CHECK", "repair") get_settings.cache_clear() + probe = {"texts": [], "labels": [{"kind": "title", "text": "Exam Score by Study Hours"}]} + backend.script = lambda job, theme: FakeOutcome(probe=probe) script = { "root": [ { @@ -516,8 +640,8 @@ async def test_a_kept_text_reaches_the_next_create_plot( {"call": "plot_pipeline", "args": {}}, {"text": "Here is your plot."}, ], - "adapter": [{"json": SCATTER_PLAN}], - "reviewer": [{"json": VERDICT_OK}], + "adapter": [{"json": SCATTER_PLAN}] * 3, + "reviewer": [{"json": VERDICT_OK}] * 3, } fake = swap_models("gemini", script) sid = await open_session(client) @@ -525,15 +649,20 @@ async def test_a_kept_text_reaches_the_next_create_plot( await client.post( f"/v1/sessions/{sid}/messages", headers=HEADERS, json={"text": "swap the axes, title Lernzeit"} ) - assert (await _session_state(client, sid))[PENDING_EXPECTED_TEXTS] == ["Lernzeit"] + kept = await _session_state(client, sid) + assert (kept[PENDING_REQUEST], kept[PENDING_EXPECTED_TEXTS]) == ("Title: Lernzeit", ["Lernzeit"]) await client.post(f"/v1/sessions/{sid}/dataset/confirm", headers=HEADERS) + # The confirmation is no binding change: the note and its texts survive it. + assert (await _session_state(client, sid))[PENDING_EXPECTED_TEXTS] == ["Lernzeit"] events = await create_plot(client, sid) - assert next(data for name, data in events if name == "plot")["status"] == "ok" + assert next(data for name, data in events if name == "plot")["status"] in ("ok", "needs_attention") assert isinstance(fake, ScriptedLlm) adapter = [r for r in fake.requests if (r.config.labels or {})["agent_kind"] == "adapter"] text = "".join(part.text or "" for part in adapter[0].contents[-1].parts or []) assert EXPECTED_HEADING in text and "Lernzeit" in text + shows = get_services().versions.all(sid)[-1].result.shows + assert shows is not None and shows.requested_not_found == ["Lernzeit"] # the kept text was checked assert (await _session_state(client, sid))[PENDING_EXPECTED_TEXTS] is None get_settings.cache_clear()