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..0448b13ec03 100644 --- a/agents/anyplot/pipeline.py +++ b/agents/anyplot/pipeline.py @@ -136,9 +136,11 @@ 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_TEXTS, MAX_FEEDBACK, MAX_LINE_CHARS, MAX_NOTE_CHARS, @@ -166,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_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 @@ -568,8 +577,39 @@ 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] = 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 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. + """ + if args.change_request or args.expected_texts: + return list(args.expected_texts) + return kept_texts(pending) MIN_KEPT_NOTE_CHARS = 40 @@ -608,19 +648,45 @@ 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)}) + # 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, 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, 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: - 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_KEYS: + if state.get(key) is not None: + state[key] = None + 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 def render_adapt_request(request: AdaptRequest, view: SessionView) -> str: 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 58ecb4797e1..27e1eec5a43 100644 --- a/agents/anyplot/session_state.py +++ b/agents/anyplot/session_state.py @@ -68,6 +68,18 @@ `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 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" VERSIONS = PREFIX + "versions" @@ -204,7 +216,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 } @@ -243,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_REQUEST: None} + 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 new file mode 100644 index 00000000000..c6af8f348f8 --- /dev/null +++ b/changelog.d/agent-pending-expected-texts.md @@ -0,0 +1,11 @@ +### 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 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 956614a5c1a..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` (`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`, 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 fb0693e4907..c40da1cf1f2 100644 --- a/tests/unit/agents/runtime/test_data_questions.py +++ b/tests/unit/agents/runtime/test_data_questions.py @@ -21,11 +21,29 @@ 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, + kept_texts, + merged_request, + 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 CONFIRMED, DATASET_ID, DATASET_VERSION, PENDING_REQUEST, narrows, read_session +from agents.anyplot.session_state import ( + BINDINGS, + CONFIRMED, + DATASET_ID, + DATASET_VERSION, + PENDING_BINDINGS, + 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 @@ -400,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"), ], ) @@ -408,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", [ @@ -421,26 +445,226 @@ 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] = {BINDINGS: []} + args = PipelineArgs(change_request="Title: CO2 pro Kopf", expected_texts=["CO2 pro Kopf"]) + keep_pending_request(state, args, "data_unconfirmed") + + 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"] + settle_pending_request(state, taken, shipped=True) + assert state[PENDING_REQUEST] is None and state[PENDING_EXPECTED_TEXTS] is None + + @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( + ("stored", "expected"), + [ + (["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_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, backend: FakeBackend + ) -> None: + """`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": [ + { + "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}] * 3, + "reviewer": [{"json": VERDICT_OK}] * 3, + } + 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"} + ) + 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"] 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() async def test_a_note_survives_a_binding_change_that_asks_for_confirmation( self, client: httpx.AsyncClient, swap_models