diff --git a/agents/README.md b/agents/README.md index e23a6f3b00..4624396224 100644 --- a/agents/README.md +++ b/agents/README.md @@ -58,7 +58,7 @@ Three rules hold for everything here: | Variable | Default | Meaning | |---|---|---| | `AGENT_PROVIDER` | `anthropic-vertex` | `anthropic-vertex` (Claude on Vertex AI) or `gemini` | -| `AGENT_MODEL` | `claude-haiku-5-5` | Model of every agent; a `gemini-` id, or for `anthropic-vertex` a model in `CLAUDE_MODELS` (forced tool use with thinking disabled) | +| `AGENT_MODEL` | `claude-haiku-5-5` | Model of every agent; a `gemini-` id, or for `anthropic-vertex` a model in `CLAUDE_MODELS` (accepts disabled thinking) | | `AGENT_JUDGE_MODEL` | `claude-haiku-5-5` | Model of the scope and dataset judge; same rule | | `AGENT_LOCATION` | `eu` | Vertex AI location: `eu`, `us` or `global` | | `AGENT_PROJECT`, else `GOOGLE_CLOUD_PROJECT` | `anyplot` | Project that serves and bills the model calls | @@ -73,6 +73,8 @@ Three rules hold for everything here: | `AGENT_RUNS_PER_MINUTE` | `1` | Runs that may start within any 60 seconds (a sliding window) | | `AGENT_QUEUE_MAX_WAIT_S` | `600` | Longest wait in the run queue, after which the run ends with `capacity`; the queue holds rate x wait / 60 entries (10) and answers `503 capacity` beyond that. Through the BFF the whole turn, this wait included, ends by `AGENT_TURN_MAX_S` (890 s, see `docs/reference/api.md`), which the full wait plus the 180 s run fits into | | `AGENT_MAX_ATTEMPTS` | `2` | Adapter attempts per pipeline run, 1 to 3: the first and at most one repair round by default; the eval harness sets 1 or 3 (`--max-attempts`) to measure no repair or a second repair round | +| `AGENT_ROOT_THINKING_BUDGET` | `0` | For measurements, Claude only: 0 keeps the root's thinking off; 1 to 4,096 turns on adaptive thinking and adds that many tokens to the root's output cap | +| `AGENT_ADAPTER_THINKING_BUDGET` | `0` | The same for the adapter; the eval harness sets both with `--thinking-budget ROOT,ADAPTER` | | `AGENT_MAX_LLM_CALLS` | `12` | LLM calls per request | | `AGENT_REQUEST_TOKEN_BUDGET` | `160000` | Cost-weighted tokens per request (see [Token budgets and output caps](#token-budgets-and-output-caps)) | | `AGENT_DAILY_TOKEN_BUDGET` | `2000000` | Cost-weighted tokens per user and day | @@ -117,6 +119,8 @@ The output caps per model call are constants in `anyplot/models.py`, set with am On Gemini, thinking tokens count toward the cap, and the soft deadline bounds the adapter's caps: at spike X's fitted rate, a call that uses all 8,192 tokens still leaves the repair attempt its 75 s. Gemini's answers are larger: its plan calls in spike X needed a median of about 4,700 output tokens, and 11 of 112 needed more than 8,192. Claude Haiku 5.5 runs at about 0.0029 s per output token, so a call that uses all 16,384 takes about 49 s, inside the 60 s the deadline check reserves for an adapter call. +Claude runs with thinking off. A thinking measurement (`AGENT_ROOT_THINKING_BUDGET`, `AGENT_ADAPTER_THINKING_BUDGET`) turns on adaptive thinking for the root or the adapter and adds its budget to that agent's caps in this table, so the answer keeps its whole headroom however much the thinking uses. Haiku 5.5 takes no thinking budget of its own, so the value is room, not a limit the model sees, and its depth follows the agent's effort. The largest budget, 4,096, keeps the adapter's full-file cap at 20,480, below the Anthropic SDK's limit for a request that does not stream (about 21,300). + ## Run it locally ### Before you begin @@ -365,6 +369,7 @@ Before the first case the harness renders the catalogue file of the first case o | `--static` | Run only the model-free static stage and write `-static.json` instead of a matrix report (see [Run the stress cases](#run-the-stress-cases)); the renderer is `fake` unless `--renderer` or `AGENT_RENDERER` names another | | `--repeats N` | Runs per case, to see how stable a result is | | `--max-attempts N` | Adapter attempts per run, 1 to 3 (`AGENT_MAX_ATTEMPTS`, 2 by default), so 1 measures the first attempt alone and 3 a second repair round; the report's stamp records the value as `max_attempts` | +| `--thinking-budget ROOT,ADAPTER` | Claude only: adaptive thinking for the root and the adapter, each value (0 to 4,096, 0 off) added to that agent's output cap (`AGENT_ROOT_THINKING_BUDGET`, `AGENT_ADAPTER_THINKING_BUDGET`); the stamp records it as `thinking_budget`, the summary names it, and the thinking tokens are priced as output | | `--budget-usd X` | Stop before a case that could take the estimated list-price cost past X (the spend so far plus the costliest case so far), or once the cost has passed X (exit code 3) | | `--out DIR` | Output directory, `agents/evals/reports/` by default (git-ignored) | | `--baseline FILE`, `--no-baseline` | The report to compare with; by default `evals/baselines/claude-haiku-5-5.json` when it exists | @@ -379,7 +384,7 @@ For its own process the harness lifts the run queue's start rate and the service A run writes to `--out`: -- `-.json`: the report. It holds the stamp (provider, models, location, attempt bound, renderer, ADK version, commit, prompt hashes, prices), the summary, and one record per case and repeat: status and reason, the exception class when the pipeline ended in an error, attempts, gate failures by gate, validator rejections, edit-apply failures, the reviewer's verdict, LLM calls by agent, tokens by kind, `model_version`, the cost at list price, time to the first event, end-to-end time and render times. Since report schema 2, each record also says where the run stopped (`stage`, such as `adapter_truncated`, `edit_apply` or `reviewer_defects`, and `stages` per attempt), which attempt shipped and which the reviewer saw last, an `attempt_log` with each attempt's adapter outcome, finish reason and plan shape, every model call's finish reason and tokens, and the answers that failed their schema, by agent kind and outcome (`answer_outcomes`: `repaired` or `refused`) and by broken rule (`answer_rules`, such as `defects.*.observed:string_too_long`); a schema-1 baseline still loads, and a report without the answer counts reads as none. The date is the UTC date. +- `-.json`: the report. It holds the stamp (provider, models, location, attempt bound, thinking budgets, renderer, ADK version, commit, prompt hashes, prices), the summary, and one record per case and repeat: status and reason, the exception class when the pipeline ended in an error, attempts, gate failures by gate, validator rejections, edit-apply failures, the reviewer's verdict, LLM calls by agent, tokens by kind, `model_version`, the cost at list price, time to the first event, end-to-end time and render times. Since report schema 2, each record also says where the run stopped (`stage`, such as `adapter_truncated`, `edit_apply` or `reviewer_defects`, and `stages` per attempt), which attempt shipped and which the reviewer saw last, an `attempt_log` with each attempt's adapter outcome, finish reason and plan shape, every model call's finish reason and tokens, and the answers that failed their schema, by agent kind and outcome (`answer_outcomes`: `repaired` or `refused`) and by broken rule (`answer_rules`, such as `defects.*.observed:string_too_long`), the edits each tolerant matching tier applied (`edit_tolerant`), and the labels of the imports the validator banned (`banned_imports`, such as `io` or `pandas.io`: the longest prefix of the path in a closed vocabulary of module names, anything else `?`, so no word the model writes as an import reaches the log); a schema-1 baseline still loads, and a report without these counts reads as none. The date is the UTC date. - `-.md`: the Markdown summary, with pass rates by perturbation, library and spec, and the diff against the baseline. - `renders/`, `gallery.html` and `gallery-all.html`: the shipped PNG of every run, a review page with 30 renders sampled at random from the runs that passed, and a list of every run. On the review page, tick **Accept** or **Reject** for each plot and select **Export judgements as JSON**; the export counts your accepts and rejects. The page never names the model, but its folder may, so compare two arms with the blind gallery instead (see [Run spike X](#run-spike-x)). The next run in the same directory rewrites both pages, so give each run its own `--out` when you want to keep its gallery. diff --git a/agents/anyplot/code/edits.py b/agents/anyplot/code/edits.py index bb7ace7a2f..5e1cfeb06f 100644 --- a/agents/anyplot/code/edits.py +++ b/agents/anyplot/code/edits.py @@ -1,16 +1,41 @@ """The edit applier: an `AdaptPlan` applied to the working form, or the reasons it cannot be. Edits apply in order, each to the text the previous ones produced. An edit's `find` -must occur exactly once in that text (overlapping occurrences count, so `"aa"` in -`"aaa"` is two) and may not overlap a protected region: +must match exactly once in that text and may not overlap a protected region: * the `THEME = os.getenv("ANYPLOT_THEME", ...)` assignment, -* every module-level assignment whose value tests `THEME == ...` or `THEME != ...` - (`PAGE_BG`, `ELEVATED_BG`, `INK*`, `GRID`, `BRAND`, ...), +* every theme token: a module-level assignment that picks a fixed value per theme + (`PAGE_BG`, `ELEVATED_BG`, `INK*`, `GRID`, `BRAND`, ...; see `regions.py` for the + exact rule, which leaves a data-dependent `x = INK if THEME == "dark" else ...` to + the adapter), * the placeholder line `df = load_user_data()` once it is present, including one an earlier edit of the same plan introduced, * the final savefig statement. +**Matching** goes from strict to tolerant, Aider style, and never fuzzy: the first +tier that finds `find` at all decides, and more than one match there is a failure, so +a looser tier never picks among the candidates of a stricter one. + +1. `exact`: `find` as a substring (overlapping occurrences count, so `"aa"` in + `"aaa"` is two). +2. `trailing_whitespace`: spaces and tabs before each line break ignored on both + sides, and `\\r\\n` or `\\r` read as `\\n`; a `find` may still start or end inside + a line. +3. `whitespace`: whole lines whose words agree once indentation and runs of blanks + outside string literals are ignored. A literal's text must agree exactly, so + `"North America"` never matches `"North America"`, and a line that starts inside a + multi-line literal is never matched this way. The replacement is re-indented by + the one indentation the code has over `find` (or `find` over the code); when the + lines do not share one such shift, the tier finds nothing, because Python's + indentation is meaning. + +A `find` that matches nothing now but exactly once in the code before the plan, in +any tier, failed because an earlier edit of the same plan changed that code +(`stale_match`), and its repair line says so. + +`AppliedPlan.tolerant` names the tier of every edit a tolerant tier applied, so the +attribution log shows how often they help. + A protected region covers the whole lines of its statement, so an edit that only appends a comment to such a line is refused too; an edit may end right before one or start right after one. The `IMPRINT` palette list is not protected: the adapter may @@ -19,9 +44,10 @@ problem; the plan as a whole then yields no code. `full_code` replaces everything; the caller decides whether a full file is allowed (from attempt 2 on), the applier only applies it. After either kind of plan, `protected_drift` compares the result with the -base: the protected statements must keep their exact text, and THEME and the theme -tokens may not be bound again anywhere (a later `PAGE_BG = ...` passes the overlap -check above but would recolour the plot). +base: the protected statements must keep their meaning (their syntax tree, so a +re-stated token with the same value in other quotes, spacing or comment passes), and +THEME and the theme tokens may not be bound again anywhere (a later `PAGE_BG = ...` +passes the overlap check above but would recolour the plot). Failure lines are repair feedback (`Line`): one line each, at most `MAX_LINE_CHARS` characters, quoting at most `MAX_QUOTE_CHARS` characters of code. Next to them, @@ -33,9 +59,10 @@ import re from collections import Counter from dataclasses import dataclass, field +from typing import Literal from ..schemas import MAX_FEEDBACK, MAX_LINE_CHARS, AdaptPlan -from .regions import PLACEHOLDER_FUNC, THEME_NAME, Region, SourceIndex, Span, find_regions +from .regions import PLACEHOLDER, PLACEHOLDER_FUNC, THEME_NAME, Region, SourceIndex, Span, find_regions MAX_QUOTE_CHARS = 80 @@ -53,6 +80,8 @@ _PLACEHOLDER_LINE = re.compile( rf"^[ \t]*df[ \t]*=[ \t]*{PLACEHOLDER_FUNC}\(\)[ \t]*(?:#[^\r\n]*)?(?:\r\n|\r|\n|$)", re.M ) +_NEWLINE = re.compile(r"\r\n|\r|\n") +_LINE_END_BLANKS = " \t" _KIND_NAMES = { "theme": "THEME assignment", @@ -63,6 +92,7 @@ FAILURE_KINDS: frozenset[str] = frozenset( { "zero_match", + "stale_match", "multi_match", *(f"protected:{kind}" for kind in _KIND_NAMES), "drift:theme", @@ -72,9 +102,14 @@ "working_unparseable", } ) -"""The content-free kind of every failure line: no match, several matches, an overlap with -a protected region (by region kind), a protected statement changed or a theme name bound -again after the plan applied (`drift:*`), or a working form that does not parse.""" +"""The content-free kind of every failure line: no match, a match only in the code before an +earlier edit of the same plan changed it (`stale_match`), several matches, an overlap with a +protected region (by region kind), a protected statement changed or a theme name bound again +after the plan applied (`drift:*`), or a working form that does not parse.""" + +MatchTier = Literal["exact", "trailing_whitespace", "whitespace"] +TOLERANT_TIERS: tuple[MatchTier, ...] = ("trailing_whitespace", "whitespace") +"""The tiers after `exact`, in the order they are tried (the module docstring, "Matching").""" @dataclass(frozen=True, slots=True) @@ -83,12 +118,15 @@ class AppliedPlan: `kinds` holds the `FAILURE_KINDS` entry of every failure, the ones past the `MAX_FEEDBACK` lines included; trailer lines (the drift reminder, the overflow - count) have none. + count) have none. `tolerant` names the `TOLERANT_TIERS` entry of every edit that + applied through a tolerant tier, in plan order, also when another edit of the + plan failed or the plan drifted, so the log counts every match a tier found. """ code: str | None failures: list[str] = field(default_factory=list) kinds: tuple[str, ...] = () + tolerant: tuple[str, ...] = () def apply_plan(working: str, plan: AdaptPlan) -> AppliedPlan: @@ -100,7 +138,7 @@ def apply_plan(working: str, plan: AdaptPlan) -> AppliedPlan: if not drift: return applied lines = _with_drift_reminder([line for _, line in drift]) - return AppliedPlan(None, lines[:MAX_FEEDBACK], tuple(kind for kind, _ in drift)) + return AppliedPlan(None, lines[:MAX_FEEDBACK], tuple(kind for kind, _ in drift), applied.tolerant) def protected_drift(base: str, code: str) -> list[str]: @@ -108,11 +146,13 @@ def protected_drift(base: str, code: str) -> list[str]: The text checks per edit cannot see a `full_code` plan, nor an edit that adds `PAGE_BG = "#FF00FF"` further down. So after every plan the THEME assignment, each - theme token assignment and the final savefig statement must keep their exact - source text, and THEME and the token names may be bound no more often than in - `base` (assignment, augmented assignment, `del`, `global`, import alias, function, - class, parameter, loop or `with` target, walrus, `except ... as`, match capture). - A `code` that does not parse is left to the validator. + theme token assignment and the final savefig statement must keep their meaning, + compared as syntax trees: a full file that re-states a token with the same value + in other quotes, spacing, line breaks or comment passes, any other value does not. + THEME and the token names may be bound no more often than in `base` (assignment, + augmented assignment, `del`, `global`, import alias, function, class, parameter, + loop or `with` target, walrus, `except ... as`, match capture). A `code` that does + not parse is left to the validator. """ return _with_drift_reminder([line for _, line in _drift(base, code)]) @@ -134,22 +174,23 @@ def _drift(base: str, code: str) -> list[tuple[str, str]]: before, after = find_regions(base, before_tree), find_regions(code, after_tree) failures: list[tuple[str, str]] = [] - def text(source: str, span: Span) -> str: - return source[span.start : span.end].strip() - if before.theme is not None and ( - after.theme is None or text(base, before.theme.span) != text(code, after.theme.span) + after.theme is None or _meaning(base, before.theme.span) != _meaning(code, after.theme.span) ): failures.append(("drift:theme", "the plan changes or removes the protected THEME assignment")) - after_tokens = {region.name: text(code, region.span) for region in after.theme_tokens} + after_tokens = {region.name: _meaning(code, region.span) for region in after.theme_tokens} for region in before.theme_tokens: - if after_tokens.get(region.name) != text(base, region.span): + if after_tokens.get(region.name) != _meaning(base, region.span): failures.append( - ("drift:theme_token", f"the plan changes or removes the protected theme token {region.name}") + ( + "drift:theme_token", + f"the plan changes or removes the protected theme token {region.name}; " + "copy its line unchanged from the current code", + ) ) if before.savefig is not None and before.savefig.statement is not None: after_savefig = after.savefig.statement if after.savefig is not None else None - if after_savefig is None or text(base, before.savefig.statement) != text(code, after_savefig): + if after_savefig is None or _meaning(base, before.savefig.statement) != _meaning(code, after_savefig): failures.append(("drift:savefig", "the plan changes or removes the protected final savefig statement")) names = {THEME_NAME, *(name for region in before.theme_tokens for name in region.name.split(", "))} @@ -161,6 +202,19 @@ def text(source: str, span: Span) -> str: return [(kind, _line(failure)) for kind, failure in failures] +def _meaning(source: str, span: Span) -> str: + """A protected statement as its syntax tree without positions: comments, spacing and quotes do not count. + + Every protected region is a whole module-level statement, so its lines parse on + their own; the stripped text stands in should they ever not. + """ + text = source[span.start : span.end] + try: + return ast.dump(ast.parse(text)) + except SyntaxError: + return text.strip() + + def _bindings(tree: ast.Module, names: set[str]) -> Counter[str]: """How often each of `names` is bound or deleted anywhere in `tree`.""" counts: Counter[str] = Counter() @@ -231,23 +285,29 @@ def _apply(working: str, plan: AdaptPlan) -> AppliedPlan: text = working failures: list[str] = [] kinds: list[str] = [] + tolerant: list[str] = [] total = len(plan.edits) for number, edit in enumerate(plan.edits, start=1): label = f"edit {number}/{total}" - matches = _occurrences(text, edit.find) + tier, matches = _locate(text, edit.find, edit.replace) + if not matches and text != working and len(_locate(working, edit.find, edit.replace)[1]) == 1: + failures.append(_stale_failure(label, edit.find)) + kinds.append("stale_match") + continue if len(matches) != 1: failures.append(_count_failure(label, len(matches), edit.find)) kinds.append("zero_match" if not matches else "multi_match") continue - start = matches[0] - end = start + len(edit.find) + start, end, replace = matches[0] hit = next((r for r in [*protected, *_placeholders(text)] if r.span.overlaps(start, end)), None) if hit is not None: failures.append(_protected_failure(label, text, start, end, hit)) kinds.append(f"protected:{hit.kind}") continue - text = text[:start] + edit.replace + text[end:] - protected = _shift(protected, end, len(edit.replace) - (end - start)) + text = text[:start] + replace + text[end:] + protected = _shift(protected, end, len(replace) - (end - start)) + if tier != "exact": + tolerant.append(tier) if len(failures) > MAX_FEEDBACK: hidden = len(failures) - (MAX_FEEDBACK - 1) @@ -255,7 +315,22 @@ def _apply(working: str, plan: AdaptPlan) -> AppliedPlan: *failures[: MAX_FEEDBACK - 1], f"{hidden} more edits failed in the same way; fix the ones above first", ] - return AppliedPlan(None if failures else text, failures, tuple(kinds)) + return AppliedPlan(None if failures else text, failures, tuple(kinds), tuple(tolerant)) + + +Match = tuple[int, int, str] +"""One place `find` matched: its start and end in the text, and the replacement to put there.""" + + +def _locate(text: str, find: str, replace: str) -> tuple[MatchTier, list[Match]]: + """The matches of `find` in the first tier that has any (module docstring, "Matching"), with that tier.""" + exact = [(start, start + len(find), replace) for start in _occurrences(text, find)] + if exact: + return "exact", exact + trailing = _trailing_whitespace_matches(text, find, replace) + if trailing: + return "trailing_whitespace", trailing + return "whitespace", _whitespace_matches(text, find, replace) def _occurrences(text: str, find: str) -> list[int]: @@ -268,6 +343,180 @@ def _occurrences(text: str, find: str) -> list[int]: return found +def _line_spans(source: str) -> list[tuple[int, int, int]]: + """Each line as `(start, content_end, end)`: `content_end` before its line break, `end` after it.""" + spans: list[tuple[int, int, int]] = [] + start = 0 + for match in _NEWLINE.finditer(source): + spans.append((start, match.start(), match.end())) + start = match.end() + spans.append((start, len(source), len(source))) + return spans + + +def _without_line_end_blanks(source: str) -> tuple[str, list[int], list[int]]: + """`source` without the spaces and tabs before each line break and with every break written `\\n`. + + Also returns, per kept character, its start and end offset in `source`, so a match + in the result maps back to the exact span it covers there (a `\\r\\n` is one kept + character two source characters wide). + """ + kept: list[str] = [] + starts: list[int] = [] + ends: list[int] = [] + for start, content_end, end in _line_spans(source): + content = source[start:content_end].rstrip(_LINE_END_BLANKS) + kept.append(content) + starts.extend(range(start, start + len(content))) + ends.extend(range(start + 1, start + len(content) + 1)) + if end > content_end: + kept.append("\n") + starts.append(content_end) + ends.append(end) + return "".join(kept), starts, ends + + +def _trailing_whitespace_matches(text: str, find: str, replace: str) -> list[Match]: + """Tier 2: the matches once blanks before line breaks are gone and line breaks are `\\n` on both sides.""" + normal_find = _without_line_end_blanks(find)[0] + if not normal_find.strip(): + return [] + normal_text, starts, ends = _without_line_end_blanks(text) + return [ + (starts[position], ends[position + len(normal_find) - 1], replace) + for position in _occurrences(normal_text, normal_find) + ] + + +def _whitespace_matches(text: str, find: str, replace: str) -> list[Match]: + """Tier 3: whole lines whose words agree, with `replace` moved by the one indentation shift between them.""" + find_lines = _NEWLINE.split(find) + ends_with_break = len(find_lines) > 1 and find_lines[-1] == "" + if ends_with_break: + find_lines.pop() + find_words, _ = _line_words(find_lines) + if not any(find_words): + return [] + text_lines = _line_spans(text) + text_words, in_literal = _line_words([text[start:content_end] for start, content_end, _ in text_lines]) + count = len(find_lines) + matches: list[Match] = [] + for first in range(len(text_lines) - count + 1): + if text_words[first : first + count] != find_words or any(in_literal[first : first + count]): + continue + window = text_lines[first : first + count] + indents = [ + (_indent(text[start:content_end]), _indent(line)) + for (start, content_end, _), line, words in zip(window, find_lines, find_words, strict=True) + if words + ] + moved = _reindent(replace, indents) + if moved is None: + continue + last_start, last_content_end, last_end = window[-1] + end = last_end if ends_with_break else last_content_end + if ends_with_break and last_end == last_content_end: + continue # find ends with a line break that the code's last line does not have + matches.append((window[0][0], end, moved)) + if len(matches) >= MAX_COUNTED_MATCHES: + break + return matches + + +def _line_words(lines: list[str]) -> tuple[list[str], list[bool]]: + """Each line's words for the whitespace tier, and whether the line starts inside a string literal. + + Outside string literals, indentation and trailing blanks go and every run of blanks + becomes one space, as `" ".join(line.split())` would; a comment collapses the same + way. A literal's text, quotes included, stays verbatim, so two lines that differ in + the blanks inside a string do not agree. The scan carries an open literal from one + line to the next (a triple-quoted one, or a quoted one continued by a backslash), + which marks the lines that start inside it. + """ + words: list[str] = [] + starts_inside: list[bool] = [] + quote: str | None = None # the open literal's delimiter: ', ", ''' or """ + for line in lines: + starts_inside.append(quote is not None) + kept: list[str] = [] + blank = False + position = 0 + while position < len(line): + char = line[position] + if quote is not None: + if char == "\\": + kept.append(line[position : position + 2]) + position += 2 + elif line.startswith(quote, position): + kept.append(quote) + position += len(quote) + quote = None + else: + kept.append(char) + position += 1 + continue + if char.isspace(): + blank = True + position += 1 + continue + if blank and kept: + kept.append(" ") + blank = False + if char == "#": + kept.append(" ".join(line[position:].split())) + break + if char in "'\"": + quote = char * 3 if line.startswith(char * 3, position) else char + kept.append(quote) + position += len(quote) + continue + kept.append(char) + position += 1 + if quote is not None and len(quote) == 1 and not line.endswith("\\"): + quote = None # a one-quote literal ends with its line unless a backslash continues it + words.append("".join(kept)) + return words, starts_inside + + +def _indent(line: str) -> str: + return line[: len(line) - len(line.lstrip(" \t"))] + + +def _reindent(replace: str, indents: list[tuple[str, str]]) -> str | None: + """`replace` shifted by the indentation the code has over `find` (or `find` over the code). + + `indents` pairs each non-blank matched line's indentation in the code with its + indentation in `find`. They must share one shift (the code's indentation is a + prefix plus `find`'s, or `find`'s a prefix plus the code's, with the same prefix + on every line); otherwise, or when a line of `replace` lacks the indentation a + shift would remove, there is no safe placement and the match is not used. + """ + shifts: set[tuple[str, str]] = set() + for code_indent, find_indent in indents: + if code_indent.endswith(find_indent): + shifts.add(("add", code_indent[: len(code_indent) - len(find_indent)])) + elif find_indent.endswith(code_indent): + shifts.add(("remove", find_indent[: len(find_indent) - len(code_indent)])) + else: + return None + if len(shifts) != 1: + return None + direction, prefix = shifts.pop() + if not prefix: + return replace + moved: list[str] = [] + for line in (replace[start:end] for start, _, end in _line_spans(replace) if end > start): + if not line.strip(): + moved.append(line) + elif direction == "add": + moved.append(prefix + line) + elif line.startswith(prefix): + moved.append(line[len(prefix) :]) + else: + return None + return "".join(moved) + + def _placeholders(text: str) -> list[Region]: index = SourceIndex(text) regions = [] @@ -308,13 +557,25 @@ def _count_failure(label: str, count: int, find: str) -> str: ) +def _stale_failure(label: str, find: str) -> str: + return _line( + f"{label}: find matches the code only as it was before an earlier edit of this plan changed it; " + "write each find against the code after the edits before it, or merge the two edits into one. " + f"find = {_quote(find)}" + ) + + def _protected_failure(label: str, text: str, start: int, end: int, region: Region) -> str: index = SourceIndex(text) first, last = index.line_of(start), index.line_of(max(start, end - 1)) where = f"line {first}" if first == last else f"lines {first}-{last}" - what = _KIND_NAMES[region.kind] - name = "" if region.kind in ("placeholder", "theme") else f" {region.name}" region_line = index.line_of(region.span.start) + if region.kind == "placeholder": + return _line( + f"{label}: find ({where}) overlaps {_placeholder_advice(index, region)} find = {_quote(text[start:end])}" + ) + what = _KIND_NAMES[region.kind] + name = "" if region.kind == "theme" else f" {region.name}" return _line( f"{label}: find ({where}) overlaps the protected {what}{name} at line {region_line}; " "the THEME assignment, the theme tokens, the df = load_user_data() line and the final savefig " @@ -322,6 +583,22 @@ def _protected_failure(label: str, text: str, start: int, end: int, region: Regi ) +def _placeholder_advice(index: SourceIndex, region: Region) -> str: + """How to work around the placeholder: name the line below it, which an insertion anchors on.""" + line = index.line_of(region.span.start) + below = next((number for number in range(line + 1, index.line_count + 1) if index.line_text(number).strip()), None) + head = ( + f"the protected {PLACEHOLDER} line at line {line}; keep that line exactly as it is and out of find and replace" + ) + if below is None: # never in a working form, where the final savefig follows + return f"{head}." + anchor = _quote(index.line_text(below).rstrip("\r\n")) + return ( + f"{head}; to change df or add statements after it, set find to line {below} ({anchor}) " + "and write the new statements in front of that line in replace." + ) + + def _line(text: str) -> str: """One feedback line of at most `MAX_LINE_CHARS` (quotes already escape their newlines).""" text = text.replace("\r", " ").replace("\n", " ") diff --git a/agents/anyplot/code/regions.py b/agents/anyplot/code/regions.py index f49d65f08c..357a5b0541 100644 --- a/agents/anyplot/code/regions.py +++ b/agents/anyplot/code/regions.py @@ -4,9 +4,15 @@ follows conventions that the adapter must keep and the normaliser rewrites: * the theme block: `THEME = os.getenv("ANYPLOT_THEME", "light")`, then module-level - tokens whose value tests the theme, such as + tokens that pick a fixed value per theme, such as `PAGE_BG = "#FAF8F1" if THEME == "light" else "#1A1A17"` (`PAGE_BG`, `ELEVATED_BG`, - `INK`, `INK_SOFT`, `INK_MUTED`, `GRID`, `BRAND`, ...); + `INK`, `INK_SOFT`, `INK_MUTED`, `GRID`, `BRAND`, ...). A token's value tests the + theme and is built only from literals, `THEME` and earlier tokens + (`EDGE = INK_SOFT if THEME == "light" else PAGE_BG` is one). An assignment whose + branch reads anything else, such as box-basic's + `_tight_text_color = INK if THEME == "dark" else IMPRINT_PALETTE[tightest_idx]`, is + plot code that follows the data, not a token: the adapter must be able to rewrite it + (in the 650 matplotlib and seaborn files it is the only one of 2,745); * the Imprint palette list, `IMPRINT = [...]` (124 files) or `IMPRINT_PALETTE = [...]` (89 files); * one final `plt.savefig(f"plot-{THEME}.png", dpi=..., facecolor=PAGE_BG)` statement @@ -269,6 +275,22 @@ def tests_theme(value: ast.expr) -> bool: ) +def is_theme_constant(value: ast.expr, known: set[str]) -> bool: + """Whether `value` is built only from literals and the names in `known` (THEME and earlier tokens). + + A conditional expression and a comparison are walked part by part; any other node + must be a literal (`ast.literal_eval` accepts it: strings, numbers, and lists, + tuples or dicts of them). + """ + if isinstance(value, ast.IfExp): + return all(is_theme_constant(part, known) for part in (value.test, value.body, value.orelse)) + if isinstance(value, ast.Compare): + return all(is_theme_constant(part, known) for part in (value.left, *value.comparators)) + if isinstance(value, ast.Name): + return value.id in known + return _literal(value)[1] + + def is_placeholder(node: ast.AST) -> bool: """The statement `df = load_user_data()`.""" return ( @@ -409,6 +431,7 @@ def find_regions(code: str, tree: ast.Module | None = None) -> Regions: theme: Region | None = None tokens: list[Region] = [] + known = {THEME_NAME} # the names a token's value may read: THEME and the tokens above it palettes: list[Palette] = [] imprint: Palette | None = None module_savefig: dict[int, Span] = {} # id(call) -> statement lines, for module-level savefig statements @@ -418,8 +441,10 @@ def find_regions(code: str, tree: ast.Module | None = None) -> Regions: continue names = _assigned_names(stmt) if names is not None and isinstance(stmt, ast.Assign | ast.AnnAssign) and stmt.value is not None: - if THEME_NAME not in names and tests_theme(stmt.value): + value = stmt.value + if THEME_NAME not in names and tests_theme(value) and is_theme_constant(value, known): tokens.append(Region("theme_token", ", ".join(names), index.lines_span(stmt))) + known.update(names) palette = _palette(index, stmt, names) if palette is not None: if imprint is None and palette.name in IMPRINT_NAMES: diff --git a/agents/anyplot/code/validate.py b/agents/anyplot/code/validate.py index e6525baf99..98b1f71e94 100644 --- a/agents/anyplot/code/validate.py +++ b/agents/anyplot/code/validate.py @@ -16,15 +16,41 @@ Decisions this module takes where the design leaves room: * **Import surface.** COMMON plus the science stack (`scipy.{stats, interpolate, signal, - cluster.hierarchy, spatial, optimize, special, integrate, ndimage}`, `sklearn` without - `datasets`, `statsmodels.{api, nonparametric, tsa, graphics}`) plus, for matplotlib - and seaborn, `matplotlib`, `seaborn` and the bundled `mpl_toolkits.{mplot3d, - axes_grid1, axisartist}`. The four extra scipy modules, `statsmodels.graphics` and - the toolkits were admitted after the readiness sweep showed them behind most of the - catalogue's benign import blocks; each was walked for file or network helpers and - has none. Third-party plotting helpers (cartopy, qrcode, wordcloud, matplotlib_venn, - squarify, adjustText) stay banned: they are not in the agents image, and admitting - one is an owner decision, not a validator default. + cluster.hierarchy, spatial, optimize, special, integrate, ndimage, fft, linalg, + constants}`, `sklearn` without `datasets`, `statsmodels.{api, nonparametric, tsa, + graphics}`) plus, for matplotlib and seaborn, `matplotlib`, `seaborn` and the bundled + `mpl_toolkits.{mplot3d, axes_grid1, axisartist}`. The four extra scipy modules, + `statsmodels.graphics` and the toolkits were admitted after the readiness sweep showed + them behind most of the catalogue's benign import blocks; each was walked for file or + network helpers and has none. The pure computing modules of the standard library + (`warnings`, `typing`, `calendar`, `decimal`, `fractions`, `zoneinfo`, `enum`, + `dataclasses`, `operator`, `bisect`, `heapq`, `numbers`, `cmath`) and + `scipy.{fft, linalg, constants}` were admitted by the owner (2026-10-10) for FFTs and + similar work; the walk behind them is at `COMMON_IMPORTS`, and the names it found are + banned below. Third-party plotting helpers (cartopy, qrcode, wordcloud, + matplotlib_venn, squarify, adjustText) stay banned: they are not in the agents image, + and admitting one is an owner decision, not a validator default. A `banned-import` + message names the module and what to use instead (`IMPORT_ALTERNATIVES`), and the + finding carries a label of the path for the attribution log (`Finding.module`): its + longest prefix in the closed vocabulary `LOGGED_MODULES`, else `?`. An import path + is code the model wrote, so a path outside the vocabulary (a word from the data + written as a module name) never reaches the log. +* **Re-exports and private names.** An allowed module can hand out a module the import + rules keep out (`statistics.sys`, `enum.bltns`, which is `builtins`, + `dataclasses.inspect`) or a private helper that runs `exec` (`dataclasses._FuncBuilder`). + So the names in `ESCAPE_ATTRIBUTES` are banned as attributes on any object and as + `from` imports, and a private name (one leading underscore) is banned on anything an + import bound, and as a `from` import. Both are banned as a component of an import + path below its root too, where an alias would hide them from the attribute check + (`import scipy.fft._backend as backend`). No catalogue file uses either. +* **A module is never a value.** A name bound by `import X [as Y]` may only be the + receiver of an attribute (`calendar.month_abbr`); as a value (`[calendar][0]`, + `f(dataclasses)`, `c = collections`) it would reach the attribute checks through an + expression the resolver cannot follow, so `c._locale` would pass the private-name + rule. No catalogue file uses a module as a value. A module reached through an + attribute chain (`[np.lib][0]`) or a `from` import cannot be told from a plain value + without importing it, so the dangerous private names the walk found are in + `ESCAPE_ATTRIBUTES` as well. * **Attribute names are banned outright, on any object.** `.format`, `.eval`, `.query`, `.show`, `.use`, `.load`, `.read_*`, `.to_csv` and the rest are findings wherever they appear, called or not, because the receiver's type is not knowable without executing @@ -93,8 +119,8 @@ * **Placeholder (ADAPTATION).** Exactly one `df = load_user_data()` statement, anywhere; any other use of the name `load_user_data` is a separate finding. -Rule ids: `size`, `encoding`, `syntax`, `banned-import`, `star-import`, -`relative-import`, `os-use`, `banned-name`, `banned-call`, `banned-attribute`, `dunder`, +Rule ids: `size`, `encoding`, `syntax`, `banned-import` (with `Finding.module`), `star-import`, +`relative-import`, `os-use`, `module-value`, `banned-name`, `banned-call`, `banned-attribute`, `dunder`, `banned-statement`, `url-literal`, `string-length`, `savefig-target` (SECURITY); `placeholder-count`, `placeholder-use`, `rng`, `literal-data`, `palette-prefix` (ADAPTATION). Findings are sorted by line; messages never echo more than about 60 @@ -105,8 +131,9 @@ import ast import re +import sys from collections import defaultdict -from collections.abc import Iterable +from collections.abc import Container, Iterable from dataclasses import dataclass from typing import Literal, TypeVar @@ -125,6 +152,32 @@ PLACEHOLDER = "df = load_user_data()" THEME_VARIABLE = "ANYPLOT_THEME" +# The pure standard modules from `warnings` on were admitted by the owner (2026-10-10). +# Each was walked for public names that read, load, save, open, fetch or evaluate, and +# for the modules it re-exports (Python 3.13.12): +# * warnings: `formatwarning`, `showwarning` and `warn_explicit` read a line of any file +# through `linecache` (the first returns it as text) or write to a file object, so +# they are in `BANNED_IO`; re-exports `sys`. +# * typing: `get_type_hints` and `ForwardRef` evaluate annotation strings, which is +# `eval`; re-exports `sys` and `types` (whose `CodeType` and `FunctionType` build +# code): all in `ESCAPE_ATTRIBUTES`. +# * calendar: `LocaleTextCalendar`, `LocaleHTMLCalendar` and `different_locale` set the +# process locale through `_locale.setlocale`, which the `locale` ban keeps out: in +# `ESCAPE_ATTRIBUTES`. `main` reaches them through its `-L` option, but a column may be +# called `main`, so it is banned on the calendar module only: in `MODULE_ESCAPES`. The +# rest prints to standard output only; re-exports `sys`, `datetime`, `_locale`. +# * zoneinfo: `reset_tzpath` points the time zone search at any directory, +# `available_timezones` lists it, `ZoneInfo.from_file` reads a file object: in +# `BANNED_IO`. `ZoneInfo(key)` reads the time zone database only and refuses keys +# that leave it. +# * enum: `bltns` is the `builtins` module and `global_enum` writes into a module's +# namespace: in `ESCAPE_ATTRIBUTES`; re-exports `sys`. +# * dataclasses: re-exports `inspect` (source reads) and `sys`; the private +# `_FuncBuilder` runs `exec` on text, which the private-name rule covers. +# * operator: `attrgetter` and `methodcaller` look attributes up by string, which is +# `getattr`: in `ESCAPE_ATTRIBUTES`. +# * decimal, fractions, numbers, cmath, bisect, heapq: nothing (fractions re-exports +# `sys`, `re`, `math`, `operator`, `numbers` and `functools`). COMMON_IMPORTS: frozenset[str] = frozenset( { "numpy", @@ -138,12 +191,28 @@ "textwrap", "colorsys", "re", + "warnings", + "typing", + "calendar", + "decimal", + "fractions", + "zoneinfo", + "enum", + "dataclasses", + "operator", + "bisect", + "heapq", + "numbers", + "cmath", } ) # Shared by every Python plotting library: the science stack minus its data loaders. # Every package here was walked for public names that read, load, save, open or fetch # (2026-10-09, scipy 1.18, statsmodels 0.14): none exist; `scipy.ndimage.imread` is -# long gone. `scipy.io` and `scipy.datasets` stay out. +# long gone. `scipy.io` and `scipy.datasets` stay out. `scipy.fft`, `scipy.linalg` and +# `scipy.constants` were walked on 2026-10-10 (scipy 1.18.1): computation only, the +# CODATA table of `scipy.constants` is parsed from a string inside the module, and the +# private submodules they re-export fall under the private-name rule. SHARED_SCIENCE_IMPORTS: frozenset[str] = frozenset( { "scipy.stats", @@ -155,6 +224,9 @@ "scipy.special", "scipy.integrate", "scipy.ndimage", + "scipy.fft", + "scipy.linalg", + "scipy.constants", "sklearn", "statsmodels.api", "statsmodels.nonparametric", @@ -238,8 +310,132 @@ "rc_file", "rc_params_from_file", "urlopen", + "formatwarning", + "showwarning", + "warn_explicit", + "reset_tzpath", + "available_timezones", + "from_file", } ) +# Banned as attributes on any object, as `from` imports and as an import path's component +# below its root: modules an allowed module re-exports that the import rules keep out +# (`statistics.sys.modules`, `enum.bltns`, `dataclasses.inspect`), and the reflection and +# the locale setters the stdlib walk found (see COMMON_IMPORTS). The private-name rule +# needs the import resolver, which an expression defeats (`[np.lib][0]`), so the private +# helpers a walk of the allowed modules found dangerous are banned here by name as well: +# the re-exported `sys` (collections) and `_locale` (calendar), the stdlib RNG +# (`statistics._random`), the time zone path and file readers (`zoneinfo._tzpath`, +# `_common`), import by name (`warnings._getcategory`), `exec` on text +# (`dataclasses._FuncBuilder`), and numpy's file opener (`numpy.lib._datasource.open`). +ESCAPE_ATTRIBUTES: frozenset[str] = frozenset( + { + "sys", + "os", + "builtins", + "bltns", + "modules", + "inspect", + "subprocess", + "importlib", + "pathlib", + "shutil", + "socket", + "linecache", + "get_type_hints", + "ForwardRef", + "CodeType", + "FunctionType", + "attrgetter", + "methodcaller", + "global_enum", + "different_locale", + "LocaleTextCalendar", + "LocaleHTMLCalendar", + "_sys", + "_locale", + "_random", + "_tzpath", + "_common", + "_getcategory", + "_FuncBuilder", + "_datasource", + } +) +MODULE_ESCAPES: frozenset[str] = frozenset({"calendar.main"}) +"""Escapes banned only on their own module, because the bare name is plausible data: `calendar.main` +sets the process locale through its `-L` option, while `df.main` is a column. Checked against the +resolved attribute path, `from` imports and import paths; the `module-value` rule keeps the module +from reaching it through an expression the resolver cannot follow.""" +_DATA_FROM_DF = "the data comes only from df = load_user_data(), and the plot reads and writes no files" +IMPORT_ALTERNATIVES: dict[str, str] = { + **dict.fromkeys( + ("io", "csv", "json", "pickle", "sqlite3", "urllib", "requests", "http", "pathlib", "shutil", "glob"), + _DATA_FROM_DF, + ), + **dict.fromkeys(("tempfile", "zipfile", "tarfile", "PIL", "imageio", "sklearn.datasets"), _DATA_FROM_DF), + **dict.fromkeys(("scipy.io", "scipy.datasets", "pandas.io", "numpy.lib.format"), _DATA_FROM_DF), + "os": 'os only as THEME = os.getenv("ANYPLOT_THEME", ...) after a plain import os', + "sys": "a plot needs nothing from the interpreter: remove the import", + "random": "values come from df; a seeded np.random.default_rng() is only for jitter or subsampling", + "secrets": "values come from df; a seeded np.random.default_rng() is only for jitter or subsampling", + "locale": "format numbers with f-strings or matplotlib.ticker.FuncFormatter, which need no locale", + "string": "use str methods, f-strings and literals", + "dateutil": "parse dates with pandas.to_datetime or datetime, time zones with zoneinfo", + "pytz": "use zoneinfo for time zones", + "statsmodels.formula": "fit with statsmodels.api on arrays taken from df", + "scipy": "import the scipy submodule itself, such as scipy.stats or scipy.fft", +} +"""What a `banned-import` repair line suggests instead, by module path or its longest known prefix.""" +DEFAULT_IMPORT_ALTERNATIVE = ( + "use numpy, pandas, scipy's computing modules, sklearn, statsmodels.api, the plotting library, or pure " + "standard modules such as math, datetime, collections, re, warnings or zoneinfo" +) +# Third-party roots a banned import is logged under: the catalogue's banned plotting +# helpers, the catalogue's other plotting libraries, and data and science packages a +# model may reach for. A root missing here is logged as `?`; add it when a run shows one. +THIRD_PARTY_MODULES: frozenset[str] = frozenset( + { + "adjustText", + "cartopy", + "matplotlib_venn", + "qrcode", + "squarify", + "wordcloud", + "altair", + "bokeh", + "highcharts_core", + "lets_plot", + "plotly", + "plotnine", + "pygal", + "IPython", + "geopandas", + "networkx", + "openpyxl", + "polars", + "pyarrow", + "shapely", + "skimage", + "sympy", + "xarray", + "yaml", + } +) +LOGGED_MODULES: frozenset[str] = frozenset( + { + *sys.stdlib_module_names, + *COMMON_IMPORTS, + *SHARED_SCIENCE_IMPORTS, + *DENIED_IMPORTS, + *(module for modules in LIBRARY_IMPORTS.values() for module in modules), + *IMPORT_ALTERNATIVES, + *THIRD_PARTY_MODULES, + } +) +"""The closed vocabulary of `Finding.module`: no word in it comes from a model or a user.""" +UNKNOWN_MODULE = "?" +"""The label of a banned import with no prefix in `LOGGED_MODULES`.""" # `read_*` (pandas readers), `fetch_*` (sklearn downloaders), `load*` (`np.load`, # `np.loadtxt`, `sns.load_dataset`, sklearn's `load_*`). _BANNED_IO_PATTERN = re.compile(r"^(?:read_\w+|fetch_\w+|load\w*)$") @@ -272,11 +468,43 @@ @dataclass(frozen=True, slots=True) class Finding: - """One validator result: a short kebab-case rule id, one message line, a 1-based line.""" + """One validator result: a short kebab-case rule id, one message line, a 1-based line. + + A `banned-import` finding also carries the label of the module path it refused + (`module_label`, a word of a closed vocabulary) for the attribution log; every other + finding has None. + """ rule: str message: str line: int | None + module: str | None = None + + +def _longest_prefix(path: str, known: Container[str]) -> str | None: + """The longest dotted prefix of `path` that is in `known` (`scipy.io.matlab` → `scipy.io`), else None.""" + parts = path.split(".") + for length in range(len(parts), 0, -1): + prefix = ".".join(parts[:length]) + if prefix in known: + return prefix + return None + + +def module_label(path: str) -> str: + """The label of a module path for the attribution log: its longest prefix in `LOGGED_MODULES`, else `?`. + + The path is code the model wrote, so only a word of the closed vocabulary is logged: + `os.path` is `os`, `pandas.io.common` is `pandas.io`, and `patient_records` is `?`. + The function is idempotent, so a reader re-checks a logged label with it. + """ + return _longest_prefix(path, LOGGED_MODULES) or UNKNOWN_MODULE + + +def import_alternative(path: str) -> str: + """What to use instead of the module `path`: the entry of its longest known prefix, else the default.""" + prefix = _longest_prefix(path, IMPORT_ALTERNATIVES) + return IMPORT_ALTERNATIVES[prefix] if prefix is not None else DEFAULT_IMPORT_ALTERNATIVE def validate_security(code: str, *, library: str) -> list[Finding]: @@ -374,6 +602,11 @@ def _snippet(node: ast.AST) -> str: return text +def _io_hint(name: str) -> str: + """For a reader or loader (`read_*`, `load*`, `fetch_*`): where the data comes from instead.""" + return f" → {_DATA_FROM_DF}" if _BANNED_IO_PATTERN.match(name) else "" + + def _module_allowed(path: str, allowed: Iterable[str]) -> bool: return any(path == prefix or path.startswith(prefix + ".") for prefix in allowed) @@ -440,8 +673,13 @@ def __init__(self, tree: ast.Module) -> None: # -- shared -------------------------------------------------------------------------- - def add(self, rule: str, message: str, node: ast.AST | None) -> None: - self.findings.append(Finding(rule, message[:MAX_MESSAGE_CHARS], self._line(node))) + def add(self, rule: str, message: str, node: ast.AST | None, *, module: str | None = None) -> None: + label = module_label(module) if module is not None else None + self.findings.append(Finding(rule, message[:MAX_MESSAGE_CHARS], self._line(node), label)) + + def ban_import(self, path: str, reason: str, node: ast.AST) -> None: + """A `banned-import` finding whose message names the module and what to use instead.""" + self.add("banned-import", f"import of '{path}' {reason} → {import_alternative(path)}", node, module=path) def result(self) -> list[Finding]: unique = dict.fromkeys(self.findings) @@ -505,6 +743,7 @@ def _is_call_receiver(self, node: ast.AST, methods: frozenset[str]) -> bool: def security(self, library: str, allowed_imports: frozenset[str]) -> None: self._check_imports(library, allowed_imports) self._check_os_use() + self._check_module_values() self._check_names() self._check_attributes() self._check_calls() @@ -524,7 +763,7 @@ def permitted(path: str) -> bool: if alias.name == "os": self.os_names.add(alias.asname or "os") elif not permitted(alias.name): - self.add("banned-import", f"import of '{alias.name}' is not allowed with {library}", alias) + self.ban_import(alias.name, f"is not allowed with {library}", alias) else: self._check_module_path(alias.name, alias) for from_node in self.of(ast.ImportFrom): @@ -538,33 +777,72 @@ def permitted(path: str) -> bool: continue path = f"{from_node.module}.{alias.name}" if not module_banned and (denied(path) or not (permitted(from_node.module) or permitted(path))): - self.add("banned-import", f"import of '{path}' is not allowed with {library}", alias) + self.ban_import(path, f"is not allowed with {library}", alias) self._check_function_name(alias.name, alias) + if path in MODULE_ESCAPES: + self.add("banned-attribute", f"'{path}' is not allowed", alias) def _check_module_path(self, path: str, node: ast.AST) -> bool: - """An import path whose component is a banned name (`pandas.io`, `numpy.lib.format`). + """An import path whose component is a banned name (`pandas.io`, `numpy.lib.format`), or + below its root an escape name or a private module (`scipy.fft._backend`). The alias would hide the component from the attribute check: `common.get_handle` - after `import pandas.io.common as common` spells no `.io`. + after `import pandas.io.common as common` spells no `.io`, and `backend.set_backend` + after `import scipy.fft._backend as backend` no `._backend`. The root itself is + left to the allowlist, which keeps its message. """ - for part in path.split("."): + escape = next((name for name in MODULE_ESCAPES if _module_allowed(path, [name])), None) + if escape is not None: + self.ban_import(path, f"reaches the banned name '{escape}'", node) + return True + for position, part in enumerate(path.split(".")): if part in BANNED_ATTRIBUTES or part in BANNED_IO or _BANNED_IO_PATTERN.match(part): - self.add("banned-import", f"import of '{path}' reaches the banned name '{part}'", node) + self.ban_import(path, f"reaches the banned name '{part}'", node) + return True + if position and part in ESCAPE_ATTRIBUTES: + self.ban_import(path, f"reaches the banned name '{part}'", node) + return True + if position and part.startswith("_"): + self.ban_import(path, f"reaches the private module '{part}'", node) return True return False def _check_function_name(self, name: str, node: ast.AST) -> None: - """A function reached by a `from` import: the attribute set and the I/O set apply.""" - if name in BANNED_ATTRIBUTES: + """A name reached by a `from` import: the attribute, escape and I/O sets apply, and no private name.""" + if name in BANNED_ATTRIBUTES or name in ESCAPE_ATTRIBUTES: self.add("banned-attribute", f"'{name}' is not allowed", node) elif name in BANNED_IO or _BANNED_IO_PATTERN.match(name): - self.add("banned-call", f"'{name}' is not allowed", node) + self.add("banned-call", f"'{name}' is not allowed{_io_hint(name)}", node) + elif name.startswith("_") and not _DUNDER_NAME.match(name): + self.add("banned-attribute", f"the private name '{name}' is not allowed", node) def _check_os_use(self) -> None: for name in self.of(ast.Name): if name.id in self.os_names and not self._is_theme_lookup(name): self.add("os-use", f"'{name.id}' may only be used as {name.id}.getenv('{THEME_VARIABLE}', ...)", name) + def _check_module_values(self) -> None: + """A name bound by `import X [as Y]` only as the receiver of an attribute (module docstring). + + `os` has its own, stricter rule (`os-use`). + """ + modules = { + alias.asname or alias.name.split(".")[0] + for import_node in self.of(ast.Import) + for alias in import_node.names + if alias.name != "os" + } + for name in self.of(ast.Name): + if name.id not in modules or not isinstance(name.ctx, ast.Load): + continue + parent = self.parents.get(name) + if not (isinstance(parent, ast.Attribute) and parent.value is name): + self.add( + "module-value", + f"the module '{name.id}' may only be used as {name.id}., never as a value", + name, + ) + def _is_theme_lookup(self, name: ast.Name) -> bool: """`os.getenv("ANYPLOT_THEME", ...)` or `os.environ.get("ANYPLOT_THEME", ...)` around `name`.""" node: ast.AST = name @@ -609,13 +887,18 @@ def _check_attributes(self) -> None: for attribute in self.of(ast.Attribute): if _DUNDER_NAME.match(attribute.attr): self.add("dunder", f"dunder attribute '.{attribute.attr}' is not allowed", attribute) - elif attribute.attr in BANNED_ATTRIBUTES: + elif attribute.attr in BANNED_ATTRIBUTES or attribute.attr in ESCAPE_ATTRIBUTES: self.add("banned-attribute", f"attribute '.{attribute.attr}' is not allowed", attribute) elif attribute.attr in BANNED_IO or _BANNED_IO_PATTERN.match(attribute.attr): # `df.load` / `df.read_count`: a column reached as an attribute, not a module function. column_like = self.resolve(attribute.value) is None and attribute.attr.startswith(("read_", "load")) - hint = "; a column of that name must be subscripted" if column_like else "" + hint = "; a column of that name must be subscripted" if column_like else _io_hint(attribute.attr) self.add("banned-call", f"'{_snippet(attribute)}' is not allowed{hint}", attribute) + elif self.resolve(attribute) in MODULE_ESCAPES: + self.add("banned-attribute", f"attribute '{_snippet(attribute)}' is not allowed", attribute) + elif attribute.attr.startswith("_") and self.resolve(attribute.value) is not None: + # `collections._sys`, `dataclasses._FuncBuilder`: private helpers of an imported name. + self.add("banned-attribute", f"the private attribute '{_snippet(attribute)}' is not allowed", attribute) def _check_calls(self) -> None: for call in self.of(ast.Call): diff --git a/agents/anyplot/models.py b/agents/anyplot/models.py index 45540cca6b..ba1a6d0816 100644 --- a/agents/anyplot/models.py +++ b/agents/anyplot/models.py @@ -23,15 +23,32 @@ and the context filter trimming history stay harmless. Haiku 5.5 accepts disabled thinking at effort `high` or below, so every kind stays at `low` or `medium`. +For measurements only, `AGENT_ROOT_THINKING_BUDGET` and `AGENT_ADAPTER_THINKING_BUDGET` +(0, off, by default; at most `settings.MAX_THINKING_BUDGET`) turn thinking on for that +kind (`thinking_budget`). Haiku 5.5 takes no thinking budget of its own +(`budget_tokens` is a 400), so the config asks for ADK's adaptive mode +(`thinking_budget=-1`, sent as `thinking: {type: "adaptive", display: "summarized"}`), +the depth follows the kind's effort, and the budget is added to the kind's output cap, +so the answer keeps its whole measured headroom however much the thinking uses. The +summary comes back as thought parts, which the answer guard, ADK's output parsing and +the stream translator all skip. With both at 0 every request is unchanged. + ADK 2.11 sends no response schema to Claude: `basic.py` sets `config.response_schema` for a tool-less agent with an `output_schema`, and `anthropic_llm.py` never reads it, so the adapter and reviewer would get free text. -`VertexClaude` closes that gap with a forced tool call: when a request carries a -response schema and no tools, it adds one tool whose input schema is the response -schema, forces `tool_choice` to it, and turns the returned `tool_use` input back -into the JSON text ADK's `validate_schema` parses. Claude Opus 5.5, Sonnet 5.5 and -Fable 5.1 answer a forced `tool_choice` (and disabled thinking) with a 400, so the -settings accept only the Claude models in `settings.CLAUDE_MODELS`. +`VertexClaude` closes that gap with Claude's structured outputs: when a request +carries a response schema and no tools, it sets `output_config.format` to that +schema (`structured_output_format`), so constrained decoding makes the answer text +valid JSON of exactly that shape, and ADK's `validate_schema` parses it as it is. +The judge answers the same way. The grammar enforces types, enums, required fields +and `additionalProperties: false`; what it cannot enforce (string lengths, item +counts, a verdict's `ok` agreeing with its defects) the SDK's `transform_schema` +moves into the field descriptions as hints, and `schemas.repair_plan` and +`schemas.repair_verdict` mend after the strict check. Unlike a forced tool call, the +format lets a thinking model think before it answers (on Haiku 5.5 a forced tool +call skips thinking), and Claude Opus 5.5, Sonnet 5.5 and Fable 5.1 accept it. +Those three refuse disabled thinking with a 400, so the settings still accept only +the Claude models in `settings.CLAUDE_MODELS`. **Gemini** (`gemini`). `google.adk.models.google_llm.Gemini` with `client_kwargs={"enterprise": True, "project": ..., "location": ...}`, HTTP retries @@ -61,6 +78,7 @@ from functools import cached_property from typing import Any, Literal, Protocol +from anthropic import transform_schema from google.adk.models.anthropic_llm import AnthropicGenerateContentConfig, Claude from google.adk.models.base_llm import BaseLlm from google.adk.models.google_llm import Gemini @@ -155,16 +173,15 @@ types.HarmCategory.HARM_CATEGORY_DANGEROUS_CONTENT, ) RETRY_ATTEMPTS = 3 - -STRUCTURED_TOOL = "submit_result" -"""Name of the forced tool that carries a Claude structured answer.""" +ADAPTIVE_THINKING = -1 +"""ADK's `thinking_budget` for Claude's adaptive thinking; 0 is disabled, a positive budget is the +`budget_tokens` mode that Haiku 5.5 refuses.""" JUDGE_MAX_OUTPUT_TOKENS = 256 -"""The scope and dataset judge's cap. Its forced tool answer (`verdict` and `lang`) took -59 tokens in every one of the rerun's 244 runs, 5 below the old cap of 64, and the +"""The scope and dataset judge's cap. Its answer (`verdict` and `lang`) took 59 tokens as a +forced tool call in every one of the rerun's 244 runs, 5 below the old cap of 64, and the Gemini judge 15 to 22 in spike X. 256 is 4.3 times the Claude answer and leaves room for a Gemini judge's minimal thinking; a finished answer costs the same under any cap.""" -JUDGE_TOOL = "submit_verdict" def _labels(kind: str) -> dict[str, str]: @@ -215,12 +232,24 @@ def response_json_schema(schema: Any) -> dict[str, Any]: return result +def structured_output_format(schema: Any) -> dict[str, Any]: + """The `output_config.format` that constrains a Claude answer to `schema`. + + `anthropic.transform_schema` (the SDK's own helper behind `messages.parse`) makes + the inlined schema strict: `additionalProperties: false` on every object, and the + keywords constrained decoding does not support (`minLength`, `maxLength`, + `maxItems`, defaults) moved into the field's description, where the model still + reads them; the schema's own validators check them afterwards. + """ + return {"type": "json_schema", "schema": transform_schema(response_json_schema(schema))} + + def _has_tools(llm_request: LlmRequest) -> bool: return bool(llm_request.config and llm_request.config.tools) class VertexClaude(Claude): - """ADK's `Claude` with a client from the settings and structured output through a forced tool.""" + """ADK's `Claude` with a client from the settings and structured output through `output_config.format`.""" vertex_project: str vertex_region: str @@ -237,18 +266,18 @@ def _anthropic_client(self) -> Any: def _build_anthropic_kwargs( self, llm_request: LlmRequest, messages: Any, tools: Any, tool_choice: Any, thinking: Any ) -> dict[str, Any]: - """ADK's request arguments, plus the forced structured-answer tool when a response schema is set.""" + """ADK's request arguments, plus the structured-output format when a response schema is set. + + The format joins ADK's `output_config` (which holds the effort) rather than + replacing it. A request with tools (the root) gets no format: its answer is chat. + """ kwargs: dict[str, Any] = super()._build_anthropic_kwargs(llm_request, messages, tools, tool_choice, thinking) schema = llm_request.config.response_schema if llm_request.config else None if schema is not None and not _has_tools(llm_request): - kwargs["tools"] = [ - { - "name": STRUCTURED_TOOL, - "description": "Submit the final answer. Call this exactly once with the complete answer.", - "input_schema": response_json_schema(schema), - } - ] - kwargs["tool_choice"] = {"type": "tool", "name": STRUCTURED_TOOL} + kwargs["output_config"] = { + **(kwargs.get("output_config") or {}), + "format": structured_output_format(schema), + } return kwargs async def generate_content_async( @@ -257,22 +286,7 @@ async def generate_content_async( structured = bool(llm_request.config and llm_request.config.response_schema) and not _has_tools(llm_request) # A structured answer is read whole, so it never streams. async for response in super().generate_content_async(llm_request, stream=stream and not structured): - yield _structured_text(response) if structured else response - - -def _structured_text(response: LlmResponse) -> LlmResponse: - """Replace the forced tool call with the JSON text of its input, as a schema-bound Gemini answer reads.""" - if response.content is None or not response.content.parts: - return response - parts: list[types.Part] = [] - for part in response.content.parts: - call = part.function_call - if call is not None and call.name == STRUCTURED_TOOL: - parts.append(types.Part(text=json.dumps(call.args or {}, ensure_ascii=False))) - elif call is None: - parts.append(part) - response.content = types.Content(role=response.content.role or "model", parts=parts) - return response + yield response # --- Gemini ------------------------------------------------------------------------------ @@ -292,6 +306,19 @@ def _retry_options() -> types.HttpRetryOptions: # --- Public factories -------------------------------------------------------------------- +def thinking_budget(kind: ModelKind, settings: AgentSettings | None = None) -> int: + """The output room a thinking measurement grants `kind` on Claude; 0 keeps its thinking off. + + Only the root and the adapter take one (`AGENT_ROOT_THINKING_BUDGET`, + `AGENT_ADAPTER_THINKING_BUDGET`); the reviewer and every Gemini kind have 0. + """ + settings = settings or get_settings() + if settings.provider != "anthropic-vertex": + return 0 + budgets: dict[ModelKind, int] = {"root": settings.root_thinking_budget, "adapter": settings.adapter_thinking_budget} + return budgets.get(kind, 0) + + def make_model(kind: ModelKind, settings: AgentSettings | None = None) -> BaseLlm: """The ADK model of one agent kind for the configured provider.""" settings = settings or get_settings() @@ -300,7 +327,7 @@ def make_model(kind: ModelKind, settings: AgentSettings | None = None) -> BaseLl if settings.provider == "anthropic-vertex": return VertexClaude( model=settings.model, - max_tokens=MAX_OUTPUT_TOKENS[kind], + max_tokens=MAX_OUTPUT_TOKENS[kind] + thinking_budget(kind, settings), vertex_project=settings.project, vertex_region=settings.location, ) @@ -317,11 +344,13 @@ def make_content_config(kind: ModelKind, settings: AgentSettings | None = None) if kind not in MODEL_KINDS: raise ValueError(f"unknown model kind {kind!r}") if settings.provider == "anthropic-vertex": + budget = thinking_budget(kind, settings) return AnthropicGenerateContentConfig( - max_output_tokens=MAX_OUTPUT_TOKENS[kind], + max_output_tokens=MAX_OUTPUT_TOKENS[kind] + budget, effort=CLAUDE_EFFORT[kind], # thinking_budget 0 is ADK's `thinking: {type: "disabled"}`; unset, Haiku 5.5 thinks. - thinking_config=types.ThinkingConfig(thinking_budget=0), + # A measurement's budget turns on adaptive thinking (Haiku 5.5 refuses `budget_tokens`). + thinking_config=types.ThinkingConfig(thinking_budget=ADAPTIVE_THINKING if budget else 0), labels=_labels(kind), ) return types.GenerateContentConfig( @@ -333,19 +362,22 @@ def make_content_config(kind: ModelKind, settings: AgentSettings | None = None) ) -def allow_full_file(config: types.GenerateContentConfig) -> None: +def allow_full_file(config: types.GenerateContentConfig, settings: AgentSettings | None = None) -> None: """Widen one adapter request's config for an answer that may be a full file. - The cap becomes `ADAPTER_FULL_MAX_OUTPUT_TOKENS` on Claude, which keeps thinking - disabled, and `GEMINI_ADAPTER_FULL_MAX_OUTPUT_TOKENS` on Gemini, whose thinking - level drops to `GEMINI_ADAPTER_FULL_THINKING`. The provider is read from the - config's type, not from the settings, so the request decides. ADK copies the - agent's config per request only shallowly (`_copy_request_scoped_fields`), so the - thinking config is replaced, never mutated: a change to the shared object would - reach every later run. + The cap becomes `ADAPTER_FULL_MAX_OUTPUT_TOKENS` on Claude, which keeps its thinking + as it was (off, or a measurement's adaptive thinking with the adapter's thinking + budget added to the cap again), and `GEMINI_ADAPTER_FULL_MAX_OUTPUT_TOKENS` on + Gemini, whose thinking level drops to `GEMINI_ADAPTER_FULL_THINKING`. The provider + is read from the config's type, not from the settings, so the request decides. ADK + copies the agent's config per request only shallowly (`_copy_request_scoped_fields`), + so the thinking config is replaced, never mutated: a change to the shared object + would reach every later run. """ if isinstance(config, AnthropicGenerateContentConfig): - config.max_output_tokens = ADAPTER_FULL_MAX_OUTPUT_TOKENS + thinking = config.thinking_config.thinking_budget if config.thinking_config else 0 + room = thinking_budget("adapter", settings) if thinking else 0 + config.max_output_tokens = ADAPTER_FULL_MAX_OUTPUT_TOKENS + room return config.max_output_tokens = GEMINI_ADAPTER_FULL_MAX_OUTPUT_TOKENS config.thinking_config = types.ThinkingConfig(thinking_level=GEMINI_ADAPTER_FULL_THINKING) @@ -440,7 +472,7 @@ async def _within_budget(attempt: Attempt, text: str, rubric: str, timeout_s: fl @dataclass class AnthropicJudge: - """The judge on Claude through Vertex AI, answering through a forced tool call.""" + """The judge on Claude through Vertex AI, answering under a structured-output format.""" model: str project: str @@ -460,18 +492,12 @@ async def _attempt(self, text: str, rubric: str) -> JudgeVerdict: thinking={"type": "disabled"}, system=rubric, messages=[{"role": "user", "content": text}], - tools=[ - { - "name": JUDGE_TOOL, - "description": "Submit the verdict.", - "input_schema": response_json_schema(_JudgeAnswer), - } - ], - tool_choice={"type": "tool", "name": JUDGE_TOOL}, + output_config={"format": structured_output_format(_JudgeAnswer)}, ) - answer = next((block.input for block in message.content if getattr(block, "type", "") == "tool_use"), None) - if not isinstance(answer, dict): - raise ValueError("the judge answered without a verdict") + # A refusal or a cut-off answer need not match the format; it is a failed attempt, not a verdict. + if message.stop_reason != "end_turn": + raise ValueError(f"the judge stopped with {message.stop_reason}") + answer = "".join(block.text for block in message.content if getattr(block, "type", "") == "text") usage = message.usage input_tokens = sum( int(getattr(usage, name, 0) or 0) @@ -479,7 +505,7 @@ async def _attempt(self, text: str, rubric: str) -> JudgeVerdict: ) output_tokens = int(getattr(usage, "output_tokens", 0) or 0) return JudgeVerdict( - **_JudgeAnswer.model_validate(answer).model_dump(), + **_JudgeAnswer.model_validate_json(answer).model_dump(), tokens=input_tokens + output_tokens, input_tokens=input_tokens, output_tokens=output_tokens, diff --git a/agents/anyplot/pipeline.py b/agents/anyplot/pipeline.py index e9f0cb5395..9a559eab1f 100644 --- a/agents/anyplot/pipeline.py +++ b/agents/anyplot/pipeline.py @@ -58,8 +58,10 @@ Every step also writes one content-free attribution line (`plugins/ledger.attribution`): `pipeline_adapt` (the answer's outcome, `plan`, `schema`, `truncated` or `full_code_refused`, the plan's shape, and the call's finish reason and output tokens), -`pipeline_check` (edit-apply failure count and kinds, blocking validator rule ids, -ADAPTATION rule ids), `pipeline_render` (render and wall time, the gate ids that +`pipeline_check` (edit-apply failure count and kinds, how many edits a tolerant +matching tier applied, blocking validator rule ids, the labels of banned imports +(`validate.module_label`: a word of a closed vocabulary, or `?`), ADAPTATION rule +ids), `pipeline_render` (render and wall time, the gate ids that failed or reported), `pipeline_review` (the verdict, its defect ids and the call's finish reason) and `pipeline_result` (status, reason, attempts, the `Stage` that ended each attempt and the run, the shipped and the last reviewed attempt, the number of @@ -436,6 +438,12 @@ class CheckResult: defects: list[str] blocking_rules: list[str] adaptation_rules: list[str] + banned_imports: list[str] = field(default_factory=list) + """The module labels of the `banned-import` findings (`validate.module_label`), at most `MAX_LOGGED_MODULES`.""" + + +MAX_LOGGED_MODULES = 8 +"""Banned module labels one `pipeline_check` line names; a plan that imports more is rare and says enough.""" def _data_texts(data_csv: str) -> frozenset[str]: @@ -497,7 +505,10 @@ def _check( + f" → {ADAPTATION_TARGETS.get(f.rule, 'derive it from df')}. Likely cause: the adaptation ({f.rule})." for f in soft ] - return CheckResult(not blocking, blocking, defects, blocking_rules, [f.rule for f in soft]) + modules = list(dict.fromkeys(f.module for f in blocking_findings if f.rule == "banned-import" and f.module)) + return CheckResult( + not blocking, blocking, defects, blocking_rules, [f.rule for f in soft], modules[:MAX_LOGGED_MODULES] + ) def finish(run: Run) -> PlotResult: @@ -800,6 +811,7 @@ async def _attempts( outcome="edits_failed", edit_failures=len(applied.failures), edit_failure_kinds=dict(sorted(Counter(applied.kinds).items())), + edit_tolerant=dict(sorted(Counter(applied.tolerant).items())), ) feedback, previous_plan = applied.failures, plan _end_attempt(run, "edit_apply") @@ -820,7 +832,9 @@ async def _attempts( attempt=attempt, outcome="ok" if check.may_render else "rejected", edit_failures=0, + edit_tolerant=dict(sorted(Counter(applied.tolerant).items())), validator=check.blocking_rules, + banned_imports=check.banned_imports, adaptation=check.adaptation_rules, ) if not check.may_render: diff --git a/agents/anyplot/prompts/adapter.md b/agents/anyplot/prompts/adapter.md index a424ecc291..57db481417 100644 --- a/agents/anyplot/prompts/adapter.md +++ b/agents/anyplot/prompts/adapter.md @@ -37,12 +37,14 @@ It returns a pandas DataFrame with exactly the loader columns, typed: `integer` ## Lines you never touch -The server refuses an edit whose `find` overlaps any of these lines: +The server refuses an edit whose `find` overlaps any of these lines, and a full file that changes them: -- the `THEME = os.getenv("ANYPLOT_THEME", ...)` line and every theme token computed from `THEME` (`PAGE_BG`, `ELEVATED_BG`, `INK`, `INK_SOFT`, `INK_MUTED`, `GRID`, `BRAND`, ...); -- the `df = load_user_data()` line once it exists; +- the `THEME = os.getenv("ANYPLOT_THEME", ...)` line and every theme token, a module-level name set to fixed values per theme, such as `PAGE_BG = "#FAF8F1" if THEME == "light" else "#1A1A17"` (`PAGE_BG`, `ELEVATED_BG`, `INK`, `INK_SOFT`, `INK_MUTED`, `GRID`, `BRAND`, ...). A line that picks a value from the data by theme, such as `label_color = INK if THEME == "dark" else IMPRINT[i]`, is ordinary plot code that you may change; +- the `df = load_user_data()` line once it exists. Keep it out of `find` and `replace`, also to append to it: to transform `df` or add statements after it, use the line below it as `find` and write the new statements in front of that line in `replace`; - the final `savefig` statement. +In a full file, copy each of these lines unchanged from the current code. + The canvas is fixed by the server: never change `figsize`, `dpi`, `set_size_inches` or the `savefig` arguments, and never add `bbox_inches`. ## The palette @@ -61,7 +63,7 @@ These rules replace the catalogue's own rules where they differ: - **Callouts and statistics.** A label or callout that states a number must compute it from `df` in an f-string (`f"r = {df[x].corr(df[y]):.2f}"`, written with the column variables of the code). A callout target comes from a rule over `df` (the maximum, the latest row), and a conditional claim is drawn only when `df` meets it. Remove a callout that cannot be derived. - **Density.** Adapt marker sizes, line widths and alpha to the user's row count (see the VQ-03 table below). - **Dates.** Use `matplotlib.dates.AutoDateLocator` with `ConciseDateFormatter` instead of a fixed locator, unless the change request asks for a specific one. -- **Safe code.** Use only the imports the code already has, plus `numpy`, `pandas`, `math`, `statistics`, `datetime`, `collections`, `itertools`, `functools`, `textwrap`, `colorsys` and `re` when needed. No file or network access, no `os` beyond the THEME line, no `.format(...)` (use f-strings), no `eval`, `exec`, `getattr` or other reflection, no names with double underscores, no `class` statements, and no string literal longer than 200 characters. +- **Safe code.** Use only the imports the code already has, plus, when needed, `numpy`, `pandas`, the plotting library, scipy's computing modules (`scipy.stats`, `scipy.interpolate`, `scipy.signal`, `scipy.optimize`, `scipy.fft`, `scipy.linalg`, ...) and these standard modules: `math`, `cmath`, `statistics`, `decimal`, `fractions`, `numbers`, `datetime`, `calendar`, `zoneinfo`, `collections`, `itertools`, `functools`, `operator`, `bisect`, `heapq`, `typing`, `warnings`, `textwrap`, `colorsys` and `re`. Never import `io`, `sys`, `pathlib`, `random`, `locale` or `string`. No file or network access, no `os` beyond the THEME line, no `.format(...)` (use f-strings), no `eval`, `exec`, `getattr`, `operator.attrgetter` or other reflection, no names with double underscores or private names of a module (`module._name`), no `class` statements, and no string literal longer than 200 characters. ## Change requests diff --git a/agents/anyplot/schemas.py b/agents/anyplot/schemas.py index 72de03dbf1..f907fa64f4 100644 --- a/agents/anyplot/schemas.py +++ b/agents/anyplot/schemas.py @@ -310,8 +310,10 @@ def _ok_matches_defects(self) -> Self: def _json_list(value: Any) -> Any: """A list the model sent as JSON text, parsed; anything else unchanged. - A forced tool call sometimes carries a nested array as a string; the items are - the model's own, only their wrapping differs. + An answer without constrained decoding (Claude's forced tool call before the + answers moved to `output_config.format`, which now rules this out on Claude) + sometimes carried a nested array as a string; the items are the model's own, + only their wrapping differs. """ if isinstance(value, str): try: diff --git a/agents/anyplot/settings.py b/agents/anyplot/settings.py index 4bd74517db..53bdff5c6a 100644 --- a/agents/anyplot/settings.py +++ b/agents/anyplot/settings.py @@ -24,6 +24,8 @@ | `AGENT_RUNS_PER_MINUTE` | `1` | Runs that may start within any 60 seconds (a sliding window) | | `AGENT_QUEUE_MAX_WAIT_S` | `600` | Longest wait in the run queue; it also sizes the queue (`AGENT_RUNS_PER_MINUTE` x this / 60 entries) | | `AGENT_MAX_ATTEMPTS` | `2` | Adapter attempts per pipeline run, 1 to 3: the first and at most one repair round by default; the eval harness sets 1 or 3 to measure no repair or a second repair round | +| `AGENT_ROOT_THINKING_BUDGET` | `0` | For measurements, Claude only: 0 keeps the root's thinking off; 1 to 4,096 turns on adaptive thinking and adds that many tokens to its output cap | +| `AGENT_ADAPTER_THINKING_BUDGET` | `0` | The same for the adapter; the eval harness sets both with `--thinking-budget ROOT,ADAPTER` | | `AGENT_MAX_LLM_CALLS` | `12` | LLM calls per request (the `RunConfig` cap) | | `AGENT_REQUEST_TOKEN_BUDGET` | `160000` | Cost-weighted tokens per request, in input-token equivalents: an uncached input token counts 1, a cache read 0.1, a cache write 1.25, an output token 5 (`plugins/ledger.BUDGET_WEIGHTS`, the list-price ratios) | | `AGENT_DAILY_TOKEN_BUDGET` | `2000000` | Cost-weighted tokens per user and day, weighted the same way: about 54 median runs on a warm cache, where the run cap binds first, and about 26 for a user whose every request starts cold | @@ -44,7 +46,8 @@ The output caps of the model calls are constants in `models.py`, not settings, each with ample headroom above the measured answers: root and reviewer 4,096 tokens, the adapter 8,192 for an edit-only call and 16,384 when a full file is allowed (12,288 on Gemini, -which the soft deadline bounds), and the judge 256. +which the soft deadline bounds), and the judge 256. A thinking measurement on Claude adds +its `AGENT_*_THINKING_BUDGET` to the cap of its kind. No `.env` file is read here: the service runs on Cloud Run environment variables only, and the tests stay hermetic. `adk web` is different: it walks up from the agent @@ -88,9 +91,16 @@ LOCATIONS: tuple[str, ...] = ("eu", "us", "global") MODEL_PREFIXES: dict[str, str] = {"anthropic-vertex": "claude-", "gemini": "gemini-"} CLAUDE_MODELS: tuple[str, ...] = ("claude-haiku-5-5",) -"""Claude models that accept a forced `tool_choice` with thinking disabled at effort low and -medium (`models.VertexClaude`); Claude Opus 5.5, Sonnet 5.5 and Fable 5.1 answer both with a 400.""" +"""Claude models that accept disabled thinking at effort low and medium (`models.make_content_config`); +Claude Opus 5.5, Sonnet 5.5 and Fable 5.1 answer disabled thinking with a 400. The structured +answers (`output_config.format`) would work on all of them.""" FIXTURE_PATTERN = r"^[a-z0-9][a-z0-9-]{0,79}$" +MAX_THINKING_BUDGET = 4_096 +"""The most output room a thinking measurement may add to a Claude cap (`AGENT_*_THINKING_BUDGET`). + +It keeps the adapter's full-file cap (16,384 plus 4,096) below the Anthropic SDK's limit +for a request that does not stream (about 21,300 tokens), and a full-file call that uses +all of it at about 60 s, the `pipeline.ADAPTER_P95_S` the deadline reserves.""" def _split_list(value: Any) -> Any: @@ -177,6 +187,19 @@ class AgentSettings(BaseSettings): (`--max-attempts`) sets 1 to measure no repair and 3 to measure a second repair round. Every attempt after the first needs `ADAPTER_P95_S` plus `RENDER_P95_S` left of the soft deadline.""" + root_thinking_budget: int = Field(default=0, ge=0, le=MAX_THINKING_BUDGET) + """Claude thinking of the root, for measurements (`AGENT_ROOT_THINKING_BUDGET`): 0 (the default) + keeps it off. Above 0 the root thinks adaptively at its effort and its output cap grows by this + many tokens, so the reply keeps the whole measured headroom (`models.make_content_config`). + Claude Haiku 5.5 takes no thinking budget of its own (`budget_tokens` is a 400), so this is the + room thinking may use, not a limit the model sees. The root replays its thinking blocks after a + tool result; an account that enforces the history-editing check of preserved thinking may refuse + that call, which a measurement run shows as pipeline errors.""" + + adapter_thinking_budget: int = Field(default=0, ge=0, le=MAX_THINKING_BUDGET) + """The same for the adapter (`AGENT_ADAPTER_THINKING_BUDGET`). The adapter is single-turn, so + nothing is replayed; its structured answer (`output_config.format`) lets it think first.""" + max_llm_calls: PositiveInt = 12 """LLM calls per request (`AGENT_MAX_LLM_CALLS`), the `RunConfig` cap.""" @@ -337,9 +360,13 @@ def _consistent(self) -> Self: if self.provider == "anthropic-vertex" and value not in CLAUDE_MODELS: raise ValueError( f"{name}={value!r} is not a supported Claude model ({', '.join(CLAUDE_MODELS)}): " - "structured answers use a forced tool call with thinking disabled, which newer " - "Claude models refuse with a 400" + "the agents run with thinking disabled, which newer Claude models refuse with a 400" ) + if self.provider != "anthropic-vertex" and (self.root_thinking_budget or self.adapter_thinking_budget): + raise ValueError( + "AGENT_ROOT_THINKING_BUDGET and AGENT_ADAPTER_THINKING_BUDGET apply to Claude only; " + "Gemini thinks by a level per agent kind" + ) if self.is_development and self.cloud_run_service: raise ValueError( "ENVIRONMENT=development is refused on Cloud Run (K_SERVICE is set): it would skip the caller check" diff --git a/agents/anyplot/sub_agents/adapter.py b/agents/anyplot/sub_agents/adapter.py index 816622d2b5..3b0a9ccd3b 100644 --- a/agents/anyplot/sub_agents/adapter.py +++ b/agents/anyplot/sub_agents/adapter.py @@ -9,8 +9,10 @@ scope; the callback removes it, so the adapter reads nothing but the request.) The answer is bound to `AdaptPlan` (`output_schema`); on Claude the model factory -turns that into a forced tool call. `schema_guard` mends an answer that misses only a -formal rule (`schemas.repair_plan`) and blanks one that still fails the schema, so +turns that into a structured-output format (`models.structured_output_format`), whose +grammar enforces the shape but not the lengths and counts. `schema_guard` mends an +answer that misses only such a formal rule (`schemas.repair_plan`) and blanks one that +still fails the schema, so the pipeline repairs it instead of ADK ending the run. On the second attempt, when a full file is allowed, the callback widens the request through `models.allow_full_file`: the output cap rises to `ADAPTER_FULL_MAX_OUTPUT_TOKENS` @@ -102,9 +104,9 @@ def schema_guard( outcome (`repaired` or `refused`) and the broken rules (`schema_errors`). An answer that ended at the output limit (`FinishReason.MAX_TOKENS`) is blanked - without a check: every field of `AdaptPlan` has a default, so a Claude tool input - cut off to `{}` or to its first edits would pass as a plan that silently drops the - rest. The pipeline reads the finish reason from the request ledger and tells the + without a check: every field of `AdaptPlan` has a default, so an answer cut off and + then mended to `{}` or to its first edits would pass as a plan that silently drops + the rest. The pipeline reads the finish reason from the request ledger and tells the repair the answer was cut off. """ diff --git a/agents/evals/matrix.py b/agents/evals/matrix.py index 8cce5700a0..0e7a5d791a 100644 --- a/agents/evals/matrix.py +++ b/agents/evals/matrix.py @@ -29,7 +29,12 @@ the reviewed attempt, an `attempt_log` (per attempt: the adapter's outcome, finish reason, output tokens and plan shape, the check with its edit-failure kinds, the render gates and the review verdict), every model call's finish reason and tokens -(`calls`, counted per agent kind in `finish_reasons`), and the edit-failure kinds. +(`calls`, counted per agent kind in `finish_reasons`), the edit-failure kinds, how +many edits a tolerant matching tier applied (`edit_tolerant`, by tier, also in a plan +that another edit failed), and the module +labels of the banned imports the validator refused (`banned_imports`: the longest +prefix of the path in the validator's closed vocabulary, such as `io` or `pandas.io`, +anything else `?`), so a run shows which modules the adapter reaches for. Answers that failed their schema are counted by agent kind and outcome (`answer_outcomes`: `repaired` or `refused`) and by broken rule (`answer_rules`, list indices written `*`), from the guard's `answer_schema` lines. @@ -78,7 +83,11 @@ attempt bound (`AGENT_MAX_ATTEMPTS`, 2 by default, recorded in the stamp as `max_attempts`); a three-attempt run fits the request budget too, at about 117k weighted tokens for the costliest run simulated on a cold prompt cache with a second -review. +review. `--thinking-budget ROOT,ADAPTER` measures Claude's adaptive thinking for the +root and the adapter: each value (0 to 4,096, 0 off) is added to that agent's output +cap (`AGENT_ROOT_THINKING_BUDGET`, `AGENT_ADAPTER_THINKING_BUDGET`, recorded in the stamp +as `thinking_budget`), and the thinking tokens are priced as output; the defaults +change no request. The harness sets `ENVIRONMENT=development` for the `remote` and `local` renderers unless it runs on Cloud Run (`K_SERVICE`), because a developer's renderer token is accepted @@ -154,6 +163,9 @@ MAX_ATTEMPTS_CHOICES: tuple[int, ...] = (1, 2, 3) """The values `--max-attempts` takes: 1 to `agents.anyplot.schemas.MAX_ATTEMPTS`, written out because importing that module builds the agents before `configure` has set the environment (a test keeps them equal).""" +MAX_THINKING_BUDGET = 4_096 +"""The largest `--thinking-budget` value: `agents.anyplot.settings.MAX_THINKING_BUDGET`, written out for the +same reason (a test keeps them equal).""" ATTRIBUTION_LOGGER = "anyplot.agents.attribution" MODEL_TOKEN_KINDS: tuple[str, ...] = ("prompt", "candidates", "thoughts", "cached", "cache_write", "tool_use_prompt") EXIT_OK, EXIT_REGRESSION, EXIT_SETUP, EXIT_STOPPED, EXIT_OUTAGE = 0, 1, 2, 3, 4 @@ -216,6 +228,15 @@ def parse_args(argv: list[str] | None) -> argparse.Namespace: choices=MAX_ATTEMPTS_CHOICES, help="adapter attempts per pipeline run, AGENT_MAX_ATTEMPTS (default: the settings, 2)", ) + parser.add_argument( + "--thinking-budget", + type=thinking_budgets, + metavar="ROOT,ADAPTER", + help=( + "Claude only, for measurements: extra output tokens for adaptive thinking of the root and the adapter, " + f"0 to {MAX_THINKING_BUDGET} each, 0 off (AGENT_ROOT_THINKING_BUDGET, AGENT_ADAPTER_THINKING_BUDGET)" + ), + ) parser.add_argument( "--budget-usd", type=float, @@ -243,11 +264,23 @@ def parse_args(argv: list[str] | None) -> argparse.Namespace: return args +def thinking_budgets(value: str) -> tuple[int, int]: + """`--thinking-budget ROOT,ADAPTER`: two whole numbers from 0 to `MAX_THINKING_BUDGET`.""" + parts = value.split(",") + if len(parts) != 2 or not all(part.strip().isdigit() for part in parts): + raise argparse.ArgumentTypeError("expected ROOT,ADAPTER, two whole numbers such as 0,2048") + root, adapter = (int(part) for part in parts) + if root > MAX_THINKING_BUDGET or adapter > MAX_THINKING_BUDGET: + raise argparse.ArgumentTypeError(f"each budget is at most {MAX_THINKING_BUDGET}") + return root, adapter + + def configure(args: argparse.Namespace, environ: MutableMapping[str, str]) -> None: """Write the flags into the `AGENT_*` environment the settings read; runs before any `agents.anyplot` import. `--provider` alone selects that provider's default model and judge; `--model` alone - implies its provider; `--max-attempts` sets `AGENT_MAX_ATTEMPTS`. The environment's + implies its provider; `--max-attempts` sets `AGENT_MAX_ATTEMPTS`; `--thinking-budget` + sets `AGENT_ROOT_THINKING_BUDGET` and `AGENT_ADAPTER_THINKING_BUDGET`. The environment's own `AGENT_*` values stay for everything no flag names. With the `remote` or `local` renderer the process runs as `ENVIRONMENT=development` (outside Cloud Run), whatever the shell exported: only development accepts a developer's renderer token and the @@ -275,6 +308,10 @@ def configure(args: argparse.Namespace, environ: MutableMapping[str, str]) -> No environ["AGENT_LOCATION"] = args.location if args.max_attempts is not None: environ["AGENT_MAX_ATTEMPTS"] = str(args.max_attempts) + if args.thinking_budget is not None: + root, adapter = args.thinking_budget + environ["AGENT_ROOT_THINKING_BUDGET"] = str(root) + environ["AGENT_ADAPTER_THINKING_BUDGET"] = str(adapter) configured = environ.get("AGENT_RENDERER") fallback = "fake" if getattr(args, "static", False) else "remote" renderer = args.renderer or (configured if configured in RENDERERS else fallback) @@ -561,11 +598,20 @@ def base_record(case: "EvalCase", repeat: int) -> dict[str, Any]: "calls": [], "finish_reasons": {}, "edit_failure_kinds": {}, + "edit_tolerant": {}, + "banned_imports": {}, "answer_outcomes": {}, "answer_rules": {}, } +def module_path(value: Any) -> str: + """A logged banned-import label, re-checked against the validator's closed vocabulary, else `?`.""" + from agents.anyplot.code.validate import module_label + + return module_label(str(value)) + + def _int(value: Any) -> int: return value if isinstance(value, int) and not isinstance(value, bool) and value > 0 else 0 @@ -619,7 +665,9 @@ def _book_attempt(record: dict[str, Any], hook: str, line: dict[str, Any]) -> No elif hook == "pipeline_check": entry["check"] = str(line.get("outcome")) entry["edit_failure_kinds"] = dict(line.get("edit_failure_kinds") or {}) + entry["edit_tolerant"] = dict(line.get("edit_tolerant") or {}) entry["validator"] = [str(rule) for rule in line.get("validator") or []] + entry["banned_imports"] = [module_path(module) for module in line.get("banned_imports") or []] entry["adaptation"] = [str(rule) for rule in line.get("adaptation") or []] elif hook == "pipeline_render": entry["render"] = { @@ -650,6 +698,8 @@ def book_attribution(record: dict[str, Any], lines: list[dict[str, Any]], settin adapter: Counter[str] = Counter(record["adapter_outcomes"]) agents: Counter[str] = Counter(record["llm_calls_by_agent"]) kinds: Counter[str] = Counter(record["edit_failure_kinds"]) + tolerant: Counter[str] = Counter(record["edit_tolerant"]) + modules: Counter[str] = Counter(record["banned_imports"]) answers: Counter[str] = Counter(record["answer_outcomes"]) rules: Counter[str] = Counter(record["answer_rules"]) finishes = {kind: Counter[str](reasons) for kind, reasons in record["finish_reasons"].items()} @@ -688,7 +738,9 @@ def book_attribution(record: dict[str, Any], lines: list[dict[str, Any]], settin elif hook == "pipeline_check": record["edit_apply_failures"] += _int(line.get("edit_failures")) kinds.update({str(kind): _int(count) for kind, count in (line.get("edit_failure_kinds") or {}).items()}) + tolerant.update({str(tier): _int(count) for tier, count in (line.get("edit_tolerant") or {}).items()}) validator.update(str(rule) for rule in line.get("validator") or []) + modules.update(module_path(module) for module in line.get("banned_imports") or []) if line.get("outcome") == "loader_failed": validator["loader"] += 1 _book_attempt(record, hook, line) @@ -720,6 +772,8 @@ def book_attribution(record: dict[str, Any], lines: list[dict[str, Any]], settin record["adapter_outcomes"] = dict(sorted(adapter.items())) record["llm_calls_by_agent"] = dict(sorted(agents.items())) record["edit_failure_kinds"] = dict(sorted((kind, count) for kind, count in kinds.items() if count)) + record["edit_tolerant"] = dict(sorted((tier, count) for tier, count in tolerant.items() if count)) + record["banned_imports"] = dict(sorted(modules.items())) record["answer_outcomes"] = dict(sorted(answers.items())) record["answer_rules"] = dict(sorted(rules.items())) record["finish_reasons"] = {kind: dict(sorted(reasons.items())) for kind, reasons in sorted(finishes.items())} @@ -999,6 +1053,7 @@ def build_stamp( "judge_model": settings.judge_model, "location": settings.location, "max_attempts": settings.max_attempts, + "thinking_budget": {"root": settings.root_thinking_budget, "adapter": settings.adapter_thinking_budget}, "renderer": settings.renderer, "render_url": getattr(settings, "render_url", None), "adk_version": _version("google-adk"), diff --git a/agents/evals/report.py b/agents/evals/report.py index 5b23e13b03..398ef63032 100644 --- a/agents/evals/report.py +++ b/agents/evals/report.py @@ -13,7 +13,9 @@ `edit_failure_kinds`), splits the adapter outcome `truncated` (an answer cut off at the output limit) from `schema`, and counts an unparseable file as one `syntax` validator rejection instead of two; later schema-2 runs also count the answers that -failed their schema (`answer_outcomes`, `answer_rules`). Every reader here takes a schema-1 report too: +failed their schema (`answer_outcomes`, `answer_rules`), the edits a tolerant +matching tier applied (`edit_tolerant`) and the module labels of banned imports +(`banned_imports`). Every reader here takes a schema-1 report too: a field it lacks reads as empty, so a schema-1 baseline still diffs against a schema-2 run (`SUPPORTED_SCHEMAS`). @@ -174,10 +176,12 @@ def summarize(runs: list[dict[str, Any]]) -> dict[str, Any]: "tokens": dict(sorted(tokens.items())), "gate_failures": _counter(runs, "gate_failures"), "validator_rejections": _counter(runs, "validator_rejections"), + "banned_imports": _counter(runs, "banned_imports"), "adapter_outcomes": _counter(runs, "adapter_outcomes"), "adapter_outcomes_by_attempt": _adapter_by_attempt(runs), "edit_apply_failures": sum(int(run.get("edit_apply_failures") or 0) for run in runs), "edit_failure_kinds": _counter(runs, "edit_failure_kinds"), + "edit_tolerant": _counter(runs, "edit_tolerant"), "answer_outcomes": _counter(runs, "answer_outcomes"), "answer_rules": _counter(runs, "answer_rules"), "finish_reasons": _finish_reasons(runs), @@ -412,6 +416,13 @@ def summary_markdown(report: dict[str, Any], diff: Diff | None = None) -> str: f"commit `{str(stamp.get('git_sha') or '')[:12]}`.", "", ] + thinking = stamp.get("thinking_budget") or {} + if any(thinking.values()): + lines += [ + f"**Thinking measurement:** adaptive Claude thinking with {thinking.get('root', 0):,} extra output tokens " + f"for the root and {thinking.get('adapter', 0):,} for the adapter (0 is off).", + "", + ] if report.get("stopped"): lines += [f"**Stopped early** ({report['stopped']}): the rates cover the runs that finished.", ""] lines += ["## Results", ""] @@ -449,6 +460,10 @@ def summary_markdown(report: dict[str, Any], diff: Diff | None = None) -> str: counts = [ *(["Gate " + name, str(count)] for name, count in (summary.get("gate_failures") or {}).items()), *(["Validator " + name, str(count)] for name, count in (summary.get("validator_rejections") or {}).items()), + *( + ["Validator banned import, " + name, str(count)] + for name, count in (summary.get("banned_imports") or {}).items() + ), *(["Adapter " + name, str(count)] for name, count in (summary.get("adapter_outcomes") or {}).items()), *( [f"Adapter, attempt {attempt}, {name}", str(count)] @@ -460,6 +475,10 @@ def summary_markdown(report: dict[str, Any], diff: Diff | None = None) -> str: ["Edit-apply failure " + name, str(count)] for name, count in (summary.get("edit_failure_kinds") or {}).items() ), + *( + ["Edit applied by tolerant match, " + name, str(count)] + for name, count in (summary.get("edit_tolerant") or {}).items() + ), *(["Reviewer " + name, str(count)] for name, count in (summary.get("reviewer") or {}).items()), *(["Answer " + name, str(count)] for name, count in (summary.get("answer_outcomes") or {}).items()), *(["Answer rule, " + name, str(count)] for name, count in (summary.get("answer_rules") or {}).items()), diff --git a/changelog.d/agents-adapter-hardening.md b/changelog.d/agents-adapter-hardening.md new file mode 100644 index 0000000000..96e7c6bb0c --- /dev/null +++ b/changelog.d/agents-adapter-hardening.md @@ -0,0 +1,70 @@ +### Added + +- **A thinking measurement for the agent root and adapter.** + `AGENT_ROOT_THINKING_BUDGET` and `AGENT_ADAPTER_THINKING_BUDGET` (0, off, by + default; at most 4,096), set by the eval harness with `--thinking-budget + ROOT,ADAPTER`, turn on Claude's adaptive thinking for that agent and add the + budget to its output cap, so the answer keeps its headroom. Claude Haiku 5.5 + takes no thinking budget of its own, so the value is room, not a limit; the + stamp records it and the summary names the measurement. With both at 0 no + request changes. (#12124) + +### Changed + +- **Claude answers every structured question under a structured-output + format.** The adapter plan, the reviewer verdict and the judge verdict use + `output_config.format` with the schema made strict by the SDK's + `transform_schema`, instead of a forced tool call, so constrained decoding + guarantees the JSON shape, the types, the enums and the required fields; the + repairs keep mending the lengths and counts a grammar cannot enforce. Unlike + a forced tool call, the format lets a thinking model think first. (#12124) +- **The agent validator admits pure computing modules.** By the owner's + decision, adapted code may import `warnings`, `typing`, `calendar`, + `decimal`, `fractions`, `zoneinfo`, `enum`, `dataclasses`, `operator`, + `bisect`, `heapq`, `numbers`, `cmath`, `scipy.fft`, `scipy.linalg` and + `scipy.constants`. Each was walked for names that read, write, fetch, + evaluate or set the process locale, and those stay banned, as do `io`, `sys`, + `pathlib`, `random`, `locale` and `string`. `calendar.main` is banned on the + calendar module only, so a column read as `df.main` still passes. A banned import's repair line names the module and + what to use instead, and the eval record counts banned imports by module. The + log names a module only by a word of a fixed list (the standard library, the + allowlists and a few known packages), and any other import as `?`, so no word + the model writes as an import path reaches the log. (#12124) +- **Edits match tolerantly, never fuzzily.** After an exact match, the edit + applier tries the same text with the blanks before line breaks ignored, then + whole lines whose words agree once indentation and spaces outside string + literals are ignored, with the replacement re-indented; a literal's text must + agree exactly. The first tier with any match decides, and the + unique-match and protected-region checks stay. A `find` that only matches the + code before an earlier edit of the same plan gets its own repair line, and + the eval record counts the edits each tier applied. (#12124) + +### Fixed + +- **Box-basic's repair no longer fails on a protected line it must change.** + The edit applier protected every theme conditional, including box-basic + matplotlib's data-dependent `_tight_text_color`, so 12 of the 13 + `drift:theme_token` failures in the rerun of spike X refused a correct full + file. A theme token is now a value picked per theme from literals and + earlier tokens only, and the drift check compares meaning, so a token + re-stated in other quotes or spacing passes. (#12124) +- **The placeholder failure says how to add code after `df = + load_user_data()`.** The repair line and the adapter prompt now say to keep + the line out of `find` and `replace` and name the line below it to anchor an + insertion on, the cause of the rerun's 5 `protected:placeholder` failures. + (#12124) + +### Security + +- **The agent validator closes module re-exports and private helpers.** An + allowed module could hand out a module the import rules keep out + (`statistics.sys.modules`, `collections._sys`, `enum.bltns`) or a private + helper that runs `exec` (`dataclasses._FuncBuilder`); some of these passed + the validator before. Such names are now banned as attributes, as `from` + imports and as components of an import path below its root, and so is any + private name or private submodule of an imported module. A module bound by + `import` may no longer be used as a value, so `[calendar][0]._locale` cannot + launder it, and the dangerous private helpers a walk of the allowed modules + found (`_locale`, `_random`, `_tzpath`, `_FuncBuilder`, numpy's `_datasource` + and a few more) are banned by name on any object. No catalogue file changes + eligibility. (#12124) diff --git a/docs/concepts/agent-network.md b/docs/concepts/agent-network.md index 1055934219..328786bba6 100644 --- a/docs/concepts/agent-network.md +++ b/docs/concepts/agent-network.md @@ -65,17 +65,17 @@ Only one agent talks to the user. Everything the research shows an LLM does not | `anyplot` (root) | `Agent` with no `mode` (chat root); the only user-facing agent | `AGENT_MODEL` (claude-haiku-5-5; gemini-3.8-flash on the Gemini arm), effort `low` with thinking disabled on Claude, `thinking_level=LOW` on Gemini, `max_output_tokens` 4096 | `static_instruction` from `agents/anyplot/prompts/root.md`: scope list, fixed-refusal rule, reply-language rule, tool rules, the "never" list. An `InstructionProvider` adds only server-validated values (locale, spec id, library, dataset status, whether bindings are complete, plot versions); catalogue text such as the spec title never enters it, because that block has instruction priority, and the root reads it fenced through `get_spec_brief` | `get_dataset_profile`, `get_spec_brief`, `get_current_code(version)`, `set_bindings`, `plot_pipeline`. Phase 2 adds `find_specs`, `get_spec_knowledge`, `select_spec` | Chat text | | `adapter_` | `Agent(mode="single_turn")`, one per enabled library, run with `ctx.run_node` inside the pipeline, `include_contents='none'` | `AGENT_MODEL`, effort `medium` on Claude, `thinking_level=MEDIUM` on Gemini; `max_output_tokens` 8192 for edit-only calls (on Gemini thinking tokens count toward the cap, and a call cut off at 8192 still leaves the repair its 75 s of the soft deadline), and when a full file is allowed 16384 on Claude and 12288 on Gemini, with `thinking_level=LOW` there; every cap leaves ample headroom above the largest measured answer (see `agents/README.md`) | `static_instruction` (never `instruction`, because the verbatim prompt files contain `{THEME}`, `{lang}` and `{lib}` braces that ADK would template): `prompts/adapter.md` plus `prompts/default-style-guide.md` and `prompts/library/.md` verbatim, about 8k to 10k tokens; a cached prefix on Claude through the App's `ContextCacheConfig` (5-minute lifetime), and above the 6,144-token implicit-cache minimum of Gemini 3.8 Flash | none | `AdaptPlan` | | `reviewer` | `Agent(mode="single_turn")`, no tools; its `before_model_callback` appends the PNG of the rendered theme as ordinary user content after a label that names the theme (on Gemini at `media_resolution=MEDIUM`, 560 tokens), loaded by `render_id` from the render store, never from state | `AGENT_MODEL`, effort `low` on Claude, LOW on Gemini, `max_output_tokens` 4096 | `static_instruction` from `prompts/reviewer.md`: a reduced checklist (VQ-01, VQ-02, VQ-03, VQ-06, VQ-07, SC-01, SC-03, DQ-03 for stale claims, AR-09 for clipping), the theme-readability and defect-grammar sections adapted from `prompts/workflow-prompts/ai-quality-review.md`, and the style guide | none | `Verdict{ok, defects[]}` with fixed ids, rendered into the existing `DEFECT_RE` grammar on the server; an image defect is filed under the rendered theme whatever theme the reviewer named, a `code` defect stays `code` | -| Scope judge (not an agent) | A direct model call inside `ScopeGuardPlugin`, client built by `make_judge_client()`: an `AsyncAnthropicVertex` client answering through a forced tool call on Claude, `genai.Client(enterprise=True, project=..., location=settings.location)` on Gemini | `AGENT_JUDGE_MODEL` (claude-haiku-5-5; gemini-3.5-flash-lite on the Gemini arm), JSON schema, at most 256 output tokens, 4 s budget with one retry after a 0.5 s back-off, on Gemini its own safety filters off | `prompts/scope_judge.md` | none | `{verdict: in_scope|out_of_scope|attack|needs_data, lang}` | +| Scope judge (not an agent) | A direct model call inside `ScopeGuardPlugin`, client built by `make_judge_client()`: an `AsyncAnthropicVertex` client answering under a structured-output format (`output_config.format`) on Claude, `genai.Client(enterprise=True, project=..., location=settings.location)` on Gemini | `AGENT_JUDGE_MODEL` (claude-haiku-5-5; gemini-3.5-flash-lite on the Gemini arm), JSON schema, at most 256 output tokens, 4 s budget with one retry after a 0.5 s back-off, on Gemini its own safety filters off | `prompts/scope_judge.md` | none | `{verdict: in_scope|out_of_scope|attack|needs_data, lang}` | The root's "never" list, enforced by the stream translator and the evals: never write or run code (code changes happen only through `plot_pipeline`; code answers are short prose that references lines from `get_current_code`; the translator strips fenced code blocks longer than 10 lines); never invent spec ids; never paste data rows (naming a single value the user asks about is fine); never emit links (a scheme or `www.`; a bare web address such as `jana-novak.de` is data), HTML or markdown images; never claim success unless `PlotResult.status` is `ok` or `needs_attention`. Rules for every agent: -- Models come only from `make_model()` in `agents/anyplot/models.py`, which builds the model of the configured provider. On `anthropic-vertex` (the default) it returns `VertexClaude`, ADK's `Claude` with an `AsyncAnthropicVertex` client built from the settings, thinking disabled, and a forced tool call for the adapter's and the reviewer's response schemas, because ADK 2.11 sends no response schema to Claude. On `gemini` it returns `Gemini(model=..., client_kwargs={"enterprise": True, "project": ..., "location": settings.location}, retry_options=HttpRetryOptions(attempts=3, initial_delay=1))`. The bare `Gemini()` default is the retiring 2.5 Flash and is never used. +- Models come only from `make_model()` in `agents/anyplot/models.py`, which builds the model of the configured provider. On `anthropic-vertex` (the default) it returns `VertexClaude`, ADK's `Claude` with an `AsyncAnthropicVertex` client built from the settings, thinking disabled (for measurements only, `AGENT_ROOT_THINKING_BUDGET` and `AGENT_ADAPTER_THINKING_BUDGET`, 0 by default and at most 4,096, turn on adaptive thinking for the root or the adapter and add the budget to that agent's output cap; the eval harness sets them with `--thinking-budget`), and Claude's structured outputs for the adapter's and the reviewer's response schemas, because ADK 2.11 sends no response schema to Claude: the request's `output_config.format` carries the schema made strict by the SDK's `transform_schema`, so constrained decoding enforces types, enums and required fields, and the Pydantic schema with its repairs checks the lengths and counts the grammar cannot. On `gemini` it returns `Gemini(model=..., client_kwargs={"enterprise": True, "project": ..., "location": settings.location}, retry_options=HttpRetryOptions(attempts=3, initial_delay=1))`. The bare `Gemini()` default is the retiring 2.5 Flash and is never used. - No `FallbackModel` in phase 1. A fallback would need a second model id or the `global` endpoint, which breaks the pins; persistent 429s map to `error{code:"capacity"}`. A named fallback on `eu` is an explicit owner decision; switching `AGENT_PROVIDER` is a configuration change, not a fallback. - Low-cardinality `labels` in every agent's generation config. On Gemini: explicit `safety_settings` at BLOCK_MEDIUM_AND_ABOVE, and no temperature, top_p or thinking budget on Gemini 3. - `agents/anyplot/agent.py` keeps an explicit registry `ALL_AGENTS = [root_agent, *ADAPTERS.values(), reviewer]`, because the adapters and the reviewer are started with `ctx.run_node` and a tree walk from the root would miss them. A unit test iterates the registry and `App.plugins` and asserts: no `code_executor`, `google_search`, `url_context`, `AgentTool`, `LoggingPlugin` or chat-mode sub-agents; no agent has a non-empty string `instruction`; every model string and client location comes from settings. -- Settings defaults in `AgentSettings` (`agents/anyplot/settings.py`, read from `AGENT_*` variables): `provider="anthropic-vertex"`, `model="claude-haiku-5-5"`, `judge_model="claude-haiku-5-5"`, `location="eu"`. The Gemini arm sets `AGENT_PROVIDER=gemini`, `AGENT_MODEL=gemini-3.8-flash` and `AGENT_JUDGE_MODEL=gemini-3.5-flash-lite`. The settings refuse `-latest` aliases, model ids that do not match the provider, Claude models outside `CLAUDE_MODELS` (newer Claude models refuse the forced tool call with thinking disabled), any location other than `eu`, `us` or `global`, and `renderer="local"` outside development. +- Settings defaults in `AgentSettings` (`agents/anyplot/settings.py`, read from `AGENT_*` variables): `provider="anthropic-vertex"`, `model="claude-haiku-5-5"`, `judge_model="claude-haiku-5-5"`, `location="eu"`. The Gemini arm sets `AGENT_PROVIDER=gemini`, `AGENT_MODEL=gemini-3.8-flash` and `AGENT_JUDGE_MODEL=gemini-3.5-flash-lite`. The settings refuse `-latest` aliases, model ids that do not match the provider, Claude models outside `CLAUDE_MODELS` (newer Claude models refuse disabled thinking with a 400), any location other than `eu`, `us` or `global`, and `renderer="local"` outside development. Deviations from the first sketch, each evidence-backed: there is no data-intake LLM agent in step 1 (about 320 of 325 specs declare typed `## Data` role bullets, so default bindings are deterministic and confirmed in UI dropdowns; parsing must be code anyway because `thousands='.'` silently corrupts `DD.MM.YYYY` dates); the reviewer is tool-less (GA image path, native `response_schema`, nothing persisted in session events, no `load_artifacts` listing of `data.csv`); the domain knowledge lives in tools and prompts rather than in a separate expert agent. @@ -207,9 +207,9 @@ Schemas live in `agents/anyplot/schemas.py`: `ColumnProfile{name≤64, dtype, mi - **Normaliser** (per runtime; matplotlib and seaborn first): drop `bbox_inches`, keep the figure size and rescale the dpi so the output hits 3200x1800 or 2400x2400 exactly (within the canvas gate's 16 px when the aspect is a hair off and no dpi is exact), strip the `sys.path` guard, the catalogue header and the catalogue title (the title string becomes `""`; the call that shows it stays). The canvas is never an LLM repair target. - **Readiness scan** on the normalised original: `blocked` (the 13 map specs, a library that is not enabled, a SECURITY validator failure, no `THEME` block, a non-literal savefig target), `coupled` (literal limits, data-coordinate annotations, literal statistics, short palettes, hard-coded date locators; these findings go to the adapter as hints), or `clean`. -- **Edits** apply in order. Each `find` must match exactly once and may not overlap a protected region. Protected regions are found by AST: the `THEME` assignment and every module-level assignment whose value tests `THEME == ...` (`PAGE_BG`, `ELEVATED_BG`, `INK*`, `GRID`, `BRAND`), the placeholder line, and the final savefig statement. The `IMPRINT` palette list is not protected: it stays one literal list, assigned once, that keeps its existing entries in order and may only be extended with further positions from `core/palette.py`, which the validator checks. With more than eight groups the adapter keeps palette colors for the seven largest and draws the rest in the theme's muted ink as one "Other" group, which the reviewer accepts under VQ-07. +- **Edits** apply in order. Each `find` must match exactly once and may not overlap a protected region. Matching goes from strict to tolerant and is never fuzzy: an exact substring first, then the same with the blanks before line breaks ignored, then whole lines whose words agree once indentation and runs of blanks outside string literals are ignored, with the replacement moved by the one indentation shift between them. A string literal's text must agree exactly, and a line inside a multi-line literal never matches this way. The first tier that finds anything decides, so more than one match there is a failure. A `find` that matches, in any tier, only the code before an earlier edit of the same plan changed it is reported as such (`stale_match`). Protected regions are found by AST: the `THEME` assignment, every theme token (a module-level assignment that tests `THEME == ...` and is built only from literals, `THEME` and earlier tokens: `PAGE_BG`, `ELEVATED_BG`, `INK*`, `GRID`, `BRAND`), the placeholder line, and the final savefig statement. A theme conditional that reads the data, such as box-basic's `_tight_text_color = INK if THEME == "dark" else IMPRINT_PALETTE[tightest_idx]`, is plot code the adapter may change. After a plan, a protected statement must keep its meaning, compared as a syntax tree, so a full file that re-states a token in other quotes or spacing passes. The `IMPRINT` palette list is not protected: it stays one literal list, assigned once, that keeps its existing entries in order and may only be extended with further positions from `core/palette.py`, which the validator checks. With more than eight groups the adapter keeps palette colors for the seven largest and draws the rest in the theme's muted ink as one "Other" group, which the reviewer accepts under VQ-07. - **Loader**: exactly one `df = load_user_data()` in the working form. The server replaces it with `pd.read_csv("data.csv", dtype=..., parse_dates=[...])` using a relative path, so the exported code also works in notebooks. -- **Validator profiles.** The SECURITY rules run on the normalised original (eligibility) and on every adapted working form: `ast.parse` on at most 48 KB; imports only from COMMON (numpy, pandas, math, statistics, datetime, collections, itertools, functools, textwrap, colorsys, re; not `string`) or the per-library allowlist (the plotting submodules, for matplotlib and seaborn also the bundled `mpl_toolkits.{mplot3d,axes_grid1,axisartist}`, `scipy.{stats,interpolate,signal,cluster.hierarchy,spatial,optimize,special,integrate,ndimage}`, sklearn without `datasets`, `statsmodels.{api,nonparametric,tsa,graphics}`; third-party helpers such as cartopy or adjustText stay out); never `statsmodels.formula`, `scipy.io`, `scipy.datasets`, sys, pathlib, importlib, subprocess, socket, urllib, requests, shutil, ctypes or pickle; `os` only as `getenv("ANYPLOT_THEME", ...)`; banned names `eval`, `exec`, `compile`, `open`, `__import__`, `input`, `breakpoint`, `globals`, `locals`, `vars`, `getattr`, `setattr`, `delattr`, `memoryview`, `exit`, `quit` and the three-argument `type()`; every name, attribute or keyword matching `^__\w+__$` and any string literal containing a dunder identifier; banned attributes `.format`, `.format_map`, pandas `.eval` and `.query`, `memmap`, `ctypeslib`, `f2py`, `matplotlib.use`, `show`; banned calls: pandas `read_*`, `ExcelFile`, `HDFStore` and the I/O writers (`to_csv`, `to_pickle`, `to_parquet`, `to_excel`, `to_sql`, `to_hdf`, `to_feather`, `to_json`, `to_html`, `to_clipboard`, but not `to_datetime` or `to_numpy`), `np.load*`, `save*`, `savetxt`, `fromfile`, `fromregex`, `DataSource`, `loadtxt`, `genfromtxt`, `ndarray.tofile`, `plt.imread`, `matplotlib.image.imread`, `matplotlib.cbook.get_sample_data`, `mpl.rc_file`, `load_dataset`, `fetch_*`, `urlopen`; no class, async, await, yield, global or nonlocal statements, no star or relative imports; no `http(s)://`, `ftp://`, `file:` or `data:` literals; string literals at most 200 characters and at most 2,000 characters of new literal text per plan; the savefig target is exactly `f"plot-{THEME}.png"`; `def` and `lambda` stay allowed for formatters. The ADAPTATION rules run on adapted code only: exactly one placeholder; no RNG used to create data (a seeded `np.random.default_rng()` is allowed for jitter and subsampling, and the row-fidelity gate catches fabricated data); numeric literal lists of at most 20 items; the palette prefix preserved. SECURITY failures reject the code and get a repair attempt while one is allowed; ADAPTATION failures become defect lines. Every bypass named here is a validator-corpus test. +- **Validator profiles.** The SECURITY rules run on the normalised original (eligibility) and on every adapted working form: `ast.parse` on at most 48 KB; imports only from COMMON (numpy, pandas, math, statistics, datetime, collections, itertools, functools, textwrap, colorsys, re, and since the owner's decision of 2026-10-10 the pure computing modules warnings, typing, calendar, decimal, fractions, zoneinfo, enum, dataclasses, operator, bisect, heapq, numbers and cmath; not `string`, `locale` or `random`) or the per-library allowlist (the plotting submodules, for matplotlib and seaborn also the bundled `mpl_toolkits.{mplot3d,axes_grid1,axisartist}`, `scipy.{stats,interpolate,signal,cluster.hierarchy,spatial,optimize,special,integrate,ndimage,fft,linalg,constants}`, sklearn without `datasets`, `statsmodels.{api,nonparametric,tsa,graphics}`; third-party helpers such as cartopy or adjustText stay out); never `statsmodels.formula`, `scipy.io`, `scipy.datasets`, sys, pathlib, importlib, subprocess, socket, urllib, requests, shutil, ctypes, pickle or io; `os` only as `getenv("ANYPLOT_THEME", ...)`; a `banned-import` message names the module and what to use instead, and the finding carries a label of the path for the attribution log (its longest prefix in a closed vocabulary of module names, else `?`); each admitted module was walked for names that read, write, fetch or evaluate, and those are banned (`warnings.formatwarning`, `showwarning` and `warn_explicit`, `zoneinfo.reset_tzpath`, `available_timezones` and `ZoneInfo.from_file`, `typing.get_type_hints` and `ForwardRef`, `operator.attrgetter` and `methodcaller`, `enum.global_enum`, calendar's `LocaleTextCalendar`, `LocaleHTMLCalendar` and `different_locale`, which set the process locale, and `calendar.main`, which reaches them and is banned on the calendar module only, because a column may be called `main`); module re-exports that reach what the import rules keep out are banned as attributes on any object and as `from` imports (`sys`, `os`, `builtins`, enum's `bltns`, `modules`, `inspect`, `subprocess`, `importlib`, `pathlib`, `shutil`, `socket`, `linecache`, `CodeType`, `FunctionType`), and so is any private name (one leading underscore) of an imported module, such as `collections._sys` or `dataclasses._FuncBuilder`; both rules cover the components of an import path below its root as well, so `import scipy.fft._backend as backend` is refused; a module bound by `import` is only ever the receiver of an attribute, never a value, so `[calendar][0]` is refused; and the dangerous private helpers a walk of the allowed modules found (`_sys`, `_locale`, `_random`, `_tzpath`, `_common`, `_getcategory`, `_FuncBuilder` and numpy's `_datasource`) are banned by name on any object, so a module reached through an attribute chain or a `from` import cannot hand them out either; banned names `eval`, `exec`, `compile`, `open`, `__import__`, `input`, `breakpoint`, `globals`, `locals`, `vars`, `getattr`, `setattr`, `delattr`, `memoryview`, `exit`, `quit` and the three-argument `type()`; every name, attribute or keyword matching `^__\w+__$` and any string literal containing a dunder identifier; banned attributes `.format`, `.format_map`, pandas `.eval` and `.query`, `memmap`, `ctypeslib`, `f2py`, `matplotlib.use`, `show`; banned calls: pandas `read_*`, `ExcelFile`, `HDFStore` and the I/O writers (`to_csv`, `to_pickle`, `to_parquet`, `to_excel`, `to_sql`, `to_hdf`, `to_feather`, `to_json`, `to_html`, `to_clipboard`, but not `to_datetime` or `to_numpy`), `np.load*`, `save*`, `savetxt`, `fromfile`, `fromregex`, `DataSource`, `loadtxt`, `genfromtxt`, `ndarray.tofile`, `plt.imread`, `matplotlib.image.imread`, `matplotlib.cbook.get_sample_data`, `mpl.rc_file`, `load_dataset`, `fetch_*`, `urlopen`; no class, async, await, yield, global or nonlocal statements, no star or relative imports; no `http(s)://`, `ftp://`, `file:` or `data:` literals; string literals at most 200 characters and at most 2,000 characters of new literal text per plan; the savefig target is exactly `f"plot-{THEME}.png"`; `def` and `lambda` stay allowed for formatters. The ADAPTATION rules run on adapted code only: exactly one placeholder; no RNG used to create data (a seeded `np.random.default_rng()` is allowed for jitter and subsampling, and the row-fidelity gate catches fabricated data); numeric literal lists of at most 20 items; the palette prefix preserved. SECURITY failures reject the code and get a repair attempt while one is allowed; ADAPTATION failures become defect lines. Every bypass named here is a validator-corpus test. ### Render @@ -317,7 +317,7 @@ Testing a newer model version, the other provider's arm, a different judge model Built on 2026-10-10: the harness, the two fixture generators and the 145 fixture cases (the 122 of the full matrix and the 23 stress cases with their model-free `--static` stage), the reports, the diff, the review gallery, the blind two-run gallery with its scoring, the model-free catalogue eligibility sweep `agents/evals/eligibility.py` (`agents/README.md`, "Run the regression harness"), and the judge-only scope eval `agents/evals/scope.py` with its refusal recall and false-refusal rate (`agents/README.md`, "Run the scope eval"; its first run passed both gates). Not built yet: `--prompts `, `sync_cases.py`, the Gemini baseline, and the CI workflow. - **Fixture cases**, each a directory with `case.json` (spec, library, bindings, expected `accepted`, `rejected` or `unknown`, tags, the perturbation and the source of the data), `data.csv`, and an optional `change_request.txt`. Two sources: the committed synthetic fixtures in `agents/evals/fixtures/cases//`, which are the spike-X matrix (10 specs times matplotlib and seaborn times 6 perturbation datasets, written by the seeded generator `agents/evals/make_fixtures.py`) plus two hand-written cases and the 23 stress cases of `agents/evals/make_stress.py` (tag `stress`, left out of `full`; see [Bounds](#bounds)), and the promoted feedback cases synced from the private bucket into the gitignored `agents/evals/.cases/` directory (never committed, because they hold users' data). The tag `smoke` marks the 12 cases of the smoke set. The scope set is the judge-only case file `agents/evals/scope.evalset.json` (see Evals below); the injection and code-escape sets stay as flow evals. -- **Harness** (`agents/evals/matrix.py`, the spike-X runner made permanent): runs every case through the real `/v1` flow in-process (httpx `ASGITransport` against `agents.main:app`, the same plugins and run queue, the models from the settings, and the render backend `--renderer` names: `remote` against a deployed renderer, `local` in Docker, or `fake` for a dry run of everything but the render) with candidate settings that are passed only to the harness: `--provider` (alone it selects that provider's default models; the settings refuse a model from the other provider), `--model`, `--judge-model`, `--location`, `--cases smoke|full|stress|`, `--static` (only the model-free static stage of `agents/evals/stress.py`: parse, eligibility, bindings, loader, prompt sizes and the stand-in renders), `--repeats N`, `--budget-usd` (stop before a case that could take the estimated list-price cost past it), and `--gcloud-token` (mint the developer's renderer token with the gcloud CLI and renew it before its one-hour expiry). Before the first case it renders each library's catalogue file once through the configured backend, without a model call, and stops with exit code 2 when that fails; during the run a pipeline that ended in `RendererUnavailable` (the `error` class on its `pipeline_result` attribution line), or three errors in a row, stop it with exit code 4, so an outage never becomes a matrix of failed runs or a baseline. Per case it records the status and reason, attempts, gate failures by gate, validator rejections, edit-apply failures, the reviewer verdict, LLM calls, `model_version`, tokens (prompt, candidates, thoughts, cached, cache writes, the judge's input and output), cost at list price, the time to the first event, the end-to-end time and the render time, and, since report schema 2, the stage that stopped each attempt and the run (for example `adapter_truncated`, `edit_apply` or `reviewer_defects`, never a bare `validation`), the shipped and the last reviewed attempt, each attempt's adapter outcome, finish reason and plan shape, the edit-failure kinds, every model call's finish reason and tokens, and the answers that failed their schema, by agent kind and outcome (`answer_outcomes`: `repaired` or `refused`) and by broken rule (`answer_rules`). It reads them from the stream and from the content-free attribution lines that the pipeline, the Budget plugin and the judges write per request id. It writes `/-.json`, a Markdown summary, and a review gallery of 30 renders sampled at random from the runs that passed, with an accept and reject pair each and a JSON export, and prints a diff table against `agents/evals/baselines/.json` (or `--baseline`) over the cases both reports hold: pass rate, accept-match rate (cases whose outcome matches the case's expected outcome), cost per successful plot, p50 and p95 latency, and every case that flipped. A run passes when it ships a render that passed every deterministic gate (no padded canvas, no ADAPTATION finding); every rate counts harness errors as runs that did not pass. The owner's judgement is the acceptance measure: to compare two arms, `python -m agents.evals.report blind` merges both reports into one gallery of the same cases with the arm hidden in a separate key file, and `report score` gives the acceptance per arm. The exit code is 1 when the pass rate on the shared cases drops below the baseline minus a tolerance (5 percentage points by default) or a run crashed in the harness, 2 for a setup error, 3 when the budget stopped the run, and 4 for an outage. +- **Harness** (`agents/evals/matrix.py`, the spike-X runner made permanent): runs every case through the real `/v1` flow in-process (httpx `ASGITransport` against `agents.main:app`, the same plugins and run queue, the models from the settings, and the render backend `--renderer` names: `remote` against a deployed renderer, `local` in Docker, or `fake` for a dry run of everything but the render) with candidate settings that are passed only to the harness: `--provider` (alone it selects that provider's default models; the settings refuse a model from the other provider), `--model`, `--judge-model`, `--location`, `--cases smoke|full|stress|`, `--static` (only the model-free static stage of `agents/evals/stress.py`: parse, eligibility, bindings, loader, prompt sizes and the stand-in renders), `--repeats N`, `--budget-usd` (stop before a case that could take the estimated list-price cost past it), and `--gcloud-token` (mint the developer's renderer token with the gcloud CLI and renew it before its one-hour expiry). Before the first case it renders each library's catalogue file once through the configured backend, without a model call, and stops with exit code 2 when that fails; during the run a pipeline that ended in `RendererUnavailable` (the `error` class on its `pipeline_result` attribution line), or three errors in a row, stop it with exit code 4, so an outage never becomes a matrix of failed runs or a baseline. Per case it records the status and reason, attempts, gate failures by gate, validator rejections, edit-apply failures, the reviewer verdict, LLM calls, `model_version`, tokens (prompt, candidates, thoughts, cached, cache writes, the judge's input and output), cost at list price, the time to the first event, the end-to-end time and the render time, and, since report schema 2, the stage that stopped each attempt and the run (for example `adapter_truncated`, `edit_apply` or `reviewer_defects`, never a bare `validation`), the shipped and the last reviewed attempt, each attempt's adapter outcome, finish reason and plan shape, the edit-failure kinds, every model call's finish reason and tokens, the answers that failed their schema, by agent kind and outcome (`answer_outcomes`: `repaired` or `refused`) and by broken rule (`answer_rules`), the edits each tolerant matching tier applied (`edit_tolerant`), and the module labels of banned imports (`banned_imports`). It reads them from the stream and from the content-free attribution lines that the pipeline, the Budget plugin and the judges write per request id. It writes `/-.json`, a Markdown summary, and a review gallery of 30 renders sampled at random from the runs that passed, with an accept and reject pair each and a JSON export, and prints a diff table against `agents/evals/baselines/.json` (or `--baseline`) over the cases both reports hold: pass rate, accept-match rate (cases whose outcome matches the case's expected outcome), cost per successful plot, p50 and p95 latency, and every case that flipped. A run passes when it ships a render that passed every deterministic gate (no padded canvas, no ADAPTATION finding); every rate counts harness errors as runs that did not pass. The owner's judgement is the acceptance measure: to compare two arms, `python -m agents.evals.report blind` merges both reports into one gallery of the same cases with the arm hidden in a separate key file, and `report score` gives the acceptance per arm. The exit code is 1 when the pass rate on the shared cases drops below the baseline minus a tolerance (5 percentage points by default) or a run crashed in the harness, 2 for a setup error, 3 when the budget stopped the run, and 4 for an outage. - **Baselines.** `agents/evals/baselines/claude-haiku-5-5.json` is committed with the default model: the rerun of spike X on main in the evening of 2026-10-10 (commit `6d0de1e0acd7`, 244 runs, 122 cases with two repeats each). `gemini-3.8-flash.json` follows with the Gemini arm, so the price and quality comparison is a diff between two reports. `--save-baseline` refuses a run that stopped early, holds promoted cases, or has a run that ended in an error. A model bump is a PR that changes the `AGENT_MODEL` default and adds the new baseline file; a unit test asserts that a baseline exists for the pinned model once any baseline is committed. The same mechanism covers judge-model bumps and ADK upgrades, because the ADK version is part of the report stamp. - **CI.** `.github/workflows/agents-eval.yml` has `workflow_dispatch` inputs `provider`, `model`, `judge_model`, `location`, `cases` (smoke or full) and `repeats`, and a nightly `smoke` run on the pinned model. It authenticates through Workload Identity Federation, sets `GOOGLE_CLOUD_LOCATION=eu`, uploads the report as an artifact and writes the diff table into the job summary. It mirrors the candidate-versus-baseline pattern of `review-retest.yml` (see [Review retest](../workflows/review-retest.md)). A lifecycle probe in the nightly run calls `models.get` on the pinned model and fails loudly on a 404 or a retirement notice, so a forced bump of the pinned model, or of the Gemini arm's short-term 3.8 Flash tier, is noticed before users are. - **Cost.** A full run is 122 cases times at most 2 attempts. Measured on 2026-10-10 at list price for the full matrix: $0.52 on the Claude Haiku 5.5 arm ($0.0051 per passed plot, median $0.0041, 4.77 model calls per run) and $15.89 on the Gemini 3.8 Flash arm ($0.169 per passed plot, median $0.124, 4.80 model calls per run, $3.99 of it on failed runs); `smoke` (12 cases) is about a tenth of a full run. Both fit inside the spend cap. @@ -387,7 +387,7 @@ Owner tasks 1 to 6 come first. Then run the spikes: | X | Adaptation: 10 specs times matplotlib and seaborn times 6 datasets (renamed, x10, n=12, n=5000, date, decimal comma; the cases are in `agents/evals/fixtures/cases/` and run with `agents/evals/matrix.py`, see `agents/README.md`) on both arms, `claude-haiku-5-5` and gemini-3.8-flash, so the owner picks the phase-1 default by cost and acceptance; plotly only on figure-level gates because the phase-1 image has no Chrome. Pass means all deterministic gates plus an owner blind accept or reject on a sample of 30 renders (`python -m agents.evals.report blind` shows both arms' renders of the same cases without naming the arm). Also reports post-normaliser eligibility under the SECURITY profile and the files blocked for a missing `THEME` block or a non-literal savefig target (`agents/evals/eligibility.py`, no model call) | Under $25 | | C | Does a NodeTool `Event(message)` arrive before the function response under `StreamingMode.NONE`? | Yes | | F | Does a plugin's `on_tool_error` catch a `ValueError`? Is the content returned when `before_run` halts emitted? | Both observed | -| A′ | Does the reviewer read a canary string from callback-injected images on a single-turn agent with `include_contents='none'` while its structured output still applies (the native `response_schema` on Gemini, the forced tool call on Claude)? | Exact match; fallback is a `view_render` tool returning Parts, which reintroduces the Preview multimodal-function-response path | +| A′ | Does the reviewer read a canary string from callback-injected images on a single-turn agent with `include_contents='none'` while its structured output still applies (the native `response_schema` on Gemini, the `output_config.format` on Claude)? | Exact match; fallback is a `view_render` tool returning Parts, which reintroduces the Preview multimodal-function-response path | Scaffold `agents/`, run `adk web --port 8002 --session_service_uri memory:// --artifact_service_uri memory:// agents` with a development-only `fixture` state seed (snapshot, dataset store and bindings from `agents/evals/fixtures`; refused in production) and the full stack (agents on :8001, the API on :8000, the app on :3000) with `AGENT_RENDERER=local`. diff --git a/tests/unit/agents/code/test_edits.py b/tests/unit/agents/code/test_edits.py index 604f0df943..849d680636 100644 --- a/tests/unit/agents/code/test_edits.py +++ b/tests/unit/agents/code/test_edits.py @@ -155,7 +155,6 @@ def test_overlapping_occurrences_count() -> None: ('THEME = os.getenv("ANYPLOT_THEME", "light")', "THEME assignment"), ('PAGE_BG = "#FAF8F1"', "theme token PAGE_BG"), ('else "#F0EFE8"\nIMPRINT', "theme token INK"), - ("df = load_user_data()", "placeholder line"), ("dpi=400, facecolor=PAGE_BG)", "final savefig statement"), ("ax.set_ylim(0, 100)\nplt.savefig", "final savefig statement"), ], @@ -168,6 +167,19 @@ def test_protected_regions_refuse_edits(find: str, what: str) -> None: assert "anchor the edit on other lines" in result.failures[0] +@pytest.mark.parametrize("find", ["df = load_user_data()", "df = load_user_data()\nfig, ax"]) +def test_placeholder_overlap_names_the_line_to_anchor_on(find: str) -> None: + """The rerun's five `protected:placeholder` failures were edits that extended the line; say how to add after it.""" + result = apply((find, find.replace("load_user_data()", "load_user_data().dropna()"))) + + failure = result.failures[0] + assert result.kinds == ("protected:placeholder",) + assert "overlaps the protected df = load_user_data() line at line 10" in failure + assert "keep that line exactly as it is and out of find and replace" in failure + assert 'set find to line 11 ("fig, ax = plt.subplots(figsize=(8, 4.5), dpi=400)")' in failure + assert "write the new statements in front of that line in replace" in failure + + def test_imprint_is_not_protected_and_neighbours_are_fine() -> None: result = apply( ('IMPRINT = ["#009E73", "#C475FD"]', 'IMPRINT = ["#009E73", "#C475FD", "#4467A3"]'), @@ -202,7 +214,7 @@ def test_placeholder_introduced_by_an_earlier_edit_is_protected() -> None: ("df = load_user_data()\nfig", "df = load_user_data().dropna()\nfig"), working=working, ) - assert result.code is None and "placeholder line" in result.failures[0] + assert result.code is None and "protected df = load_user_data() line" in result.failures[0] def test_failures_skip_and_report_every_bad_edit() -> None: @@ -286,3 +298,191 @@ def test_kinds_count_the_edits_past_the_feedback_cap(self) -> None: def test_a_clean_plan_has_no_kinds(self) -> None: assert apply(("ax.set_ylim(0, 100)", "ax.set_ylim(0, 50)")).kinds == () + + +DATA_THEME_LINE = '_tight_color = INK if THEME == "dark" else IMPRINT[int(df["y"].argmin()) % 2]\n' +WITH_DATA_THEME_LINE = WORKING.replace("ax.set_ylim(0, 100)\n", "ax.set_ylim(0, 100)\n" + DATA_THEME_LINE) + + +class TestThemeTokenDrift: + """The rerun's 13 `drift:theme_token` failures: 12 from box-basic's data-dependent conditional, 1 from a token.""" + + def test_a_data_dependent_theme_conditional_is_plot_code(self) -> None: + """A full file may rewrite it, as box-basic's repair needs; it reads the data, not just the theme.""" + rewritten = WITH_DATA_THEME_LINE.replace( + DATA_THEME_LINE, '_tight_color = INK if THEME == "dark" else IMPRINT[0]\n' + ) + + assert protected_drift(WITH_DATA_THEME_LINE, rewritten) == [] + assert apply_plan(WITH_DATA_THEME_LINE, AdaptPlan(full_code=rewritten)).code == rewritten + + def test_an_edit_may_change_a_data_dependent_theme_conditional(self) -> None: + result = apply(("% 2]", "% len(IMPRINT)]"), working=WITH_DATA_THEME_LINE) + + assert result.failures == [] and result.code is not None and "% len(IMPRINT)]" in result.code + + @pytest.mark.parametrize( + "restated", + [ + "PAGE_BG = '#FAF8F1' if THEME == 'light' else '#1A1A17'", # other quotes + 'PAGE_BG = "#FAF8F1" if THEME=="light" else "#1A1A17" # page', # spacing and a comment + 'PAGE_BG = (\n "#FAF8F1" if THEME == "light" else "#1A1A17"\n)', # line breaks + ], + ) + def test_a_token_restated_with_the_same_value_is_no_drift(self, restated: str) -> None: + full = WORKING.replace('PAGE_BG = "#FAF8F1" if THEME == "light" else "#1A1A17"', restated) + + assert protected_drift(WORKING, full) == [] + assert apply_plan(WORKING, AdaptPlan(full_code=full)).code == full + + def test_a_token_with_another_value_drifts_and_says_to_copy_the_line(self) -> None: + full = WORKING.replace('else "#1A1A17"\nINK', 'else "#000000"\nINK') + + result = apply_plan(WORKING, AdaptPlan(full_code=full)) + + assert result.kinds == ("drift:theme_token",) + assert "theme token PAGE_BG; copy its line unchanged from the current code" in result.failures[0] + + +class TestTolerantMatching: + """Exact, then trailing whitespace, then whitespace-normalised whole lines; never fuzzy.""" + + def test_exact_matches_need_no_tolerance(self) -> None: + assert apply(("ax.set_ylim(0, 100)", "ax.set_ylim(0, 50)")).tolerant == () + + def test_trailing_blanks_in_find_or_code_are_ignored(self) -> None: + working = WORKING.replace("ax.set_ylim(0, 100)\n", "ax.set_ylim(0, 100) \n") + + result = apply( + ('ax.plot(df["x"], df["y"], color=IMPRINT[0]) \nax.set_ylim(0, 100)\n', "ax.grid()\n"), working=working + ) + + assert result.failures == [] and result.tolerant == ("trailing_whitespace",) + assert result.code == WORKING.replace( + 'ax.plot(df["x"], df["y"], color=IMPRINT[0])\nax.set_ylim(0, 100)\n', "ax.grid()\n" + ) + + def test_windows_line_breaks_in_find_are_read_as_newlines(self) -> None: + result = apply(("color=IMPRINT[0])\r\nax.set_ylim(0, 100)", "color=IMPRINT[0])\nax.set_ylim(0, 50)")) + + assert result.failures == [] and result.tolerant == ("trailing_whitespace",) + assert result.code == WORKING.replace("ax.set_ylim(0, 100)", "ax.set_ylim(0, 50)") + + def test_a_block_written_with_less_indentation_is_found_and_the_replacement_indented(self) -> None: + working = WORKING.replace( + "ax.set_ylim(0, 100)\n", "for low in (0, 10):\n ax.set_ylim(low, 100)\n ax.grid(True)\n" + ) + + result = apply( + ("ax.set_ylim(low, 100)\nax.grid(True)\n", "ax.set_ylim(low, 90)\nax.grid(False)\n"), working=working + ) + + assert result.failures == [] and result.tolerant == ("whitespace",) + assert result.code == working.replace( + " ax.set_ylim(low, 100)\n ax.grid(True)\n", " ax.set_ylim(low, 90)\n ax.grid(False)\n" + ) + + def test_a_block_written_with_more_indentation_is_moved_back(self) -> None: + result = apply( + (' ax.plot(df["x"], df["y"], color=IMPRINT[0])\n ax.set_ylim(0, 100)\n', " ax.set_ylim(0, 50)\n") + ) + + assert result.failures == [] and result.tolerant == ("whitespace",) + assert result.code == WORKING.replace( + 'ax.plot(df["x"], df["y"], color=IMPRINT[0])\nax.set_ylim(0, 100)\n', "ax.set_ylim(0, 50)\n" + ) + + def test_lines_without_one_shared_indentation_shift_do_not_match(self) -> None: + working = WORKING.replace("ax.set_ylim(0, 100)\n", "for low in (0, 10):\n ax.set_ylim(low, 100)\n") + + result = apply((" for low in (0, 10):\n ax.set_ylim(low, 100)\n", "pass\n"), working=working) + + assert result.kinds == ("zero_match",) and result.tolerant == () + + def test_words_must_agree_exactly(self) -> None: + """Never fuzzy: one changed character is no match, however close.""" + assert apply(("ax.set_ylim(0, 10)\n", "x\n")).kinds == ("zero_match",) + assert apply(("ax.set_ylim(0,100)\n", "x\n")).kinds == ("zero_match",) + + def test_several_tolerant_matches_are_a_multi_match(self) -> None: + """The first tier that finds anything decides; two candidates there are never resolved by guessing.""" + grid = "ax.grid(True, alpha=0.3)" + working = WORKING.replace("ax.set_ylim(0, 100)\n", f"if df.empty:\n {grid}\nelse:\n {grid}\n") + + trailing = apply((f"{grid} \n", "x\n"), working=working) + spaced = apply(("ax.grid(True, alpha=0.3)\n", "ax.grid(False)\n"), working=working) + + assert trailing.kinds == ("multi_match",) and spaced.kinds == ("multi_match",) + assert trailing.failures[0].startswith("edit 1/1: find matches 2 times") + + def test_a_tolerant_match_still_respects_the_protected_regions(self) -> None: + result = apply( + (' ax.set_ylim(0, 100)\n plt.savefig(f"plot-{THEME}.png", dpi=400, facecolor=PAGE_BG)', " x") + ) + + assert result.kinds == ("protected:savefig",) + + def test_a_replacement_that_lacks_the_indentation_to_remove_is_no_match(self) -> None: + """Moving `replace` back by four spaces is impossible when a line of it has none: no safe placement.""" + assert apply((" ax.set_ylim(0, 100)\n", "ax.set_ylim(0, 50)\n")).kinds == ("zero_match",) + + @pytest.mark.parametrize( + ("code", "find"), + [ + ('ax.set_title("North America")\n', 'ax.set_title("North America")\n'), + ('ax.set_title("North America")\n', 'ax.set_title("North America")\n'), + ("ax.text(0, 0, '# of runs')\n", "ax.text(0, 0, '# of runs')\n"), + ('ax.text(0, 0, "say \\"a b\\"")\n', 'ax.text(0, 0, "say \\"a b\\"")\n'), + ], + ) + def test_blanks_inside_a_string_literal_must_agree(self, code: str, find: str) -> None: + """A literal's text is the plot's text: other blanks inside it are other code, not a tolerable typo.""" + working = WORKING.replace("ax.set_ylim(0, 100)\n", code) + + assert apply((find, "x\n"), working=working).kinds == ("zero_match",) + + def test_blanks_between_tokens_still_vary_next_to_a_string_literal(self) -> None: + working = WORKING.replace("ax.set_ylim(0, 100)\n", 'ax.set_title("North America", fontsize=12) # top\n') + + result = apply( + ('ax.set_title("North America", fontsize=12) # top\n', 'ax.set_title("Europe")\n'), working=working + ) + + assert result.failures == [] and result.tolerant == ("whitespace",) + assert result.code == working.replace( + 'ax.set_title("North America", fontsize=12) # top\n', 'ax.set_title("Europe")\n' + ) + + def test_a_failed_plan_still_names_the_tiers_its_other_edits_matched_in(self) -> None: + """The log counts every tolerant match, also in a plan that a sibling edit or a drift fails.""" + sibling_failed = apply(("ax.set_ylim(0, 100)\n", "ax.set_ylim(0, 50)\n"), ("not in the code", "x")) + drifted = apply(("ax.set_ylim(0, 100)\n", 'ax.set_ylim(0, 50)\nINK = "#000000"\n')) + + assert (sibling_failed.code, sibling_failed.kinds, sibling_failed.tolerant) == ( + None, + ("zero_match",), + ("whitespace",), + ) + assert (drifted.code, drifted.kinds, drifted.tolerant) == (None, ("drift:rebind",), ("whitespace",)) + + def test_a_line_inside_a_multi_line_string_literal_is_never_matched_by_its_words(self) -> None: + working = WORKING.replace("ax.set_ylim(0, 100)\n", 'note = """first\n second line\n"""\n') + + assert apply(("second line\n", "x\n"), working=working).kinds == ("zero_match",) + + +def test_a_find_that_an_earlier_edit_already_changed_is_a_stale_match() -> None: + result = apply( + ("ax.set_ylim(0, 100)", "ax.set_ylim(0, 50)"), ("ax.set_ylim(0, 100)\nplt", "ax.set_ylim(0, 90)\nplt") + ) + + assert result.kinds == ("stale_match",) and set(result.kinds) <= FAILURE_KINDS + assert result.failures[0].startswith("edit 2/2: find matches the code only as it was before an earlier edit") + assert "merge the two edits into one" in result.failures[0] + + +def test_a_stale_find_is_recognised_in_a_tolerant_tier_too() -> None: + """A stale find written with other blanks than the code gets the stale-match line, not the zero-match one.""" + result = apply(("ax.set_ylim(0, 100)", "ax.set_ylim(0, 50)"), ("ax.set_ylim(0, 100)\n", "ax.set_ylim(0, 90)\n")) + + assert result.kinds == ("stale_match",) diff --git a/tests/unit/agents/code/test_regions.py b/tests/unit/agents/code/test_regions.py index 4d34776745..f57d5c30e6 100644 --- a/tests/unit/agents/code/test_regions.py +++ b/tests/unit/agents/code/test_regions.py @@ -14,7 +14,7 @@ ) from agents.anyplot.code.regions import tests_theme as value_tests_theme # not named test* for pytest -from .conftest import CATALOGUE, source +from .conftest import CATALOGUE, CATALOGUE_FILES, source def text(code: str, span: Span) -> str: @@ -109,6 +109,40 @@ def test_tests_theme(value: str, expected: bool) -> None: assert value_tests_theme(ast.parse(value, mode="eval").body) is expected +def test_a_theme_token_picks_fixed_values_and_a_data_dependent_conditional_is_not_one() -> None: + """The catalogue's shapes: literals, lists, earlier tokens; box-basic's `_tight_text_color` reads the data.""" + code = source(""" + THEME = os.getenv("ANYPLOT_THEME", "light") + INK = "#1A1A17" if THEME == "light" else "#F0EFE8" + BANDS = [0.30, 0.22] if THEME == "light" else [0.45, 0.32] + EDGE = INK if THEME == "light" else "#999999" + IS_DARK = THEME == "dark" + LATER = LATE if THEME == "light" else "#000" + LATE = "#FFF" if THEME == "light" else "#000" + _tight = INK if THEME == "dark" else IMPRINT[best] + ALPHA = 0.3 if THEME == "light" else min(0.5, 0.4) + """) + + assert [token.name for token in find_regions(code).theme_tokens] == ["INK", "BANDS", "EDGE", "IS_DARK", "LATE"] + + +@pytest.mark.skipif(not CATALOGUE_FILES, reason="plots/ is not checked out") +def test_every_catalogue_theme_conditional_but_one_is_a_token() -> None: + """Measured on 2026-10-10: 2,745 theme-testing assignments in the 650 files, one of them data-dependent.""" + narrowed = [] + for path in CATALOGUE_FILES: + code = path.read_text(encoding="utf-8") + tree = ast.parse(code) + tokens = {token.name for token in find_regions(code, tree).theme_tokens} + for stmt in tree.body: + if isinstance(stmt, ast.Assign) and len(stmt.targets) == 1 and isinstance(stmt.targets[0], ast.Name): + name = stmt.targets[0].id + if name != "THEME" and value_tests_theme(stmt.value) and name not in tokens: + narrowed.append(f"{path.parts[-4]}/{path.name}:{name}") + + assert narrowed == ["box-basic/matplotlib.py:_tight_text_color"] + + @pytest.mark.parametrize( ("statement", "expected"), [ diff --git a/tests/unit/agents/code/test_validate.py b/tests/unit/agents/code/test_validate.py index c3ca2f98ff..9b4fefd132 100644 --- a/tests/unit/agents/code/test_validate.py +++ b/tests/unit/agents/code/test_validate.py @@ -18,6 +18,7 @@ PLACEHOLDER, Finding, Profile, + module_label, validate_adaptation, validate_security, ) @@ -277,7 +278,11 @@ def test_relative_import(self, body: str) -> None: assert security_rules(body) == {"relative-import"} def test_import_inside_a_function_is_checked_too(self) -> None: - assert security_rules("def f():\n import subprocess\n return subprocess\n") == {"banned-import"} + # Returning the module is a finding of its own: a module is never a value. + assert security_rules("def f():\n import subprocess\n return subprocess\n") == { + "banned-import", + "module-value", + } def test_from_import_of_a_banned_function(self) -> None: assert security_rules("from numpy import load\n") == {"banned-call"} @@ -303,11 +308,203 @@ def test_import_path_with_a_banned_component(self, body: str, expected: set[str] def test_banned_component_message_names_the_component(self) -> None: findings = validate_security(plot("import numpy.lib.format as fmt\n"), library="matplotlib") - assert [(f.rule, f.message) for f in findings] == [ - ("banned-import", "import of 'numpy.lib.format' reaches the banned name 'format'") + assert [(f.rule, f.message, f.module) for f in findings] == [ + ( + "banned-import", + "import of 'numpy.lib.format' reaches the banned name 'format' → the data comes only from " + "df = load_user_data(), and the plot reads and writes no files", + "numpy.lib.format", + ) + ] + + @pytest.mark.parametrize( + ("body", "module", "label", "alternative"), + [ + ("import locale\n", "locale", "locale", "format numbers with f-strings or matplotlib.ticker.FuncFormatter"), + ("import io\n", "io", "io", "the data comes only from df = load_user_data()"), + ("from io import StringIO\n", "io", "io", "the data comes only from df = load_user_data()"), # banned part + ("import random\n", "random", "random", "a seeded np.random.default_rng() is only for jitter"), + ("from os import getenv\n", "os.getenv", "os", 'os only as THEME = os.getenv("ANYPLOT_THEME", ...)'), + ("import string\n", "string", "string", "use str methods, f-strings and literals"), + ("import scipy\n", "scipy", "scipy", "import the scipy submodule itself, such as scipy.stats or scipy.fft"), + ( + "from sklearn.datasets import load_iris\n", + "sklearn.datasets.load_iris", + "sklearn.datasets", + "the data comes only from df", + ), + ("import yaml\n", "yaml", "yaml", "pure standard modules such as math, datetime, collections, re"), + ("import jane_doe_records\n", "jane_doe_records", "?", "pure standard modules such as math, datetime"), + ], + ) + def test_banned_import_message_names_the_module_and_what_to_use( + self, body: str, module: str, label: str, alternative: str + ) -> None: + """The rerun's 17 banned-import repairs got no hint; the line now names the module and the way around it. + + The repair line goes back to the model and quotes the path; the log gets only its label. + """ + finding = next(f for f in validate_security(plot(body), library="matplotlib") if f.rule == "banned-import") + + assert finding.message.startswith(f"import of '{module}' ") + assert " → " in finding.message and alternative in finding.message.split(" → ", 1)[1] + assert finding.module == label + + def test_only_banned_imports_carry_a_module(self) -> None: + findings = validate_security(plot("import sys\nx = eval('1')\n"), library="matplotlib") + + assert {(f.rule, f.module) for f in findings} == {("banned-import", "sys"), ("banned-name", None)} + + @pytest.mark.parametrize( + ("path", "label"), + [ + ("scipy.io.matlab", "scipy.io"), + ("pandas.io.common", "pandas.io"), + ("numpy.lib.format", "numpy.lib.format"), + ("os.path", "os"), + ("cartopy.crs", "cartopy"), + ("a" * 45 + ".b", "?"), + ("möbius", "?"), + ("data_2024.Q1", "?"), + ("Patient_Smith_Diagnosis", "?"), + ("os_jane_doe", "?"), # a prefix of a component is not a prefix of the path + ("?", "?"), + ], + ) + def test_module_label_is_a_word_of_a_closed_vocabulary(self, path: str, label: str) -> None: + """A word the model writes as an import path never reaches the log: only known module names do.""" + assert module_label(path) == label + assert module_label(label) == label # idempotent, so the eval harness re-checks a logged label with it + + def test_a_reader_call_says_where_the_data_comes_from(self) -> None: + findings = validate_security(plot("x = pd.read_csv('data.csv')\n"), library="matplotlib") + + assert [f.message for f in findings if f.rule == "banned-call"] == [ + "'pd.read_csv' is not allowed → the data comes only from df = load_user_data(), " + "and the plot reads and writes no files" ] +PURE_MODULE_USES = [ + "import warnings, typing, calendar, decimal, fractions, zoneinfo, enum, dataclasses\n", + "import operator, bisect, heapq, numbers, cmath\n", + "import warnings\nwarnings.filterwarnings('ignore', category=UserWarning)\n", + "import warnings\nwith warnings.catch_warnings():\n warnings.simplefilter('ignore')\n", + "from scipy.fft import rfft, rfftfreq\nspectrum = np.abs(rfft(df['a'].to_numpy()))\n", + "import scipy.fft\nfreqs = scipy.fft.rfftfreq(8, d=1.0)\n", + "import scipy.linalg as la\nq, r = la.qr(np.eye(2))\n", + "from scipy import constants\nc = constants.c\n", + "from zoneinfo import ZoneInfo\ntz = ZoneInfo('Europe/Berlin')\n", + "from operator import itemgetter\nrows = sorted([(1, 2)], key=itemgetter(1))\n", + "import heapq\ntop = heapq.nlargest(3, [1, 2, 3])\n", + "from bisect import bisect_left\ni = bisect_left([1, 2], 2)\n", + "from decimal import Decimal\nx = Decimal('1.5')\n", + "from fractions import Fraction\nx = Fraction(1, 3)\n", + "import calendar\nnames = list(calendar.month_abbr)[1:]\n", + "from enum import Enum\nLevel = Enum('Level', 'LOW HIGH')\n", + "from dataclasses import dataclass, field\n", + "from typing import Any, NamedTuple\nPoint = NamedTuple('Point', [('x', float)])\n", + "import cmath\nz = cmath.exp(1j)\n", + "v = df._values\n", # a private attribute of a value, not of an imported module + "top = df.main.max()\n", # a column called main: calendar.main is banned on the calendar module only +] +ESCAPES = [ + ("import warnings\nline = warnings.formatwarning('', UserWarning, 'f', 1)\n", "banned-call"), + ("import warnings\nwarnings.showwarning('m', UserWarning, 'f', 1)\n", "banned-call"), + ("import warnings\nwarnings.warn_explicit('m', UserWarning, 'f', 1)\n", "banned-call"), + ("from warnings import formatwarning\n", "banned-call"), + ("import zoneinfo\nzoneinfo.reset_tzpath(to=['etc'])\n", "banned-call"), + ("import zoneinfo\nnames = zoneinfo.available_timezones()\n", "banned-call"), + ("from zoneinfo import ZoneInfo\nz = ZoneInfo.from_file(None)\n", "banned-call"), + ("import typing\nhints = typing.get_type_hints(len)\n", "banned-attribute"), + ("from typing import get_type_hints\n", "banned-attribute"), + ("import typing\nref = typing.ForwardRef('x')\n", "banned-attribute"), + ("import typing\nbuild = typing.types.CodeType\n", "banned-attribute"), + ("import operator\ngetter = operator.attrgetter('real')\n", "banned-attribute"), + ("from operator import methodcaller\n", "banned-attribute"), + ("import enum\nb = enum.bltns\n", "banned-attribute"), + ("import enum\nmark = enum.global_enum\n", "banned-attribute"), + ("import dataclasses\nsource = dataclasses.inspect\n", "banned-attribute"), + ("import dataclasses\nbuilder = dataclasses._FuncBuilder\n", "banned-attribute"), + ("from dataclasses import _FuncBuilder\n", "banned-attribute"), + ("import calendar\nc = calendar._locale\n", "banned-attribute"), + ("import scipy.fft\nb = scipy.fft._backend\n", "banned-attribute"), + # calendar's locale classes set the process locale, which the `locale` ban keeps out. + ("import calendar\ncal = calendar.LocaleTextCalendar(locale='de_DE')\n", "banned-attribute"), + ("from calendar import LocaleHTMLCalendar\n", "banned-attribute"), + ( + "import calendar\nwith calendar.different_locale('de_DE'):\n names = list(calendar.month_abbr)\n", + "banned-attribute", + ), + ("import calendar\ncalendar.main(['cal', '-L', 'de_DE', '-e', 'utf-8'])\n", "banned-attribute"), + ("import calendar as cal\nrun = cal.main\n", "banned-attribute"), + ("from calendar import main\n", "banned-attribute"), + ("from calendar import main as show_month\n", "banned-attribute"), + ("import calendar.main\n", "banned-import"), + # A private or escape component of an import path, which an alias would hide from the attribute check. + ("import scipy.fft._backend as backend\nbackend.set_global_backend('scipy')\n", "banned-import"), + ("from scipy.fft._pocketfft import r2c\n", "banned-import"), + ("import scipy.linalg._decomp\n", "banned-import"), + ("import collections.sys as s\n", "banned-import"), + # A module laundered through an expression the resolver cannot follow (second review round). + ("import calendar\nc = [calendar][0]\nc._locale.setlocale(0, 'de_DE')\n", "module-value"), + ("import calendar\nc = [calendar][0]\nc._locale.setlocale(0, 'de_DE')\n", "banned-attribute"), + ("import dataclasses\nd = [dataclasses][0]\nb = d._FuncBuilder\n", "module-value"), + ("import dataclasses\nd = [dataclasses][0]\nb = d._FuncBuilder\n", "banned-attribute"), + ("import collections as c\nm = (lambda module: module)(c)\n", "module-value"), + ("import statistics\ns = statistics\n", "module-value"), + ("m = [np.lib][0]\nf = m._datasource.open('x')\n", "banned-attribute"), + ("import statistics\nr = [statistics.mean][0].__globals__\n", "dunder"), + ("import zoneinfo\nz = [zoneinfo.ZoneInfo][0]\np = z._tzpath\n", "banned-attribute"), + ("import warnings\nw = [warnings.warn][0]\ng = w._getcategory\n", "banned-attribute"), + ("from statistics import mean\nr = [mean][0]._random\n", "banned-attribute"), + # Routes the walk found in modules allowed before (2026-10-10): they passed the validator then. + ('import collections\nx = collections._sys.modules["builtins"].exec("y = 1")\n', "banned-attribute"), + ('import statistics\nx = statistics.sys.modules["os"].system("true")\n', "banned-attribute"), + ("import matplotlib\nrun = matplotlib.subprocess\n", "banned-attribute"), + ("import matplotlib\nm = matplotlib.os\n", "banned-attribute"), + ("import warnings\nw = warnings.sys\n", "banned-attribute"), +] + + +class TestPureModules: + """The owner's 2026-10-10 additions: allowed for computing, with the names that read, write or reflect banned.""" + + @pytest.mark.parametrize("body", PURE_MODULE_USES) + def test_allowed(self, body: str) -> None: + assert security_rules(body) == set() + + @pytest.mark.parametrize(("body", "rule"), ESCAPES) + def test_escape_is_caught(self, body: str, rule: str) -> None: + assert rule in security_rules(body) + + def test_a_private_module_in_an_import_path_is_named_and_logged_by_its_known_prefix(self) -> None: + findings = validate_security(plot("import scipy.fft._backend as backend\n"), library="matplotlib") + + banned = [finding for finding in findings if finding.rule == "banned-import"] + assert [finding.module for finding in banned] == ["scipy.fft"] + assert "reaches the private module '_backend'" in banned[0].message + + @pytest.mark.parametrize( + "body", + [ + "import locale\n", + "import random\n", + "import sys\n", + "import pathlib\n", + "import string\n", + "import io\n", + "from os import path\n", + "from sklearn.datasets import make_blobs\n", + "import scipy.io\n", + "import scipy.datasets\n", + "import statsmodels.formula.api\n", + ], + ) + def test_still_banned(self, body: str) -> None: + assert "banned-import" in security_rules(body) + + # --- os --------------------------------------------------------------------------------- diff --git a/tests/unit/agents/evals/test_matrix.py b/tests/unit/agents/evals/test_matrix.py index 34242264cb..9d087e1133 100644 --- a/tests/unit/agents/evals/test_matrix.py +++ b/tests/unit/agents/evals/test_matrix.py @@ -10,6 +10,7 @@ import json import os import subprocess +from datetime import UTC, datetime from pathlib import Path from types import SimpleNamespace from typing import Any @@ -23,7 +24,7 @@ from agents.anyplot.run_queue import RunQueue from agents.anyplot.schemas import MAX_ATTEMPTS from agents.anyplot.services import Services -from agents.anyplot.settings import AgentSettings, get_settings +from agents.anyplot.settings import MAX_THINKING_BUDGET, AgentSettings, get_settings from agents.evals import matrix from agents.evals.cases import EvalCase, load_cases, select_cases from agents.evals.matrix import ( @@ -131,7 +132,9 @@ async def test_two_fixtures_through_the_real_flow( "full_code": False, "check": "ok", "edit_failure_kinds": {}, + "edit_tolerant": {}, "validator": [], + "banned_imports": [], "adaptation": [], "render": {"passed": True, "canvas_ok": True, "gates": []}, "review": "ok", @@ -167,6 +170,7 @@ async def test_two_fixtures_through_the_real_flow( 2, 2, ) + assert stamp["thinking_budget"] == {"root": 0, "adapter": 0} # off unless a measurement asks assert result.report_path.name == f"{stamp['date']}-claude-haiku-5-5.json" assert json.loads(result.report_path.read_text())["runs"][1]["case_id"] == "scatter-basic-matplotlib" assert "## Runs that did not pass" in result.summary_path.read_text() @@ -378,6 +382,34 @@ def test_configure_writes_the_attempt_bound_only_when_the_flag_names_one() -> No def test_the_flag_takes_exactly_the_settings_range() -> None: assert matrix.MAX_ATTEMPTS_CHOICES == tuple(range(1, MAX_ATTEMPTS + 1)) + assert matrix.MAX_THINKING_BUDGET == MAX_THINKING_BUDGET + + +def test_configure_writes_the_thinking_budgets_only_when_the_flag_names_them() -> None: + measured: dict[str, str] = {} + configure(parse_args(["--renderer", "fake", "--thinking-budget", "0,2048"]), measured) + assert (measured["AGENT_ROOT_THINKING_BUDGET"], measured["AGENT_ADAPTER_THINKING_BUDGET"]) == ("0", "2048") + + unset: dict[str, str] = {} + configure(parse_args(["--renderer", "fake"]), unset) + assert "AGENT_ROOT_THINKING_BUDGET" not in unset and "AGENT_ADAPTER_THINKING_BUDGET" not in unset + + for value in ("2048", "1,2,3", "-1,0", "0,4097", "a,b"): + with pytest.raises(SystemExit): + parse_args(["--thinking-budget", value]) + + +def test_the_stamp_and_the_summary_name_a_thinking_measurement(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("AGENT_ADAPTER_THINKING_BUDGET", "2048") + settings = AgentSettings() + + stamp = matrix.build_stamp(settings, MatrixConfig(), [], datetime(2026, 10, 10, tzinfo=UTC)) + + assert stamp["thinking_budget"] == {"root": 0, "adapter": 2048} + page = summary_markdown({"schema": 2, "stamp": stamp, "summary": summarize([]), "runs": []}) + assert "adaptive Claude thinking with 0 extra output tokens for the root and 2,048 for the adapter" in page + plain = summary_markdown({"schema": 2, "stamp": {"thinking_budget": {"root": 0, "adapter": 0}}, "summary": {}}) + assert "Thinking measurement" not in plain def test_configure_refuses_contradictions() -> None: @@ -723,6 +755,48 @@ def test_book_attribution_counts_answers_that_failed_their_schema_by_rule() -> N assert "| Answer rule, reviewer defects.*.observed:string_too_long | 2 |" in page +def test_book_attribution_counts_the_edits_a_tolerant_tier_applied() -> None: + settings = AgentSettings(provider="gemini", model="gemini-3.8-flash", judge_model="gemini-3.5-flash-lite") + record = matrix.base_record(two_fixtures()[1], 1) + record["turns"] = 1 + check = {"hook": "pipeline_check", "outcome": "ok", "edit_failures": 0, "validator": [], "adaptation": []} + lines: list[dict[str, Any]] = [ + {**check, "attempt": 1, "edit_tolerant": {"trailing_whitespace": 1, "whitespace": 2}}, + {**check, "attempt": 2, "edit_tolerant": {"whitespace": 1}}, + {**check, "attempt": 3}, # a line from before the field existed + ] + + matrix.book_attribution(record, lines, settings) + + assert record["edit_tolerant"] == {"trailing_whitespace": 1, "whitespace": 3} + assert [entry["edit_tolerant"] for entry in record["attempt_log"]] == [ + {"trailing_whitespace": 1, "whitespace": 2}, + {"whitespace": 1}, + {}, + ] + page = summary_markdown({"schema": 2, "stamp": {}, "summary": summarize([record]), "runs": []}) + assert "| Edit applied by tolerant match, whitespace | 3 |" in page + + +def test_book_attribution_names_the_banned_modules_content_free() -> None: + """The next run shows which modules the adapter reaches for; a value outside the validator's vocabulary is `?`.""" + settings = AgentSettings(provider="gemini", model="gemini-3.8-flash", judge_model="gemini-3.5-flash-lite") + record = matrix.base_record(two_fixtures()[1], 1) + record["turns"] = 1 + check = {"hook": "pipeline_check", "outcome": "rejected", "edit_failures": 0, "adaptation": []} + lines: list[dict[str, Any]] = [ + {**check, "attempt": 1, "validator": ["banned-import", "banned-call"], "banned_imports": ["io"]}, + {**check, "attempt": 2, "validator": ["banned-import"], "banned_imports": ["io", "x y", "jane_doe_1984"]}, + ] + + matrix.book_attribution(record, lines, settings) + + assert record["banned_imports"] == {"?": 2, "io": 2} + assert [entry["banned_imports"] for entry in record["attempt_log"]] == [["io"], ["io", "?", "?"]] + page = summary_markdown({"schema": 2, "stamp": {}, "summary": summarize([record]), "runs": []}) + assert "| Validator banned import, io | 2 |" in page + + def test_a_baseline_is_refused_for_runs_that_ended_in_an_error(tmp_path: Path) -> None: good = {"case_id": "a-matplotlib-x", "origin": "fixtures", "status": "ok", "reason": None} report = {"stamp": {"model": "claude-haiku-5-5"}, "stopped": None, "runs": [good]} diff --git a/tests/unit/agents/runtime/fakes.py b/tests/unit/agents/runtime/fakes.py index 4db326ed96..c41490f9ad 100644 --- a/tests/unit/agents/runtime/fakes.py +++ b/tests/unit/agents/runtime/fakes.py @@ -3,7 +3,7 @@ `ScriptedLlm` stands in for any ADK model and answers from a per-kind queue (the kind comes from the `agent_kind` label every generation config carries). `FakeAnthropic` stands in for the `AsyncAnthropicVertex` client behind the real `VertexClaude`, so -the Claude arm exercises the forced-tool structured output end to end. +the Claude arm exercises the structured output (`output_config.format`) end to end. """ import json @@ -144,20 +144,26 @@ def kind(kwargs: dict[str, Any]) -> str: return "unknown" def respond(self, kwargs: dict[str, Any]) -> Message: - """The next scripted answer; `stop` overrides its stop reason (`max_tokens` marks a cut-off answer).""" + """The next scripted answer; `stop` overrides its stop reason (`max_tokens` marks a cut-off answer). + + A structured answer (`json`) is JSON text, as `output_config.format` returns it. Cut + off at `max_tokens`, it is that text without its closing quotes and brackets, the + truncated JSON constrained decoding leaves at the limit: `{"edits": [{"find": "a` + for an edit, `{` for an empty object. `thought` puts a summarized thinking block + before the answer, as adaptive thinking returns one. + """ kind = self.kind(kwargs) queue = self.script.get(kind, []) item = queue.pop(0) if queue else {"text": "fallback"} if "json" in item: - choice = kwargs.get("tool_choice") or {} - assert choice.get("type") == "tool", "a structured answer must be a forced tool call" - block = { - "type": "tool_use", - "id": f"toolu_{len(self.calls)}", - "name": choice["name"], - "input": item["json"], - } - stop = "tool_use" + output_format = (kwargs.get("output_config") or {}).get("format") or {} + assert output_format.get("type") == "json_schema", "a structured answer must be constrained by a format" + assert "tool_choice" not in kwargs or not isinstance(kwargs["tool_choice"], dict), "no forced tool call" + text = json.dumps(item["json"], ensure_ascii=False) + if item.get("stop") == "max_tokens": + text = text.rstrip('"]}') + block = {"type": "text", "text": text} + stop = "end_turn" elif "call" in item: block = { "type": "tool_use", @@ -170,13 +176,14 @@ def respond(self, kwargs: dict[str, Any]) -> Message: block = {"type": "text", "text": item["text"]} stop = "end_turn" stop = item.get("stop", stop) + thinking = [{"type": "thinking", "thinking": item["thought"], "signature": "c2ln"}] if "thought" in item else [] return Message.model_validate( { "id": f"msg_{len(self.calls)}", "type": "message", "role": "assistant", "model": "claude-haiku-5-5", - "content": [block], + "content": [*thinking, block], "stop_reason": stop, "stop_sequence": None, "usage": {"input_tokens": 100, "output_tokens": 20}, diff --git a/tests/unit/agents/runtime/test_models.py b/tests/unit/agents/runtime/test_models.py index c111fcf9a5..6181764ff0 100644 --- a/tests/unit/agents/runtime/test_models.py +++ b/tests/unit/agents/runtime/test_models.py @@ -17,7 +17,6 @@ GEMINI_ADAPTER_FULL_MAX_OUTPUT_TOKENS, GEMINI_ADAPTER_MAX_OUTPUT_TOKENS, JUDGE_MAX_OUTPUT_TOKENS, - STRUCTURED_TOOL, AnthropicJudge, FakeJudge, GeminiJudge, @@ -29,9 +28,11 @@ make_judge_client, make_model, response_json_schema, + structured_output_format, + thinking_budget, ) from agents.anyplot.schemas import MAX_FULL_CODE_CHARS, AdaptPlan, Verdict -from agents.anyplot.settings import AgentSettings +from agents.anyplot.settings import MAX_THINKING_BUDGET, AgentSettings from .fakes import SCATTER_PLAN, FakeAnthropic @@ -170,20 +171,28 @@ async def run(self, request: LlmRequest, answer: dict) -> tuple[list, FakeAnthro responses = [response async for response in model.generate_content_async(request, stream=True)] return responses, client - async def test_schema_becomes_a_forced_tool_and_the_answer_json_text(self) -> None: + async def test_schema_becomes_an_output_format_next_to_the_effort(self) -> None: responses, client = await self.run(self.request(AdaptPlan), SCATTER_PLAN) call = client.calls[0] - assert call["tool_choice"] == {"type": "tool", "name": STRUCTURED_TOOL} - assert call["tools"][0]["input_schema"] == response_json_schema(AdaptPlan) - assert "$ref" not in json.dumps(call["tools"][0]["input_schema"]) - assert call["output_config"] == {"effort": "medium"} + assert call["output_config"] == {"effort": "medium", "format": structured_output_format(AdaptPlan)} + assert not isinstance(call.get("tools"), list) and not isinstance(call.get("tool_choice"), dict) assert "thinking" not in call or call["thinking"] is None or not isinstance(call["thinking"], dict) assert len(responses) == 1 parts = responses[0].content.parts assert [part.function_call for part in parts] == [None] assert AdaptPlan.model_validate_json(parts[0].text).title == SCATTER_PLAN["title"] + async def test_a_request_with_tools_gets_no_format(self) -> None: + client = FakeAnthropic({"unknown": [{"text": "chat"}]}) + model = make_model("root", CLAUDE) + model.__dict__["_anthropic_client"] = client + + responses = [response async for response in model.generate_content_async(self.request(AdaptPlan, tools=True))] + + assert responses[0].content.parts[0].text == "chat" + assert "format" not in (client.calls[0].get("output_config") or {}) + async def test_verdict_schema_round_trip(self) -> None: responses, _ = await self.run(self.request(Verdict), {"ok": True, "defects": []}) @@ -195,6 +204,90 @@ def test_inlined_schema_has_no_definitions(self) -> None: assert "$defs" not in schema assert schema["properties"]["defects"]["items"]["properties"]["id"]["enum"][0] == "VQ-01" + @pytest.mark.parametrize("schema", [AdaptPlan, Verdict]) + def test_the_format_is_strict_and_keeps_the_limits_as_hints(self, schema: type) -> None: + """Constrained decoding needs `additionalProperties: false` and refuses length and count keywords.""" + output_format = structured_output_format(schema) + text = json.dumps(output_format) + + assert output_format["type"] == "json_schema" + assert "$ref" not in text and "$defs" not in text + + def objects(node: object) -> list[dict]: + if isinstance(node, dict): + found = [node] if node.get("type") == "object" else [] + return found + [item for value in node.values() for item in objects(value)] + if isinstance(node, list): + return [item for value in node for item in objects(value)] + return [] + + assert all(item["additionalProperties"] is False for item in objects(output_format["schema"])) + for keyword in ("minLength", "maxLength", "maxItems", "minimum", "maximum"): + assert f'"{keyword}"' not in text + assert "maxItems: 5" in text # the limit stays as a hint in the description + + +THINKING = AgentSettings( + location="global", project="anyplot-eval", root_thinking_budget=2048, adapter_thinking_budget=4096 +) +NON_STREAMING_LIMIT = 128_000 * 600 // 3600 +"""The largest `max_tokens` the Anthropic SDK sends without streaming (`_calculate_nonstreaming_timeout`).""" + + +class TestThinkingMeasurement: + """`AGENT_*_THINKING_BUDGET`: adaptive Claude thinking with its room added to the cap, off by default.""" + + def test_off_by_default(self) -> None: + assert [thinking_budget(kind, CLAUDE) for kind in ("root", "adapter", "reviewer")] == [0, 0, 0] + assert thinking_budget("adapter", GEMINI) == 0 + + def test_a_budget_turns_on_adaptive_thinking_and_adds_its_room_to_the_cap(self) -> None: + configs = {kind: make_content_config(kind, THINKING) for kind in ("root", "adapter", "reviewer")} + + assert {kind: config.max_output_tokens for kind, config in configs.items()} == { + "root": 4096 + 2048, + "adapter": 8192 + 4096, + "reviewer": 4096, + } + assert {kind: config.thinking_config.thinking_budget for kind, config in configs.items()} == { + "root": -1, + "adapter": -1, + "reviewer": 0, + } + assert make_model("adapter", THINKING).max_tokens == 8192 + 4096 + + def test_a_full_file_keeps_the_thinking_room_below_the_sdk_limit(self) -> None: + request = LlmRequest(config=make_content_config("adapter", THINKING).model_copy()) + + allow_full_file(request.config, THINKING) + + assert request.config.max_output_tokens == ADAPTER_FULL_MAX_OUTPUT_TOKENS + 4096 + assert request.config.thinking_config is not None and request.config.thinking_config.thinking_budget == -1 + assert ADAPTER_FULL_MAX_OUTPUT_TOKENS + MAX_THINKING_BUDGET < NON_STREAMING_LIMIT + + async def test_the_adapter_thinks_before_its_structured_answer(self) -> None: + client = FakeAnthropic({"unknown": [{"thought": "plan the edits first", "json": SCATTER_PLAN}]}) + model = make_model("adapter", THINKING) + model.__dict__["_anthropic_client"] = client + config = make_content_config("adapter", THINKING) + config.response_schema = AdaptPlan + request = LlmRequest( + model="claude-haiku-5-5", + contents=[types.Content(role="user", parts=[types.Part(text="adapt this")])], + config=config, + ) + + responses = [response async for response in model.generate_content_async(request)] + + call = client.calls[0] + assert call["thinking"] == {"type": "adaptive", "display": "summarized"} + assert call["max_tokens"] == 8192 + 4096 + assert call["output_config"]["effort"] == "medium" and call["output_config"]["format"]["type"] == "json_schema" + parts = responses[0].content.parts + assert parts[0].thought and parts[0].text == "plan the edits first" + answer = "".join(part.text for part in parts if part.text and not part.thought) + assert AdaptPlan.model_validate_json(answer).title == SCATTER_PLAN["title"] + class TestJudge: async def test_retry_once_then_answer(self) -> None: @@ -267,7 +360,7 @@ def test_factory_follows_the_provider(self) -> None: assert isinstance(gemini, GeminiJudge) assert (gemini.model, gemini.location) == ("gemini-3.5-flash-lite", "eu") - async def test_anthropic_judge_parses_the_forced_tool(self) -> None: + async def test_anthropic_judge_answers_under_the_format(self) -> None: judge = make_judge_client(CLAUDE) assert isinstance(judge, AnthropicJudge) client = FakeAnthropic({"unknown": [{"json": {"verdict": "out_of_scope", "lang": "en"}}]}) @@ -278,7 +371,23 @@ async def test_anthropic_judge_parses_the_forced_tool(self) -> None: assert verdict.verdict == "out_of_scope" assert verdict.tokens == 120 assert (verdict.input_tokens, verdict.output_tokens) == (100, 20) # the split the eval harness prices - assert client.calls[0]["tool_choice"]["type"] == "tool" + call = client.calls[0] + schema = call["output_config"]["format"]["schema"] + assert schema["properties"]["verdict"]["enum"] == ["in_scope", "out_of_scope", "attack", "needs_data"] + assert schema["additionalProperties"] is False and schema["required"] == ["verdict", "lang"] + assert "tools" not in call and "tool_choice" not in call + assert call["thinking"] == {"type": "disabled"} + + async def test_anthropic_judge_retries_a_refusal_then_fails_closed(self) -> None: + judge = make_judge_client(CLAUDE) + assert isinstance(judge, AnthropicJudge) + refusal = {"json": {"verdict": "in_scope", "lang": "en"}, "stop": "refusal"} + client = FakeAnthropic({"unknown": [refusal, dict(refusal)]}) + judge.__dict__["_client"] = client + + with pytest.raises(JudgeUnavailable, match="failed twice"): + await judge.judge("x", "# Scope judge") + assert len(client.calls) == 2 def test_adapter_full_cap(self) -> None: """Claude's full-file cap holds a plan at the schema's 24 KB limit; Gemini's is bounded by the deadline.""" diff --git a/tests/unit/agents/runtime/test_pipeline.py b/tests/unit/agents/runtime/test_pipeline.py index 9cf01fa16a..1b7583bd27 100644 --- a/tests/unit/agents/runtime/test_pipeline.py +++ b/tests/unit/agents/runtime/test_pipeline.py @@ -410,3 +410,17 @@ def test_each_soft_finding_gets_the_target_its_rule_checks(self) -> None: assert "derive it from df" not in palette assert "take the values from df" in lines["rng"] and "take the values from df" in lines["literal-data"] assert all(len(line) <= MAX_LINE_CHARS for line in check.defects) + + def test_banned_imports_name_their_module_for_the_log_and_the_way_around_for_the_repair(self) -> None: + code = 'import io\nimport locale\nimport io as again\ndf = load_user_data()\nplt.savefig(f"plot-{THEME}.png")\n' + + check = _check(code, library="matplotlib", palette=[], base=code) + + assert check.banned_imports == ["io", "locale"] # once each, in order + lines = [line for line in check.blocking if "banned-import" in line] + assert lines[0].startswith("validator banned-import (line 1): import of 'io' is not allowed with matplotlib → ") + assert "the data comes only from df = load_user_data()" in lines[0] + assert "matplotlib.ticker.FuncFormatter" in lines[1] + + def test_a_clean_check_names_no_module(self) -> None: + assert _check("x = 1\n", library="matplotlib", palette=[], base="x = 1\n").banned_imports == [] diff --git a/tests/unit/agents/runtime/test_service_flow.py b/tests/unit/agents/runtime/test_service_flow.py index 18481e4bfd..be93d9c954 100644 --- a/tests/unit/agents/runtime/test_service_flow.py +++ b/tests/unit/agents/runtime/test_service_flow.py @@ -3,7 +3,8 @@ The renderer is the fake backend, the judge is scripted, and every agent's model is replaced: `ScriptedLlm` for the Gemini arm (ADK sends the response schema natively), the real `VertexClaude` over a fake Anthropic client for the Claude arm (the -structured answer is a forced tool call). Both arms must produce the same stream. +structured answer is JSON text under `output_config.format`). Both arms must produce +the same stream. The runtime's run queue here allows 1,000 starts a minute, so back-to-back turns in one test never wait for the rate window; `test_run_queue_flow.py` drives the queue @@ -177,8 +178,9 @@ async def test_create_plot_streams_to_an_ok_plot_result( else: assert isinstance(fake, FakeAnthropic) choices = [call.get("tool_choice") for call in fake.calls] - forced = [choice for choice in choices if isinstance(choice, dict) and choice.get("type") == "tool"] - assert len(forced) == 2 # the adapter and the reviewer answered through the forced tool + formats = [call for call in fake.calls if "format" in (call.get("output_config") or {})] + assert len(formats) == 2 # the adapter and the reviewer answered under a structured-output format + assert not any(isinstance(choice, dict) and choice.get("type") == "tool" for choice in choices) # The root's closing reply after the shipped plot is the budget-exempt call: it carries no tools. assert not isinstance(choices[-1], dict) and not fake.calls[-1].get("tools") assert all(call.get("thinking") == {"type": "disabled"} for call in fake.calls) @@ -405,7 +407,8 @@ async def test_gate_feedback_goes_to_the_next_attempt_and_the_last_one_is_review assert len(backend.jobs) == attempts -# A plan cut off mid-string: Gemini's text stops, Claude's forced tool input stops before a required field. +# A plan cut off mid-string: both providers send the truncated JSON text (FakeAnthropic cuts Claude's +# answer before its closing quotes and brackets, so it stops after `study_ho` as Gemini's does). CUT_OFF = { "gemini": {"text": '{"edits": [{"find": "np.random.seed(42)\\nstudy_ho', "finish_reason": "MAX_TOKENS"}, "anthropic-vertex": {"json": {"edits": [{"find": "np.random.seed(42)\nstudy_ho"}]}, "stop": "max_tokens"}, @@ -442,7 +445,7 @@ async def test_the_attribution_log_names_the_stage_of_every_attempt( assert not any("study_ho" in record.getMessage() for record in caplog.records) -# Claude's forced tool input cut off to `{}`: schema-valid (every AdaptPlan field has a default), still not a plan. +# An answer cut off before its first field: Gemini's text is empty, Claude's is `{` (FakeAnthropic cuts `{}`). CUT_OFF_EMPTY = { "gemini": {"text": "", "finish_reason": "MAX_TOKENS"}, "anthropic-vertex": {"json": {}, "stop": "max_tokens"}, @@ -630,10 +633,11 @@ async def test_failed_edits_are_logged_by_kind_without_their_text( assert (plot["status"], plot["attempts"]) == ("ok", 2) check = attribution_lines(caplog, "pipeline_check")[0] - assert (check["outcome"], check["edit_failures"], check["edit_failure_kinds"]) == ( + assert (check["outcome"], check["edit_failures"], check["edit_failure_kinds"], check["edit_tolerant"]) == ( "edits_failed", 1, {"zero_match": 1}, + {}, ) (result,) = attribution_lines(caplog, "pipeline_result") assert result["stages"] == ["edit_apply", "reviewer_ok"] diff --git a/tests/unit/agents/test_settings.py b/tests/unit/agents/test_settings.py index f240b4ec79..918af36e63 100644 --- a/tests/unit/agents/test_settings.py +++ b/tests/unit/agents/test_settings.py @@ -5,7 +5,7 @@ import pytest from pydantic import ValidationError -from agents.anyplot.settings import AgentSettings, get_settings +from agents.anyplot.settings import MAX_THINKING_BUDGET, AgentSettings, get_settings @pytest.fixture(autouse=True) @@ -63,10 +63,24 @@ def test_development_is_refused_on_cloud_run(self, monkeypatch: pytest.MonkeyPat AgentSettings() @pytest.mark.parametrize("model", ["claude-opus-5-5", "claude-sonnet-5-5", "claude-fable-5-1"]) - def test_claude_models_without_forced_tool_use_are_refused(self, model: str) -> None: + def test_claude_models_that_refuse_disabled_thinking_are_refused(self, model: str) -> None: with pytest.raises(ValidationError, match="not a supported Claude model"): AgentSettings(model=model) + def test_thinking_budgets_are_off_bounded_and_claude_only(self) -> None: + assert (AgentSettings().root_thinking_budget, AgentSettings().adapter_thinking_budget) == (0, 0) + assert AgentSettings(adapter_thinking_budget=MAX_THINKING_BUDGET).adapter_thinking_budget == 4096 + for field_name, value in (("adapter_thinking_budget", 4097), ("root_thinking_budget", -1)): + with pytest.raises(ValidationError): + AgentSettings(**{field_name: value}) + with pytest.raises(ValidationError, match="apply to Claude only"): + AgentSettings( + provider="gemini", + model="gemini-3.8-flash", + judge_model="gemini-3.5-flash-lite", + root_thinking_budget=1024, + ) + def test_get_settings_is_cached(self) -> None: assert get_settings() is get_settings()