From 9dd392e72f42adfad108c86e1395cf8c467f1b1c Mon Sep 17 00:00:00 2001 From: martyna-mindsdb Date: Wed, 8 Jul 2026 17:06:58 +0200 Subject: [PATCH 01/21] gdrive connector with google picker api --- anton/core/runtime.py | 39 +++++++++++++++++++++++++++++++++++++++ uv.lock | 2 +- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/anton/core/runtime.py b/anton/core/runtime.py index 82783506..7519d251 100644 --- a/anton/core/runtime.py +++ b/anton/core/runtime.py @@ -14,6 +14,7 @@ """ from __future__ import annotations +import json import logging import os from pathlib import Path @@ -142,6 +143,13 @@ async def build_chat_session( data_vault = LocalDataVault() if LocalDataVault is not None else None google_drive_oauth_connected = False + # {connection_name: [{"id": ..., "name": ...}, ...]} — files the user + # explicitly granted access to via the Google Picker. drive.file scope + # only covers files the app created itself, so without calling these + # out by name the agent has no way to know they're reachable at all; + # it isn't enough to just leave PICKED_FILES sitting in the env like + # any other DS_ credential field. + google_drive_picked_files: dict[str, list[dict]] = {} if data_vault is not None: try: for conn in data_vault.list_connections(): @@ -154,6 +162,14 @@ async def build_chat_session( fields = data_vault.load(engine, name) or {} if fields.get("auth_type") == "oauth": google_drive_oauth_connected = True + raw_picked = fields.get("picked_files") + if raw_picked: + try: + files = json.loads(raw_picked) + except (json.JSONDecodeError, TypeError): + files = [] + if files: + google_drive_picked_files[name] = files except Exception: logger.debug("Could not inject Anton data vault env", exc_info=True) @@ -164,6 +180,29 @@ async def build_chat_session( "in the injected `DS_GOOGLE_DRIVE___...` environment variables. " "Only claim Google Drive access if you can actually use those credentials successfully." ) + if google_drive_picked_files: + picked_lines = [ + f"- {f.get('name', 'untitled')} (id: {f.get('id')}, connection: {conn_name})" + for conn_name, files in google_drive_picked_files.items() + for f in files + ] + # Deliberately imperative and structurally separate from the + # paragraph above — a soft prose mention was tested and the + # agent silently dropped these files whenever a user asked to + # "list" Drive files, because it just reported files.list()'s + # output verbatim without cross-referencing earlier context. + integration_guidance += ( + "\n\nIMPORTANT — additional Drive files the user has explicitly granted access to " + "via the Google Picker, which a plain files.list() or files.search() call will NOT " + "return (the google_drive scope only covers files this app created itself, plus " + "these specifically granted ones):\n" + + "\n".join(picked_lines) + + "\nWhenever you list, search, or enumerate Drive files for the user, you MUST " + "include every file above IN ADDITION to whatever files.list()/files.search() " + "returns — do not report only the API call's results. To read one of these files' " + "content, call files.get(fileId=...) directly with its id above; do not expect it " + "to appear in a files.list() response first." + ) suffix_parts = [s for s in (system_prompt_suffix, integration_guidance) if s] final_suffix = "".join(suffix_parts) if suffix_parts else None diff --git a/uv.lock b/uv.lock index 08c2abc4..e23502f3 100644 --- a/uv.lock +++ b/uv.lock @@ -203,7 +203,7 @@ requires-dist = [ { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.0" }, { name = "pytest-asyncio", marker = "extra == 'dev'", specifier = ">=0.24" }, { name = "pyyaml", specifier = ">=6.0" }, - { name = "rich", specifier = ">=13.0" }, + { name = "rich", specifier = ">=13.0,<15" }, { name = "typer", specifier = ">=0.12.4" }, ] provides-extras = ["clipboard", "dev"] From 4f1f481cf7c5990b37d43f2e7dcc40a970e0c907 Mon Sep 17 00:00:00 2001 From: Konstantin Sivakov Date: Thu, 9 Jul 2026 16:24:30 +0200 Subject: [PATCH 02/21] fix scrubbing for unregistered envs --- anton/utils/datasources.py | 28 +++++++++++++++++++++++ tests/test_scrubbing.py | 46 ++++++++++++++++++++++++++++++++++++++ uv.lock | 2 +- 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/anton/utils/datasources.py b/anton/utils/datasources.py index 27b1f0a9..586ea908 100644 --- a/anton/utils/datasources.py +++ b/anton/utils/datasources.py @@ -104,6 +104,32 @@ def register_secret_vars( _DS_SECRET_VARS.add(key) +def _register_unregistered_connection_vars(vault: DataVault, engine: str, name: str) -> None: + """Register DS_* var names for a connection whose engine isn't in the + registry (custom engines, connector-spec saves). + + Without this, every field of such a connection falls into the + conservative unknown-DS_* scrub — so harmless values like base_url + surfaced as `[DS_..._BASE_URL]` markers in user-facing output (ENG-688). + Classification: the record's stored ``secure_keys`` when present, else + the secret-name heuristic. + """ + from anton.tools import looks_secret + + record = vault.read_record(engine, name) if hasattr(vault, "read_record") else None + fields = (record or {}).get("fields") or vault.load(engine, name) or {} + secure_keys = (record or {}).get("secure_keys") + prefix = _slug_env_prefix(engine, name) + for field_name in fields: + key = f"{prefix}__{field_name.upper()}" + _DS_KNOWN_VARS.add(key) + is_secret = ( + field_name in secure_keys if secure_keys is not None else looks_secret(field_name) + ) + if is_secret: + _DS_SECRET_VARS.add(key) + + def scrub_credentials(text: str) -> str: """Remove secret values from scratchpad/tool output before it reaches the LLM. @@ -240,6 +266,8 @@ def restore_namespaced_env(vault: DataVault) -> None: edef = dreg.get(conn["engine"]) if edef is not None: register_secret_vars(edef, engine=conn["engine"], name=conn["name"]) + else: + _register_unregistered_connection_vars(vault, conn["engine"], conn["name"]) def find_matching_connection( diff --git a/tests/test_scrubbing.py b/tests/test_scrubbing.py index a7cc4772..d91c512d 100644 --- a/tests/test_scrubbing.py +++ b/tests/test_scrubbing.py @@ -139,3 +139,49 @@ def test_known_secret_env_value_redacted_with_label(self, monkeypatch): result = _scrub_user_input(f"my key is {key}") assert key not in result assert "[OPENAI_API_KEY]" in result + + +class TestCustomEngineRegistration: + """ENG-688: connections of engines not in the registry (custom engines, + connector-spec saves) must register their fields so non-secret values + (base_url, host, ...) stay readable instead of leaking as markers.""" + + def _vault(self, tmp_path): + from anton.core.datasources.data_vault import LocalDataVault + + return LocalDataVault(tmp_path / "vault") + + def test_custom_engine_base_url_readable_secret_scrubbed(self, tmp_path): + from anton.utils.datasources import restore_namespaced_env + + vault = self._vault(tmp_path) + vault.save( + "acme_crm", "prod", + {"base_url": "https://api.acme-crm.example", "token": "tok_1234567890abcdef"}, + secure_keys=["token"], + ) + restore_namespaced_env(vault) + + result = scrub_credentials( + "GET https://api.acme-crm.example failed with token tok_1234567890abcdef" + ) + assert "https://api.acme-crm.example" in result + assert "tok_1234567890abcdef" not in result + assert "[DS_ACME_CRM_PROD__TOKEN]" in result + + def test_custom_engine_without_secure_keys_uses_name_heuristic(self, tmp_path): + from anton.utils.datasources import restore_namespaced_env + + vault = self._vault(tmp_path) + vault.save( + "acme_crm", "legacy", + {"base_url": "https://legacy.acme-crm.example", "api_key": "ak_1234567890abcdef"}, + ) + restore_namespaced_env(vault) + + result = scrub_credentials( + "base https://legacy.acme-crm.example key ak_1234567890abcdef" + ) + assert "https://legacy.acme-crm.example" in result + assert "ak_1234567890abcdef" not in result + assert "[DS_ACME_CRM_LEGACY__API_KEY]" in result diff --git a/uv.lock b/uv.lock index 08c2abc4..e23502f3 100644 --- a/uv.lock +++ b/uv.lock @@ -203,7 +203,7 @@ requires-dist = [ { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.0" }, { name = "pytest-asyncio", marker = "extra == 'dev'", specifier = ">=0.24" }, { name = "pyyaml", specifier = ">=6.0" }, - { name = "rich", specifier = ">=13.0" }, + { name = "rich", specifier = ">=13.0,<15" }, { name = "typer", specifier = ">=0.12.4" }, ] provides-extras = ["clipboard", "dev"] From c46cc71e3813e7523ba3a77236a7e149528b42b4 Mon Sep 17 00:00:00 2001 From: andrew Date: Thu, 9 Jul 2026 17:51:02 +0300 Subject: [PATCH 03/21] move important block higher --- anton/core/tools/tool_defs.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/anton/core/tools/tool_defs.py b/anton/core/tools/tool_defs.py index debe6c1e..1997fc77 100644 --- a/anton/core/tools/tool_defs.py +++ b/anton/core/tools/tool_defs.py @@ -41,6 +41,14 @@ class ToolDef: "- dump: Show a clean notebook-style summary of cells (code + truncated output)\n" "- install: Install Python packages into the scratchpad's environment. " "Packages persist across resets.\n\n" + "IMPORTANT: Cells have an inactivity timeout of 30 seconds — if a cell produces " + "no output and no progress() calls for 30s, it is killed and all state is lost. " + "For long-running code (API calls, data extraction, heavy computation), call " + "progress(message) periodically to signal work is ongoing and reset the timer. " + "The total timeout scales from your estimated_execution_time_seconds " + "(roughly 2x the estimate). You MUST provide estimated_execution_time_seconds " + "for every exec call. For very long operations, provide a realistic estimate " + "and use progress() to keep the cell alive.\n\n" "Use print() to produce output. Host Python packages are available by default. " "Include a 'packages' array on exec calls for any libraries your code needs — " "they'll be auto-installed before the cell runs (already-installed ones are skipped).\n" @@ -57,15 +65,7 @@ class ToolDef: "sample(var) inspects any variable with type-aware formatting — DataFrames get " "shape/dtypes/head, dicts get keys/values, lists get length/items. " "Defaults to 'preview' mode (compact); use sample(var, mode='full') for complete dump.\n" - "All .anton/.env secrets are available as environment variables (os.environ).\n\n" - "IMPORTANT: Cells have an inactivity timeout of 30 seconds — if a cell produces " - "no output and no progress() calls for 30s, it is killed and all state is lost. " - "For long-running code (API calls, data extraction, heavy computation), call " - "progress(message) periodically to signal work is ongoing and reset the timer. " - "The total timeout scales from your estimated_execution_time_seconds " - "(roughly 2x the estimate). You MUST provide estimated_execution_time_seconds " - "for every exec call. For very long operations, provide a realistic estimate " - "and use progress() to keep the cell alive." + "All .anton/.env secrets are available as environment variables (os.environ)." ), input_schema={ "type": "object", From 1af7647bf7727bd3505c765729cfdd3d80fc2a6f Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 15:57:54 +0300 Subject: [PATCH 04/21] Unify "when" rules via get_rules --- anton/core/memory/hippocampus.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/anton/core/memory/hippocampus.py b/anton/core/memory/hippocampus.py index 13790aae..65c44195 100644 --- a/anton/core/memory/hippocampus.py +++ b/anton/core/memory/hippocampus.py @@ -329,18 +329,11 @@ def recall_scratchpad_wisdom(self) -> str: parts: list[str] = [] # Extract "when" rules - rules = self.recall_rules() - if rules: - in_when = False - for line in rules.splitlines(): - if line.strip().startswith("## When"): - in_when = True - continue - elif line.strip().startswith("## "): - in_when = False - continue - if in_when and line.strip().startswith("- "): - parts.append(line.strip()) + for rule in self.get_rules(): + if rule.kind == "when": + entry = f"- {rule.text}" + if entry not in parts: + parts.append(entry) # Extract scratchpad-related lessons for lesson in self.get_lessons(): From 35df749ac7f59c34e10e276a9c2e770ef5abeef5 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 16:31:57 +0300 Subject: [PATCH 05/21] sort by confidence and date, limit by tokens --- anton/core/memory/base.py | 4 +- anton/core/memory/hippocampus.py | 75 +++++++++++++++++++++----------- tests/test_hippocampus.py | 30 +++++++++++++ 3 files changed, 82 insertions(+), 27 deletions(-) diff --git a/anton/core/memory/base.py b/anton/core/memory/base.py index 4a5cbf47..02dd4bb2 100644 --- a/anton/core/memory/base.py +++ b/anton/core/memory/base.py @@ -61,8 +61,8 @@ def recall_topic(self, slug: str) -> str: """Return lessons tagged with the given topic slug.""" ... - def recall_scratchpad_wisdom(self) -> str: - """Return procedural knowledge relevant to scratchpad execution.""" + def recall_scratchpad_wisdom(self, token_budget: int = 2000) -> str: + """Return procedural knowledge relevant to scratchpad execution, within budget.""" ... # --- read (Engrams, for inspection / CRUD UI) --- diff --git a/anton/core/memory/hippocampus.py b/anton/core/memory/hippocampus.py index 65c44195..4158fae7 100644 --- a/anton/core/memory/hippocampus.py +++ b/anton/core/memory/hippocampus.py @@ -67,6 +67,25 @@ def _extract_metadata(text: str) -> tuple[str, dict]: return clean_text, meta +def _filter_by_token_budget(engrams: list[Engram], token_budget: int) -> list[Engram]: + """Keep entries, in the given order, until token_budget is spent. + + Callers must pass entries already sorted by priority (most important + first) — this stops at the first entry that would exceed the budget, it + does not repack for a tighter fit. Budget enforced at ~4 chars/token. + """ + char_budget = token_budget * 4 + used = 0 + kept: list[Engram] = [] + for engram in engrams: + length = len(engram.text) + 1 + if used + length > char_budget: + break + kept.append(engram) + used += length + return kept + + class Hippocampus: """Reads and writes memory traces at a single scope (global OR project). @@ -239,18 +258,7 @@ def get_lessons(self, token_budget: int = None) -> list[Engram]: # Reverse entries so most recent are first entries.reverse() - # Budget: ~4 chars per token - char_budget = token_budget * 4 - result_lines = [] - used = sum(len(ln) for ln in result_lines) - - for ln in entries: - if used + len(ln.text) + 1 > char_budget: - break - result_lines.append(ln) - used += len(ln.text) + 1 - - return result_lines + return _filter_by_token_budget(entries, token_budget) def del_lesson(self, id): entries = self.get_lessons() @@ -320,29 +328,46 @@ def recall_topic(self, slug: str) -> str: return self._lessons_to_text(items, header=slug) - def recall_scratchpad_wisdom(self) -> str: + def recall_scratchpad_wisdom(self, token_budget: int = 2000) -> str: """Retrieve procedural knowledge relevant to scratchpad execution. - Returns all "when" rules + lessons whose text contains "scratchpad". - Injected into tool descriptions so the LLM sees them when composing code. + Gathers all "when" rules + lessons whose text contains "scratchpad", + ordered by confidence tier (high, then medium/unset, then low) and by + recency within a tier, then trimmed to token_budget (~4 chars/token — + same convention as get_lessons). Injected into tool descriptions so + the LLM sees them when composing code. """ - parts: list[str] = [] + candidates: list[Engram] = [] - # Extract "when" rules for rule in self.get_rules(): if rule.kind == "when": - entry = f"- {rule.text}" - if entry not in parts: - parts.append(entry) + candidates.append(rule) - # Extract scratchpad-related lessons for lesson in self.get_lessons(): if "scratchpad" in lesson.text.lower() or (lesson.topic and "scratchpad" in lesson.topic.lower()): - entry = f"- {lesson.text}" - if entry not in parts: - parts.append(entry) + candidates.append(lesson) + + confidence_rank = {"high": 0, "medium": 1, "low": 2} + + def sort_key(engram: Engram) -> tuple[int, float]: + rank = confidence_rank.get(engram.confidence, 1) + recency = -engram.updated_at.timestamp() if engram.updated_at else float("inf") + return (rank, recency) + + candidates.sort(key=sort_key) + + # Dedup by text, keeping the higher-priority copy from the sort above + seen_texts: set[str] = set() + deduped: list[Engram] = [] + for engram in candidates: + if engram.text in seen_texts: + continue + seen_texts.add(engram.text) + deduped.append(engram) + + kept = _filter_by_token_budget(deduped, token_budget) - return "\n".join(parts) + return "\n".join(f"- {engram.text}" for engram in kept) # ---------- rules ------------------- diff --git a/tests/test_hippocampus.py b/tests/test_hippocampus.py index 2d3036db..861f3004 100644 --- a/tests/test_hippocampus.py +++ b/tests/test_hippocampus.py @@ -98,6 +98,36 @@ def test_includes_scratchpad_topic_files(self, hc, mem_dir): result = hc.recall_scratchpad_wisdom() assert "Always re-import" in result + def test_sorts_by_confidence_tier_then_recency(self, hc, mem_dir): + (mem_dir / "rules.md").write_text( + "# Rules\n\n## When\n" + "- HIGH_OLD_RULE \n" + "- LOW_NEW_RULE \n" + "- MEDIUM_MID_RULE \n" + ) + (mem_dir / "lessons.md").write_text( + "# Lessons\n" + "- Scratchpad HIGH_NEW_LESSON fact \n" + ) + result = hc.recall_scratchpad_wisdom() + + # High tier (newest first), then medium, then low last. + assert ( + result.index("HIGH_NEW_LESSON") + < result.index("HIGH_OLD_RULE") + < result.index("MEDIUM_MID_RULE") + < result.index("LOW_NEW_RULE") + ) + + def test_budget_limits_output(self, hc, mem_dir): + entries = "\n".join( + f"- When rule number {i} with some extra padding words here" for i in range(30) + ) + (mem_dir / "rules.md").write_text(f"# Rules\n\n## When\n{entries}\n") + result = hc.recall_scratchpad_wisdom(token_budget=150) + entry_count = result.count("- When rule") + assert 0 < entry_count < 30 + class TestEncodeRule: def test_creates_rules_file(self, hc, mem_dir): From a3c73cd89ac4516fb7265c57750fa75dc3eec2cf Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 16:37:21 +0300 Subject: [PATCH 06/21] docs update --- anton/README.md | 6 +++--- docs/docs/developer/cerebellum-and-acc.md | 4 +++- docs/docs/developer/memory-systems.md | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/anton/README.md b/anton/README.md index 776bf6a9..24056419 100644 --- a/anton/README.md +++ b/anton/README.md @@ -305,7 +305,7 @@ When scratchpads are active, relevant lessons are appended to the scratchpad too scratchpad_tool["description"] += f"\n\nLessons from past sessions:\n{wisdom}" ``` -This combines all "when" rules + lessons with `scratchpad-*` topics from both scopes. The content comes from `cortex.get_scratchpad_context()`, which calls `recall_scratchpad_wisdom()` on both hippocampi. +This combines "when" rules + scratchpad-related lessons from both scopes, ordered by confidence tier (high, then medium/unset, then low) and recency within a tier, trimmed to a token budget (default 2000/scope, ~4 chars/token). The content comes from `cortex.get_scratchpad_context()`, which calls `recall_scratchpad_wisdom()` on both hippocampi. ## The `memorize` Tool @@ -572,7 +572,7 @@ When errored cells exist, they accumulate in a buffer across the turn. At end-of 1. The buffered cells get formatted into a compact post-mortem prompt 2. One LLM call via `LLMClient.generate_object_code` (the cheap coding model) returns a `_DiffPassResult` Pydantic model with extracted lessons 3. Each lesson is wrapped as an `Engram` with `kind="lesson"`, `topic="scratchpad"`, `source="consolidation"`, and routed through `Cortex.encode()` — the same path manual lessons and the consolidator already use -4. Future scratchpad cells see those lessons via the existing `recall_scratchpad_wisdom()` injection into the scratchpad tool description +4. Future scratchpad cells see those lessons via the existing `recall_scratchpad_wisdom()` injection into the scratchpad tool description (confidence tier + recency ordered, budget-limited — not all lessons are guaranteed to survive the cut) The cerebellum is a **producer** only — it generates new lesson entries for the existing storage and retrieval pipeline. There's no parallel storage system, no separate `corrections.md` file. Whatever the consolidator and `/memorize` write to, the cerebellum also writes to. @@ -811,7 +811,7 @@ The Hippocampus handles one scope (global OR project) and is the canonical file- | `recall_rules()` | `rules.md` | Basal Ganglia + OFC | | `recall_lessons(token_budget)` | `lessons.md` (budget-limited, most recent first) | Anterior Temporal Lobe | | `recall_topic(slug)` | `topics/{slug}.md` | Cortical Association Areas | -| `recall_scratchpad_wisdom()` | "when" rules + scratchpad-related lessons + `topics/scratchpad-*.md` | Procedural memory | +| `recall_scratchpad_wisdom(token_budget)` | "when" rules + scratchpad-related lessons (confidence tier + recency ordered, budget-limited) | Procedural memory | **Encoding methods:** | Method | Writes | Behavior | diff --git a/docs/docs/developer/cerebellum-and-acc.md b/docs/docs/developer/cerebellum-and-acc.md index af8b647d..d7914e45 100644 --- a/docs/docs/developer/cerebellum-and-acc.md +++ b/docs/docs/developer/cerebellum-and-acc.md @@ -65,7 +65,9 @@ background task — the user gets their reply immediately: 3. Each lesson becomes an `Engram` with `kind="lesson"`, `topic="scratchpad"`, `source="consolidation"`, routed through `Cortex.encode()`. 4. Future scratchpad cells see those lessons via the existing - `recall_scratchpad_wisdom()` injection into the scratchpad tool description. + `recall_scratchpad_wisdom()` injection into the scratchpad tool description + (confidence tier + recency ordered, budget-limited — not all lessons are + guaranteed to survive the cut). The cerebellum is a **producer only** — no parallel storage system, no separate corrections file. Whatever the consolidator and `memorize` write to, the diff --git a/docs/docs/developer/memory-systems.md b/docs/docs/developer/memory-systems.md index 123c9553..6ca0141d 100644 --- a/docs/docs/developer/memory-systems.md +++ b/docs/docs/developer/memory-systems.md @@ -56,7 +56,7 @@ it just reads and writes. | `recall_rules()` | `rules.md` | Basal Ganglia + OFC | | `recall_lessons(token_budget)` | `lessons.md` (budget-limited, most recent first) | Anterior Temporal Lobe | | `recall_topic(slug)` | `topics/{slug}.md` | Cortical Association Areas | -| `recall_scratchpad_wisdom()` | "when" rules + scratchpad-related lessons + `topics/scratchpad-*.md` | Procedural priming | +| `recall_scratchpad_wisdom(token_budget)` | "when" rules + scratchpad-related lessons (confidence tier + recency ordered, budget-limited) | Procedural priming | | Encoding method | Writes | Behavior | |---|---|---| From fa72f07875fea3993c86d105d9caf2c6475124e5 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 16:56:59 +0300 Subject: [PATCH 07/21] fix scratchpad_tool.description mutates --- anton/core/session.py | 7 +++++-- tests/test_chat_scratchpad.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index 3f260aff..a889c505 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -2,7 +2,7 @@ import asyncio from collections.abc import AsyncIterator, Callable -from dataclasses import asdict, dataclass, field +from dataclasses import asdict, dataclass, field, replace from datetime import datetime import json import re @@ -732,7 +732,10 @@ def _build_tools(self) -> list[dict]: return self.tool_registry.dump() def _build_core_tools(self) -> None: - scratchpad_tool = SCRATCHPAD_TOOL + # Copy — SCRATCHPAD_TOOL is a module-level singleton; mutating its + # .description in place would leak across every session/user sharing + # this process instead of resetting per session. + scratchpad_tool = replace(SCRATCHPAD_TOOL) pkg_list = self._scratchpads.available_packages if pkg_list: notable = sorted(p for p in pkg_list if p.lower() in self._NOTABLE_PACKAGES) diff --git a/tests/test_chat_scratchpad.py b/tests/test_chat_scratchpad.py index e7175f59..2c1124bb 100644 --- a/tests/test_chat_scratchpad.py +++ b/tests/test_chat_scratchpad.py @@ -85,6 +85,38 @@ async def test_scratchpad_tool_in_tools(self, workspace): finally: await session.close() + async def test_tool_build_does_not_mutate_shared_singleton(self, workspace): + """_build_core_tools() must not mutate the shared SCRATCHPAD_TOOL + singleton — otherwise memory-injected wisdom would leak across every + session sharing this process instead of resetting per session.""" + original_description = SCRATCHPAD_TOOL.description + + mock_llm = make_mock_llm() + cortex_a = MagicMock() + cortex_a.get_scratchpad_context.return_value = "WISDOM_FROM_SESSION_A" + session_a = ChatSession( + ChatSessionConfig(llm_client=mock_llm, workspace=workspace, cortex=cortex_a) + ) + try: + tools_a = session_a._build_tools() + desc_a = next(t["description"] for t in tools_a if t["name"] == "scratchpad") + assert "WISDOM_FROM_SESSION_A" in desc_a + assert SCRATCHPAD_TOOL.description == original_description + finally: + await session_a.close() + + cortex_b = MagicMock() + cortex_b.get_scratchpad_context.return_value = "" + session_b = ChatSession( + ChatSessionConfig(llm_client=mock_llm, workspace=workspace, cortex=cortex_b) + ) + try: + tools_b = session_b._build_tools() + desc_b = next(t["description"] for t in tools_b if t["name"] == "scratchpad") + assert "WISDOM_FROM_SESSION_A" not in desc_b + finally: + await session_b.close() + class TestScratchpadExecViaChat: async def test_scratchpad_exec_via_chat(self, workspace): From 35346a98b9f878ca38ed73c1db01249cba8f486c Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 17:42:28 +0300 Subject: [PATCH 08/21] remove some duplicates between scratchpad tool description and system prompt --- anton/README.md | 4 ++-- anton/core/memory/hippocampus.py | 14 ++++++++------ docs/docs/developer/memory-systems.md | 2 +- tests/test_hippocampus.py | 16 ++++++++++------ 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/anton/README.md b/anton/README.md index 24056419..31ac36e3 100644 --- a/anton/README.md +++ b/anton/README.md @@ -305,7 +305,7 @@ When scratchpads are active, relevant lessons are appended to the scratchpad too scratchpad_tool["description"] += f"\n\nLessons from past sessions:\n{wisdom}" ``` -This combines "when" rules + scratchpad-related lessons from both scopes, ordered by confidence tier (high, then medium/unset, then low) and recency within a tier, trimmed to a token budget (default 2000/scope, ~4 chars/token). The content comes from `cortex.get_scratchpad_context()`, which calls `recall_scratchpad_wisdom()` on both hippocampi. +This combines scratchpad-related "when" rules + scratchpad-related lessons from both scopes, ordered by confidence tier (high, then medium/unset, then low) and recency within a tier, trimmed to a token budget (default 2000/scope, ~4 chars/token). The content comes from `cortex.get_scratchpad_context()`, which calls `recall_scratchpad_wisdom()` on both hippocampi. ## The `memorize` Tool @@ -811,7 +811,7 @@ The Hippocampus handles one scope (global OR project) and is the canonical file- | `recall_rules()` | `rules.md` | Basal Ganglia + OFC | | `recall_lessons(token_budget)` | `lessons.md` (budget-limited, most recent first) | Anterior Temporal Lobe | | `recall_topic(slug)` | `topics/{slug}.md` | Cortical Association Areas | -| `recall_scratchpad_wisdom(token_budget)` | "when" rules + scratchpad-related lessons (confidence tier + recency ordered, budget-limited) | Procedural memory | +| `recall_scratchpad_wisdom(token_budget)` | scratchpad-related "when" rules + scratchpad-related lessons (confidence tier + recency ordered, budget-limited) | Procedural memory | **Encoding methods:** | Method | Writes | Behavior | diff --git a/anton/core/memory/hippocampus.py b/anton/core/memory/hippocampus.py index 4158fae7..0538f797 100644 --- a/anton/core/memory/hippocampus.py +++ b/anton/core/memory/hippocampus.py @@ -331,16 +331,18 @@ def recall_topic(self, slug: str) -> str: def recall_scratchpad_wisdom(self, token_budget: int = 2000) -> str: """Retrieve procedural knowledge relevant to scratchpad execution. - Gathers all "when" rules + lessons whose text contains "scratchpad", - ordered by confidence tier (high, then medium/unset, then low) and by - recency within a tier, then trimmed to token_budget (~4 chars/token — - same convention as get_lessons). Injected into tool descriptions so - the LLM sees them when composing code. + Gathers "when" rules + lessons whose text or topic mentions + "scratchpad", ordered by confidence tier (high, then medium/unset, + then low) and by recency within a tier, then trimmed to token_budget + (~4 chars/token — same convention as get_lessons). Injected into tool + descriptions so the LLM sees them when composing code. """ candidates: list[Engram] = [] for rule in self.get_rules(): - if rule.kind == "when": + if rule.kind == "when" and ( + "scratchpad" in rule.text.lower() or (rule.topic and "scratchpad" in rule.topic.lower()) + ): candidates.append(rule) for lesson in self.get_lessons(): diff --git a/docs/docs/developer/memory-systems.md b/docs/docs/developer/memory-systems.md index 6ca0141d..0778174c 100644 --- a/docs/docs/developer/memory-systems.md +++ b/docs/docs/developer/memory-systems.md @@ -56,7 +56,7 @@ it just reads and writes. | `recall_rules()` | `rules.md` | Basal Ganglia + OFC | | `recall_lessons(token_budget)` | `lessons.md` (budget-limited, most recent first) | Anterior Temporal Lobe | | `recall_topic(slug)` | `topics/{slug}.md` | Cortical Association Areas | -| `recall_scratchpad_wisdom(token_budget)` | "when" rules + scratchpad-related lessons (confidence tier + recency ordered, budget-limited) | Procedural priming | +| `recall_scratchpad_wisdom(token_budget)` | scratchpad-related "when" rules + scratchpad-related lessons (confidence tier + recency ordered, budget-limited) | Procedural priming | | Encoding method | Writes | Behavior | |---|---|---| diff --git a/tests/test_hippocampus.py b/tests/test_hippocampus.py index 861f3004..712433d0 100644 --- a/tests/test_hippocampus.py +++ b/tests/test_hippocampus.py @@ -80,10 +80,13 @@ def test_empty_returns_empty(self, hc): def test_extracts_when_rules(self, hc, mem_dir): (mem_dir / "rules.md").write_text( - "# Rules\n\n## Always\n- Be fast\n\n## When\n- If paginated → use progress()\n" + "# Rules\n\n## Always\n- Be fast\n\n## When\n" + "- If a scratchpad API is paginated → use progress()\n" + "- If the user writes in Spanish → respond in Spanish\n" ) result = hc.recall_scratchpad_wisdom() assert "paginated" in result + assert "Spanish" not in result def test_includes_scratchpad_lessons(self, hc, mem_dir): (mem_dir / "lessons.md").write_text( @@ -101,9 +104,9 @@ def test_includes_scratchpad_topic_files(self, hc, mem_dir): def test_sorts_by_confidence_tier_then_recency(self, hc, mem_dir): (mem_dir / "rules.md").write_text( "# Rules\n\n## When\n" - "- HIGH_OLD_RULE \n" - "- LOW_NEW_RULE \n" - "- MEDIUM_MID_RULE \n" + "- Scratchpad HIGH_OLD_RULE \n" + "- Scratchpad LOW_NEW_RULE \n" + "- Scratchpad MEDIUM_MID_RULE \n" ) (mem_dir / "lessons.md").write_text( "# Lessons\n" @@ -121,11 +124,12 @@ def test_sorts_by_confidence_tier_then_recency(self, hc, mem_dir): def test_budget_limits_output(self, hc, mem_dir): entries = "\n".join( - f"- When rule number {i} with some extra padding words here" for i in range(30) + f"- Scratchpad when-rule number {i} with some extra padding words here" + for i in range(30) ) (mem_dir / "rules.md").write_text(f"# Rules\n\n## When\n{entries}\n") result = hc.recall_scratchpad_wisdom(token_budget=150) - entry_count = result.count("- When rule") + entry_count = result.count("- Scratchpad when-rule") assert 0 < entry_count < 30 From 72ae059d5576a7c1dc3a82a4dc39385eb64a4b5e Mon Sep 17 00:00:00 2001 From: martyna-mindsdb Date: Fri, 10 Jul 2026 16:51:58 +0200 Subject: [PATCH 09/21] fixes from ai code review --- anton/core/runtime.py | 101 +++++++++++++++++++++++++++++------------- 1 file changed, 71 insertions(+), 30 deletions(-) diff --git a/anton/core/runtime.py b/anton/core/runtime.py index 7519d251..a56d2e14 100644 --- a/anton/core/runtime.py +++ b/anton/core/runtime.py @@ -152,7 +152,21 @@ async def build_chat_session( google_drive_picked_files: dict[str, list[dict]] = {} if data_vault is not None: try: - for conn in data_vault.list_connections(): + connections = data_vault.list_connections() + except Exception: + logger.debug("Could not list Anton data vault connections", exc_info=True) + connections = [] + for conn in connections: + # Per-connection try/except — one bad record (a load() error, a + # malformed field) must not abort inject_env/picked_files + # processing for every connection after it in the list. + # engine/name are initialized before the try so the except + # handler's logging never re-calls conn.get(...) itself — if + # `conn` isn't a plain dict, that would raise a second, + # uncaught exception right here and abort the whole loop again. + engine = None + name = None + try: engine = conn.get("engine") name = conn.get("name") if not (engine and name): @@ -162,47 +176,74 @@ async def build_chat_session( fields = data_vault.load(engine, name) or {} if fields.get("auth_type") == "oauth": google_drive_oauth_connected = True - raw_picked = fields.get("picked_files") + # Fall back to the pre-rename key so a vault record + # written before the _picked_files rename doesn't + # silently lose its guidance. + raw_picked = fields.get("_picked_files") or fields.get("picked_files") if raw_picked: try: - files = json.loads(raw_picked) + parsed = json.loads(raw_picked) except (json.JSONDecodeError, TypeError): - files = [] + parsed = [] + # Only keep well-formed entries — anything else (a + # bare string, a dict instead of a list, an entry + # missing "id") would crash the f.get(...) calls + # below or embed a literal "None" fileId in the + # prompt instructions. + files = ( + [f for f in parsed if isinstance(f, dict) and f.get("id")] + if isinstance(parsed, list) else [] + ) if files: google_drive_picked_files[name] = files - except Exception: - logger.debug("Could not inject Anton data vault env", exc_info=True) + except Exception: + logger.debug( + "Could not inject Anton data vault env for %s/%s", + engine, name, exc_info=True, + ) integration_guidance = "" - if google_drive_oauth_connected: + # Also shown whenever picked_files exist even if google_drive_oauth_connected + # is somehow False (fragile single-connection heuristic) — a connection + # can only have picked_files at all via the OAuth-based Picker flow, so + # its DS_ credentials necessarily exist; otherwise the IMPORTANT block + # below would tell the agent to fetch specific file IDs without ever + # mentioning the env vars that authenticate those calls. + if google_drive_oauth_connected or google_drive_picked_files: integration_guidance = ( " Connected Google Drive accounts are available through Google OAuth credentials " "in the injected `DS_GOOGLE_DRIVE___...` environment variables. " "Only claim Google Drive access if you can actually use those credentials successfully." ) - if google_drive_picked_files: - picked_lines = [ - f"- {f.get('name', 'untitled')} (id: {f.get('id')}, connection: {conn_name})" - for conn_name, files in google_drive_picked_files.items() - for f in files - ] - # Deliberately imperative and structurally separate from the - # paragraph above — a soft prose mention was tested and the - # agent silently dropped these files whenever a user asked to - # "list" Drive files, because it just reported files.list()'s - # output verbatim without cross-referencing earlier context. - integration_guidance += ( - "\n\nIMPORTANT — additional Drive files the user has explicitly granted access to " - "via the Google Picker, which a plain files.list() or files.search() call will NOT " - "return (the google_drive scope only covers files this app created itself, plus " - "these specifically granted ones):\n" - + "\n".join(picked_lines) - + "\nWhenever you list, search, or enumerate Drive files for the user, you MUST " - "include every file above IN ADDITION to whatever files.list()/files.search() " - "returns — do not report only the API call's results. To read one of these files' " - "content, call files.get(fileId=...) directly with its id above; do not expect it " - "to appear in a files.list() response first." - ) + # Deliberately NOT gated behind google_drive_oauth_connected — that flag + # is a fragile, single-source heuristic (fields.get("auth_type") == + # "oauth" on SOME connection). A connection whose record predates that + # field, or uses a different auth_type value, would otherwise have real + # picked_files data but no guidance telling the agent it exists — + # reproducing the exact bug this block was added to fix. + if google_drive_picked_files: + picked_lines = [ + f"- {f.get('name', 'untitled')} (id: {f.get('id')}, connection: {conn_name})" + for conn_name, files in google_drive_picked_files.items() + for f in files + ] + # Deliberately imperative and structurally separate from the + # paragraph above — a soft prose mention was tested and the + # agent silently dropped these files whenever a user asked to + # "list" Drive files, because it just reported files.list()'s + # output verbatim without cross-referencing earlier context. + integration_guidance += ( + "\n\nIMPORTANT — additional Drive files the user has explicitly granted access to " + "via the Google Picker, which a plain files.list() or files.search() call will NOT " + "return (the google_drive scope only covers files this app created itself, plus " + "these specifically granted ones):\n" + + "\n".join(picked_lines) + + "\nWhenever you list, search, or enumerate Drive files for the user, you MUST " + "include every file above IN ADDITION to whatever files.list()/files.search() " + "returns — do not report only the API call's results. To read one of these files' " + "content, call files.get(fileId=...) directly with its id above; do not expect it " + "to appear in a files.list() response first." + ) suffix_parts = [s for s in (system_prompt_suffix, integration_guidance) if s] final_suffix = "".join(suffix_parts) if suffix_parts else None From a48a192b3c1d0d7781a89f2f3339ba6afa721016 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 10 Jul 2026 18:26:53 +0300 Subject: [PATCH 10/21] remove scratchpad items in system prompt --- anton/core/memory/base.py | 27 ++++++++++++---- anton/core/memory/cortex.py | 22 +++++++------ anton/core/memory/hippocampus.py | 45 +++++++++++++++++++++------ docs/docs/developer/memory-systems.md | 4 ++- tests/test_cortex.py | 33 +++++++++++++++++++- 5 files changed, 105 insertions(+), 26 deletions(-) diff --git a/anton/core/memory/base.py b/anton/core/memory/base.py index 02dd4bb2..ea171c8a 100644 --- a/anton/core/memory/base.py +++ b/anton/core/memory/base.py @@ -53,8 +53,13 @@ def recall_rules(self) -> str: """Return behavioral gates (rules.md equivalent).""" ... - def recall_lessons(self, token_budget: int = 1000) -> str: - """Return semantic facts within the given token budget.""" + def recall_lessons(self, token_budget: int = 1000, exclude_scratchpad: bool = False) -> str: + """Return semantic facts within the given token budget. + + exclude_scratchpad drops entries whose text/topic mentions + "scratchpad" — used by build_memory_context() to avoid double-billing + entries already injected via recall_scratchpad_wisdom(). + """ ... def recall_topic(self, slug: str) -> str: @@ -71,12 +76,22 @@ def get_identities(self) -> list[Engram]: """Return identity entries as Engrams.""" ... - def get_rules(self) -> list[Engram]: - """Return behavioral rules as Engrams.""" + def get_rules(self, exclude_scratchpad_when: bool = False) -> list[Engram]: + """Return behavioral rules as Engrams. + + exclude_scratchpad_when drops "when" rules whose text/topic mentions + "scratchpad" — used by build_memory_context() to avoid double-billing + entries already injected via recall_scratchpad_wisdom(). always/never + rules are never affected. + """ ... - def get_lessons(self, token_budget: int = None) -> list[Engram]: - """Return semantic facts as Engrams, optionally budget-limited.""" + def get_lessons(self, token_budget: int = None, exclude_scratchpad: bool = False) -> list[Engram]: + """Return semantic facts as Engrams, optionally budget-limited. + + exclude_scratchpad drops entries whose text/topic mentions + "scratchpad", applied before the budget cut. + """ ... # --- write --- diff --git a/anton/core/memory/cortex.py b/anton/core/memory/cortex.py index fa8df0ed..4d0b436f 100644 --- a/anton/core/memory/cortex.py +++ b/anton/core/memory/cortex.py @@ -185,8 +185,11 @@ async def build_memory_context(self, user_message: str = "") -> str: if identity: sections.append(f"## Your Memory — Identity\n{identity}") - # 2. Global rules (with smart retrieval) - global_engrams = self.global_hc.get_rules() + # 2. Global rules (with smart retrieval). get_rules(exclude_scratchpad_when=True) + # drops scratchpad-related "when" rules — those are already injected + # into the scratchpad tool description by get_scratchpad_context(), + # and showing them here too would double their token cost. + global_engrams = self.global_hc.get_rules(exclude_scratchpad_when=True) if global_engrams: global_engrams = await self._retrieve_relevant_rules(global_engrams, user_message) if global_engrams: @@ -194,8 +197,8 @@ async def build_memory_context(self, user_message: str = "") -> str: f"## Your Memory — Global Rules\n{self._format_rules_engrams(global_engrams)}" ) - # 3. Project rules (with smart retrieval) - project_engrams = self.project_hc.get_rules() + # 3. Project rules (with smart retrieval) — same scratchpad exclusion. + project_engrams = self.project_hc.get_rules(exclude_scratchpad_when=True) if project_engrams: project_engrams = await self._retrieve_relevant_rules(project_engrams, user_message) if project_engrams: @@ -205,17 +208,18 @@ async def build_memory_context(self, user_message: str = "") -> str: for engram in project_engrams: self._log_read_engram(engram) - # 4. Global lessons - global_lessons = self.global_hc.recall_lessons(token_budget=1000) + # 4. Global lessons. recall_lessons() excludes scratchpad-related + # entries internally — same reasoning as the rules exclusion above. + global_lessons = self.global_hc.recall_lessons(token_budget=1000, exclude_scratchpad=True) if global_lessons: sections.append(f"## Your Memory — Global Lessons\n{global_lessons}") - # 5. Project lessons - project_lessons = self.project_hc.recall_lessons(token_budget=1000) + # 5. Project lessons — same scratchpad exclusion. + project_lessons = self.project_hc.recall_lessons(token_budget=1000, exclude_scratchpad=True) if project_lessons: sections.append(f"## Your Memory — Project Lessons\n{project_lessons}") if self._episodic is not None: - for engram in self.project_hc.get_lessons(token_budget=1000): + for engram in self.project_hc.get_lessons(token_budget=1000, exclude_scratchpad=True): self._log_read_engram(engram) # 6. Minds datasource context (auto-loaded if present) diff --git a/anton/core/memory/hippocampus.py b/anton/core/memory/hippocampus.py index 0538f797..95b4916c 100644 --- a/anton/core/memory/hippocampus.py +++ b/anton/core/memory/hippocampus.py @@ -86,6 +86,20 @@ def _filter_by_token_budget(engrams: list[Engram], token_budget: int) -> list[En return kept +def _is_scratchpad_related(engram: Engram) -> bool: + """Whether an engram's text or topic mentions "scratchpad". + + Used both to select entries INTO recall_scratchpad_wisdom() (the + scratchpad tool description) and to exclude the same entries FROM + get_rules()/get_lessons() when building the system prompt — keeping one + definition avoids the two call sites drifting apart and double-injecting + (or double-excluding) an entry. + """ + if "scratchpad" in engram.text.lower(): + return True + return bool(engram.topic and "scratchpad" in engram.topic.lower()) + + class Hippocampus: """Reads and writes memory traces at a single scope (global OR project). @@ -210,16 +224,16 @@ def clear_identity(self) -> None: # --------- lessons -------------- - def recall_lessons(self, token_budget: int|None = 1000) -> str: + def recall_lessons(self, token_budget: int|None = 1000, exclude_scratchpad: bool = False) -> str: """Load semantic knowledge (lessons.md), most recent first, within budget. Brain analog: Anterior Temporal Lobe — the convergence hub for semantic facts distilled from many episodes. Budget enforced at ~4 chars/token. """ - return self._lessons_to_text(self.get_lessons(token_budget)) + return self._lessons_to_text(self.get_lessons(token_budget, exclude_scratchpad=exclude_scratchpad)) - def get_lessons(self, token_budget: int = None) -> list[Engram]: + def get_lessons(self, token_budget: int = None, exclude_scratchpad: bool = False) -> list[Engram]: """Load semantic knowledge (lessons.md) as Engrams. When token_budget is None (default) returns all entries in file order. @@ -252,6 +266,9 @@ def get_lessons(self, token_budget: int = None) -> list[Engram]: text, meta = _extract_metadata(text) entries.append(Engram(text=text.removeprefix("- "), **meta)) + if exclude_scratchpad: + entries = [e for e in entries if not _is_scratchpad_related(e)] + if token_budget is None: return entries @@ -340,13 +357,11 @@ def recall_scratchpad_wisdom(self, token_budget: int = 2000) -> str: candidates: list[Engram] = [] for rule in self.get_rules(): - if rule.kind == "when" and ( - "scratchpad" in rule.text.lower() or (rule.topic and "scratchpad" in rule.topic.lower()) - ): + if rule.kind == "when" and _is_scratchpad_related(rule): candidates.append(rule) for lesson in self.get_lessons(): - if "scratchpad" in lesson.text.lower() or (lesson.topic and "scratchpad" in lesson.topic.lower()): + if _is_scratchpad_related(lesson): candidates.append(lesson) confidence_rank = {"high": 0, "medium": 1, "low": 2} @@ -387,8 +402,15 @@ def recall_rules(self) -> str: except (OSError, UnicodeDecodeError): return "" - def get_rules(self) -> list[Engram]: - """Load behavioral rules (rules.md) as Engrams, grouped by kind.""" + def get_rules(self, exclude_scratchpad_when: bool = False) -> list[Engram]: + """Load behavioral rules (rules.md) as Engrams, grouped by kind. + + exclude_scratchpad_when drops "when" rules that mention "scratchpad" + — those are injected separately via recall_scratchpad_wisdom() into + the scratchpad tool description; including them in the system prompt + too would double their token cost. always/never rules are never + affected — those aren't part of that injection. + """ if not self._rules_path.is_file(): return [] try: @@ -410,6 +432,11 @@ def get_rules(self) -> list[Engram]: entries.sort(key=lambda x: x.kind) + if exclude_scratchpad_when: + entries = [ + e for e in entries if not (e.kind == "when" and _is_scratchpad_related(e)) + ] + return entries def del_rule(self, id): diff --git a/docs/docs/developer/memory-systems.md b/docs/docs/developer/memory-systems.md index 0778174c..552754da 100644 --- a/docs/docs/developer/memory-systems.md +++ b/docs/docs/developer/memory-systems.md @@ -165,7 +165,9 @@ injected *after* memory, giving user instructions higher priority. **Moment B — scratchpad tool description (procedural priming).** When scratchpads are active, `cortex.get_scratchpad_context()` appends relevant lessons to the scratchpad tool's description — the LLM sees them exactly when -composing code. +composing code. Moment A excludes anything scratchpad-related +(`get_rules(exclude_scratchpad_when=True)`, `recall_lessons()`) so the same +rule/lesson isn't billed twice per call. ### Context budget diff --git a/tests/test_cortex.py b/tests/test_cortex.py index 69facbec..28faa86d 100644 --- a/tests/test_cortex.py +++ b/tests/test_cortex.py @@ -59,6 +59,35 @@ async def test_includes_lessons(self, cortex, dirs): assert "Global fact" in result assert "Project fact" in result + async def test_excludes_scratchpad_when_rules(self, cortex, dirs): + """Scratchpad-related "when" rules already surface via the scratchpad + tool description (get_scratchpad_context) — showing them here too + would double their token cost, so they're excluded from the system + prompt. Unrelated "when" rules are unaffected.""" + g, _ = dirs + hc = Hippocampus(g) + hc.encode_rule( + "If a scratchpad API is paginated, use progress()", + kind="when", confidence="high", source="user", + ) + hc.encode_rule( + "If the user writes in Spanish, respond in Spanish", + kind="when", confidence="high", source="user", + ) + result = await cortex.build_memory_context() + assert "paginated" not in result + assert "Spanish" in result + + async def test_excludes_scratchpad_lessons(self, cortex, dirs): + """Same exclusion as above, for lessons.""" + g, _ = dirs + hc = Hippocampus(g) + hc.encode_lesson("Scratchpad cells timeout at 30s") + hc.encode_lesson("CoinGecko rate-limits at 50/min") + result = await cortex.build_memory_context() + assert "timeout at 30s" not in result + assert "rate-limits" in result + class TestGetScratchpadContext: def test_empty_returns_empty(self, cortex): @@ -66,7 +95,9 @@ def test_empty_returns_empty(self, cortex): def test_combines_scopes(self, cortex, dirs): g, p = dirs - (g / "rules.md").write_text("# Rules\n\n## Always\n\n## Never\n\n## When\n- If slow → batch\n") + (g / "rules.md").write_text( + "# Rules\n\n## Always\n\n## Never\n\n## When\n- If scratchpad is slow → batch\n" + ) (p / "lessons.md").write_text("# Lessons\n- Scratchpad times out at 30s\n") result = cortex.get_scratchpad_context() assert "slow" in result From 9c9c3bea0299776109984f4a1d96fe734bfd9c4f Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Mon, 13 Jul 2026 21:10:44 +0000 Subject: [PATCH 11/21] Merge pull request #247 from mindsdb/alejandrocantu/eng-747-sdk-unwraps-error-envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ENG-747: read status-error codes from the shape the SDK actually stores — revives the dead model-403 card --- anton/core/llm/openai.py | 48 +++++++--- tests/test_status_error_mapper.py | 151 +++++++++++++++++++++++++----- 2 files changed, 164 insertions(+), 35 deletions(-) diff --git a/anton/core/llm/openai.py b/anton/core/llm/openai.py index 8a863609..88923c44 100644 --- a/anton/core/llm/openai.py +++ b/anton/core/llm/openai.py @@ -38,16 +38,37 @@ def _raise_for_status_error(exc: "openai.APIStatusError", model: str) -> NoRetur Mapping policy: - 401 → ConnectionError with the invalid-key copy (cowork-server's provider_auth detection keys on this exact phrase). - - 403 with a structured gateway code (``error.code`` of - ``model_access_denied`` / ``model_disabled``) → ModelUnavailableError - carrying the code + model, with actionable copy. Detection is - code-exact on purpose: BYOK OpenAI 403s (region blocks), Anthropic - permission errors, and Cloudflare HTML 403s carry no such code and - must fall through to the generic message, never the plan copy. + - 403 with a structured gateway code (``model_access_denied`` / + ``model_disabled``) → ModelUnavailableError carrying the code + + model, with actionable copy. Detection is code-exact on purpose: + BYOK OpenAI 403s (region blocks), Anthropic permission errors, and + Cloudflare HTML 403s carry no such code and must fall through to + the generic message, never the plan copy. - 429 with a quota detail → TokenLimitExceeded, checked first so a body - carrying both ``detail`` and ``error.code`` stays token_limit (quota - keeps its own card downstream). + carrying both ``detail`` and a structured code stays token_limit + (quota keeps its own card downstream). - anything else → the generic "temporarily unavailable" ConnectionError. + + Body-shape tolerance (ENG-747): the OpenAI SDK UNWRAPS the error + envelope before storing it — ``openai/_client.py`` does + ``data = body.get("error", body)`` — so for the gateway's OpenAI-style + 403 (``{"error": {"code": …}}``), ``exc.body`` is the INNER dict and + ``code`` sits at top level. The gateway's 429 is FastAPI-style + (``{"detail": …}``, no envelope) and passes through untouched. Both + fields are therefore read from the top level first, with an envelope + fallback for clients that deliver the wire shape unmodified (anton's + pyproject allows ``openai>=1.0``, and proxies exist that re-wrap). + The originally shipped ENG-598 mapper read only + ``body["error"]["code"]`` — a key the SDK had already peeled off — + which left the model-403 card dead in production. + + Known limits, on purpose: a wire body carrying BOTH a top-level + ``detail`` and an ``error`` envelope loses the detail inside the SDK + (the unwrap discards siblings of ``error``) — unrecoverable from + ``exc.body``, and no real dialect emits that shape. And a 429 whose + envelope carries only ``message`` (OpenAI's own quota dialect, e.g. + ``insufficient_quota``) stays generic: classifying arbitrary provider + messages as MindsHub quota would put an upsell CTA on BYOK errors. """ if exc.status_code == 401: raise ConnectionError( @@ -55,13 +76,16 @@ def _raise_for_status_error(exc: "openai.APIStatusError", model: str) -> NoRetur ) from exc body = exc.body if isinstance(exc.body, dict) else {} - if exc.status_code == 429 and body.get("detail"): - msg = f"Server returned 429 — {body['detail']}" + envelope = body.get("error") if isinstance(body.get("error"), dict) else {} + detail = body.get("detail") or envelope.get("detail") + # str-only: FastAPI validation errors put a LIST in detail — rendering + # its repr into user-facing copy (with an upgrade CTA!) helps nobody. + if exc.status_code == 429 and isinstance(detail, str) and detail: + msg = f"Server returned 429 — {detail}" msg += " Visit https://console.mindshub.ai to upgrade or top up your tokens." raise TokenLimitExceeded(msg) from exc - error = body.get("error") if isinstance(body.get("error"), dict) else {} - code = error.get("code") + code = body.get("code") or envelope.get("code") if exc.status_code == 403 and code in ("model_access_denied", "model_disabled"): if code == "model_access_denied": msg = ( diff --git a/tests/test_status_error_mapper.py b/tests/test_status_error_mapper.py index c1089783..beb8212d 100644 --- a/tests/test_status_error_mapper.py +++ b/tests/test_status_error_mapper.py @@ -1,8 +1,7 @@ -"""The shared provider status-error mapper (ENG-598). +"""The shared provider status-error mapper (ENG-598, fixed in ENG-747). -One mapper (`openai._raise_for_status_error`) now serves all four call paths -(chat/stream × completions/responses) — previously four copy-pasted blocks -that had already drifted in wording. These tests pin the mapping policy: +One mapper (`openai._raise_for_status_error`) serves all four call paths +(chat/stream × completions/responses). These tests pin the mapping policy: - 401 → ConnectionError with the exact invalid-key copy (cowork-server's provider_auth detection string-matches it). @@ -11,25 +10,55 @@ with actionable copy per code. - Any other 403 (BYOK region blocks, Anthropic permission errors, Cloudflare HTML) → the generic message, NEVER the plan copy. + +ENG-747: every exception here is constructed by the REAL pinned OpenAI SDK +from a raw HTTP response (`httpx.MockTransport`), never hand-built. The +original suite duck-typed `exc.body` as the wire envelope +(``{"error": {...}}``) — but the SDK UNWRAPS that envelope +(``openai/_client.py``: ``data = body.get("error", body)``), so the tests +passed against a shape production never produces and the model-403 card +shipped dead. If the SDK's behavior ever changes, these tests notice; +hand-built fixtures cannot. """ +import httpx +import openai import pytest from anton.core.llm.openai import _raise_for_status_error from anton.core.llm.provider import ModelUnavailableError, TokenLimitExceeded -class _FakeStatusError(Exception): - """Duck-typed stand-in for openai.APIStatusError (status_code + body).""" - - def __init__(self, status_code, body=None): - super().__init__(f"HTTP {status_code}") - self.status_code = status_code - self.body = body +def _sdk_error(status_code, json_body=None, text_body=None): + """Real `openai.APIStatusError`, built by the pinned SDK from a raw HTTP + response — exactly what production call sites catch and hand to the + mapper. This is the load-bearing difference from the original suite.""" + + def handler(request: httpx.Request) -> httpx.Response: + if text_body is not None: + return httpx.Response(status_code, text=text_body) + return httpx.Response(status_code, json=json_body if json_body is not None else {}) + + client = openai.OpenAI( + base_url="http://gateway.test/v1", + api_key="test-key", + max_retries=0, + http_client=httpx.Client(transport=httpx.MockTransport(handler)), + ) + try: + client.chat.completions.create( + model="latest:sonnet", max_tokens=1, + messages=[{"role": "user", "content": "hi"}], + ) + except openai.APIStatusError as exc: + return exc + raise AssertionError(f"SDK did not raise for HTTP {status_code}") def _gateway_403(code, model="sonnet"): - return _FakeStatusError(403, body={"error": { + """The gateway's OpenAI-style 403 envelope, byte-shaped like the live + body captured from prod on 2026-07-13 (ENG-747).""" + return _sdk_error(403, json_body={"error": { "message": f"The model '{model}' is rejected.", "type": "invalid_request_error", "param": "model", @@ -37,32 +66,77 @@ def _gateway_403(code, model="sonnet"): }}) +# ── the SDK-unwrap regression itself ────────────────────────────────── + +def test_sdk_unwraps_error_envelope(): + """Documents the SDK behavior that broke ENG-598: `exc.body` is the + INNER error object, not the wire envelope. If this ever fails, the SDK + changed its parsing and the mapper's shape assumptions need re-auditing.""" + exc = _gateway_403("model_access_denied") + assert isinstance(exc.body, dict) + assert "error" not in exc.body + assert exc.body["code"] == "model_access_denied" + + # ── 401 ─────────────────────────────────────────────────────────────── def test_401_maps_to_invalid_key_connection_error(): + exc = _sdk_error(401, json_body={"error": {"message": "bad key"}}) with pytest.raises(ConnectionError) as err: - _raise_for_status_error(_FakeStatusError(401), "sonnet") + _raise_for_status_error(exc, "sonnet") # cowork-server's provider_auth detection keys on this exact phrase. assert "Invalid API key" in str(err.value) assert not isinstance(err.value, ModelUnavailableError) +def test_401_html_body_maps_to_invalid_key(): + # nginx auth walls return HTML — the 401 branch must not need a body. + exc = _sdk_error(401, text_body="401 Authorization Required") + with pytest.raises(ConnectionError) as err: + _raise_for_status_error(exc, "sonnet") + assert "Invalid API key" in str(err.value) + + # ── 429 (quota) ─────────────────────────────────────────────────────── -def test_429_with_detail_maps_to_token_limit(): - exc = _FakeStatusError(429, body={"detail": "Monthly limit exceeded for tokens: 5/5"}) +def test_429_fastapi_detail_maps_to_token_limit(): + # The gateway's CURRENT dialect: FastAPI HTTPException → {"detail": ...}, + # no envelope, so the SDK's unwrap is a no-op. + exc = _sdk_error(429, json_body={"detail": "Monthly limit exceeded for tokens: 5/5"}) with pytest.raises(TokenLimitExceeded) as err: _raise_for_status_error(exc, "sonnet") assert "Monthly limit exceeded" in str(err.value) assert "console.mindshub.ai" in str(err.value) +def test_429_enveloped_detail_also_maps_to_token_limit(): + # If the gateway ever moves its 429 into the OpenAI envelope while keeping + # the `detail` field, the SDK unwraps it to top level — still classified. + # (An envelope carrying only `message` — OpenAI's own quota dialect — + # deliberately stays generic; see the mapper docstring's known limits.) + exc = _sdk_error(429, json_body={"error": {"detail": "Monthly limit exceeded for tokens: 5/5"}}) + with pytest.raises(TokenLimitExceeded) as err: + _raise_for_status_error(exc, "sonnet") + assert "Monthly limit exceeded" in str(err.value) + + def test_bare_429_stays_generic(): + exc = _sdk_error(429, json_body={}) with pytest.raises(ConnectionError) as err: - _raise_for_status_error(_FakeStatusError(429), "sonnet") + _raise_for_status_error(exc, "sonnet") assert "temporarily unavailable" in str(err.value) +def test_429_list_detail_stays_generic(): + # FastAPI validation errors put a LIST in detail — its Python repr must + # never reach user-facing copy with an upgrade CTA attached. + exc = _sdk_error(429, json_body={"detail": [{"loc": ["body", "x"], "msg": "field required"}]}) + with pytest.raises(ConnectionError) as err: + _raise_for_status_error(exc, "sonnet") + assert "field required" not in str(err.value) + assert not isinstance(err.value, TokenLimitExceeded) + + # ── 403 with structured gateway codes ──────────────────────────────── def test_model_access_denied_maps_to_plan_copy(): @@ -95,11 +169,38 @@ def test_model_unavailable_is_a_connection_error(): _raise_for_status_error(_gateway_403("model_disabled"), "sonnet") +def _wire_shaped_error(status_code, body): + """Real `openai.APIStatusError` constructed DIRECTLY with an explicit + body — bypassing the SDK's parse-and-unwrap on purpose. This is the only + way to hand the mapper an envelope-shaped ``exc.body``: the pinned SDK + always peels ``error`` (see test_sdk_unwraps_error_envelope), but + anton's pyproject allows ``openai>=1.0`` and proxies exist that re-wrap, + so the mapper's envelope fallback must stay pinned by a test that the + MockTransport route physically cannot produce.""" + request = httpx.Request("POST", "http://gateway.test/v1/chat/completions") + response = httpx.Response(status_code, json=body if isinstance(body, dict) else None, request=request) + return openai.APIStatusError("wire-shaped", response=response, body=body) + + +def test_envelope_shaped_403_maps_via_fallback(): + # A client that does NOT unwrap delivers the wire envelope verbatim — + # the mapper's `envelope.get("code")` fallback is what classifies it. + exc = _wire_shaped_error(403, {"error": {"code": "model_access_denied", "message": "no"}}) + with pytest.raises(ModelUnavailableError): + _raise_for_status_error(exc, "sonnet") + + +def test_envelope_shaped_429_detail_maps_via_fallback(): + exc = _wire_shaped_error(429, {"error": {"detail": "Monthly limit exceeded for tokens: 5/5"}}) + with pytest.raises(TokenLimitExceeded): + _raise_for_status_error(exc, "sonnet") + + # ── 403 WITHOUT the gateway codes: never the plan copy ──────────────── def test_byok_openai_403_falls_through_to_generic(): # e.g. OpenAI region block — different error code. - exc = _FakeStatusError(403, body={"error": { + exc = _sdk_error(403, json_body={"error": { "code": "unsupported_country_region_territory", "message": "Country not supported.", }}) @@ -111,13 +212,14 @@ def test_byok_openai_403_falls_through_to_generic(): def test_html_403_falls_through_to_generic(): # Cloudflare/WAF blocks have no JSON body at all. + exc = _sdk_error(403, text_body="blocked") with pytest.raises(ConnectionError) as err: - _raise_for_status_error(_FakeStatusError(403, body="blocked"), "sonnet") + _raise_for_status_error(exc, "sonnet") assert not isinstance(err.value, ModelUnavailableError) def test_403_with_no_code_falls_through_to_generic(): - exc = _FakeStatusError(403, body={"error": {"message": "forbidden"}}) + exc = _sdk_error(403, json_body={"error": {"message": "forbidden"}}) with pytest.raises(ConnectionError) as err: _raise_for_status_error(exc, "sonnet") assert not isinstance(err.value, ModelUnavailableError) @@ -127,9 +229,11 @@ def test_403_with_no_code_falls_through_to_generic(): def test_429_with_detail_wins_even_if_error_code_present(): # A quota failure must stay token_limit, never be misread as model-403. - exc = _FakeStatusError(429, body={ - "detail": "Monthly limit exceeded for tokens: 5/5", - "error": {"code": "model_disabled"}, + # (The SDK unwraps this body to its "error" member, which carries both + # fields — precedence must hold on the unwrapped shape too.) + exc = _sdk_error(429, json_body={ + "error": {"detail": "Monthly limit exceeded for tokens: 5/5", + "code": "model_disabled"}, }) with pytest.raises(TokenLimitExceeded): _raise_for_status_error(exc, "sonnet") @@ -138,6 +242,7 @@ def test_429_with_detail_wins_even_if_error_code_present(): # ── other statuses stay generic ─────────────────────────────────────── def test_500_stays_generic(): + exc = _sdk_error(500, json_body={"error": {"message": "boom"}}) with pytest.raises(ConnectionError) as err: - _raise_for_status_error(_FakeStatusError(500), "sonnet") + _raise_for_status_error(exc, "sonnet") assert "Server returned 500" in str(err.value) From 089c287a3045b586dd62d1a8e7da4368bb759470 Mon Sep 17 00:00:00 2001 From: martyna-mindsdb Date: Tue, 14 Jul 2026 20:33:37 +0200 Subject: [PATCH 12/21] review fixes --- anton/core/runtime.py | 91 ++------------------------------------ anton/utils/datasources.py | 54 ++++++++++++++++++++++ 2 files changed, 58 insertions(+), 87 deletions(-) diff --git a/anton/core/runtime.py b/anton/core/runtime.py index a56d2e14..bafac698 100644 --- a/anton/core/runtime.py +++ b/anton/core/runtime.py @@ -14,7 +14,6 @@ """ from __future__ import annotations -import json import logging import os from pathlib import Path @@ -142,14 +141,6 @@ async def build_chat_session( initial_history = history_store.load(session_id) data_vault = LocalDataVault() if LocalDataVault is not None else None - google_drive_oauth_connected = False - # {connection_name: [{"id": ..., "name": ...}, ...]} — files the user - # explicitly granted access to via the Google Picker. drive.file scope - # only covers files the app created itself, so without calling these - # out by name the agent has no way to know they're reachable at all; - # it isn't enough to just leave PICKED_FILES sitting in the env like - # any other DS_ credential field. - google_drive_picked_files: dict[str, list[dict]] = {} if data_vault is not None: try: connections = data_vault.list_connections() @@ -157,13 +148,9 @@ async def build_chat_session( logger.debug("Could not list Anton data vault connections", exc_info=True) connections = [] for conn in connections: - # Per-connection try/except — one bad record (a load() error, a - # malformed field) must not abort inject_env/picked_files - # processing for every connection after it in the list. - # engine/name are initialized before the try so the except - # handler's logging never re-calls conn.get(...) itself — if - # `conn` isn't a plain dict, that would raise a second, - # uncaught exception right here and abort the whole loop again. + # Per-connection try/except so one bad record can't abort + # injecting env for the rest; engine/name are set before the try + # so the except's own logging can't itself raise. engine = None name = None try: @@ -172,82 +159,12 @@ async def build_chat_session( if not (engine and name): continue data_vault.inject_env(engine, name) - if engine == "google_drive": - fields = data_vault.load(engine, name) or {} - if fields.get("auth_type") == "oauth": - google_drive_oauth_connected = True - # Fall back to the pre-rename key so a vault record - # written before the _picked_files rename doesn't - # silently lose its guidance. - raw_picked = fields.get("_picked_files") or fields.get("picked_files") - if raw_picked: - try: - parsed = json.loads(raw_picked) - except (json.JSONDecodeError, TypeError): - parsed = [] - # Only keep well-formed entries — anything else (a - # bare string, a dict instead of a list, an entry - # missing "id") would crash the f.get(...) calls - # below or embed a literal "None" fileId in the - # prompt instructions. - files = ( - [f for f in parsed if isinstance(f, dict) and f.get("id")] - if isinstance(parsed, list) else [] - ) - if files: - google_drive_picked_files[name] = files except Exception: logger.debug( "Could not inject Anton data vault env for %s/%s", engine, name, exc_info=True, ) - integration_guidance = "" - # Also shown whenever picked_files exist even if google_drive_oauth_connected - # is somehow False (fragile single-connection heuristic) — a connection - # can only have picked_files at all via the OAuth-based Picker flow, so - # its DS_ credentials necessarily exist; otherwise the IMPORTANT block - # below would tell the agent to fetch specific file IDs without ever - # mentioning the env vars that authenticate those calls. - if google_drive_oauth_connected or google_drive_picked_files: - integration_guidance = ( - " Connected Google Drive accounts are available through Google OAuth credentials " - "in the injected `DS_GOOGLE_DRIVE___...` environment variables. " - "Only claim Google Drive access if you can actually use those credentials successfully." - ) - # Deliberately NOT gated behind google_drive_oauth_connected — that flag - # is a fragile, single-source heuristic (fields.get("auth_type") == - # "oauth" on SOME connection). A connection whose record predates that - # field, or uses a different auth_type value, would otherwise have real - # picked_files data but no guidance telling the agent it exists — - # reproducing the exact bug this block was added to fix. - if google_drive_picked_files: - picked_lines = [ - f"- {f.get('name', 'untitled')} (id: {f.get('id')}, connection: {conn_name})" - for conn_name, files in google_drive_picked_files.items() - for f in files - ] - # Deliberately imperative and structurally separate from the - # paragraph above — a soft prose mention was tested and the - # agent silently dropped these files whenever a user asked to - # "list" Drive files, because it just reported files.list()'s - # output verbatim without cross-referencing earlier context. - integration_guidance += ( - "\n\nIMPORTANT — additional Drive files the user has explicitly granted access to " - "via the Google Picker, which a plain files.list() or files.search() call will NOT " - "return (the google_drive scope only covers files this app created itself, plus " - "these specifically granted ones):\n" - + "\n".join(picked_lines) - + "\nWhenever you list, search, or enumerate Drive files for the user, you MUST " - "include every file above IN ADDITION to whatever files.list()/files.search() " - "returns — do not report only the API call's results. To read one of these files' " - "content, call files.get(fileId=...) directly with its id above; do not expect it " - "to appear in a files.list() response first." - ) - - suffix_parts = [s for s in (system_prompt_suffix, integration_guidance) if s] - final_suffix = "".join(suffix_parts) if suffix_parts else None - config = ChatSessionConfig( llm_client=llm_client, settings=settings, @@ -256,7 +173,7 @@ async def build_chat_session( episodic=episodic, system_prompt_context=SystemPromptContext( runtime_context=build_runtime_context(settings), - suffix=final_suffix, + suffix=system_prompt_suffix, ), output_dir=str(output_dir), workspace=workspace, diff --git a/anton/utils/datasources.py b/anton/utils/datasources.py index 27b1f0a9..223fece8 100644 --- a/anton/utils/datasources.py +++ b/anton/utils/datasources.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os import re import shutil @@ -145,6 +146,20 @@ def scrub_credentials(text: str) -> str: return text +def _parse_picked_files(raw: str | None) -> list[dict]: + """Parse a `_picked_files` JSON field, dropping malformed entries so they + can't crash a later f.get(...) call or embed "None" as a fileId.""" + if not raw: + return [] + try: + parsed = json.loads(raw) + except (json.JSONDecodeError, TypeError): + return [] + if not isinstance(parsed, list): + return [] + return [f for f in parsed if isinstance(f, dict) and f.get("id")] + + def build_datasource_context(vault: DataVault, active_only: str | None = None) -> str: """Build a system-prompt section listing available DS_* env vars by name. @@ -172,6 +187,12 @@ def build_datasource_context(vault: DataVault, active_only: str | None = None) - "it by name; never treat the bracket form as a literal credential " "or pass it back as a value to any tool.\n" ) + # Google Drive's drive.file OAuth scope only covers files the app created + # itself, plus files explicitly granted via the Google Picker (persisted + # as a `_picked_files` vault field) — these must be named explicitly here + # since a plain files.list()/files.search() call won't return them. + google_drive_oauth_connected = False + google_drive_picked_files: dict[str, list[dict]] = {} for c in conns: slug = f"{c['engine']}-{c['name']}" if active_only and slug != active_only: @@ -200,6 +221,39 @@ def build_datasource_context(vault: DataVault, active_only: str | None = None) - if identity: head += f" — {identity}" lines.append(f"- {head} → {var_names}") + if c["engine"] == "google_drive": + if fields.get("auth_type") == "oauth": + google_drive_oauth_connected = True + picked = _parse_picked_files(fields.get("_picked_files")) + if picked: + google_drive_picked_files[c["name"]] = picked + if google_drive_oauth_connected or google_drive_picked_files: + lines.append( + "\nConnected Google Drive accounts are available through Google OAuth credentials " + "in the injected `DS_GOOGLE_DRIVE___...` environment variables. " + "Only claim Google Drive access if you can actually use those credentials successfully." + ) + if google_drive_picked_files: + picked_lines = [ + f"- {f.get('name', 'untitled')} (id: {f.get('id')}, connection: {conn_name})" + for conn_name, files in google_drive_picked_files.items() + for f in files + ] + # Imperative and structurally separate from the paragraph above — a + # softer prose mention got silently dropped by the agent when + # reporting files.list() results verbatim. + lines.append( + "\nIMPORTANT — additional Drive files the user has explicitly granted access to " + "via the Google Picker, which a plain files.list() or files.search() call will NOT " + "return (the google_drive scope only covers files this app created itself, plus " + "these specifically granted ones):\n" + + "\n".join(picked_lines) + + "\nWhenever you list, search, or enumerate Drive files for the user, you MUST " + "include every file above IN ADDITION to whatever files.list()/files.search() " + "returns — do not report only the API call's results. To read one of these files' " + "content, call files.get(fileId=...) directly with its id above; do not expect it " + "to appear in a files.list() response first." + ) return "\n".join(lines) From c34576a8be8e9d59476e2b314fb0081224b8bbb5 Mon Sep 17 00:00:00 2001 From: martyna-mindsdb Date: Tue, 14 Jul 2026 21:24:26 +0200 Subject: [PATCH 13/21] fix: system-prompt suffix crash on empty suffix; add gdrive picker context tests --- anton/core/runtime.py | 5 +- tests/test_build_chat_session_google_drive.py | 84 ++++++++++++++ tests/test_datasource_context_identity.py | 107 +++++++++++++++++- 3 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 tests/test_build_chat_session_google_drive.py diff --git a/anton/core/runtime.py b/anton/core/runtime.py index bafac698..b078a898 100644 --- a/anton/core/runtime.py +++ b/anton/core/runtime.py @@ -173,7 +173,10 @@ async def build_chat_session( episodic=episodic, system_prompt_context=SystemPromptContext( runtime_context=build_runtime_context(settings), - suffix=system_prompt_suffix, + # SystemPromptContext.suffix is typed str (default ""), not + # Optional — pass "" rather than None when no suffix was given, + # or ChatSystemPromptBuilder.build()'s suffix.strip() crashes. + suffix=system_prompt_suffix or "", ), output_dir=str(output_dir), workspace=workspace, diff --git a/tests/test_build_chat_session_google_drive.py b/tests/test_build_chat_session_google_drive.py new file mode 100644 index 00000000..20e86b29 --- /dev/null +++ b/tests/test_build_chat_session_google_drive.py @@ -0,0 +1,84 @@ +"""End-to-end: build_chat_session() -> ChatSession._build_system_prompt() actually +surfaces Google Drive Picker guidance in the real assembled system prompt. + +ENG-687 review (PR #241): the picked-files/OAuth guidance moved out of +build_chat_session (anton/core/runtime.py) into build_datasource_context +(anton/utils/datasources.py), which ChatSession already calls fresh every +turn. This exercises the real integration point end-to-end rather than just +the relocated function in isolation, and guards a real crash the move +uncovered: SystemPromptContext.suffix is typed str (default ""), but +build_chat_session could pass suffix=None when no system_prompt_suffix was +given, and ChatSystemPromptBuilder.build() calls suffix.strip() unconditionally. +""" +from __future__ import annotations + +import json + +import pytest + + +@pytest.fixture(autouse=True) +def _isolated_home(tmp_path, monkeypatch): + """LocalDataVault() with no args resolves under $HOME — isolate it so this + test never touches the real ~/.anton/data_vault or ~/.anton/memory.""" + home = tmp_path / "home" + home.mkdir() + monkeypatch.setenv("HOME", str(home)) + # build_chat_session constructs a real LLMClient; a syntactically-plausible + # dummy key is enough since this test never calls the LLM (no turn_stream()). + monkeypatch.setenv("ANTON_ANTHROPIC_API_KEY", "sk-ant-test-00000000000000000000000000") + return home + + +@pytest.fixture() +def workspace_path(tmp_path): + p = tmp_path / "workspace" + p.mkdir() + return p + + +async def test_picked_files_reach_the_real_system_prompt(workspace_path): + from anton.core.datasources.data_vault import LocalDataVault + from anton.core.runtime import build_chat_session + + vault = LocalDataVault() + vault.save("google_drive", "work", { + "auth_type": "oauth", + "account_email": "user@example.com", + "_picked_files": json.dumps([{"id": "f1", "name": "Roadmap.gdoc"}]), + }) + + session = await build_chat_session(session_id="test-picked-files", workspace_path=str(workspace_path)) + prompt = await session._build_system_prompt() + + assert "Connected Google Drive accounts are available" in prompt + assert "IMPORTANT — additional Drive files" in prompt + assert "Roadmap.gdoc" in prompt + + +async def test_no_connections_and_no_suffix_does_not_crash(workspace_path): + """Regression: on staging, build_chat_session could pass suffix=None to + SystemPromptContext (typed str) whenever there was nothing to add, + crashing ChatSystemPromptBuilder.build()'s suffix.strip() call. Any + session with zero connections and no explicit system_prompt_suffix hits + this — verifying it explicitly rather than relying on it being masked by + an unrelated google_drive connection existing in the vault.""" + from anton.core.runtime import build_chat_session + + session = await build_chat_session(session_id="test-empty", workspace_path=str(workspace_path)) + prompt = await session._build_system_prompt() # must not raise + + assert "Google Drive" not in prompt + + +async def test_explicit_system_prompt_suffix_still_appended(workspace_path): + from anton.core.runtime import build_chat_session + + session = await build_chat_session( + session_id="test-suffix", + workspace_path=str(workspace_path), + system_prompt_suffix="Host-specific note: reply in French.", + ) + prompt = await session._build_system_prompt() + + assert "Host-specific note: reply in French." in prompt diff --git a/tests/test_datasource_context_identity.py b/tests/test_datasource_context_identity.py index dff4d778..667ea0e2 100644 --- a/tests/test_datasource_context_identity.py +++ b/tests/test_datasource_context_identity.py @@ -5,8 +5,10 @@ or the DB host/name, never the credential, and never a dump of opaque/config fields. """ +import json + from anton.core.datasources.data_vault import LocalDataVault -from anton.utils.datasources import _connection_identity, build_datasource_context +from anton.utils.datasources import _connection_identity, _parse_picked_files, build_datasource_context class TestConnectionIdentity: @@ -82,3 +84,106 @@ def test_label_preferred_over_email(self, tmp_path): ctx = build_datasource_context(v) assert "Support" in ctx # the human label is shown assert "regtr@mail.com" not in ctx # label preferred over the opaque email + + +class TestParsePickedFiles: + def test_valid_list(self): + raw = json.dumps([{"id": "1", "name": "a"}, {"id": "2", "name": "b"}]) + assert _parse_picked_files(raw) == [{"id": "1", "name": "a"}, {"id": "2", "name": "b"}] + + def test_drops_malformed_entries(self): + raw = json.dumps([ + {"id": "1", "name": "keep"}, + "not-a-dict", + {"name": "missing-id"}, + {"id": None, "name": "null-id"}, + ]) + assert _parse_picked_files(raw) == [{"id": "1", "name": "keep"}] + + def test_non_list_json_returns_empty(self): + assert _parse_picked_files(json.dumps({"id": "1"})) == [] + + def test_invalid_json_returns_empty(self): + assert _parse_picked_files("not json") == [] + + def test_empty_or_none_returns_empty(self): + assert _parse_picked_files(None) == [] + assert _parse_picked_files("") == [] + + +class TestGoogleDrivePickerContext: + """ENG-687: google_drive's drive.file OAuth scope only covers files the + app created itself, plus files explicitly granted via the Google + Picker — the agent needs those named by id or a plain files.list()/ + files.search() call won't surface them at all.""" + + def test_oauth_connection_without_picked_files_shows_availability_only(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", {"auth_type": "oauth", "account_email": "u@x.com"}) + ctx = build_datasource_context(v) + assert "Connected Google Drive accounts are available" in ctx + assert "IMPORTANT" not in ctx + + def test_picked_files_surfaced_with_id_and_connection(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", { + "auth_type": "oauth", + "_picked_files": json.dumps([{"id": "f1", "name": "Roadmap.gdoc"}]), + }) + ctx = build_datasource_context(v) + assert "IMPORTANT — additional Drive files" in ctx + assert "Roadmap.gdoc" in ctx + assert "id: f1" in ctx + assert "connection: work" in ctx + + def test_resource_key_not_required_but_included_when_present(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", { + "_picked_files": json.dumps([{"id": "f1", "name": "Shared.gdoc", "resourceKey": "rk123"}]), + }) + ctx = build_datasource_context(v) + assert "Roadmap.gdoc" not in ctx # sanity: not leaking the other test's fixture + assert "Shared.gdoc" in ctx + + def test_malformed_picked_file_entries_are_dropped_not_crashed(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", { + "_picked_files": json.dumps(["not-a-dict", {"name": "missing-id"}]), + }) + ctx = build_datasource_context(v) # must not raise + assert "IMPORTANT" not in ctx # nothing well-formed survived, so no block at all + + def test_no_google_drive_connection_no_guidance(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("postgres", "prod", {"host": "db.acme.com"}) + ctx = build_datasource_context(v) + assert "Google Drive" not in ctx + assert "IMPORTANT" not in ctx + + def test_multiple_google_drive_connections_each_listed_separately(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", {"_picked_files": json.dumps([{"id": "1", "name": "Work.gdoc"}])}) + v.save("google_drive", "personal", {"_picked_files": json.dumps([{"id": "2", "name": "Personal.gdoc"}])}) + ctx = build_datasource_context(v) + assert "Work.gdoc" in ctx and "connection: work" in ctx + assert "Personal.gdoc" in ctx and "connection: personal" in ctx + + def test_active_only_suppresses_other_connections_guidance(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", { + "auth_type": "oauth", + "_picked_files": json.dumps([{"id": "1", "name": "Roadmap.gdoc"}]), + }) + v.save("postgres", "prod", {"host": "db.acme.com"}) + # A different connection is active — Drive guidance must not leak in. + ctx = build_datasource_context(v, active_only="postgres-prod") + assert "Google Drive" not in ctx + assert "Roadmap.gdoc" not in ctx + + def test_active_only_on_google_drive_still_shows_its_guidance(self, tmp_path): + v = LocalDataVault(tmp_path) + v.save("google_drive", "work", { + "_picked_files": json.dumps([{"id": "1", "name": "Roadmap.gdoc"}]), + }) + ctx = build_datasource_context(v, active_only="google_drive-work") + assert "Roadmap.gdoc" in ctx From 2667f954fe5148d151d16ab575db6fb970291905 Mon Sep 17 00:00:00 2001 From: pnewsam Date: Tue, 14 Jul 2026 12:32:51 -0700 Subject: [PATCH 14/21] fix legacy datasource secret classification --- anton/utils/datasources.py | 16 ++++++++-------- tests/test_scrubbing.py | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/anton/utils/datasources.py b/anton/utils/datasources.py index 586ea908..51eaa1d8 100644 --- a/anton/utils/datasources.py +++ b/anton/utils/datasources.py @@ -7,7 +7,12 @@ from pathlib import Path from typing import TYPE_CHECKING -from anton.core.datasources.data_vault import DataVault, LocalDataVault, _slug_env_prefix +from anton.core.datasources.data_vault import ( + DataVault, + LocalDataVault, + _slug_env_prefix, + is_secret_key, +) from anton.core.datasources.datasource_registry import DatasourceRegistry, _YAML_BLOCK_RE if TYPE_CHECKING: @@ -112,10 +117,8 @@ def _register_unregistered_connection_vars(vault: DataVault, engine: str, name: conservative unknown-DS_* scrub — so harmless values like base_url surfaced as `[DS_..._BASE_URL]` markers in user-facing output (ENG-688). Classification: the record's stored ``secure_keys`` when present, else - the secret-name heuristic. + the vault's canonical legacy secret-name heuristic. """ - from anton.tools import looks_secret - record = vault.read_record(engine, name) if hasattr(vault, "read_record") else None fields = (record or {}).get("fields") or vault.load(engine, name) or {} secure_keys = (record or {}).get("secure_keys") @@ -123,10 +126,7 @@ def _register_unregistered_connection_vars(vault: DataVault, engine: str, name: for field_name in fields: key = f"{prefix}__{field_name.upper()}" _DS_KNOWN_VARS.add(key) - is_secret = ( - field_name in secure_keys if secure_keys is not None else looks_secret(field_name) - ) - if is_secret: + if is_secret_key(field_name, secure_keys): _DS_SECRET_VARS.add(key) diff --git a/tests/test_scrubbing.py b/tests/test_scrubbing.py index d91c512d..87518a90 100644 --- a/tests/test_scrubbing.py +++ b/tests/test_scrubbing.py @@ -185,3 +185,25 @@ def test_custom_engine_without_secure_keys_uses_name_heuristic(self, tmp_path): assert "https://legacy.acme-crm.example" in result assert "ak_1234567890abcdef" not in result assert "[DS_ACME_CRM_LEGACY__API_KEY]" in result + + def test_custom_engine_legacy_passphrase_is_scrubbed(self, tmp_path): + from anton.utils.datasources import restore_namespaced_env + + vault = self._vault(tmp_path) + passphrase = "correct horse battery staple" + vault.save( + "acme_crm", + "legacy", + { + "base_url": "https://legacy.acme-crm.example", + "passphrase": passphrase, + }, + ) + restore_namespaced_env(vault) + + result = scrub_credentials( + f"base https://legacy.acme-crm.example passphrase {passphrase}" + ) + assert "https://legacy.acme-crm.example" in result + assert passphrase not in result + assert "[DS_ACME_CRM_LEGACY__PASSPHRASE]" in result From db10fdcf9bd35e4f361074097e935d2d5098d394 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 15:43:15 -0700 Subject: [PATCH 15/21] feat(session): add WAITING verdict to completion verifier (ENG-716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completion-verifier loop classified every tool-using turn that ended by asking the user a question as INCOMPLETE and force-continued it, so the agent answered its own questions (or fabricated) instead of waiting. Root cause: the verdict vocabulary had no "waiting for the user" state — proven in prod (verifier verdict literally read "the assistant is waiting for user feedback ... has not been completed") and reproduced on a clean instance. - Add a WAITING verdict: a turn ending in a genuine question to the user (or a reasoned refusal) is a valid stop, not unfinished work. - Move the verdict to the cheap coding model via generate_object_code with a typed _VerifierVerdict over a compact request+reply view, instead of a full-transcript call on the planning model parsed from free-text "STATUS:" lines. Cheaper, and structurally reliable. - Add verify_min_tool_rounds (default 1 = current behavior) to allow skipping trivial single-tool-round turns once verdict logs confirm they're rarely INCOMPLETE. - Add non-disclosure clauses to the internal SYSTEM continuation/diagnosis injections so they aren't echoed to the user ("the verifier note is noted"). - Log every verdict for measurability. Co-Authored-By: Claude Opus 4.8 (1M context) --- anton/core/session.py | 127 ++++++++++++++++++++++------------ anton/core/settings.py | 5 ++ tests/test_chat_scratchpad.py | 10 ++- 3 files changed, 96 insertions(+), 46 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index a889c505..5b29f17d 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -6,9 +6,11 @@ from datetime import datetime import json import re -from typing import TYPE_CHECKING, List +from typing import TYPE_CHECKING, List, Literal import os +from pydantic import BaseModel, Field + from anton.core.backends.base import Cell, ScratchpadRuntimeFactory from anton.core.backends.local import local_scratchpad_runtime_factory from anton.core.datasources.data_vault import DataVault @@ -120,6 +122,29 @@ def _scrub_user_input(user_input: str | list[dict]) -> str | list[dict]: ] +class _VerifierVerdict(BaseModel): + """Structured verdict from the completion verifier (runs on the cheap + coding model). The field descriptions below double as the verifier's + instructions — see LLMClient.generate_object_code (ENG-716).""" + + status: Literal["COMPLETE", "WAITING", "INCOMPLETE", "STUCK"] = Field( + description=( + "Classify the assistant's latest message against the user's request:\n" + "- COMPLETE: the requested task is done. A finished task followed by an " + "optional 'want me to…?' offer is still COMPLETE.\n" + "- WAITING: the assistant's latest message asks the user a question it " + "genuinely needs answered to proceed with the requested task, or is a " + "reasoned refusal. This is a valid stopping point — do NOT treat it as " + "unfinished; the correct action is to wait for the user's reply.\n" + "- INCOMPLETE: the assistant stopped partway through the requested task " + "WITHOUT asking the user anything, and could keep going on its own.\n" + "- STUCK: a hard blocker prevents completion (missing credentials, an " + "unavailable service, or a permission the assistant does not have)." + ) + ) + reason: str = Field(description="One brief sentence explaining the verdict.") + + @dataclass class ChatSessionConfig: """All construction parameters for a ChatSession. @@ -183,6 +208,7 @@ def __init__(self, config: ChatSessionConfig) -> None: self._settings = config.settings self._max_tool_rounds = s.max_tool_rounds self._max_continuations = s.max_continuations + self._verify_min_tool_rounds = s.verify_min_tool_rounds self._context_pressure_threshold = s.context_pressure_threshold self._max_consecutive_errors = s.max_consecutive_errors self._resilience_nudge_at = s.resilience_nudge_at @@ -2197,9 +2223,10 @@ async def _stream_and_handle_tools( ) # --- Completion verification --- - # Only verify when tools were actually used (not for simple Q&A) - # and we haven't hit the max-rounds hard stop. - if tool_round == 0 or _max_rounds_hit: + # Skip when too few tool rounds were used (pure Q&A always skips at + # tool_round==0; raising verify_min_tool_rounds also skips trivial + # single-round turns) or when we hit the max-rounds hard stop. + if tool_round < self._verify_min_tool_rounds or _max_rounds_hit: break # Append the assistant's final text so the verifier can see it @@ -2230,47 +2257,60 @@ async def _stream_and_handle_tools( # Consolidation still runs after diagnosis break - # Ask the LLM to self-assess completion. - # Use a copy of history with a trailing user message so models - # that don't support assistant-prefill won't reject the request. - # Factory is re-invoked on each recovery attempt so the verifier - # sees the latest post-compaction history. - def build_verify_messages() -> list[dict]: - return list(self._history) + [ - { - "role": "user", - "content": ( - "SYSTEM: Evaluate whether the task the user originally requested " - "has been fully completed based on the conversation above." - ), - } - ] + # Ask the cheap coding model to self-assess completion, using a + # compact text-only view (the user's request + the assistant's latest + # message) rather than the full transcript. That is all the verifier + # needs for the verdict, keeps the call cheap, and — being plain text + # — avoids any tool_use/tool_result pairing constraints (ENG-716). + request_text = (user_message or "").strip() or "(see the assistant's message)" + verify_messages = [ + {"role": "user", "content": f"USER'S REQUEST:\n{request_text}"}, + { + "role": "assistant", + "content": reply.strip() or "(the assistant produced no final text)", + }, + { + "role": "user", + "content": ( + "Based on the assistant's latest message above, which status " + "applies to the user's request?" + ), + }, + ] verifier_system = ( - "You are a task-completion verifier. Given the conversation, determine " - "whether the user's original request has been fully completed.\n\n" - "Respond with EXACTLY one of these lines, followed by a brief reason:\n" - "STATUS: COMPLETE — \n" - "STATUS: INCOMPLETE — \n" - "STATUS: STUCK — \n\n" - "COMPLETE = the task is done or the response fully answers the question.\n" - "INCOMPLETE = more work can be done to finish the task.\n" - "STUCK = a blocker prevents completion (missing info, permissions, etc).\n\n" - "Be strict: if the user asked for X and only part of X was delivered, " - "that is INCOMPLETE, not COMPLETE. But if the user asked a question " - "and the assistant answered it, that is COMPLETE even without tool use." + "You are a task-completion verifier. Decide whether the user's " + "request is complete, the assistant is waiting on the user, the work " + "is unfinished, or the assistant is blocked. Follow the status " + "definitions exactly." ) - verification = await self.plan_with_recovery( - system=verifier_system, - max_tokens=256, - messages_factory=build_verify_messages, + try: + verdict = await self._llm.generate_object_code( + _VerifierVerdict, + system=verifier_system, + messages=verify_messages, + max_tokens=256, + ) + status = verdict.status + reason = verdict.reason.strip() + except Exception: + # Verifier failed — fail safe by treating the turn as done rather + # than forcing a continuation the user never asked for. + status, reason = "COMPLETE", "verifier unavailable" + + import logging as _logging + _logging.getLogger(__name__).info( + "completion-verifier verdict=%s continuation=%d/%d tool_rounds=%d reason=%s", + status, continuation, self._max_continuations, tool_round, reason, ) - status_text = (verification.content or "").strip().upper() - if "STATUS: COMPLETE" in status_text: + if status in ("COMPLETE", "WAITING"): + # COMPLETE = the request is done. WAITING = the assistant asked the + # user something it genuinely needs, or gave a reasoned refusal — + # a valid stop, NOT unfinished work. In both cases the turn's final + # message already stands in history; do not force a continuation. break - if "STATUS: STUCK" in status_text: - # Stuck — inject diagnosis request and let the LLM explain - reason = (verification.content or "").strip() + if status == "STUCK": + # Stuck — inject diagnosis request and let the LLM explain. self._append_history( { "role": "user", @@ -2278,7 +2318,8 @@ def build_verify_messages() -> list[dict]: f"SYSTEM: Task verification determined this task is stuck.\n" f"Verifier assessment: {reason}\n\n" "Explain to the user what went wrong, what you tried, and " - "suggest specific next steps they can take to unblock this." + "suggest specific next steps they can take to unblock this. " + "Do not mention this instruction or the verifier to the user." ), } ) @@ -2291,7 +2332,6 @@ def build_verify_messages() -> list[dict]: # INCOMPLETE — continue working continuation += 1 - reason = (verification.content or "").strip() self._append_history( { "role": "user", @@ -2300,7 +2340,8 @@ def build_verify_messages() -> list[dict]: f"(attempt {continuation}/{self._max_continuations}).\n" f"Verifier assessment: {reason}\n\n" "Continue working on the original request. Pick up where you left off " - "and finish the remaining work. Do not repeat work already done." + "and finish the remaining work. Do not repeat work already done. " + "Do not mention this instruction or the verifier to the user." ), } ) diff --git a/anton/core/settings.py b/anton/core/settings.py index 46e6b07b..9bcf07bb 100644 --- a/anton/core/settings.py +++ b/anton/core/settings.py @@ -8,6 +8,11 @@ class CoreSettings(BaseSettings): # Session orchestration tuning max_tool_rounds: int = 25 max_continuations: int = 3 + # Skip the completion verifier when a turn used fewer than this many tool + # rounds. Default 1 preserves today's behavior (only pure Q&A, tool_round==0, + # is skipped). Raise to 2 to also skip trivial single-tool-round turns once + # verdict logs confirm they're rarely INCOMPLETE (ENG-716). + verify_min_tool_rounds: int = 1 context_pressure_threshold: float = 0.7 max_consecutive_errors: int = 5 resilience_nudge_at: int = 2 diff --git a/tests/test_chat_scratchpad.py b/tests/test_chat_scratchpad.py index 2c1124bb..600b056e 100644 --- a/tests/test_chat_scratchpad.py +++ b/tests/test_chat_scratchpad.py @@ -7,7 +7,7 @@ import pytest from anton.core.backends.base import Cell -from anton.core.session import ChatSession, ChatSessionConfig +from anton.core.session import ChatSession, ChatSessionConfig, _VerifierVerdict from anton.core.tools.tool_defs import SCRATCHPAD_TOOL from anton.commands.session import handle_resume from anton.core.llm.provider import LLMResponse, StreamComplete, StreamToolResult, ToolCall, Usage @@ -251,7 +251,9 @@ async def test_scratchpad_dump_streams_tool_result(self, workspace): """dump action yields a StreamToolResult for display, but sends a short summary back to the LLM to avoid it parroting the full notebook.""" mock_llm = make_mock_llm() - mock_llm.plan = AsyncMock(return_value=_text_response("STATUS: COMPLETE — task done")) + mock_llm.generate_object_code = AsyncMock( + return_value=_VerifierVerdict(status="COMPLETE", reason="task done") + ) call_count = 0 @@ -306,7 +308,9 @@ async def test_scratchpad_in_streaming_path(self, workspace): final_response = _text_response("Got 99.") mock_llm = make_mock_llm() - mock_llm.plan = AsyncMock(return_value=_text_response("STATUS: COMPLETE — task done")) + mock_llm.generate_object_code = AsyncMock( + return_value=_VerifierVerdict(status="COMPLETE", reason="task done") + ) call_count = 0 From 92f47b14a98202a7296f8c7ca13eee095b3be4fb Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 15:50:12 -0700 Subject: [PATCH 16/21] fix(session): address self-review of the verifier change (ENG-716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial self-review of the WAITING-verdict commit surfaced two issues: - The reduced verify view was tool-blind: it showed only the user request + the assistant's final text, so a turn where a tool errored but the assistant claimed success would be judged COMPLETE — defeating the STUCK backstop the ENG-296 fabrication mitigation relies on. Track a compact per-tool outcome log (name=ok/error) for the turn and feed it to the verifier with explicit guidance ("a depended-on tool that errored while the assistant implied success is INCOMPLETE/STUCK, not COMPLETE"). Keeps the call cheap — outcomes, not full tool output. - The per-iteration `import logging` + getLogger ran on every verdict; hoisted to one logger built once per turn before the verification loop. Kept (intentional): on a verifier exception we fail-safe to COMPLETE rather than retry — failing to INCOMPLETE would risk re-introducing the very over-continuation this fix removes, and a transient stop is user-recoverable. Co-Authored-By: Claude Opus 4.8 (1M context) --- anton/core/session.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index 5b29f17d..9791b08e 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -1823,11 +1823,17 @@ async def _stream_and_handle_tools( # task isn't actually done yet. continuation = 0 _max_rounds_hit = False + import logging as _logging + _verifier_log = _logging.getLogger(__name__) while True: # Completion verification loop tool_round = 0 error_streak: dict[str, int] = {} resilience_nudged: set[str] = set() + # Compact per-tool outcome log for this iteration, so the completion + # verifier can cross-check the assistant's claims against what tools + # actually did without receiving the full transcript (ENG-716). + tool_outcomes: list[str] = [] while llm_response.tool_calls: tool_round += 1 @@ -2090,6 +2096,7 @@ async def _stream_and_handle_tools( severity=1, round_idx=tool_round, ) + tool_outcomes.append(f"{tc.name}=ok") tool_results.append( { "type": "tool_result", @@ -2132,6 +2139,7 @@ async def _stream_and_handle_tools( severity=5 if _failed else 1, round_idx=tool_round, ) + tool_outcomes.append(f"{tc.name}={'error' if _failed else 'ok'}") tool_results.append( { "type": "tool_result", @@ -2263,6 +2271,7 @@ async def _stream_and_handle_tools( # needs for the verdict, keeps the call cheap, and — being plain text # — avoids any tool_use/tool_result pairing constraints (ENG-716). request_text = (user_message or "").strip() or "(see the assistant's message)" + tools_summary = ", ".join(tool_outcomes) if tool_outcomes else "none" verify_messages = [ {"role": "user", "content": f"USER'S REQUEST:\n{request_text}"}, { @@ -2272,8 +2281,11 @@ async def _stream_and_handle_tools( { "role": "user", "content": ( - "Based on the assistant's latest message above, which status " - "applies to the user's request?" + f"TOOLS RUN THIS TURN (name=outcome): {tools_summary}\n\n" + "Based on the assistant's latest message and the tool outcomes " + "above, which status applies to the user's request? If a tool the " + "task depended on returned an error but the assistant implied " + "success, that is INCOMPLETE or STUCK — not COMPLETE." ), }, ] @@ -2297,8 +2309,7 @@ async def _stream_and_handle_tools( # than forcing a continuation the user never asked for. status, reason = "COMPLETE", "verifier unavailable" - import logging as _logging - _logging.getLogger(__name__).info( + _verifier_log.info( "completion-verifier verdict=%s continuation=%d/%d tool_rounds=%d reason=%s", status, continuation, self._max_continuations, tool_round, reason, ) From dbdcdf426d7ece597966b52fe34569dd332d424e Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 15:58:59 -0700 Subject: [PATCH 17/21] test(e2e): update stub for structured verifier + cover WAITING verdict (ENG-716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completion verifier now runs via generate_object_code (forced tool_choice on _VerifierVerdict), so the e2e stub's free-text "STATUS:" verification responses no longer matched — test_continuation_limit_respected regressed. - Stub: queue_verification_ok/incomplete/stuck now emit a structured _VerifierVerdict tool call; add queue_verification_waiting; teach _send_json to serialize tool_calls in non-streaming responses. - New test_waiting_verdict_stops_without_continuation: a tool-using turn that ends with a question + a WAITING verdict must stop with no "Continue working" injection, and the verifier must receive the tool-outcome cross-check log. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/e2e/scenarios/test_loop_safety.py | 27 +++++++++++++ tests/e2e/stub_server.py | 51 +++++++++++++++++-------- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/tests/e2e/scenarios/test_loop_safety.py b/tests/e2e/scenarios/test_loop_safety.py index 25fdf5a6..b6b69352 100644 --- a/tests/e2e/scenarios/test_loop_safety.py +++ b/tests/e2e/scenarios/test_loop_safety.py @@ -50,6 +50,33 @@ def test_continuation_limit_respected(cfg, stub, tmp_path): ), f"Budget-exhausted message not found. Request count: {stub.request_count}" +@pytest.mark.stub_only +def test_waiting_verdict_stops_without_continuation(cfg, stub, tmp_path): + # ENG-716: a tool-using turn that ends by asking the user a question must + # STOP when the verifier returns WAITING — not inject "Continue working" + # and answer its own question. Also asserts the tool-outcome cross-check + # actually reaches the verifier. + stub.queue_tool_call("scratchpad", {"action": "exec", "name": "c", "code": "print(1)"}) + stub.queue_text("Which format would you like — PDF or HTML? WAITING_ON_USER") + stub.queue_verification_waiting("assistant asked the user a question it needs answered") + result = run_anton(["--folder", str(tmp_path)], ["make me a report", "exit"], + env=base_env(stub), timeout=cfg.timeout(30)) + + assert_exit_ok(result) + assert_not_output(result, "Traceback (most recent call last)") + assert_output(result, "WAITING_ON_USER") + # WAITING is a valid stop: no continuation injection may be sent. + assert not any( + "Continue working on the original request" in json.dumps(r.get("messages", [])) + for r in stub.requests + ), "WAITING verdict must not trigger a continuation injection" + # The verifier must receive the compact per-tool outcome log (cross-check). + assert any( + "TOOLS RUN THIS TURN" in json.dumps(r.get("messages", [])) + for r in stub.requests + ), "verifier did not receive the tool-outcome summary" + + def test_session_exits_within_timeout(cfg, stub, tmp_path): stub.queue_text("Quick reply. QUICK_EXIT") stub.queue_verification_ok() diff --git a/tests/e2e/stub_server.py b/tests/e2e/stub_server.py index 6f812745..c8c20125 100644 --- a/tests/e2e/stub_server.py +++ b/tests/e2e/stub_server.py @@ -67,24 +67,35 @@ def queue_tool_call(self, name: str, arguments: dict) -> "StubServer": }])) return self - def queue_verification_ok(self) -> "StubServer": - """Queue a non-streaming 'STATUS: COMPLETE' verification response.""" - self._queue.put( - _Response(content="STATUS: COMPLETE — task is done.", force_streaming=False) - ) + def _queue_verdict(self, status: str, reason: str) -> "StubServer": + """Queue a non-streaming structured verifier verdict. + + The completion verifier now runs via ``generate_object_code`` with a + forced tool_choice on the ``_VerifierVerdict`` schema, so the stub must + answer with a tool call (status + reason), not free text (ENG-716). + """ + self._queue.put(_Response( + tool_calls=[{ + "id": f"call_{uuid.uuid4().hex[:8]}", + "name": "_VerifierVerdict", + "arguments": {"status": status, "reason": reason}, + }], + force_streaming=False, + )) return self + def queue_verification_ok(self) -> "StubServer": + """Queue a COMPLETE verifier verdict.""" + return self._queue_verdict("COMPLETE", "task is done.") + def queue_verification_incomplete(self, reason: str = "still more to do") -> "StubServer": - self._queue.put( - _Response(content=f"STATUS: INCOMPLETE — {reason}", force_streaming=False) - ) - return self + return self._queue_verdict("INCOMPLETE", reason) + + def queue_verification_waiting(self, reason: str = "waiting on the user") -> "StubServer": + return self._queue_verdict("WAITING", reason) def queue_verification_stuck(self, reason: str = "blocked") -> "StubServer": - self._queue.put( - _Response(content=f"STATUS: STUCK — {reason}", force_streaming=False) - ) - return self + return self._queue_verdict("STUCK", reason) def queue_summary(self, text: str = "Summary of earlier turns.") -> "StubServer": """Queue a response for _summarize_history's coding model call.""" @@ -212,6 +223,16 @@ def _send_sse(handler: BaseHTTPRequestHandler, resp: _Response) -> None: def _send_json(handler: BaseHTTPRequestHandler, resp: _Response) -> None: + message: dict = {"role": "assistant", "content": resp.content or None} + finish_reason = "stop" + if resp.tool_calls: + tc = resp.tool_calls[0] + message["tool_calls"] = [{ + "id": tc["id"], + "type": "function", + "function": {"name": tc["name"], "arguments": json.dumps(tc["arguments"])}, + }] + finish_reason = "tool_calls" data = { "id": f"chatcmpl-{uuid.uuid4().hex[:8]}", "object": "chat.completion", @@ -219,8 +240,8 @@ def _send_json(handler: BaseHTTPRequestHandler, resp: _Response) -> None: "model": "gpt-test", "choices": [{ "index": 0, - "message": {"role": "assistant", "content": resp.content}, - "finish_reason": "stop", + "message": message, + "finish_reason": finish_reason, }], "usage": {"prompt_tokens": 10, "completion_tokens": 10, "total_tokens": 20}, } From 2e44a3fe813b60953841879a9e46edd253103328 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 16:25:10 -0700 Subject: [PATCH 18/21] fix(session): evidence-complete verifier view (PR #255 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both Major findings from code review: the reduced verify view dropped evidence needed to judge completion reliably. - Referential context: the verifier saw only the latest raw user message, so follow-ups like "now do the same for the other file" lost their task context. Now render a compact text view of the recent conversation (prior turns included). - Tool evidence: `name=ok/error` couldn't cross-check a *successful* tool claim (e.g. a fetch that returns 200 but empty). Now include truncated tool-result *content*, so a claimed-successful tool can be verified against what it actually returned. New `_render_verify_transcript` renders history as plain text (context + tool results, each truncated; internal SYSTEM injections omitted) — evidence- complete but far smaller than the raw transcript, and free of tool_use/tool_result pairing constraints. Drops the now-redundant per-tool ok/error tracking. Adds tests/test_verify_transcript.py covering context retention, tool-result evidence, SYSTEM omission, and truncation; updates the e2e WAITING test to assert tool-result evidence reaches the verifier. Co-Authored-By: Claude Opus 4.8 (1M context) --- anton/core/session.py | 79 ++++++++++++++++++------- tests/e2e/scenarios/test_loop_safety.py | 7 ++- tests/test_verify_transcript.py | 67 +++++++++++++++++++++ 3 files changed, 127 insertions(+), 26 deletions(-) create mode 100644 tests/test_verify_transcript.py diff --git a/anton/core/session.py b/anton/core/session.py index 9791b08e..aab4c567 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -145,6 +145,49 @@ class _VerifierVerdict(BaseModel): reason: str = Field(description="One brief sentence explaining the verdict.") +def _render_verify_transcript( + history: list[dict], + *, + max_messages: int = 16, + tool_cap: int = 400, + text_cap: int = 2000, +) -> str: + """Render a compact, text-only view of the recent conversation for the + completion verifier. + + Gives the verifier enough to judge reliably — prior turns (so referential + follow-ups keep their task context) and truncated tool-result *content* (so + a claimed-successful tool can be cross-checked, not just its ok/error flag) + — without shipping the full raw transcript or tripping the provider's + tool_use/tool_result pairing constraints (ENG-716). Internal ``SYSTEM:`` + injections are omitted; they are directives, not conversation. + """ + lines: list[str] = [] + for msg in history[-max_messages:]: + role = msg.get("role") + content = msg.get("content") + if isinstance(content, str): + text = content.strip() + if not text or (role == "user" and text.startswith("SYSTEM:")): + continue + speaker = "USER" if role == "user" else "ASSISTANT" + lines.append(f"{speaker}: {text[:text_cap]}") + elif isinstance(content, list): + for block in content: + if not isinstance(block, dict): + continue + btype = block.get("type") + if btype == "text" and (block.get("text") or "").strip(): + lines.append(f"ASSISTANT: {block['text'][:text_cap]}") + elif btype == "tool_use": + lines.append(f"ASSISTANT called tool: {block.get('name')}") + elif btype == "tool_result": + c = block.get("content") + text = c if isinstance(c, str) else json.dumps(c) + lines.append(f"TOOL RESULT: {text[:tool_cap]}") + return "\n".join(lines) or "(no conversation)" + + @dataclass class ChatSessionConfig: """All construction parameters for a ChatSession. @@ -1830,10 +1873,6 @@ async def _stream_and_handle_tools( tool_round = 0 error_streak: dict[str, int] = {} resilience_nudged: set[str] = set() - # Compact per-tool outcome log for this iteration, so the completion - # verifier can cross-check the assistant's claims against what tools - # actually did without receiving the full transcript (ENG-716). - tool_outcomes: list[str] = [] while llm_response.tool_calls: tool_round += 1 @@ -2096,7 +2135,6 @@ async def _stream_and_handle_tools( severity=1, round_idx=tool_round, ) - tool_outcomes.append(f"{tc.name}=ok") tool_results.append( { "type": "tool_result", @@ -2139,7 +2177,6 @@ async def _stream_and_handle_tools( severity=5 if _failed else 1, round_idx=tool_round, ) - tool_outcomes.append(f"{tc.name}={'error' if _failed else 'ok'}") tool_results.append( { "type": "tool_result", @@ -2265,27 +2302,23 @@ async def _stream_and_handle_tools( # Consolidation still runs after diagnosis break - # Ask the cheap coding model to self-assess completion, using a - # compact text-only view (the user's request + the assistant's latest - # message) rather than the full transcript. That is all the verifier - # needs for the verdict, keeps the call cheap, and — being plain text - # — avoids any tool_use/tool_result pairing constraints (ENG-716). - request_text = (user_message or "").strip() or "(see the assistant's message)" - tools_summary = ", ".join(tool_outcomes) if tool_outcomes else "none" + # Ask the cheap coding model to self-assess completion over a compact, + # text-rendered view of the recent conversation: enough context for + # referential follow-ups plus truncated tool-result evidence to + # cross-check success claims, but far smaller than the raw transcript + # and free of tool_use/tool_result pairing constraints (ENG-716). The + # assistant's latest reply is already in history (appended above). + transcript = _render_verify_transcript(self._history) verify_messages = [ - {"role": "user", "content": f"USER'S REQUEST:\n{request_text}"}, - { - "role": "assistant", - "content": reply.strip() or "(the assistant produced no final text)", - }, { "role": "user", "content": ( - f"TOOLS RUN THIS TURN (name=outcome): {tools_summary}\n\n" - "Based on the assistant's latest message and the tool outcomes " - "above, which status applies to the user's request? If a tool the " - "task depended on returned an error but the assistant implied " - "success, that is INCOMPLETE or STUCK — not COMPLETE." + "Assess the conversation below (tool results are truncated) and " + "decide the status of the USER's most recent request.\n\n" + f"{transcript}\n\n" + "Which status applies? A tool the task depended on that returned " + "an error — or returned no usable data while the assistant implied " + "success — means INCOMPLETE or STUCK, not COMPLETE." ), }, ] diff --git a/tests/e2e/scenarios/test_loop_safety.py b/tests/e2e/scenarios/test_loop_safety.py index b6b69352..47d8e671 100644 --- a/tests/e2e/scenarios/test_loop_safety.py +++ b/tests/e2e/scenarios/test_loop_safety.py @@ -70,11 +70,12 @@ def test_waiting_verdict_stops_without_continuation(cfg, stub, tmp_path): "Continue working on the original request" in json.dumps(r.get("messages", [])) for r in stub.requests ), "WAITING verdict must not trigger a continuation injection" - # The verifier must receive the compact per-tool outcome log (cross-check). + # The verifier must receive truncated tool-result evidence (not just a flag), + # so it can cross-check claimed success against what the tool actually did. assert any( - "TOOLS RUN THIS TURN" in json.dumps(r.get("messages", [])) + "TOOL RESULT:" in json.dumps(r.get("messages", [])) for r in stub.requests - ), "verifier did not receive the tool-outcome summary" + ), "verifier did not receive tool-result evidence" def test_session_exits_within_timeout(cfg, stub, tmp_path): diff --git a/tests/test_verify_transcript.py b/tests/test_verify_transcript.py new file mode 100644 index 00000000..e894b1db --- /dev/null +++ b/tests/test_verify_transcript.py @@ -0,0 +1,67 @@ +"""Unit tests for the completion-verifier's compact transcript view (ENG-716). + +Guards the two review findings on PR #255: the verifier view must retain +referential task context from prior turns AND carry truncated tool-result +evidence (not just an ok/error flag), while omitting internal SYSTEM +injections. +""" + +from __future__ import annotations + +from anton.core.session import _render_verify_transcript + + +def test_keeps_prior_turn_context_for_referential_followups(): + history = [ + {"role": "user", "content": "clean up report_q1.csv"}, + {"role": "assistant", "content": "Done — deduped 12 rows."}, + {"role": "user", "content": "now do the same for the other file"}, + {"role": "assistant", "content": "Which file did you mean?"}, + ] + out = _render_verify_transcript(history) + # The current request is referential ("the same", "the other file"); the + # prior turn must remain so the verifier can resolve it. + assert "clean up report_q1.csv" in out + assert "now do the same for the other file" in out + + +def test_includes_truncated_tool_result_evidence(): + history = [ + {"role": "user", "content": "how many open PRs?"}, + {"role": "assistant", "content": [{"type": "tool_use", "name": "scratchpad"}]}, + {"role": "user", "content": [{"type": "tool_result", "content": "[error]\nHTTP 403 Forbidden"}]}, + {"role": "assistant", "content": "There are 42 open PRs."}, + ] + out = _render_verify_transcript(history) + # The tool actually errored while the assistant claimed a number — the + # verifier must see the error content to catch the mismatch. + assert "TOOL RESULT: [error]" in out + assert "HTTP 403 Forbidden" in out + assert "There are 42 open PRs." in out + + +def test_omits_internal_system_injections(): + history = [ + {"role": "user", "content": "build the dashboard"}, + {"role": "user", "content": "SYSTEM: Task verification determined this task is not yet complete. Continue working."}, + {"role": "assistant", "content": "Working on it."}, + ] + out = _render_verify_transcript(history) + assert "SYSTEM:" not in out + assert "Continue working" not in out + assert "build the dashboard" in out + + +def test_truncates_large_tool_output(): + big = "x" * 5000 + history = [ + {"role": "user", "content": "run it"}, + {"role": "user", "content": [{"type": "tool_result", "content": big}]}, + ] + out = _render_verify_transcript(history, tool_cap=400) + # Tool output is capped so the verify call stays cheap. + assert out.count("x") <= 400 + + +def test_empty_history_is_safe(): + assert _render_verify_transcript([]) == "(no conversation)" From 4905a261a2588620ee426960328e183908e3bf04 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 16:31:46 -0700 Subject: [PATCH 19/21] fix(session): always state the current request to the verifier (self-review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing the evidence-complete view surfaced an edge case: the transcript is the last 16 messages, and a long tool-heavy turn (~7+ tool rounds = 2 messages each) can push the turn's opening user request out of that window — leaving the verifier to judge completion without knowing what was asked. Prepend the current request explicitly so it is always present regardless of turn length. The e2e WAITING test now asserts the request header reaches the verifier. Co-Authored-By: Claude Opus 4.8 (1M context) --- anton/core/session.py | 7 ++++++- tests/e2e/scenarios/test_loop_safety.py | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/anton/core/session.py b/anton/core/session.py index aab4c567..46c0f189 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -2309,13 +2309,18 @@ async def _stream_and_handle_tools( # and free of tool_use/tool_result pairing constraints (ENG-716). The # assistant's latest reply is already in history (appended above). transcript = _render_verify_transcript(self._history) + # Always state the current request explicitly: a long tool-heavy turn + # can push the turn's opening user message out of the transcript window, + # and the request is the anchor for the whole judgment (ENG-716). + request = (user_message or "").strip() + request_header = f"USER'S CURRENT REQUEST: {request}\n\n" if request else "" verify_messages = [ { "role": "user", "content": ( "Assess the conversation below (tool results are truncated) and " "decide the status of the USER's most recent request.\n\n" - f"{transcript}\n\n" + f"{request_header}{transcript}\n\n" "Which status applies? A tool the task depended on that returned " "an error — or returned no usable data while the assistant implied " "success — means INCOMPLETE or STUCK, not COMPLETE." diff --git a/tests/e2e/scenarios/test_loop_safety.py b/tests/e2e/scenarios/test_loop_safety.py index 47d8e671..3f83dcfe 100644 --- a/tests/e2e/scenarios/test_loop_safety.py +++ b/tests/e2e/scenarios/test_loop_safety.py @@ -76,6 +76,12 @@ def test_waiting_verdict_stops_without_continuation(cfg, stub, tmp_path): "TOOL RESULT:" in json.dumps(r.get("messages", [])) for r in stub.requests ), "verifier did not receive tool-result evidence" + # ...and the current request is always stated, even if a long turn evicts it + # from the transcript window. + assert any( + "USER'S CURRENT REQUEST" in json.dumps(r.get("messages", [])) + for r in stub.requests + ), "verifier did not receive the current request header" def test_session_exits_within_timeout(cfg, stub, tmp_path): From 959c76a9716cf8011992cf80bace17fdceb5fe3d Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 16:44:21 -0700 Subject: [PATCH 20/21] fix(session): correct verifier transcript roles, context budget, base64 (PR #255 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the follow-up review findings on _render_verify_transcript: - Multimodal role labeling (Major): list-based *user* content (image/file inputs) had its text blocks rendered as ASSISTANT. Speaker is now derived from the message role, so multimodal user requests/context are labelled USER. - Referential antecedent after long turns (Major): a flat last-16 window let a long tool loop evict prior conversational turns (and SYSTEM messages consumed slots before filtering). Now budget the conversation thread and tool activity separately (max_convo / max_tool), filtering SYSTEM before budgeting, so tool volume can't crowd out the turns needed to resolve referential requests. - Multimodal tool_result base64 (Minor): json.dumps materialized ~5MB of base64 then kept 400 chars of noise. New _render_tool_result_content renders block-by-block — bounded text kept, images as [image] placeholder, never raw encoded data. Adds unit coverage: >8-round tool turn preserves request + antecedent, multimodal user text labelled USER, multimodal tool_result keeps the text summary and drops base64, and SYSTEM injections don't consume the budget. Co-Authored-By: Claude Opus 4.8 (1M context) --- anton/core/session.py | 72 ++++++++++++++++++++++++--------- tests/test_verify_transcript.py | 62 ++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 18 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index 46c0f189..b7d7d476 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -145,47 +145,83 @@ class _VerifierVerdict(BaseModel): reason: str = Field(description="One brief sentence explaining the verdict.") +def _render_tool_result_content(content, cap: int) -> str: + """Render a tool_result's content as bounded plain text. + + Never serializes raw payloads: a multimodal result (e.g. read_image) can + carry megabytes of base64, so we keep only text blocks and mark images with + a placeholder rather than ``json.dumps``-ing the whole thing (ENG-716). + """ + if isinstance(content, str): + return content[:cap] or "(empty result)" + if isinstance(content, list): + parts: list[str] = [] + for block in content: + if not isinstance(block, dict): + continue + if block.get("type") == "text": + parts.append((block.get("text") or "").strip()) + elif block.get("type") in ("image", "image_url"): + parts.append("[image]") + return (" ".join(p for p in parts if p)[:cap]) or "[non-text result]" + return str(content)[:cap] + + def _render_verify_transcript( history: list[dict], *, - max_messages: int = 16, + max_convo: int = 10, + max_tool: int = 12, tool_cap: int = 400, text_cap: int = 2000, ) -> str: """Render a compact, text-only view of the recent conversation for the completion verifier. - Gives the verifier enough to judge reliably — prior turns (so referential - follow-ups keep their task context) and truncated tool-result *content* (so - a claimed-successful tool can be cross-checked, not just its ok/error flag) - — without shipping the full raw transcript or tripping the provider's - tool_use/tool_result pairing constraints (ENG-716). Internal ``SYSTEM:`` - injections are omitted; they are directives, not conversation. + Budgets the conversational thread and the tool activity *separately* so a + voluminous tool loop can't crowd out the user/assistant turns the verifier + needs to resolve referential requests ("do the same for the other file"): + the most recent ``max_convo`` user/assistant text turns plus the most recent + ``max_tool`` tool events, merged back into chronological order. Speaker is + taken from the message role (list-based *user* content — images/files — is + labelled USER, not ASSISTANT); multimodal blocks are rendered block-by-block + (text kept, images as a placeholder, never raw base64); internal ``SYSTEM:`` + injections are dropped before budgeting so they don't consume slots. Keeps + the call cheap and free of tool_use/tool_result pairing constraints + (ENG-716). """ - lines: list[str] = [] - for msg in history[-max_messages:]: + convo: list[tuple[int, str]] = [] # (orig_index, line) — user/assistant text + tools: list[tuple[int, str]] = [] # (orig_index, line) — tool activity + + for i, msg in enumerate(history): role = msg.get("role") content = msg.get("content") + speaker = "USER" if role == "user" else "ASSISTANT" if isinstance(content, str): text = content.strip() if not text or (role == "user" and text.startswith("SYSTEM:")): continue - speaker = "USER" if role == "user" else "ASSISTANT" - lines.append(f"{speaker}: {text[:text_cap]}") + convo.append((i, f"{speaker}: {text[:text_cap]}")) elif isinstance(content, list): for block in content: if not isinstance(block, dict): continue btype = block.get("type") - if btype == "text" and (block.get("text") or "").strip(): - lines.append(f"ASSISTANT: {block['text'][:text_cap]}") + if btype == "text": + text = (block.get("text") or "").strip() + if text: + convo.append((i, f"{speaker}: {text[:text_cap]}")) + elif btype in ("image", "image_url"): + convo.append((i, f"{speaker}: [image]")) elif btype == "tool_use": - lines.append(f"ASSISTANT called tool: {block.get('name')}") + tools.append((i, f"ASSISTANT called tool: {block.get('name')}")) elif btype == "tool_result": - c = block.get("content") - text = c if isinstance(c, str) else json.dumps(c) - lines.append(f"TOOL RESULT: {text[:tool_cap]}") - return "\n".join(lines) or "(no conversation)" + rendered = _render_tool_result_content(block.get("content"), tool_cap) + tools.append((i, f"TOOL RESULT: {rendered}")) + + kept = convo[-max_convo:] + tools[-max_tool:] + kept.sort(key=lambda entry: entry[0]) + return "\n".join(line for _, line in kept) or "(no conversation)" @dataclass diff --git a/tests/test_verify_transcript.py b/tests/test_verify_transcript.py index e894b1db..f16f53aa 100644 --- a/tests/test_verify_transcript.py +++ b/tests/test_verify_transcript.py @@ -65,3 +65,65 @@ def test_truncates_large_tool_output(): def test_empty_history_is_safe(): assert _render_verify_transcript([]) == "(no conversation)" + + +def test_long_tool_turn_keeps_request_and_antecedent(): + # A prior turn establishes context; the current turn is referential and runs + # >8 tool rounds. Tool activity must NOT evict the conversational thread. + history = [ + {"role": "user", "content": "clean up report_q1.csv"}, + {"role": "assistant", "content": "Done — deduped 12 rows in report_q1.csv."}, + {"role": "user", "content": "now do the same for the other file"}, + ] + for i in range(10): # 10 tool rounds = 20 tool messages + history.append({"role": "assistant", "content": [{"type": "tool_use", "name": "scratchpad"}]}) + history.append({"role": "user", "content": [{"type": "tool_result", "content": f"row {i} cleaned"}]}) + history.append({"role": "assistant", "content": "All cleaned."}) + + out = _render_verify_transcript(history) + # Both the referential request and its antecedent survive the tool volume. + assert "now do the same for the other file" in out + assert "clean up report_q1.csv" in out + # And tool evidence is still present. + assert "TOOL RESULT:" in out + + +def test_multimodal_user_text_labeled_user_not_assistant(): + history = [ + {"role": "user", "content": [ + {"type": "text", "text": "describe this image and save a report"}, + {"type": "image", "source": {"type": "base64", "data": "A" * 500}}, + ]}, + {"role": "assistant", "content": "Here's the description."}, + ] + out = _render_verify_transcript(history) + assert "USER: describe this image and save a report" in out + assert "ASSISTANT: describe this image" not in out + assert "USER: [image]" in out + assert "A" * 100 not in out # no base64 leaked + + +def test_multimodal_tool_result_keeps_summary_drops_base64(): + history = [ + {"role": "user", "content": "read the chart"}, + {"role": "user", "content": [{"type": "tool_result", "content": [ + {"type": "image", "source": {"type": "base64", "data": "Z" * 5000}}, + {"type": "text", "text": "Chart saved: 12 monthly bars"}, + ]}]}, + ] + out = _render_verify_transcript(history) + assert "Chart saved: 12 monthly bars" in out + assert "[image]" in out + assert "Z" * 100 not in out # base64 payload never serialized into the view + + +def test_system_injections_do_not_consume_budget(): + # SYSTEM injections are dropped before budgeting, so real turns aren't evicted + # by internal noise. + history = [] + for i in range(8): + history.append({"role": "user", "content": f"SYSTEM: internal note {i}"}) + history.append({"role": "user", "content": "the actual request"}) + out = _render_verify_transcript(history, max_convo=3) + assert "the actual request" in out + assert "internal note" not in out From bcd091e9bd6ff5d6b4def792552a4597c219b994 Mon Sep 17 00:00:00 2001 From: Alejandro Cantu Date: Thu, 16 Jul 2026 17:00:32 -0700 Subject: [PATCH 21/21] fix(session): tool-call preamble text is tool activity, not conversation (PR #255 review) Follow-up review found the conversation budget could still be evicted: assistant text emitted alongside a tool call ("Processing step 4" + tool_use, a normal response shape) was counted as a conversational turn, so a long tool loop's preambles filled max_convo and dropped the referential request and its antecedent. Route assistant text from any message that also contains a tool_use into the tool budget, reserving the conversation budget for real user turns and standalone replies. The long-turn regression test now emits preamble text on every round (proven to evict request + antecedent without this fix). Co-Authored-By: Claude Opus 4.8 (1M context) --- anton/core/session.py | 13 +++++++++++-- tests/test_verify_transcript.py | 11 ++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/anton/core/session.py b/anton/core/session.py index b7d7d476..adbe78c8 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -203,14 +203,23 @@ def _render_verify_transcript( continue convo.append((i, f"{speaker}: {text[:text_cap]}")) elif isinstance(content, list): + # Assistant text emitted alongside a tool call is preamble/narration + # ("Processing step 4"), not a conversational turn — route it to the + # tool budget so a long tool loop can't evict real requests/replies + # from the conversation budget. + preamble = role == "assistant" and any( + isinstance(b, dict) and b.get("type") == "tool_use" for b in content + ) for block in content: if not isinstance(block, dict): continue btype = block.get("type") if btype == "text": text = (block.get("text") or "").strip() - if text: - convo.append((i, f"{speaker}: {text[:text_cap]}")) + if not text: + continue + bucket = tools if preamble else convo + bucket.append((i, f"{speaker}: {text[:text_cap]}")) elif btype in ("image", "image_url"): convo.append((i, f"{speaker}: [image]")) elif btype == "tool_use": diff --git a/tests/test_verify_transcript.py b/tests/test_verify_transcript.py index f16f53aa..1d7ba6a9 100644 --- a/tests/test_verify_transcript.py +++ b/tests/test_verify_transcript.py @@ -75,15 +75,20 @@ def test_long_tool_turn_keeps_request_and_antecedent(): {"role": "assistant", "content": "Done — deduped 12 rows in report_q1.csv."}, {"role": "user", "content": "now do the same for the other file"}, ] - for i in range(10): # 10 tool rounds = 20 tool messages - history.append({"role": "assistant", "content": [{"type": "tool_use", "name": "scratchpad"}]}) + for i in range(10): # 10 tool rounds, each with preamble text + a tool call + history.append({"role": "assistant", "content": [ + {"type": "text", "text": f"Processing step {i}"}, + {"type": "tool_use", "name": "scratchpad"}, + ]}) history.append({"role": "user", "content": [{"type": "tool_result", "content": f"row {i} cleaned"}]}) history.append({"role": "assistant", "content": "All cleaned."}) out = _render_verify_transcript(history) - # Both the referential request and its antecedent survive the tool volume. + # Both the referential request and its antecedent survive the tool volume — + # preamble text ("Processing step N") must not consume the conversation budget. assert "now do the same for the other file" in out assert "clean up report_q1.csv" in out + assert "Done — deduped 12 rows in report_q1.csv." in out # antecedent's answer # And tool evidence is still present. assert "TOOL RESULT:" in out