diff --git a/agents/anyplot/grounding.py b/agents/anyplot/grounding.py index b88371fdda3..38a88dc8b6b 100644 --- a/agents/anyplot/grounding.py +++ b/agents/anyplot/grounding.py @@ -22,6 +22,13 @@ label or ticks draw it) must use its decimal mark, dates and three-digit groups aside. Each miss becomes a residual line, so the result says it honestly, and with `AGENT_EXPECT_CHECK=repair` a measured finding the repair round gets. + +A render without drawn texts (R, altair, pygal and lets-plot, which write no probe) is checked by the +reviewer instead where its runtime says so (`RuntimeAdapter.review_texts`, owner decision of +2026-10-29): the review request carries the requested texts, the reviewer reports each one it +cannot find in the image as a defect line under `EXPECT_LINE_ID` that quotes it, and +`reviewed_misses` reads those lines back. Only a render no readable review answered for keeps +`UNVERIFIED_LINE`. """ import re @@ -30,6 +37,8 @@ from dataclasses import dataclass from typing import Any +from core.defects import DEFECT_RE + from .schemas import MAX_SHOWN_TEXT_CHARS, ShownGroup @@ -56,9 +65,12 @@ "SC-04 ({theme}): the requested texts could not be checked, since the renderer reported no drawn texts " "→ compare them with the image. Likely cause: a renderer without the probe's text report." ) -"""The residual line of a render with requested texts but no `labels` in its probe: never a silent pass.""" +"""The residual line of a render with requested texts but no `labels` in its probe, unless a readable review +answered for them (`reviewed_misses`): never a silent pass.""" EXPECT_LINE_ID = {"text": "SC-04", "format": "SC-02"} """The rubric id a missing requested text (title, legend, label) or number format is reported under.""" +_QUOTED = re.compile(r'["“”„«»]([^"“”„«»\n]+)["“”„«»]') +"""A passage of a reviewer's line in straight or typographic double quotes or guillemets.""" _MATH_SPACE = re.compile(r"\\[,;:! ]|\\q?quad\b") """Mathtext that draws a space: read as a space.""" MATH_SYMBOLS: dict[str, str] = { @@ -85,6 +97,11 @@ """Mathtext markup characters left after the commands: dropped, so `CO$_2$` reads `CO2`.""" _DOLLAR = re.compile(r"(? dict[ShownGroup, list[str]] | None: } -def _norm(text: str) -> str: +def _norm(text: str, *, quotes: bool = False) -> str: """A text as the comparison reads it: NFKC, case-folded, without quotes and mathtext markup, one space. Mathtext is read as matplotlib draws it: a text with an even number of unescaped `$` is math, whose delimiters draw nothing, a command that draws one character (`MATH_SYMBOLS`) is that character, a spacing command is a space, and an escaped `\\$` - is a dollar sign; a lone `$` is a dollar sign too. + is a dollar sign; a lone `$` is a dollar sign too. With `quotes`, every quotation mark + stays, as `"`, so a quoted passage keeps its ends (`reviewed_misses`). """ - text = unicodedata.normalize("NFKC", text).translate(_DASHES).translate(_QUOTES) + text = unicodedata.normalize("NFKC", text).translate(_DASHES).translate(_QUOTE_MARKS if quotes else _QUOTES) delimiters = len(_DOLLAR.findall(text)) if delimiters and delimiters % 2 == 0: text = _DOLLAR.sub("", text) @@ -348,3 +366,41 @@ def expected_misses(expected: Sequence[str], drawn: DrawnTexts | None) -> list[E if (not applies and drawn.complete) or (wrong is not None and any(_wrong_mark(t, wrong) for t in applies)): misses.append(ExpectMiss(token, "format", _observed(shown, "format"))) return misses + + +def reviewed_misses(expected: Sequence[str], lines: Sequence[str]) -> list[tuple[str, str]]: + """The requested texts a review reports missing from the image, each with the first defect line that does. + + For a render without drawn texts the reviewer gets the requested texts and reports each one + it cannot find as a defect line under `EXPECT_LINE_ID` (SC-04, or SC-02 for a number format) + in the form its heading prescribes, `the requested text ""`. A line in that form names + exactly the texts it cites so, compared with every quotation mark kept (`_norm(quotes=True)`: + case, spacing and dashes aside), so a cited `Temperatur (°C)` never names a requested + `Temperatur`, a text with quotes of its own (`Zone "A"`) is still named whole, and a passage + the line quotes for what the image shows instead names nothing. A line in another form names + a text when one of its quoted passages reads as that text; when it quotes none, or the text + has quotes of its own or is a number format (a line may quote only the number it saw), when it + holds the text whole (`_contains`). Any other line, and a text no line names, is no miss: the + reviewer found it. + """ + misses: list[tuple[str, str]] = [] + for token in dict.fromkeys(" ".join(item.split()) for item in expected): + wanted = _norm(token) + if not wanted: + continue + cited = f'{REQUESTED_MARK}{_norm(token, quotes=True)}"' + loose = token.translate(_QUOTES) != token or format_unit(token) is not None + for line in lines: + match = DEFECT_RE.match(line) + if match is None or not set(EXPECT_LINE_ID.values()) & set(match["ids"].split(", ")): + continue + marked = _norm(line, quotes=True) + if REQUESTED_MARK in marked: + named = cited in marked + else: + quoted = [_norm(passage) for passage in _QUOTED.findall(line)] + named = wanted in quoted or ((loose or not quoted) and _contains(_norm(line), wanted)) + if named: + misses.append((token, line)) + break + return misses diff --git a/agents/anyplot/pipeline.py b/agents/anyplot/pipeline.py index f3f6367b16d..30733ba2df7 100644 --- a/agents/anyplot/pipeline.py +++ b/agents/anyplot/pipeline.py @@ -36,7 +36,10 @@ allowed attempt), and once more on the repaired render after a rejection (`MAX_REVIEWS` reviewer calls in all, every round with `AGENT_SHIP_BEST`); the reviewer agent sees the rendered theme's PNG, so each of its image defects is - filed under that theme, whatever theme the reviewer named; + filed under that theme, whatever theme the reviewer named. A render without drawn + texts on a runtime whose reviewer checks them (`RuntimeAdapter.review_texts`: R, + altair, pygal and lets-plot) hands the review the requested texts, and each one it + cannot find in the image is a miss like the probe's (`grounding.reviewed_misses`), in the same call; 5. **repair**: when an attempt left feedback (an answer that missed the schema or was cut off at the output limit, each with its own line, failed edits, validator findings, a failed render, a missed canvas, reviewer defects, and measured @@ -130,7 +133,7 @@ from .data.kinds import ColumnKinds, read_kinds from .data.store import StoredDataset from .dataset_context import dataset_context, file_name_for_models -from .grounding import UNVERIFIED_LINE, drawn_texts, expected_misses, plot_texts +from .grounding import EXPECT_LINE_ID, UNVERIFIED_LINE, drawn_texts, expected_misses, plot_texts, reviewed_misses from .language import DEFAULT_LANGUAGE, same_language from .plugins.ledger import RequestLedger, attribution, budget_allows, ledger_for from .policy import DATA_PREAMBLE, fence @@ -392,7 +395,12 @@ class Candidate: expect_misses: list[str] = field(default_factory=list) """The same misses as the requested texts themselves, for `PlotShows.requested_not_found`.""" expect_note: str | None = None - """`grounding.UNVERIFIED_LINE` when the run had requested texts but the probe reported no drawn texts.""" + """`grounding.UNVERIFIED_LINE` when the run had requested texts but the probe reported no drawn texts, and no + readable review of this render answered for them (`expect_review`).""" + expect_review: bool = False + """The requested texts go to this render's review, since it reports no drawn texts and its runtime's reviewer + checks them (`RuntimeAdapter.review_texts`): a readable review replaces `expect_note` with the misses it + reports (`grounding.reviewed_misses`), as `expect_lines` and `expect_misses`.""" @dataclass @@ -804,6 +812,7 @@ def render_review_request(request: ReviewRequest) -> str: ] lines += _context_lines(request.dataset_context, request.file_name) lines += _requests_lines(request.user_requests, request.change_request) + lines += _review_expected_lines(request.expected_texts, catalogue=False) if request.gate_notes: lines += ["Gate notes:", fence("tool_notes", json.dumps(request.gate_notes, ensure_ascii=False))] return "\n".join(lines) @@ -811,6 +820,30 @@ def render_review_request(request: ReviewRequest) -> str: EARLIER_REQUESTS_HEADING = "Earlier requests (the user's standing decisions, oldest first):" EXPECTED_HEADING = "Texts and number formats the user asked to see in the plot, verbatim (one per line):" +REVIEW_EXPECTED_HEADING = ( + "Texts and number formats the user asked to see in the plot, verbatim (one per line). The server cannot read " + "the texts this render draws, so look for each one in the attached render yourself: a text must stand there as " + "written (case, spacing and quotes aside); a number format (a number with one unit, such as 6,5 Mio. or 12 %) " + "must draw its unit, and the numbers it applies to must use its decimal mark. Report each one you cannot find " + "as one {ids} finding whose observed part quotes it as listed here: the plot does not show the requested " + 'text "", then what it shows instead. One you find needs no finding:' +) +"""The reviewer's heading over the requested texts (`ReviewRequest.expected_texts`): only a render without drawn +texts gets it, so a review request of a render with the probe is the same as before 2026-10-29.""" + + +def _review_expected_lines(texts: Sequence[str], *, catalogue: bool) -> list[str]: + """The requested texts for the reviewer to find in the image, fenced as user text under their heading. + + The catalogue reviewer reports a missing number format under SC-02 as the probe's check does + (`grounding.EXPECT_LINE_ID`); the checklist has no SC-02, so it reports every miss under SC-04. + """ + if not texts: + return [] + ids = EXPECT_LINE_ID["text"] + (f" ({EXPECT_LINE_ID['format']} for a number format)" if catalogue else "") + return [REVIEW_EXPECTED_HEADING.format(ids=ids), fence("user_message", "\n".join(texts))] + + CONTEXT_HEADING = ( "Dataset context (what the session knows about the data: the notes the user pasted with it and where a " "computed table comes from; use it for the title and the labels):" @@ -1221,6 +1254,9 @@ class ReviewOutcome: """Good enough to show without notes (`AGENT_STATUS_BAR`, the owner's ship bar); None means `ok`.""" visible: list[str] = field(default_factory=list) """The defect lines under `VISIBLE_DEFECT_IDS`: the render is not clean (`Candidate.visible_lines`).""" + requested: list[tuple[str, str]] = field(default_factory=list) + """The requested texts the review could not find in the image, each with the line that says so + (`grounding.reviewed_misses`); empty when the request carried none (`ReviewRequest.expected_texts`).""" async def _review(ctx: Context, scope: str, request: ReviewRequest, theme: Theme) -> ReviewOutcome | None: @@ -1232,10 +1268,13 @@ async def _review(ctx: Context, scope: str, request: ReviewRequest, theme: Theme raw = await ctx.run_node(agent, node_input=text, override_isolation_scope=scope) if not catalogue: verdict = Verdict.model_validate(raw) + lines = [_on_theme(defect, theme).as_line() for defect in verdict.defects] + requested = reviewed_misses(request.expected_texts, lines) return ReviewOutcome( - verdict.ok, - [_on_theme(defect, theme).as_line() for defect in verdict.defects], + verdict.ok and not _repairs_miss(requested), + lines, [defect.id for defect in verdict.defects], + requested=requested, ) result = parse_answer(mend_answer(ScoreAnswer.model_validate(raw).model_dump())) except (ValidationError, DynamicNodeFailError) as exc: @@ -1259,7 +1298,10 @@ async def _review(ctx: Context, scope: str, request: ReviewRequest, theme: Theme for line in result.defects if (m := DEFECT_RE.match(line)) and VISIBLE_DEFECT_IDS & set(m["ids"].split(", ")) ] - if visible: + # A requested text the review could not find in the image (a render without drawn texts), read from every + # line whatever the points: a repair finding as the probe's miss is under `AGENT_EXPECT_CHECK=repair`. + requested = reviewed_misses(request.expected_texts, result.defects) + if visible or _repairs_miss(requested): # A visible defect gets the repair round whatever the points (`VISIBLE_DEFECT_IDS`); the ship bar stays. ok = False return ReviewOutcome( @@ -1270,9 +1312,15 @@ async def _review(ctx: Context, scope: str, request: ReviewRequest, theme: Theme round(result.technical_share, 3), status_ok=status_ok, visible=visible, + requested=requested, ) +def _repairs_miss(requested: Sequence[tuple[str, str]]) -> bool: + """A requested text the review reports missing sends the round to the repair (`AGENT_EXPECT_CHECK=repair`).""" + return bool(requested) and get_settings().expect_check == "repair" + + def render_score_request(request: ReviewRequest) -> str: """The catalogue reviewer's node input: the scorer's request (`scoring.user_text`) with the gate notes and the change request added, every value fenced; the image follows from the callback.""" @@ -1287,6 +1335,7 @@ def render_score_request(request: ReviewRequest) -> str: if request.dataset_context or request.file_name: lines.append("\n".join(_context_lines(request.dataset_context, request.file_name))) lines += _requests_lines(request.user_requests, request.change_request) + lines += _review_expected_lines(request.expected_texts, catalogue=True) if request.gate_notes: lines += ["Gate notes:", fence("tool_notes", json.dumps(request.gate_notes, ensure_ascii=False))] return "\n\n".join(lines) @@ -1727,8 +1776,10 @@ async def _attempts( checking = settings.expect_check != "off" and bool(run.expected_texts) misses = expected_misses(run.expected_texts, drawn) if checking else [] expect_lines = [miss.line(run.theme) for miss in misses] - # No drawn texts to check against (a renderer without `labels`): a note, never a pass and never a repair. - expect_note = UNVERIFIED_LINE.format(theme=run.theme) if checking and drawn is None else None + # No drawn texts to check against (a renderer without `labels`): a note, never a pass. Where the runtime says + # so (R), the review finds the texts in the image instead, and only its answer lifts the note. + unverified = checking and drawn is None + expect_note = UNVERIFIED_LINE.format(theme=run.theme) if unverified else None candidate = Candidate( working=working, run_form=run_form, @@ -1746,6 +1797,7 @@ async def _attempts( expect_lines=expect_lines, expect_misses=[miss.expected for miss in misses], expect_note=expect_note, + expect_review=unverified and runtime.review_texts, ) if report.canvas_ok: run.best = candidate @@ -1818,6 +1870,7 @@ async def _attempts( plot_language=run.language, dataset_context=context, file_name=file_name, + expected_texts=run.expected_texts if candidate.expect_review else [], ) started = time.monotonic() verdict = await _review(ctx, scope, review_request, run.theme) @@ -1860,6 +1913,12 @@ async def _attempts( candidate.review_score = verdict.score candidate.review_lines = [] if verdict.ok and candidate.status_ok else list(verdict.lines) candidate.visible_lines = list(verdict.visible) + if candidate.expect_review: + # The review answered for the requested texts: the ones it could not find stay, with its own lines, as a + # measured miss does (residual, not clean), and no unverified note is left. + candidate.expect_lines = list(dict.fromkeys(line for _, line in verdict.requested)) + candidate.expect_misses = [text for text, _ in verdict.requested] + candidate.expect_note = None if verdict.score is not None and (run.best_scored is None or ship_compare(candidate, run.best_scored) >= 0): run.best_scored = candidate # The latest review's lines only: a passing review clears an earlier round's, so a later diff --git a/agents/anyplot/prompts/adapter_r.md b/agents/anyplot/prompts/adapter_r.md index f80cf4c200d..3b2dec1d37d 100644 --- a/agents/anyplot/prompts/adapter_r.md +++ b/agents/anyplot/prompts/adapter_r.md @@ -14,7 +14,7 @@ the blank lines around it are not part of it. This comment never reaches a model a scales labeller such as `scales::label_number(scale = 1e-6, accuracy = 0.1, decimal.mark = ",", big.mark = ".", suffix = " Mio.")` that writes the decimal comma and the unit -An R render has no probe, so the server cannot read its drawn texts back: the result marks the requested texts as not checked, and only your code makes them right. +An R render has no probe, so the server cannot read its drawn texts back: the reviewer looks for each requested text in the image instead, and reports one it cannot find, which then comes back to you as a finding. An R render has no probe, so no hint is ever a target measured on a render: every hint quotes the code. diff --git a/agents/anyplot/render/contract.py b/agents/anyplot/render/contract.py index b56a2e97038..ba2c6a71957 100644 --- a/agents/anyplot/render/contract.py +++ b/agents/anyplot/render/contract.py @@ -256,6 +256,13 @@ class RuntimeAdapter(Protocol): """The target of each soft ADAPTATION finding's defect line, in the language's own code; a rule without one gets the pipeline's default.""" + @property + def review_texts(self) -> bool: + """Whether the reviewer checks the user's requested texts in the image when a render reports no drawn texts + (owner decision of 2026-10-29, for a library without the text probe): False leaves such a render with the + `grounding.UNVERIFIED_LINE` note, True hands the texts to its review (`grounding.reviewed_misses`).""" + ... + def command(self, theme: Theme) -> list[str]: """The argv that renders the code file in the work directory (never a shell string).""" ... diff --git a/agents/anyplot/render/runtimes/python.py b/agents/anyplot/render/runtimes/python.py index ca1b1853c2e..b652573fe65 100644 --- a/agents/anyplot/render/runtimes/python.py +++ b/agents/anyplot/render/runtimes/python.py @@ -85,6 +85,11 @@ Imprint palette") suggested.""" +TEXTLESS_LIBRARIES = frozenset({"altair", "pygal", "letsplot"}) +"""The Python libraries that render without matplotlib and so write no probe: no drawn texts to check the user's +requested texts against (`PythonRuntime.review_texts`).""" + + class PythonRuntime: """Python plot code (matplotlib, seaborn, plotnine, altair, pygal, lets-plot), run as `plot.py` under the probe harness.""" @@ -98,6 +103,13 @@ class PythonRuntime: blocking_rules: frozenset[str] = BLOCKING_RULES adaptation_targets: Mapping[str, str] = ADAPTATION_TARGETS + @property + def review_texts(self) -> bool: + """matplotlib, seaborn and plotnine draw through matplotlib's probe, which reports the drawn texts the + requested ones are checked against, as before 2026-10-29. altair, pygal and lets-plot write no probe, as R + does not, so their reviewer finds the requested texts in the image (`TEXTLESS_LIBRARIES`).""" + return self.library in TEXTLESS_LIBRARIES + def __init__( self, *, diff --git a/agents/anyplot/render/runtimes/r.py b/agents/anyplot/render/runtimes/r.py index 312e6e71f2c..e8f523a47dc 100644 --- a/agents/anyplot/render/runtimes/r.py +++ b/agents/anyplot/render/runtimes/r.py @@ -25,7 +25,10 @@ **Outputs.** R writes `plot-.png` and no probe: the host gates R1 to R3 judge the render, and the advisory gates (G3 to G12) have nothing to read, as for altair, pygal -and lets-plot. +and lets-plot. Nor are there drawn texts to check the user's requested texts against, so +the reviewer finds them in the image instead (`review_texts`, owner decision of +2026-10-29): a run that asks for a text is `needs_attention` only when the review cannot +find it, or when no readable review answered for it. **Data runs and conversions** stay Python's: `run_data_code` runs the Python the root translated, whatever the plot's language, and a conversion reads a dropped file with the @@ -107,6 +110,8 @@ class RRuntime: """The working form's data placeholder, `df <- load_user_data()`, which the loader replaces.""" blocking_rules: frozenset[str] = BLOCKING_RULES | {DATASET_PACKAGE_RULE} adaptation_targets: Mapping[str, str] = ADAPTATION_TARGETS + review_texts = True + """R writes no probe, so the reviewer finds the requested texts in the image (module docstring, "Outputs").""" def __init__( self, diff --git a/agents/anyplot/schemas.py b/agents/anyplot/schemas.py index 69ef6c106ac..b86dad8398d 100644 --- a/agents/anyplot/schemas.py +++ b/agents/anyplot/schemas.py @@ -392,6 +392,10 @@ class ReviewRequest(_ServerContract): """The screened notes pasted with the data and the source of a computed table (`dataset_context.py`).""" file_name: str = Field(default="", max_length=MAX_FILE_NAME_CHARS) """The screened name of the dropped file, as for `AdaptRequest.file_name`.""" + expected_texts: list[ExpectedText] = Field(default_factory=list, max_length=MAX_EXPECTED_TEXTS) + """The texts and number formats the user asked to see, for the reviewer to find in the image: set only for a + render without drawn texts on a runtime whose reviewer checks them (`RuntimeAdapter.review_texts`), so every + other review request stays as it was.""" class Defect(_ModelOutput): diff --git a/agents/anyplot/settings.py b/agents/anyplot/settings.py index adc7d1562cd..4aa0f35f86f 100644 --- a/agents/anyplot/settings.py +++ b/agents/anyplot/settings.py @@ -18,7 +18,7 @@ | `AGENT_STATUS_BAR` | `0.95` | `catalogue` reviews: the ship bar; a shipped render at or above it, with no measured gate or validator finding left, shows without notes (`ok`) | | `AGENT_STATUS_SCORE` | `0` | `catalogue` reviews: the review score (0 to 100) a shipped render also needs for `ok`; 0 turns the condition off (owner decision pending) | | `AGENT_GATE_REPAIR` | `review` | When measured findings of a render (advisory gate lines, ADAPTATION findings) start a repair: `review` (they go to the self-review and, as gate notes, to the review, and a round whose render still has them after its review is repaired with them) or `first` (straight to the repair round, skipping the self-review and the review; live before 2026-10-14) | -| `AGENT_EXPECT_CHECK` | `repair` | A text or number format the user asked for (the root's `expected_texts`) that the render's drawn texts do not show: `repair` (a measured finding: the repair round gets it, and an unfixed miss is a residual line), `note` (the residual line only) or `off` (owner decision pending) | +| `AGENT_EXPECT_CHECK` | `repair` | A text or number format the user asked for (the root's `expected_texts`) that the render's drawn texts do not show, or, for a render without drawn texts on a runtime whose reviewer checks them (R, altair, pygal, lets-plot), that the review cannot find in the image: `repair` (a finding the repair round gets, and an unfixed miss is a residual line), `note` (the residual line only) or `off` (owner decision pending) | | `AGENT_SHIP_CLEAN_MARGIN` | `3` | `catalogue` reviews with `AGENT_SHIP_BEST`: a render without measured findings ships over a higher-scored one with them when it meets the ship bar or is at most this many points below it | | `AGENT_REPAIR_SELF_REVIEW` | `false` | With `AGENT_SELF_REVIEW` on, whether a repair round proofreads its render too, not only the first round | | `AGENT_SHIP_BEST` | `true` | `catalogue` reviews: review every render and ship the best-scored version, not the last | @@ -232,13 +232,16 @@ class AgentSettings(BaseSettings): expect_check: Literal["repair", "note", "off"] = "repair" """What a requested text or number format the render does not show does (`AGENT_EXPECT_CHECK`, - `grounding.expected_misses` over the root's `expected_texts`). + `grounding.expected_misses` over the root's `expected_texts`, or `grounding.reviewed_misses` when a + render without drawn texts hands them to its review, `RuntimeAdapter.review_texts`). `repair`: it is a measured finding like an advisory gate line: the self-review and the review see it, a round whose render still misses it is repaired with it, and a shipped render that still - misses it carries it as a residual line (`needs_attention`). `note`: only the residual line, no - repair round for it. `off`: no check. The choice between `repair` and `note` is a product - decision left to the owner: a repair round costs about 40 s, a false miss would spend it.""" + misses it carries it as a residual line (`needs_attention`); a miss the review reports sends its + round to the repair whatever the points, as a visible defect does. `note`: only the residual line, + no repair round for it. `off`: no check, and no texts for the review. The choice between `repair` + and `note` is a product decision left to the owner: a repair round costs about 40 s, a false miss + would spend it.""" repair_self_review: bool = False """`AGENT_REPAIR_SELF_REVIEW`: whether a repair round proofreads its render before the review, as the diff --git a/changelog.d/review-requested-texts.md b/changelog.d/review-requested-texts.md new file mode 100644 index 00000000000..7cb55a89b4e --- /dev/null +++ b/changelog.d/review-requested-texts.md @@ -0,0 +1,18 @@ +### Fixed + +- **A plot without a text probe that showed the requested text still needed attention.** + R, altair, pygal and lets-plot write no probe, so the agent chat could not + check a text the user asked for, such as a title or an axis label, and every + such request shipped as `needs attention` with "the requested texts could not + be checked", although the image showed the text (ggplot2 live test of + 2026-10-29). The reviewer now checks these texts in the image, in the review + call it already makes (owner decision of 2026-10-29). Only a render without + drawn texts on a library that opts in (`RuntimeAdapter.review_texts`) sends + the texts to the reviewer, so the reviewer prompt and every review request of + a matplotlib, seaborn or plotnine render stay unchanged. The reviewer reports + a text it cannot find as an `SC-04` line that quotes it (`SC-02` for a number + format), and the pipeline handles that line like the probe's miss: under + `AGENT_EXPECT_CHECK=repair` it sends the round to the repair whatever the + points, and if no round fixes it, the plot ships with the line and the text in + `requested_not_found`. The "could not be checked" note now appears only when + no readable review answered for the texts. (#12454) diff --git a/docs/concepts/agent-network.md b/docs/concepts/agent-network.md index e54ca726b2d..2c8deece009 100644 --- a/docs/concepts/agent-network.md +++ b/docs/concepts/agent-network.md @@ -152,6 +152,7 @@ Since 2026-10-15 the root's replies are grounded in what the server measured, no - The probe reports the content of the drawn texts by role (`labels`: titles, axis labels and legend texts first, then up to 100 tick labels per axis of each axes, each tick and axis label with the index of its axes, colorbar ticks under their own kind, texts wholly off the canvas left out, at most 400 texts of 200 characters and 20,000 characters together, and `labels_truncated` when any drawn text was left out or cut, in which case no requested text is claimed missing; `harness.drawn_labels`). `grounding.plot_texts` groups them into `PlotShows.texts`, clipped for the reply, and the shipped result carries `shows` (those texts, the review's verdict and score, and `requested_not_found`). The root reads it in its own `` block next to the adapter's `changes`, and the prompt says to describe the plot from `shows` and never to state a value, format, binding or text that no tool result holds. `shows` never reaches the page's `plot` event. A renderer older than the `labels` field yields `texts: null`, and the root then quotes no plot text. - `PipelineArgs.expected_texts` lists the texts and number formats the user wrote out for the plot, verbatim (at most five). The adapter gets them fenced as user text; `grounding.expected_misses` checks each against all drawn texts, never the clipped summary. A text, numbers in it included (`Population 2025`, `Top 10`), must stand whole in one drawn text, case, spacing, quotes, dashes and mathtext aside. A number-format example (a number with one short unit: `6,5 Mio.`, `12 %`, `CHF 6,5`) must draw its unit, and every text it applies to (the texts that draw the unit, and the tick labels of that axis of that axes, or of that colorbar, whose label or ticks draw it) must use its decimal mark, dates and three-digit groups aside. A probe without `labels` (a renderer before this change, or a failed text report) checks nothing and adds the residual line "the requested texts could not be checked" instead of a silent pass. A miss is a measured finding under `AGENT_EXPECT_CHECK=repair` (the default): the self-review and the review see it, a round whose render still misses it is repaired with it, and a shipped render that misses it carries an `SC-02` or `SC-04` residual line. +- A render without drawn texts on a runtime whose reviewer checks them (`RuntimeAdapter.review_texts`, owner decision of 2026-10-29) hands the requested texts to its review instead, in the same reviewer call. Today that is R, and altair, pygal and lets-plot among the Python libraries: none of them writes a probe. Before, every ggplot2 request for a title or an axis label shipped as `needs_attention` with the "could not be checked" line, although the image showed the text. Only such a render's review request carries the texts, under a fixed heading and fenced as user text, so the reviewer prompt and the review request of a render with the probe are unchanged. The reviewer looks for each text in the image and reports one it cannot find as a defect line that quotes it, under `SC-04`, or `SC-02` for a number format in the catalogue rubric (the checklist has only `SC-04`). `grounding.reviewed_misses` reads those lines back from every line of the answer, whatever the points: a line names a requested text when a quoted passage of it reads as that text. A reported miss is handled like the probe's: under `AGENT_EXPECT_CHECK=repair` it sends the round to the repair whatever the points, as a visible defect does, the render is not clean for `AGENT_SHIP_BEST`, and a shipped render that misses it carries the reviewer's line as a residual line and the text in `requested_not_found`. A text no line names counts as found. The "could not be checked" line stays only on a render whose review did not run or could not be read. matplotlib, seaborn and plotnine keep the old path, because they draw through matplotlib's probe (`PythonRuntime.review_texts` by library, `TEXTLESS_LIBRARIES`). - A shipped result reaches the root with two server facts beside the notes: `data_confirmed: true` and, since live test round 7 of 2026-10-16, `data_changed: false` (`tool_safety.fence_pipeline_notes`): the pipeline never changes the data. The root prompt says to claim a row, category or value added, removed, filled in or corrected only when a data tool's `changes` in the same turn says so, and to describe a plot note about an "Other" group or a remainder as part of the plot; the adapter prompt says its `changes` describe what the code does with this data, never a branch that does not run for it. The case: a donut of six energy sources that already summed to 100 % got the reply "I added 'Übrige' at 2.0 % so the shares reach 100 %", although that row was the user's. No reply-side check parses the claim: in any language a deterministic one would be a word list. - `get_dataset_profile` returns the bindings in force (`bindings`, one `role: column` per line inside a `` fence), the session block's bindings line points to it, and the root prompt says to name exactly those in the summary. The list stands in the tool result, not in the session block: in a measurement with the list in the block, the root took it for text the user had written in 1 of 6 summaries. - The adapter and the reviewer get a dataset context block, fenced as `` and capped at 1,500 characters (`dataset_context.py`): the notes pasted with the data, which the dataset route kept only after the pre-filter and the scope judge (a JSON-stat title and source among them), and where a computed table comes from (`StoredDataset.source`: the ranges and expressions of `generate_data`, or the function, call and variable names of a `run_data_code` snippet, never its comments or strings). A change of the data keeps the source, and a new paste drops it. @@ -311,7 +312,7 @@ Since live test round 5 of 2026-10-15 the parse also handles these cases: ### Render -`agents/anyplot/render/` defines `RenderJob{job_id, language, library, source, data_csv, themes, timeout_s}` and two protocols: `RuntimeAdapter` (one per library, from the registry `runtimes.runtime_for`: `file_name`, `command` as the CI command verbatim, `env`, `loader_block`, `normalise`, `validate`, `collect`, and since 2026-10-24 the language's code conventions the pipeline checks against (`regions`, `readiness`, `security`, `adaptation`, `apply_plan`, `restore_protected`, `new_literals`), its error grammar (`error_summary`), its export with the code artifact's name (`export`, `code_artifact`: `plot.py`, `plot.R`, `plot.jl`, `plot.js`, `plot.tsx`, which `schemas.ArtifactName`, the artifact route and the feedback case follow) and the render time a repair reserves (`render_p95_s`), and since 2026-10-29 the ADAPTATION rules that reject the code (`blocking_rules`), the targets of the others (`adaptation_targets`), the column kinds `loader_block` takes and what its loader changes in the data (`loader_notes`)) and `RenderBackend` with the backends `remote` (phase 1: the `anyplot-renderer` service, `AGENT_RENDERER=remote` with `AGENT_RENDER_URL`), `sandbox` (sandboxes launched inside the agents service; a stub that phase 1 does not use), `local` (development only; refuses `ENVIRONMENT=production`; always `docker run --rm --network none --read-only --tmpfs /tmp -v :/work /app/.venv/bin/python -I /opt/anyplot/harness.py plot.py` with the production renderer image, fails if Docker is missing, no bare-subprocess fallback) and `fake` (unit tests, fixture PNGs). The registry serves seven libraries and picks the runtime class by the library's language (`RUNTIME_CLASSES`). matplotlib, seaborn, plotnine, altair, pygal and lets-plot (`letsplot`) go through `PythonRuntime` (plotnine and altair since 2026-10-25, pygal and lets-plot since 2026-10-26), which hands its library to the regions, the readiness scan, the edits, the normaliser and the validator, so each library gets its own save rule and import surface. plotnine draws through matplotlib, so its renders carry the probe and its advisory gates. altair renders in vl-convert's embedded V8, pygal through cairosvg and libcairo, and lets-plot in its Kotlin/Native bridge (ImageMagick linked in): none of them draws a matplotlib figure or writes a probe, so the host gates alone judge them. ggplot2 goes through `RRuntime` since 2026-10-29: the R code profile, `plot.R` run by the renderer's R row in exec mode under the preloaded exec filter, the PNG and no probe, R's error grammar, the export with `Rscript plot.R` in its run line, a render reserve of 15 s like Python's (the first deployed R render took 5.7 s of wall time on Cloud Run), and Python's data runs and conversions, because `run_data_code` runs Python whatever the plot's language. The local Docker backend renders it with the row's environment, key for key the renderer's. +`agents/anyplot/render/` defines `RenderJob{job_id, language, library, source, data_csv, themes, timeout_s}` and two protocols: `RuntimeAdapter` (one per library, from the registry `runtimes.runtime_for`: `file_name`, `command` as the CI command verbatim, `env`, `loader_block`, `normalise`, `validate`, `collect`, and since 2026-10-24 the language's code conventions the pipeline checks against (`regions`, `readiness`, `security`, `adaptation`, `apply_plan`, `restore_protected`, `new_literals`), its error grammar (`error_summary`), its export with the code artifact's name (`export`, `code_artifact`: `plot.py`, `plot.R`, `plot.jl`, `plot.js`, `plot.tsx`, which `schemas.ArtifactName`, the artifact route and the feedback case follow) and the render time a repair reserves (`render_p95_s`), and since 2026-10-29 the ADAPTATION rules that reject the code (`blocking_rules`), the targets of the others (`adaptation_targets`), the column kinds `loader_block` takes and what its loader changes in the data (`loader_notes`), and whether the reviewer checks the requested texts of a render without drawn texts (`review_texts`)) and `RenderBackend` with the backends `remote` (phase 1: the `anyplot-renderer` service, `AGENT_RENDERER=remote` with `AGENT_RENDER_URL`), `sandbox` (sandboxes launched inside the agents service; a stub that phase 1 does not use), `local` (development only; refuses `ENVIRONMENT=production`; always `docker run --rm --network none --read-only --tmpfs /tmp -v :/work /app/.venv/bin/python -I /opt/anyplot/harness.py plot.py` with the production renderer image, fails if Docker is missing, no bare-subprocess fallback) and `fake` (unit tests, fixture PNGs). The registry serves seven libraries and picks the runtime class by the library's language (`RUNTIME_CLASSES`). matplotlib, seaborn, plotnine, altair, pygal and lets-plot (`letsplot`) go through `PythonRuntime` (plotnine and altair since 2026-10-25, pygal and lets-plot since 2026-10-26), which hands its library to the regions, the readiness scan, the edits, the normaliser and the validator, so each library gets its own save rule and import surface. plotnine draws through matplotlib, so its renders carry the probe and its advisory gates. altair renders in vl-convert's embedded V8, pygal through cairosvg and libcairo, and lets-plot in its Kotlin/Native bridge (ImageMagick linked in): none of them draws a matplotlib figure or writes a probe, so the host gates alone judge them. ggplot2 goes through `RRuntime` since 2026-10-29: the R code profile, `plot.R` run by the renderer's R row in exec mode under the preloaded exec filter, the PNG and no probe, R's error grammar, the export with `Rscript plot.R` in its run line, a render reserve of 15 s like Python's (the first deployed R render took 5.7 s of wall time on Cloud Run), and Python's data runs and conversions, because `run_data_code` runs Python whatever the plot's language. The local Docker backend renders it with the row's environment, key for key the renderer's. The renderer is a service of its own (owner decision of 2026-10-10), so the deployed agents service and a developer's `adk web` render in the same sandboxes. `agents/renderer/` has no ADK import and no `__init__.py` (`adk web agents` would list it as an agent). `POST /render` takes the `RenderJob` as JSON and answers per theme the exit code, a timeout flag, a stderr tail of at most 2,000 characters, the probe, the PNG as base64 (at most 10 MB), the attempts and a reason (`timeout`, `disk_budget`, `file_budget`, `memory`, `launcher` or `output_rejected`) with the measured value and the limit, plus the slot wait, the total time and `MemAvailable`; `POST /run-data` (since 2026-10-14) takes `{job_id, source, timeout_s}` (at most 16 KB of source and 30 s) and runs one data snippet as `snippet.py` with the harness in data mode (`harness.py --data`), in the same render slot, under the same memory floor, watchdog, kill path and launcher retry, with no theme and no matplotlib environment; it answers `{exit_code, timed_out, reason, measured, limit, csv_text, stderr_tail, wall_s, attempts}` plus the timings, the version and `MemAvailable`, where `csv_text` is the `out.csv` the harness wrote, read only after exit code 0 with `read_regular` under the 2 MB cap of computed data as strict UTF-8 (anything else is `output_rejected`). `POST /convert` (since 2026-10-15) takes `{job_id, data_base64, sheet?, timeout_s}` (at most 5 MiB of file and 15 s) and reads one binary data file with the harness in convert mode (`harness.py --convert input.bin`, the converter `convert.py` beside the harness, Arrow's system allocator), in the same slot and under the same limits; it answers `{exit_code, timed_out, reason, refusal, measured, limit, text, meta, wall_s, attempts}`, where `text` (at most 1 MiB) and `meta` (`wire.ConvertMeta`: kind, format, rows, columns, sheet, at most ten sheet names, warning counts) are read only after exit code 0 (see the conversion under [Request flow](#request-flow-for-use-with-my-data)). In data mode the harness imports everything the DATA profile allows, every module of `pandas.core`, `pandas.tseries` and `pandas.api` (pandas loads some lazily) and every time zone, reads and compiles the snippet, opens `out.csv`, and then arms an audit hook (`sys.addaudithook`) that refuses, for the rest of the process, every `open` (reading too), every `import` of a module not loaded yet, `ctypes.*`, `os.system`, `os.exec*`, `os.posix_spawn`, `os.spawn*`, `os.fork*`, `subprocess.*`, `socket.*`, every filesystem change and listing (`os.listdir`, `os.scandir`), every route to a frame (`gi_frame`, `cr_frame`, `ag_frame`, `tb_frame`, `sys._getframe`), because a frame's `f_builtins` hands out `open` and `__import__` past the validator (the Fable security review of 2026-10-14 ran `(i for i in range(1)).gi_frame.f_builtins['open']` end to end), and every `exec` and `compile` but the snippet's own run and three code builders that fill a fixed template with names they checked as identifiers (`namedtuple`, which `DataFrame.itertuples()` uses, dataclasses, scipy's distributions). CPython raises no audit event for `os.stat`, reading `os.environ` or `function.__globals__`, nor for importing a module that is loaded already; those stay the validator's; it takes `df` from the run's globals (a DataFrame, no MultiIndex columns, a non-default index reset into columns, names as text, at most 20,000 rows, 50 columns and 2 MB of CSV, 200 KB until 2026-10-14) and writes it through the handle opened before the hook. The snippet never chooses the exit code (the harness catches everything it raises, `SystemExit` included), each refusal has its own code, and a refusal the snippet caught still refuses the run. The plot mode arms an audit hook of its own (`PlotGuard`, since 2026-10-14), after the limits, the font cache and the probe are set up and the plot file is compiled: it refuses every `exec` but the plot's own run, the module bodies of an import (an import frame met before any plot frame, walking up from the caller) and the code builders (`CODE_BUILDERS`: the three above, plotnine's `Environment.eval` since 2026-10-25, and since 2026-10-26 cssselect2's selector compiler, which pygal's PNG export runs once per CSS selector of the chart and which quotes every string the stylesheet controls with `repr`); a code object or function built by hand and a `__code__` assignment; every `open` that writes anything but `plot-.png`, `probe-.json` and, since 2026-10-25, `plot-.html` (the second file of altair, and since 2026-10-26 of pygal and lets-plot, which stays in the sandbox: no backend collects it; libcairo writes pygal's PNG natively, past this hook, so the validator's pin holds that path); `subprocess.*`, `os.system`, `os.exec*`, `os.posix_spawn`, `os.spawn*`, `os.fork*`, `socket.*`, `ctypes.*` (except threadpoolctl's, which scikit-learn uses to limit BLAS threads), filesystem changes and `os.chdir`; and, since the Fable review of #12177, every route to a frame (`gi_frame`, `cr_frame`, `ag_frame`, `tb_frame`, `sys._getframe`, `sys._current_frames`) while plot code is on the stack, so neither the plot nor library code it called (`pd.Series([g]).apply(getattr, args=("gi_frame",))`) reaches a frame's `f_builtins`. Two kinds of frame reader stay allowed: an import body (an import frame met before any plot frame, as for `exec`), and a short list of library functions that read a name or a value and return no frame (`FRAME_READERS`: pandas' chained-assignment check, the caller lookup of statsmodels' graphics, and `traceback.extract_stack` and `extract_tb`, which pyparsing calls while matplotlib builds and runs its mathtext parser, and through which the interpreter prints a failed plot's traceback, so the error line still reaches the repair; and since 2026-10-25 plotnine's `Environment.capture`, which walks from `inspect.currentframe()` to the plot's own frame and keeps its `f_locals` and `f_globals`, the namespace the plot has anyway, to look column names up in `aes("x")`; every plotnine file failed without it with `call-stack is not that deep!`). A stack with no plot frame gets no frame either: a worker thread runs a callable the plot submitted (`ThreadPoolExecutor.submit(sys._current_frames)`) with no plot frame above it, and the frames it returned reached `run_plot`'s locals and let the plot disarm the guard (#12181 review). For the same reason both guards refuse `gc.get_objects`, `gc.get_referrers` and `gc.get_referents`, which hand out the guard object itself, and every callback the interpreter hands a real frame (Fable review of #12181, reproduced in both modes): `sys.settrace` and `sys.setprofile` (also raised by `threading`'s `settrace`, `setprofile` and their `_all_threads` forms) and `sys.monitoring.register_callback`, the one PEP 669 call with an audit event. A trace function walks `f_back`, which raises no event, to the locals of `run_plot` or `run_data_snippet`, where the guard sits. `signal.signal`, whose handler gets the interrupted frame and which raises no event, is replaced by a refusal before arming, like `os.mkfifo`. The guard's own state stays a plain attribute: CPython gives in-process code no state that a reference cannot reach, so the defence is that no route hands the code under test a reference. The validator bans `settrace`, `setprofile`, `settrace_all_threads`, `setprofile_all_threads` and `monitoring` as attributes on any object and as `from` imports as a second layer. `inspect.currentframe` and `logging.currentframe`, which return their caller's frame, are replaced before the guard is armed: `logging.currentframe` answers None, as in data mode, and since 2026-10-25 `inspect.currentframe` answers its caller's frame to a `FRAME_READERS` entry and None to every other caller (`frame_for_readers`, a `sys._getframe(1)` the guard judges like any other frame route, with a refusal turned into None), while data mode keeps None for every caller. Where a helper answers None, pandas reports a warning at stack level 0 and a log record names "(unknown file)". The `open` event carries no `dir_fd`, so an `os.open` write passes only with an absolute path, and the calls that raise no audit event at all (`os.mkfifo`, `os.mknod`, `_posixsubprocess.fork_exec`) are replaced by a refusal before either mode's hook is armed. Since 2026-10-28 arming a guard, in every mode that runs code in process, first installs a seccomp filter (`install_exec_deny`) under which the kernel refuses every `execve` and `execveat` of the process, of every thread already running (`SECCOMP_FILTER_FLAG_TSYNC`) and of every child, so native code the hook cannot see (lets-plot's ImageMagick delegates, libc's `system`) starts no program, whatever is on `PATH`; `fork` and threads stay allowed, a filter cannot be removed, and another architecture's or an x32 syscall is refused as well. `on` needs an `execv` of a path that cannot exist to change from ENOENT before the filter to EPERM after it, so neither a sandbox policy that already refuses `execve` nor a filter the kernel accepted but does not enforce passes for it. The end line reports it (`exec_deny`: `on`, `unavailable`, `unsupported`, or `off` when the run failed before the guard was armed), and the renderer logs it with every launch (`exec_deny` in the `sandbox` event). Every in-process run requires it since the first renderer revision with the filter logged `exec_deny: on` on 2026-10-28, which shows that Cloud Run's `sandbox do` admits it (lets-plot's row alone required it before): `executor.sandbox_env` and the local backend put `ANYPLOT_DENY_EXEC=required` into every render, data run and conversion while `RENDERER_EXEC_DENY` is `required`, its default; `optional` is the switch for a platform that stops admitting the filter, under which a run goes on and reports it. A filter that is not in place stops the run before its code starts: a render with exit code 3 (which Cloud Run's launcher reports as -13, like every non-zero exit) and a stderr line that names it, a data run with `DATA_EXIT["exec_deny"]`, which the data tool reports as no fault of the code, and a conversion with `CONVERT_EXIT["exec_deny"]`, which the agents service answers `503 convert_unavailable`; both reach the host through `status.json`. Docker's default seccomp profile lets the process add it, and `ci-image.yml` checks the routes in the built image (a running thread, libc's `system`, `subprocess`, `fexecve`, a forked child). Imports, reads and `compile` stay allowed: the plot imports its libraries lazily, matplotlib reads fonts and style sheets and compiles colour and cycler strings while it draws, and a compiled code object runs only through the refused routes. A refusal raises `PermissionError`, so library code that tolerates a read-only filesystem or a missing tool (matplotlib's cache writes, `ctypes.util.find_library`, loky's `lscpu`) carries on; the harness runs scikit-learn's two process lookups (libc for threadpoolctl, the physical cores for loky) before arming when the plot names sklearn, because refused they fail the render or oversubscribe the CPU past `RLIMIT_CPU`, and since 2026-10-26 it imports cairosvg before arming when the plot names pygal or cairosvg (`warm_cairo`), because cairocffi loads libcairo at import time through the same kind of lookups (`ctypes.util.find_library`, which runs `ldconfig`). A sweep of the 650 catalogue matplotlib and seaborn files on a developer machine (Python 3.13.12, the locked plotting stack) rendered all 474 that pass the SECURITY profile under the guard; the seven it newly failed fetch over the network, call `os.chdir` or `load_dataset`, which the validator refuses anyway. With the frame refusals, a sweep of the 572 files the readiness scan admits today (clean or coupled: 285 matplotlib, 287 seaborn) rendered 565 under the guard before and after the change, with no refusal logged; the seven failures are the plots' own (each divides by the length of a title the normaliser emptied). `tests/unit/agents/runtime/test_harness_guard.py` renders nine normalised catalogue files under the guard (scikit-learn, a statsmodels graphic, mathtext, a seaborn `KMeans` file, since 2026-10-25 the `scatter-basic` files of plotnine and altair, and since 2026-10-26 pygal's `scatter-basic` and `line-basic`, whose 4800x2700 chart the canvas rewrite saves at 3200x1800, and lets-plot's `scatter-basic`; the last six are skipped where their library is not installed, as in CI's test job; the renderer-image job of `ci-image.yml` renders a seaborn, a plotnine, an altair, a pygal and a lets-plot plot through the harness in the image, the altair one without `RLIMIT_AS` and the lets-plot one with its row's environment and 2048 MiB). The sweep has not yet been repeated in the renderer image. On a developer machine the spikes of 2026-10-26 rendered 273 of the 325 catalogue pygal files under the guard (the failures are the files' own off-target writes, `os.chdir` calls, missing modules and guard shapes, which the normaliser strips or the validator refuses, and one `RLIMIT_CPU` kill, waveform-audio) and 273 of the 284 eligible lets-plot files (10 divide by the length of the emptied title, and calibration-curve does not render with lets-plot 4.11.0 at all); plotnine and altair have no sweep yet. `POST /render/{job_id}/cancel` stops that job's render or data run; `GET /status` reports the version, the slot, stuck launchers, `MemAvailable`, whether the launcher exists and whether the run volume is in place. `agents/renderer/wire.py` is the JSON contract that both the service and the `remote` backend read. The renderer only runs code and returns its outputs; the host gates and the probe gates below stay in the agents service, and R1 turns a reason into a line that names the limit and the measured value, so the repair knows what to change. Every route needs an IAM-forwarded ID token whose `aud` is in `RENDERER_AUDIENCES` (the service URL, which the deploy adds, plus the OAuth client ids of the owner's user tokens) and whose `email` is in `RENDERER_ALLOWED_CALLERS`; the check is the one of `/v1`, reads `X-Serverless-Authorization` whenever it is present, and is skipped only in development. An unexpected error answers `500 {"detail": "internal"}` and is logged by its class only, so the `remote` backend never mistakes it for a front-end error and runs the job again. diff --git a/tests/unit/agents/runtime/test_catalogue_flow.py b/tests/unit/agents/runtime/test_catalogue_flow.py index 6f97d27f0af..83d5921743d 100644 --- a/tests/unit/agents/runtime/test_catalogue_flow.py +++ b/tests/unit/agents/runtime/test_catalogue_flow.py @@ -12,14 +12,15 @@ import pytest from agents.anyplot.grounding import UNVERIFIED_LINE -from agents.anyplot.pipeline import EARLIER_REQUESTS_HEADING, SELF_REVIEW_LINE +from agents.anyplot.pipeline import EARLIER_REQUESTS_HEADING, REVIEW_EXPECTED_HEADING, SELF_REVIEW_LINE from agents.anyplot.render.backends.fake import FakeBackend, FakeOutcome +from agents.anyplot.render.runtimes import PythonRuntime from agents.anyplot.scoring import DE_DEFAULTS, SCORED from agents.anyplot.services import get_services from agents.anyplot.settings import get_settings from .conftest import CHECKLIST_FLOW -from .fakes import ROOT_REPLY, SCATTER_PLAN, default_script +from .fakes import ROOT_REPLY, SCATTER_PLAN, VERDICT_OK, default_script from .test_service_flow import ( HEADERS, adapter_inputs, @@ -561,6 +562,8 @@ async def test_a_requested_format_the_render_misses_goes_to_the_repair( assert miss in inputs[1] and miss in inputs[2] # the proofread and the repair closing = root_inputs(fake)[-1] assert "6,0 Mio." in closing and "requested_not_found" in closing + # With the probe the review request is as before 2026-10-29: the texts are checked against the drawn ones. + assert not any(REVIEW_TASK in text for text in reviewer_inputs(fake)) @pytest.mark.parametrize("mode", ["repair", "note"]) @@ -598,7 +601,7 @@ async def test_a_renderer_without_drawn_texts_leaves_a_note_and_no_repair( script["root"] = [{"call": "plot_pipeline", "args": {"expected_texts": ["6,5 Mio."]}}, {"text": ROOT_REPLY}] script["reviewer"] = [{"json": score()}] backend.script = lambda job, theme: FakeOutcome(probe={"texts": [], "tick_overlaps": 0, "points": 0}) - swap_models("gemini", script) + fake = swap_models("gemini", script) sid = await open_session(client) events = await create_plot(client, sid) @@ -606,3 +609,178 @@ async def test_a_renderer_without_drawn_texts_leaves_a_note_and_no_repair( plot = next(data for name, data in events if name == "plot") assert (plot["status"], plot["residual_defects"]) == ("needs_attention", [UNVERIFIED_LINE.format(theme="light")]) assert len(plot["rounds"]) == 1 + # The Python runtime keeps the old path: its review request carries no requested texts. + assert REVIEW_TASK not in reviewer_inputs(fake)[0] + + +TEMPERATURE = "Temperatur (°C)" +MISS_LINE = ( + f'SC-04 (light): the plot does not show the requested text "{TEMPERATURE}" (the y axis reads "Exam Score") ' + "→ write it exactly as the user asked. Likely cause: the y axis label." +) +REVIEW_TASK = "look for each one in the attached render yourself" +"""A phrase of `REVIEW_EXPECTED_HEADING`: the review request carries the requested texts.""" + + +def missing(line: str = MISS_LINE) -> dict[str, Any]: + """A catalogue review that cannot find the requested text: one SC-04 point lost, so the points still pass.""" + return {**score(**{"SC-04": 1}), "defects": [line]} + + +@pytest.fixture +def reviewer_checks_texts(monkeypatch: pytest.MonkeyPatch) -> None: + """The fixture session's matplotlib runtime as R's (`RRuntime.review_texts`): no drawn texts (the fake's probe + has no `labels`, as R writes no probe), so the review finds the requested texts in the image.""" + monkeypatch.setattr(PythonRuntime, "review_texts", True) + + +def ask_for(script: dict[str, list[Any]], *texts: str) -> None: + script["root"] = [{"call": "plot_pipeline", "args": {"expected_texts": list(texts)}}, {"text": ROOT_REPLY}] + + +@pytest.mark.usefixtures("reviewer_checks_texts") +async def test_without_drawn_texts_a_text_the_review_confirms_needs_no_note( + client: httpx.AsyncClient, swap_models +) -> None: + """Live test of 2026-10-29: ggplot2 drew the requested y axis label, and the plot still needed attention.""" + script = default_script(plans=[SCATTER_PLAN, EMPTY_PLAN]) + ask_for(script, TEMPERATURE) + script["reviewer"] = [{"json": score()}] + fake = swap_models("gemini", script) + sid = await open_session(client) + + events = await create_plot(client, sid) + + plot = next(data for name, data in events if name == "plot") + assert (plot["status"], plot["residual_defects"]) == ("ok", []) + assert [(r["attempt"], r["outcome"]) for r in plot["rounds"]] == [(1, "ok")] + (review,) = reviewer_inputs(fake) + heading = REVIEW_EXPECTED_HEADING.format(ids="SC-04 (SC-02 for a number format)") + assert f"{heading}\n\n\n{TEMPERATURE}\n" in review + + +@pytest.mark.usefixtures("reviewer_checks_texts") +async def test_without_drawn_texts_a_text_the_review_misses_goes_to_the_repair( + client: httpx.AsyncClient, swap_models +) -> None: + """A miss the review reports is repaired as the probe's is, although its points pass the review bar.""" + script = default_script(plans=[SCATTER_PLAN, EMPTY_PLAN, SECOND_PLAN]) + ask_for(script, TEMPERATURE) + script["reviewer"] = [{"json": missing()}, {"json": score()}] + fake = swap_models("gemini", script) + sid = await open_session(client) + + events = await create_plot(client, sid) + + plot = next(data for name, data in events if name == "plot") + assert (plot["status"], plot["residual_defects"]) == ("ok", []) + assert [(r["attempt"], r["outcome"], r["shipped"]) for r in plot["rounds"]] == [ + (1, "review", False), + (2, "ok", True), + ] + assert "does not show the requested text" in adapter_inputs(fake)[2] # the repair gets the review's line + assert all(REVIEW_TASK in text for text in reviewer_inputs(fake)) # the repaired render's review checks again + + +@pytest.mark.usefixtures("reviewer_checks_texts") +@pytest.mark.parametrize("mode", ["repair", "note"]) +async def test_without_drawn_texts_a_miss_no_round_fixes_ships_as_the_reviewers_line( + client: httpx.AsyncClient, swap_models, monkeypatch: pytest.MonkeyPatch, mode: str +) -> None: + monkeypatch.setenv("AGENT_EXPECT_CHECK", mode) + get_settings.cache_clear() + plans = [SCATTER_PLAN, EMPTY_PLAN, SECOND_PLAN] if mode == "repair" else [SCATTER_PLAN, EMPTY_PLAN] + script = default_script(plans=plans) + ask_for(script, TEMPERATURE, "12 %") + script["reviewer"] = [{"json": missing()}, {"json": missing()}] + fake = swap_models("gemini", script) + sid = await open_session(client) + + events = await create_plot(client, sid) + + plot = next(data for name, data in events if name == "plot") + assert (plot["status"], plot["residual_defects"]) == ("needs_attention", [MISS_LINE]) + assert len(plot["rounds"]) == (2 if mode == "repair" else 1) + # The review named only the label: the format it found is no miss. + shipped = get_services().versions.get(sid) + assert shipped is not None and shipped.result.shows is not None + assert (shipped.result.shows.texts, shipped.result.shows.requested_not_found) == (None, [TEMPERATURE]) + assert "requested_not_found" in root_inputs(fake)[-1] + + +@pytest.mark.usefixtures("reviewer_checks_texts") +async def test_without_drawn_texts_an_unread_review_keeps_the_note(client: httpx.AsyncClient, swap_models) -> None: + """Never a silent pass: no readable review answered for the requested texts.""" + script = default_script(plans=[SCATTER_PLAN, EMPTY_PLAN]) + ask_for(script, TEMPERATURE) + unreadable = {"items": [], "defects": [], "suggestions": [], "description": ""} + script["reviewer"] = [{"json": unreadable}, {"json": unreadable}] + swap_models("gemini", script) + sid = await open_session(client) + + events = await create_plot(client, sid) + + plot = next(data for name, data in events if name == "plot") + assert plot["status"] == "needs_attention" + assert UNVERIFIED_LINE.format(theme="light") in plot["residual_defects"] + + +CHECKLIST_MISS: dict[str, Any] = { + "ok": False, + "defects": [ + { + "id": "SC-04", + "theme": "light", + "observed": f'the plot does not show the requested text "{TEMPERATURE}" (the y axis reads "Exam Score")', + "target": "write it exactly as the user asked", + "likely_cause": "the y axis label", + } + ], +} +"""A checklist verdict that cannot find the requested text: its own `Verdict` schema, rendered into a defect line.""" + + +@pytest.fixture +def checklist_flow(monkeypatch: pytest.MonkeyPatch) -> None: + """The checklist reviewer (`AGENT_REVIEWER_MODE=checklist`) with the rest of the runtime conftest's flow.""" + for name, value in CHECKLIST_FLOW.items(): + monkeypatch.setenv(name, value) + get_settings.cache_clear() + + +@pytest.mark.usefixtures("reviewer_checks_texts", "checklist_flow") +@pytest.mark.parametrize( + ("answers", "plans", "status", "rounds"), + [ + pytest.param([VERDICT_OK], [SCATTER_PLAN], "ok", 1, id="confirmed"), + pytest.param([CHECKLIST_MISS, VERDICT_OK], [SCATTER_PLAN, SECOND_PLAN], "ok", 2, id="miss-repaired"), + # A rejection without defects is no verdict (`Verdict._ok_matches_defects`). + pytest.param([{"ok": False}, {"ok": False}], [SCATTER_PLAN], "needs_attention", 1, id="unreadable"), + ], +) +async def test_without_drawn_texts_the_checklist_reviewer_answers_for_the_texts( + client: httpx.AsyncClient, + swap_models, + answers: list[dict[str, Any]], + plans: list[dict[str, Any]], + status: str, + rounds: int, +) -> None: + """The checklist path end to end (Copilot's review of #12454): its `Verdict` defects are read as the catalogue + reviewer's lines, a miss goes to the repair, and an answer that is not a verdict keeps the note.""" + script = default_script(plans=plans) + ask_for(script, TEMPERATURE) + script["reviewer"] = [{"json": answer} for answer in answers] + fake = swap_models("gemini", script) + sid = await open_session(client) + + events = await create_plot(client, sid) + + plot = next(data for name, data in events if name == "plot") + assert (plot["status"], len(plot["rounds"])) == (status, rounds) + reviews = reviewer_inputs(fake) + assert reviews and all(f"{REVIEW_EXPECTED_HEADING.format(ids='SC-04')}\n" in text for text in reviews) + if rounds == 2: + assert "does not show the requested text" in adapter_inputs(fake)[1] # the repair gets the reviewer's line + unverified = UNVERIFIED_LINE.format(theme="light") + assert (unverified in plot["residual_defects"]) is (status == "needs_attention") diff --git a/tests/unit/agents/runtime/test_grounding.py b/tests/unit/agents/runtime/test_grounding.py index 53744822130..a320790987c 100644 --- a/tests/unit/agents/runtime/test_grounding.py +++ b/tests/unit/agents/runtime/test_grounding.py @@ -34,19 +34,23 @@ from agents.anyplot.dev_fixture import snapshot_from_repo from agents.anyplot.grounding import ( GROUP_LIMITS, + REQUESTED_MARK, UNVERIFIED_LINE, drawn_texts, expected_misses, format_unit, plot_texts, + reviewed_misses, ) from agents.anyplot.pipeline import ( CONTEXT_HEADING, EXPECTED_HEADING, + REVIEW_EXPECTED_HEADING, Candidate, Run, finish, render_adapt_request, + render_review_request, render_score_request, ) from agents.anyplot.plugins.tool_safety import ( @@ -381,6 +385,29 @@ def test_requested_texts_without_drawn_texts_are_a_note_not_a_pass(self) -> None assert (result.status, result.residual_defects) == ("needs_attention", [note]) assert result.shows is not None and result.shows.requested_not_found == [] + def test_requested_texts_the_review_confirmed_need_no_note(self) -> None: + """A render without drawn texts whose review answered and named no miss (R, live test of 2026-10-29).""" + result = finish(run_shipping(expect_review=True, reviewed_ok=True, status_ok=True)) + + assert (result.status, result.residual_defects) == ("ok", []) + assert result.shows is not None and (result.shows.texts, result.shows.requested_not_found) == (None, []) + + def test_a_requested_text_the_review_missed_is_its_line(self) -> None: + line = 'SC-04 (light): the plot does not show the requested text "Temperatur (°C)" → write it. Likely cause: x.' + + result = finish( + run_shipping( + expect_review=True, + expect_lines=[line], + expect_misses=["Temperatur (°C)"], + reviewed_ok=False, + status_ok=True, + ) + ) + + assert (result.status, result.residual_defects) == ("needs_attention", [line]) + assert result.shows is not None and result.shows.requested_not_found == ["Temperatur (°C)"] + @pytest.mark.parametrize("char", ["x", "ä"]) def test_the_tool_cap_holds_a_result_at_its_schema_maximum(self, char: str) -> None: """Every list at its maximum length, every text at its character cap, in one- and two-byte UTF-8.""" @@ -443,6 +470,69 @@ def test_expected_texts_are_bounded_tool_input(self) -> None: PipelineArgs(expected_texts=["x" * 121]) +class TestReviewedMisses: + """The review's own lines for a render without drawn texts (`grounding.reviewed_misses`).""" + + MISS = 'SC-04 (light): the plot does not show the requested text "{}" (it reads "Temp") → add it. Likely cause: x.' + + def test_a_line_names_the_text_it_quotes(self) -> None: + line = self.MISS.format("Temperatur (°C)") + + assert reviewed_misses(["Temperatur (°C)", "Messreihe 2024"], [line]) == [("Temperatur (°C)", line)] + + def test_a_quoted_longer_text_is_no_miss_of_a_shorter_one(self) -> None: + assert reviewed_misses(["Temperatur"], [self.MISS.format("Temperatur (°C)")]) == [] + + def test_typographic_quotes_case_and_spacing_are_aside(self) -> None: + line = "SC-04 (dark): the title „umsatz NACH filiale“ is missing → add it. Likely cause: ggtitle()." + + assert reviewed_misses(["Umsatz nach Filiale"], [line]) == [("Umsatz nach Filiale", line)] + + def test_a_number_format_is_named_under_sc02_and_a_line_without_quotes_by_its_text(self) -> None: + line = "SC-02 (light): the y tick labels read 0.12 instead of 12 % → percent labels. Likely cause: scale." + + assert reviewed_misses(["12 %"], [line]) == [("12 %", line)] + + def test_a_cited_text_with_quotes_of_its_own_is_named_whole(self) -> None: + line = self.MISS.format('Zone "A"') + + assert reviewed_misses(['Zone "A"', "Zone"], [line]) == [('Zone "A"', line)] + + def test_a_passage_quoted_for_what_the_image_shows_names_nothing(self) -> None: + line = 'SC-04 (light): the plot does not show the requested text "Temperatur (°C)" (the y axis reads "Score")' + + assert reviewed_misses(["Score", "Temperatur (°C)"], [line]) == [("Temperatur (°C)", line)] + + def test_a_number_format_is_named_when_the_line_quotes_only_the_number_it_saw(self) -> None: + line = 'SC-02 (light): the y tick labels read "0.12" instead of 12 % → percent labels. Likely cause: scale.' + + assert reviewed_misses(["12 %"], [line]) == [("12 %", line)] + + def test_the_heading_prescribes_the_form_the_lines_are_read_in(self) -> None: + assert 'requested text ""' in REVIEW_EXPECTED_HEADING + assert REQUESTED_MARK == 'requested text "' + + def test_lines_under_other_ids_and_lines_outside_the_grammar_name_nothing(self) -> None: + lines = [ + 'VQ-01 (light): the label "Temperatur (°C)" is faint → INK. Likely cause: the colour.', + 'the plot does not show the requested text "Temperatur (°C)"', + ] + + assert reviewed_misses(["Temperatur (°C)"], lines) == [] + + def test_the_reviewer_gets_the_texts_only_when_the_request_carries_them(self) -> None: + request = ReviewRequest(render_id="r1", code="x = 1", bindings=[], profile_summary="2 rows", spec_brief="b") + texted = request.model_copy(update={"expected_texts": ["Temperatur (°C)", "12 %"]}) + fenced = "\nTemperatur (°C)\n12 %\n" + + catalogue, checklist = render_score_request(texted), render_review_request(texted) + + assert f"{REVIEW_EXPECTED_HEADING.format(ids='SC-04 (SC-02 for a number format)')}\n\n{fenced}" in catalogue + assert f"{REVIEW_EXPECTED_HEADING.format(ids='SC-04')}\n{fenced}" in checklist + for text in (render_score_request(request), render_review_request(request)): + assert "Temperatur" not in text and REVIEW_EXPECTED_HEADING[:60] not in text + + class TestHarnessLabels: def test_the_probe_reports_the_drawn_texts_by_role(self) -> None: fig = Figure(figsize=(16, 9)) diff --git a/tests/unit/agents/runtime/test_runtimes.py b/tests/unit/agents/runtime/test_runtimes.py index c2eaeb85ef5..5f1742537b6 100644 --- a/tests/unit/agents/runtime/test_runtimes.py +++ b/tests/unit/agents/runtime/test_runtimes.py @@ -44,6 +44,8 @@ def test_a_python_runtime_is_bound_to_its_library(self, library: str) -> None: assert runtime.file_name == "plot.py" and runtime.code_artifact == "plot.py" in CODE_ARTIFACTS assert runtime.placeholder == PLACEHOLDER and runtime.cpu_seconds == 45 and runtime.render_p95_s == 15.0 assert runtime.blocking_rules == BLOCKING_RULES and runtime.adaptation_targets == PYTHON_TARGETS + # matplotlib's probe reports the drawn texts; the libraries that render without it hand them to the review + assert runtime.review_texts is (library in {"altair", "pygal", "letsplot"}) assert has_runtime(library) def test_the_r_runtime_is_bound_to_ggplot2(self) -> None: @@ -56,6 +58,7 @@ def test_the_r_runtime_is_bound_to_ggplot2(self) -> None: assert runtime.cpu_seconds == 45 and runtime.render_p95_s == 15.0 assert runtime.blocking_rules == BLOCKING_RULES | {"dataset-package"} assert set(runtime.adaptation_targets) == set(PYTHON_TARGETS) + assert runtime.review_texts is True # no probe: the review finds the requested texts in the image assert has_runtime("ggplot2") with pytest.raises(ValueError, match="python library"): RRuntime(library="matplotlib")