From b0f6ebb50a114356ac47f2d9374132066a892cd9 Mon Sep 17 00:00:00 2001 From: Yuanwen Tian Date: Mon, 10 Aug 2026 14:05:42 +0800 Subject: [PATCH 1/5] feat: post-parse handoff to the build-schema agent skill (ADR-0010) The workflow was parse -> hand-author a schema -> extract. The middle step is now offered: after a parse completes on a real terminal, ade asks whether to build an extraction schema with an AI-agent session running the build-schema skill (landingai-ade plugin), collects the extraction intent plus optional baseline schema / ground truth, launches the agent seeded with the parsed job item, and on return names the delivered deliverable/schema.json and offers to run the extract. Also exposed standalone as `ade schema build JOB_ITEM_ID` so any stored parse can reach the same flow. Agents are adapters, not one agent: claude and codex are detected on PATH, a menu choice is remembered as agent.default in config.json, and a custom agent.command entry launches anything else. ade stays network-free -- non-native agents are told to use a local copy of the skill or clone the marketplace repo themselves; ade never manages skill versions. A missing agent CLI fails legibly with install instructions (non-fatally on the offer path, where the parse already succeeded). Gating keeps the machine contract byte-identical: the offer never fires under --json/--id-only, on any non-tty stream, inside an agent host or CI, or with schema_prompt: false. stdout stays payload-only; every prompt rides stderr. Subprocess launch and PATH lookup are injected ports (run_agent, which), so the suite drives the whole handoff offline -- no process is ever spawned in tests. Co-Authored-By: Claude Fable 5 --- README.md | 17 + SKILL.md | 10 + docs/adr/0010-post-parse-agent-handoff.md | 75 ++++ docs/reference/help.json | 96 ++++- src/ade_cli/agents.py | 166 ++++++++ src/ade_cli/config.py | 18 +- src/ade_cli/help.py | 30 +- src/ade_cli/main.py | 2 + src/ade_cli/parse.py | 14 +- src/ade_cli/ports.py | 20 +- src/ade_cli/schema_build.py | 478 ++++++++++++++++++++++ src/ade_cli/update.py | 9 + src/ade_cli/view.py | 15 +- tests/conftest.py | 37 ++ tests/test_parse.py | 128 ++++++ tests/test_schema_build.py | 304 ++++++++++++++ tests/test_telemetry.py | 1 + tests/test_update.py | 15 + 18 files changed, 1415 insertions(+), 20 deletions(-) create mode 100644 docs/adr/0010-post-parse-agent-handoff.md create mode 100644 src/ade_cli/agents.py create mode 100644 src/ade_cli/schema_build.py create mode 100644 tests/test_schema_build.py diff --git a/README.md b/README.md index 06095f2..dd1d76a 100644 --- a/README.md +++ b/README.md @@ -165,6 +165,23 @@ JOB=$(ade parse -d doc.pdf --wait 0 --id-only) # submit and return, exit 3 ade parse -d doc.pdf --id-only # re-run to resume; same id ``` +## Build a schema + +No schema yet? `ade schema build JOB_ITEM_ID` hands a parsed document to +an AI-agent session running the +[build-schema skill](https://github.com/landing-ai/claude-skills), which +authors and validates the schema through real extractions before +delivering it (an interactive parse also offers this step when it +finishes). You describe what to extract; the session negotiates accuracy +targets with you and iterates until the schema converges or is legibly +blocked. It needs an agent CLI on your PATH — Claude Code (`claude`, +ideally with the `landingai-ade` plugin) or Codex (`codex`); any other +agent can be declared in `~/.ade/config.json` as +`{"agent": {"name": "...", "command": ["..."]}}`. The delivered schema +lands in the named workspace as `deliverable/schema.json`, ready for +`ade extract`; set `{"schema_prompt": false}` to silence the post-parse +offer. + ## Extract | command | what it does | diff --git a/SKILL.md b/SKILL.md index c978264..e926bd8 100644 --- a/SKILL.md +++ b/SKILL.md @@ -124,6 +124,16 @@ Prefer the explicit two-step (`parse -d`, then `extract JOB_ITEM_ID`) when you want the same parse to feed several schemas — the id makes the reuse visible. +## Schema authoring is not for agents driving this CLI + +`ade schema build` (and the offer a human sees after an interactive +parse) launches a *foreground AI-agent session* running the build-schema +skill — it requires a real terminal on every stream and never fires +under `--json`, `--id-only`, or inside an agent host or CI. If you are +an agent and the user needs a schema authored, invoke the **build-schema +skill** (the `landingai-ade` plugin of landing-ai/claude-skills) +directly rather than shelling out to `ade schema build`. + ## Pending and resume Wait expiry is a normal outcome, not an error. If the poll budget diff --git a/docs/adr/0010-post-parse-agent-handoff.md b/docs/adr/0010-post-parse-agent-handoff.md new file mode 100644 index 0000000..bc40f66 --- /dev/null +++ b/docs/adr/0010-post-parse-agent-handoff.md @@ -0,0 +1,75 @@ +# The schema gap is bridged by handing off to an agent CLI, not by ade authoring schemas + +## Context + +The workflow is parse → schema → extract, and the middle step was entirely +the user's: hand-author a JSON Schema, then pass it to `extract --schema`. +The build-schema skill (the `landingai-ade` plugin of +[landing-ai/claude-skills](https://github.com/landing-ai/claude-skills), +mirrored from `schema-loop` where it is benchmark-gated) already automates +that step properly — an AI-agent session drives this very CLI through +measured iteration (real extractions, reviewer-built ground truth, +converged-or-blocked stops) and delivers the final schema at +`/deliverable/schema.json`. The CLI and the skill each knew +nothing of the other, so the natural flow — parse, build the schema, +extract — required the user to know the skill exists and wire it up +themselves. + +## Decision + +ade bridges to the skill by **launching an agent CLI as a foreground +subprocess**, in two entry points sharing one body (`schema_build.py`): + +- **`ade schema build JOB_ITEM_ID`** — the standalone command, usable on + any stored parse. +- **A post-parse offer** — after a parse summary (fresh or cached hit) on + a real terminal, an arrow-key menu (ADR-0002's pattern) asks whether to + build a schema now. **Bare Enter exits**: the default preserves the old + behavior keystroke-for-keystroke, and the summary's `next:` line remains + the machine-mode teaching. + +ade collects only what it already knows plus the extraction **intent** +(and optional existing-schema / ground-truth files); per-field accuracy +targets are deliberately *not* collected — the skill's own intake +negotiates them in conversation. The seeded prompt carries the full parse +job item id, source, workspace (`/schema-runs/-/` +by default), and skill-acquisition instructions. When the session ends, +ade looks for `deliverable/schema.json`, names it, and offers to run the +extract — a re-exec of this CLI (`update.reexec_argv`, shared with the +viewer daemons), which is mostly a free cache hit since the skill's final +iteration already ran it. + +**Agent adapters, not one agent.** A small table (`agents.py`) knows +`claude` (Claude Code — triggers the skill natively via the plugin) and +`codex`; detection is a PATH lookup through the injected `which` port. A +menu choice is remembered as `agent.default` in `config.json`; a custom +`agent.command` entry launches any other CLI. Every adapter receives the +seeded prompt as its final positional argument. + +**ade stays network-free here.** It never fetches, pins, or updates the +skill: non-native agents are told to use a local copy or shallow-clone +the marketplace repo themselves. A missing agent CLI fails legibly with +install instructions (non-fatally on the offer path — the parse +succeeded). + +**Gating.** The offer fires only when stdin, stdout, and stderr are all +terminals, never under `--json`/`--id-only`, never inside an agent host +or CI (surface.py detection — an agent driving `ade parse` must never +meet a menu), and `schema_prompt: false` silences it. The standalone +command requires a real terminal even under `--json` (the child session +inherits the streams; a piped stdout would be corrupted by its TUI) and +then requires `--intent`, resolves the agent without a menu, and never +auto-runs the extract. + +## Consequences + +- The interactive flow gains a doorway the machine contract cannot see: + stdout stays payload-only, all prompts ride stderr, and every existing + `--json` consumer is byte-identical. +- Subprocess launch and PATH lookup are ports (`run_agent`, `which`), so + the suite tests the whole handoff offline — no real process ever runs. +- Agents driving ade should invoke the build-schema skill directly; the + root SKILL.md says so. `ade schema build` is for humans at terminals. +- The deliverable contract (`deliverable/schema.json`) is the one seam + ade trusts from the skill; if the skill's output layout ever changes, + this is the single place to follow it. diff --git a/docs/reference/help.json b/docs/reference/help.json index 2dff6a2..a5cce4c 100644 --- a/docs/reference/help.json +++ b/docs/reference/help.json @@ -889,6 +889,91 @@ }, "band": "network verbs \u2014 the ADE job contracts" }, + { + "name": "schema build", + "usage": "ade schema build JOB_ITEM_ID [options] [--json]", + "summary": "Build an extraction schema for a parsed document by handing it to\nan AI-agent session running the build-schema skill (measured\niteration: real extractions, reviewer verdicts, converged-or-blocked\nstops). Ends by naming the delivered schema and offering the extract.", + "arguments": [ + { + "name": "JOB_ITEM_ID", + "required": true, + "help": "A completed parse job item id (or unambiguous prefix)." + } + ], + "flags": [ + { + "flags": "--intent", + "metavar": "TEXT", + "required": false, + "default": null, + "help": "What to extract, in plain language (prompted interactively when omitted; required with --json)." + }, + { + "flags": "--existing-schema", + "metavar": "FILE", + "required": false, + "default": null, + "help": "A schema to improve: the skill runs it unchanged first as the regression floor." + }, + { + "flags": "--ground-truth", + "metavar": "FILE", + "required": false, + "default": null, + "help": "Golden ground truth seeding the skill's eval set (JSON keyed by document filename, or flat CSV)." + }, + { + "flags": "--workspace", + "metavar": "PATH", + "required": false, + "default": null, + "help": "Skill workspace directory (default: /schema-runs/-/; an existing one resumes)." + }, + { + "flags": "--agent", + "metavar": "TEXT", + "required": false, + "default": null, + "help": "Agent CLI to launch (claude, codex, or a config-declared name); default: the configured or only available one." + } + ], + "supports_json": true, + "result": { + "shape": "object", + "keys": [ + { + "key": "status", + "what": "'schema_built'" + }, + { + "key": "job_item_id", + "what": "the parse item the schema was built against" + }, + { + "key": "agent", + "what": "the agent CLI that hosted the session" + }, + { + "key": "workspace", + "what": "the skill workspace (eval set, iteration log, dispositions live alongside the schema)" + }, + { + "key": "schema", + "what": "absolute path of the delivered schema.json" + }, + { + "key": "extract_hint", + "what": "the ready-to-run extract invocation" + }, + { + "key": "extracted", + "what": "always false \u2014 the extract run is offered interactively, never performed silently" + } + ], + "note": "Interactive terminals only: the command launches a foreground AI-agent session (claude, codex, or a config-declared CLI) seeded with the parse and your intent. Agents should invoke the build-schema skill directly instead of calling this." + }, + "band": "schema authoring \u2014 the agent handoff" + }, { "name": "history list", "usage": "ade history list [options] [--json]", @@ -1335,6 +1420,8 @@ "body": [ "parse \u2500\u252c\u2500> find \u2500\u2500> crop look at an element as an image", " \u251c\u2500> view grounded page images + markdown", + " \u251c\u2500> schema build author the extract schema with an", + " \u2502 AI-agent session (build-schema skill)", " \u2514\u2500> extract \u2500\u2500> view schema-shaped data, grounded", "", "1. parse ensure a document is parsed. Prints a job item id;", @@ -1344,10 +1431,15 @@ " type, page, box, text.", "3. crop render element regions as PNGs: one --element-id, or a", " batch by find's own filters (--type figure, --all).", - "4. extract ensure a schema extraction exists for a parse job item", + "4. schema build author the extract schema when you don't have", + " one: hands the parse to an interactive AI-agent", + " session running the build-schema skill (interactive", + " terminals only \u2014 agents should run the skill", + " directly instead).", + "5. extract ensure a schema extraction exists for a parse job item", " (or bring-your-own markdown). It becomes its own job", " item, referencing the parse \u2014 artifacts never copied.", - "5. view build the self-contained HTML viewer for a parse or an", + "6. view build the self-contained HTML viewer for a parse or an", " extract item; --element-id emits a deep link to one", " element \u2014 the citation contract.", "", diff --git a/src/ade_cli/agents.py b/src/ade_cli/agents.py new file mode 100644 index 0000000..027049a --- /dev/null +++ b/src/ade_cli/agents.py @@ -0,0 +1,166 @@ +"""Agent-CLI adapters for the build-schema handoff (schema_build.py). + +The build-schema skill runs inside an AI-agent session, not inside this +process — this module knows which agent CLIs can host one, how to find +them on PATH, and what seeded prompt hands the skill everything the CLI +already knows (the parsed job item, the intent, the workspace) without +collecting what the skill negotiates itself (accuracy targets). Pure +functions over injected lookups; no typer, no I/O. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +from typing import Callable, Mapping + +# The public marketplace mirror the seeded prompt names for agents that +# cannot trigger the skill natively; ade never fetches it itself. +SKILLS_REPO_URL = "https://github.com/landing-ai/claude-skills" +SKILL_PATH = "plugins/landingai-ade/skills/build-schema/SKILL.md" +# The skill's output contract: the final schema, byte-identical to the +# version its last iteration actually ran. +DELIVERABLE = Path("deliverable") / "schema.json" + + +@dataclass(frozen=True) +class Agent: + """One launchable agent CLI. The seeded prompt is always appended as + the final positional argument — every supported CLI opens an + interactive session seeded with its positional prompt.""" + + name: str + command: tuple[str, ...] + install_hint: str + # Whether the landingai-ade plugin can trigger the skill natively; + # other agents get the clone-and-follow instructions instead. + native_skill: bool = False + + +KNOWN_AGENTS: tuple[Agent, ...] = ( + Agent( + "claude", + ("claude",), + "npm install -g @anthropic-ai/claude-code", + native_skill=True, + ), + Agent("codex", ("codex",), "npm install -g @openai/codex"), +) + + +def custom_agent(config: Mapping) -> Agent | None: + """The config.json escape hatch: ``agent.command`` names any CLI not + in the adapter table. Malformed entries read as absent — a config + typo must degrade to detection, never crash a parse.""" + entry = config.get("agent") + if not isinstance(entry, Mapping): + return None + command = entry.get("command") + if ( + not isinstance(command, list) + or not command + or not all(isinstance(token, str) and token for token in command) + ): + return None + name = entry.get("name") + return Agent( + name if isinstance(name, str) and name else command[0], + tuple(command), + install_hint="", + ) + + +def detect(which: Callable[[str], str | None], config: Mapping) -> list[Agent]: + """The launchable agents, best first. A resolvable ``agent.command`` + override wins outright; otherwise every known agent on PATH, with a + remembered ``agent.default`` collapsing the list to that one choice + so the menu asks only once.""" + custom = custom_agent(config) + if custom is not None and which(custom.command[0]): + return [custom] + found = [agent for agent in KNOWN_AGENTS if which(agent.command[0])] + entry = config.get("agent") + default = entry.get("default") if isinstance(entry, Mapping) else None + for agent in found: + if agent.name == default: + return [agent] + return found + + +def install_message() -> str: + """The no-agent-found remediation: name the known CLIs, their install + commands, and the config escape hatch for anything else.""" + lines = ["no AI agent CLI found on PATH; install one to build schemas:"] + for agent in KNOWN_AGENTS: + lines.append(f" {agent.name}: {agent.install_hint}") + lines.append( + ' or point config.json at another agent: {"agent": ' + '{"name": "...", "command": ["...", "..."]}}' + ) + return "\n".join(lines) + + +def seeded_prompt( + *, + agent: Agent, + intent: str, + job_item_id: str, + source: str, + store_dir: Path, + workspace: Path, + environment: str | None = None, + existing_schema: Path | None = None, + ground_truth: Path | None = None, +) -> str: + """The one message the agent session starts from. It carries what ade + already knows (identifiers, paths, the intent) and how to acquire the + skill; it deliberately omits accuracy targets — the skill's own + intake negotiates those with the user in conversation.""" + if agent.native_skill: + skill_lines = [ + "Invoke the build-schema skill (from the landingai-ade plugin of " + f"{SKILLS_REPO_URL}) and follow it exactly. If the plugin is not " + "installed, acquire the skill first:", + ] + else: + skill_lines = [ + "Follow the ADE build-schema skill exactly. Use a locally " + "installed copy if one exists; otherwise acquire it first:", + ] + skill_lines += [ + f" git clone --depth 1 {SKILLS_REPO_URL} {workspace / '.claude-skills'}", + f" then follow {SKILL_PATH} from that clone.", + ] + inputs = [ + f"- Extraction intent: {intent}", + f"- Parse job item id: {job_item_id} — the document is already " + f"parsed; artifacts live in {store_dir} (parse.json, parse.md, " + "elements.json). Reuse that parse: never re-parse or pass --force.", + f"- Source document: {source}", + ] + if environment is not None: + inputs.append(f"- ADE environment: {environment}") + inputs.append(f"- Workspace: use exactly {workspace} as the skill workspace.") + if existing_schema is not None: + inputs.append( + f"- Existing schema (iteration-1 baseline): {existing_schema}" + ) + if ground_truth is not None: + inputs.append(f"- Golden ground truth: {ground_truth}") + return "\n".join( + [ + "Build an extraction schema with the ADE build-schema skill for " + "a document already parsed locally by the ade CLI.", + "", + *skill_lines, + "", + "Inputs for the skill:", + *inputs, + "", + "The ade CLI is on PATH. Check `ade auth status --json` before " + "starting and pass --json to every ade command.", + "The skill negotiates per-field accuracy targets in conversation " + "— that was deliberately not collected here; do it with the user.", + f"Deliver the final schema at {workspace / DELIVERABLE}.", + ] + ) diff --git a/src/ade_cli/config.py b/src/ade_cli/config.py index cc7c0d3..5be10ae 100644 --- a/src/ade_cli/config.py +++ b/src/ade_cli/config.py @@ -6,8 +6,13 @@ terminals can work two environments at once), then production, the stable default. ``ADE_ENDPOINT`` overrides the endpoint URL alone (the raw escape hatch); credentials still file under the resolved -environment. ``config.json`` holds only the ``oauth.`` -provider overrides (see oauth.py); ``ADE_HOME`` relocates the store. +environment. ``config.json`` holds the ``oauth.`` provider +overrides (see oauth.py), the ``update_check`` opt-out (update.py), and +the schema-handoff keys (schema_build.py): ``agent`` — a remembered +``{"default": "claude"}`` menu choice or a custom +``{"name": ..., "command": [...]}`` launcher — and ``schema_prompt: +false`` to silence the post-parse offer. ``ADE_HOME`` relocates the +store. """ from __future__ import annotations @@ -19,6 +24,7 @@ from typing import Literal from .output import EXIT_USAGE, exit_with +from .store import write_atomic ENVIRONMENTS = { "dev": "https://api.ade.dev.landing.ai", @@ -52,6 +58,14 @@ def load_config(home: Path) -> dict: return json.loads(path.read_text()) +def save_config(home: Path, config: dict) -> None: + """Publish the whole config atomically (temp-write + rename, the + shared-file posture). Callers treat failure as advisory — remembering + a preference is never worth failing the command that noticed it.""" + home.mkdir(parents=True, exist_ok=True) + write_atomic(config_path(home), json.dumps(config, indent=2) + "\n") + + @dataclass(frozen=True) class ResolvedConfig: """The endpoint in effect plus the environment whose credentials apply diff --git a/src/ade_cli/help.py b/src/ade_cli/help.py index 0b001c3..fe24f82 100644 --- a/src/ade_cli/help.py +++ b/src/ade_cli/help.py @@ -60,6 +60,7 @@ ], ), ("network verbs — the ADE job contracts", ["parse", "extract"]), + ("schema authoring — the agent handoff", ["schema build"]), ( "local read models", ["history list", "history clear", "find", "view", "crop"], @@ -79,6 +80,8 @@ "body": [ "parse ─┬─> find ──> crop look at an element as an image", " ├─> view grounded page images + markdown", + " ├─> schema build author the extract schema with an", + " │ AI-agent session (build-schema skill)", " └─> extract ──> view schema-shaped data, grounded", "", "1. parse ensure a document is parsed. Prints a job item id;", @@ -88,10 +91,15 @@ " type, page, box, text.", "3. crop render element regions as PNGs: one --element-id, or a", " batch by find's own filters (--type figure, --all).", - "4. extract ensure a schema extraction exists for a parse job item", + "4. schema build author the extract schema when you don't have", + " one: hands the parse to an interactive AI-agent", + " session running the build-schema skill (interactive", + " terminals only — agents should run the skill", + " directly instead).", + "5. extract ensure a schema extraction exists for a parse job item", " (or bring-your-own markdown). It becomes its own job", " item, referencing the parse — artifacts never copied.", - "5. view build the self-contained HTML viewer for a parse or an", + "6. view build the self-contained HTML viewer for a parse or an", " extract item; --element-id emits a deep link to one", " element — the citation contract.", "", @@ -379,6 +387,24 @@ ("environment", "the environment cleared (null with --all)"), ], }, + "schema build": { + "shape": "object", + "keys": [ + ("status", "'schema_built'"), + ("job_item_id", "the parse item the schema was built against"), + ("agent", "the agent CLI that hosted the session"), + ("workspace", "the skill workspace (eval set, iteration log, " + "dispositions live alongside the schema)"), + ("schema", "absolute path of the delivered schema.json"), + ("extract_hint", "the ready-to-run extract invocation"), + ("extracted", "always false — the extract run is offered " + "interactively, never performed silently"), + ], + "note": "Interactive terminals only: the command launches a " + "foreground AI-agent session (claude, codex, or a config-declared " + "CLI) seeded with the parse and your intent. Agents should invoke " + "the build-schema skill directly instead of calling this.", + }, "version": { "shape": "object", "keys": [ diff --git a/src/ade_cli/main.py b/src/ade_cli/main.py index 1e8c7d2..d979d96 100644 --- a/src/ade_cli/main.py +++ b/src/ade_cli/main.py @@ -25,6 +25,7 @@ ) from .parse import parse from .ports import Ports +from .schema_build import schema_app from .telemetry import LedgerGroup from .update import current_version, install_mode, update from .view import view @@ -130,6 +131,7 @@ def _force_utf8_stdio() -> None: app.add_typer(auth_app) app.add_typer(history_app) +app.add_typer(schema_app) # Top-level aliases: the same callbacks as `auth login`/`auth logout` # (identical flags and behavior), registered again at the root for muscle # memory from other CLIs. Only the help line differs — it names the alias. diff --git a/src/ade_cli/parse.py b/src/ade_cli/parse.py index ed3e990..b2449ef 100644 --- a/src/ade_cli/parse.py +++ b/src/ade_cli/parse.py @@ -20,7 +20,7 @@ import typer -from . import attach, credentials, elements, items, oauth, store +from . import attach, credentials, elements, items, oauth, schema_build, store from .config import DEFAULT_ENVIRONMENT, ENVIRONMENTS, ade_home, resolve_target from .gateway import Gateway from . import guarantee as lifecycle @@ -317,6 +317,7 @@ def emit_summary( next_line = ( f"\n next: ade view {ref} --open" f" · ade extract {ref} --schema " + f" · ade schema build {ref}" ) copy_line = "" if copy_info is not None: @@ -411,6 +412,15 @@ def maybe_keep_copy() -> dict | None: return {"kept": False, "error": error.message} return {"kept": True, "name": name} + def maybe_offer_schema() -> None: + """The post-parse doorway into `ade schema build` (schema_build.py + owns the gates: real terminal, no agent host/CI, not opted out). + Machine modes never see it, and the summary above already printed + the plain next: hint — the offer is additive, never load-bearing.""" + if as_json or id_only: + return + schema_build.offer_after_parse(ports, home, jobs, item_id) + # The guarantee: this exact invocation (source x content x params — the # id) is served from disk free — unless the last attempt failed (a # reported failure resubmits fresh, never cache-hits). live_parse gates @@ -426,6 +436,7 @@ def maybe_keep_copy() -> dict | None: completed_at=stored_meta.get("completed_at"), copy_info=maybe_keep_copy(), ) + maybe_offer_schema() return data, job_id, stored = ensure_parsed( @@ -445,6 +456,7 @@ def maybe_keep_copy() -> dict | None: endpoint_label=resolved.endpoint_label, ) emit_summary(data, job_id, cached=False, stored=stored, copy_info=maybe_keep_copy()) + maybe_offer_schema() def ensure_parsed( diff --git a/src/ade_cli/ports.py b/src/ade_cli/ports.py index 686c0d4..e78d97d 100644 --- a/src/ade_cli/ports.py +++ b/src/ade_cli/ports.py @@ -1,5 +1,6 @@ """Injected ports: the HTTP transport, the clock, terminal-ness, the -browser opener, and the raw-key reader. +browser opener, the raw-key reader, PATH lookup, and the attached +subprocess runner. Production builds real ones; tests inject fakes through the CLI seam (``ctx.obj``). No command in this slice performs HTTP, but every command @@ -8,10 +9,13 @@ from __future__ import annotations +import shutil +import subprocess import sys import time import webbrowser from dataclasses import dataclass, field +from pathlib import Path from typing import Callable, Protocol import httpx @@ -49,6 +53,13 @@ def open_browser(url: str) -> bool: return False +def run_attached(argv: list[str], cwd: Path | None = None) -> int: + """Run a foreground child that inherits this process's stdio — the + interactive agent session and the extract re-exec both need the real + terminal, so nothing is captured or redirected.""" + return subprocess.run(argv, cwd=cwd).returncode + + @dataclass class Ports: transport: httpx.BaseTransport = field(default_factory=httpx.HTTPTransport) @@ -58,6 +69,13 @@ class Ports: # (the vendored click getchar — real raw-mode reads); tests inject a # scripted reader. getchar: Callable[[], str] | None = None + # PATH lookup for agent-CLI detection; tests inject a fake resolving + # only the names the test declares present. + which: Callable[[str], str | None] = shutil.which + # Foreground subprocess inheriting this process's stdio, returning the + # exit code; tests inject a recorder — the suite never launches a + # real process. + run_agent: Callable[..., int] = run_attached # Terminal-ness of the real streams, injectable because a test runner's # captured streams are never ttys. None means detect. stdout_tty: bool | None = None diff --git a/src/ade_cli/schema_build.py b/src/ade_cli/schema_build.py new file mode 100644 index 0000000..a036ce0 --- /dev/null +++ b/src/ade_cli/schema_build.py @@ -0,0 +1,478 @@ +"""``schema build`` — hand a parsed document to the build-schema skill. + +The schema between ``parse`` and ``extract`` is the one artifact this CLI +cannot produce alone: the build-schema skill (the landingai-ade plugin of +landing-ai/claude-skills) authors it inside an AI-agent session that +drives this very CLI — real extractions, reviewer-built ground truth, +converged-or-blocked stops. This module owns the bridge, in both +directions: + +- ``ade schema build JOB_ITEM_ID`` launches an agent session (claude, + codex, or a config-declared CLI — see agents.py) seeded with the parse + item, the user's extraction intent, and a workspace; when the session + ends it looks for the skill's ``deliverable/schema.json`` and offers to + run the extract. +- ``offer_after_parse`` is the post-parse doorway into the same flow, + heavily gated: a real terminal on all three streams, no ``--json`` or + ``--id-only``, no agent host or CI (an agent driving ``ade parse`` must + never meet a menu), and not silenced by ``schema_prompt: false``. + +ade itself stays network-free here: agents are found on PATH, the skill +is acquired by the agent (the seeded prompt says how), and the extract +re-run is a re-exec of this CLI. Everything interactive rides stderr; +stdout keeps the one-payload contract. +""" + +from __future__ import annotations + +import os +import re +from datetime import datetime +from pathlib import Path + +import typer + +from . import agents, items, store, surface, term +from .config import DEFAULT_ENVIRONMENT, ade_home, load_config, save_config +from .history import resolve_or_exit +from .output import EXIT_FAILED, EXIT_USAGE, JSON_FLAG, emit, exit_with, tilde +from .ports import Ports +from .update import reexec_argv + +schema_app = typer.Typer( + name="schema", + help="Schema authoring: build an extraction schema from a parsed " + "document with an AI-agent session running the build-schema skill.", + no_args_is_help=True, +) + + +def _slug(source: str) -> str: + """A filesystem-friendly engagement slug from the parse source (a + path or URL): the filename stem, lowercased, squeezed to [a-z0-9-].""" + stem = Path(source.split("?", 1)[0].rstrip("/")).stem + slug = re.sub(r"[^a-z0-9]+", "-", stem.lower()).strip("-") + return slug or "document" + + +def _parse_item_or_exit( + jobs: store.JobStore, token: str, *, as_json: bool +) -> tuple[str, dict]: + """Resolve TOKEN to a completed parse item and return ``(item_id, + meta)`` — the same gate (and error shapes) extract's JOB_ITEM_ID form + applies, since the skill will extract against this item.""" + item_id = resolve_or_exit(jobs, token, as_json=as_json) + record = items.item_record(jobs, item_id) + if record["kind"] != "parse": + exit_with( + { + "error": "not_a_parse_item", + "job_item_id": item_id, + "kind": record["kind"], + "message": "schema build takes a parse job item id.", + }, + f"Job item {item_id} is an extract item; schema build takes a " + "parse job item id (run `ade history list`).", + as_json=as_json, + code=EXIT_FAILED, + ) + live = items.live_parse(jobs, item_id) + if live is None: + exit_with( + { + "error": "not_parsed", + "job_item_id": item_id, + "state": record["state"], + "message": "No completed parse; re-run `ade parse` to finish it.", + }, + f"Job item {item_id} has no completed parse " + f"(state: {record['state']}); re-run `ade parse` to finish it.", + as_json=as_json, + code=EXIT_FAILED, + ) + meta, _response = live + return item_id, meta + + +def _choose_agent( + ports: Ports, + home: Path, + found: list[agents.Agent], + *, + as_json: bool, +) -> agents.Agent: + """One launchable agent out of several: the arrow-key menu on a real + terminal (ADR-0002's pattern), a typed numbered fallback otherwise — + and under ``--json``, where no prompt may fire, a usage error naming + ``--agent``. The choice is remembered in config.json so the menu asks + once per machine, not once per document.""" + if as_json: + exit_with( + { + "error": "agent_ambiguous", + "agents": [agent.name for agent in found], + "message": "Several agent CLIs are available; pass --agent.", + }, + "Several agent CLIs are available " + f"({', '.join(agent.name for agent in found)}); pass --agent.", + as_json=as_json, + code=EXIT_USAGE, + ) + labels = [f"{i + 1}) {agent.name}" for i, agent in enumerate(found)] + index: int | None = None + if ports.stdin_is_tty() and os.environ.get("TERM") != "dumb": + typer.echo("Which agent should build the schema? (↑/↓ and Enter)", err=True) + try: + index = term.select(labels, getchar=ports.getchar) + except term.Unsupported: + pass # the widget erased itself; the typed fallback re-lists + else: + typer.echo("Which agent should build the schema?", err=True) + if index is None: + for label in labels: + typer.echo(f" {label}", err=True) + while True: + choice = typer.prompt("Agent", default="1", err=True).strip() + if choice.isdigit() and 1 <= int(choice) <= len(found): + index = int(choice) - 1 + break + typer.echo(f"Choose 1-{len(found)}.", err=True) + chosen = found[index] + _remember_agent(home, chosen) + return chosen + + +def _remember_agent(home: Path, chosen: agents.Agent) -> None: + """Persist the menu choice as ``agent.default`` — advisory: a config + that cannot be written just means the menu asks again next time.""" + try: + config = load_config(home) + entry = config.get("agent") + config["agent"] = { + **(entry if isinstance(entry, dict) else {}), + "default": chosen.name, + } + save_config(home, config) + typer.echo( + f"(remembered {chosen.name} as agent.default in " + f"{tilde(home / 'config.json')})", + err=True, + ) + except OSError: + pass + + +def _prompt_optional_path(label: str) -> Path | None: + """An optional file path collected interactively: bare Enter skips, + a path that does not exist re-prompts (a typo must not silently drop + the user's baseline schema or golden set).""" + while True: + raw = typer.prompt(label, default="", err=True).strip() + if not raw: + return None + path = Path(raw).expanduser() + if path.is_file(): + return path + typer.echo(f"No such file: {raw} (Enter to skip)", err=True) + + +def run_build( + ports: Ports, + home: Path, + jobs: store.JobStore, + item_id: str, + meta: dict, + *, + intent: str | None, + existing_schema: Path | None, + ground_truth: Path | None, + workspace: Path | None, + agent_name: str | None, + as_json: bool, +) -> None: + """The shared handoff body behind ``schema build`` and the post-parse + offer: collect what ade can collect, launch the agent session seeded + with the rest, then account for the deliverable.""" + source = meta.get("source") or item_id + environment = meta.get("environment", DEFAULT_ENVIRONMENT) + + config = load_config(home) + if agent_name is not None: + # An explicit --agent bypasses the remembered default: it names + # any known or config-declared agent, needing only a PATH hit. + found = [ + agent + for agent in (agents.custom_agent(config), *agents.KNOWN_AGENTS) + if agent is not None + and agent.name == agent_name + and ports.which(agent.command[0]) + ] + else: + found = agents.detect(ports.which, config) + if not found: + message = agents.install_message() + if agent_name is not None: + message = f"agent {agent_name!r} is not available.\n{message}" + exit_with( + {"error": "no_agent", "agent": agent_name, "message": message}, + message, + as_json=as_json, + code=EXIT_FAILED, + ) + agent = found[0] if len(found) == 1 else _choose_agent( + ports, home, found, as_json=as_json + ) + + if intent is None: + # --json required --intent before reaching here; this prompt is + # interactive-path only. + typer.echo( + "Describe what to extract — fields, tables, anything the " + "schema should capture (the skill refines it with you):", + err=True, + ) + intent = typer.prompt("Intent", err=True).strip() + if not as_json: + if existing_schema is None: + existing_schema = _prompt_optional_path( + "Existing schema to improve (path, Enter to skip)" + ) + if ground_truth is None: + ground_truth = _prompt_optional_path( + "Golden ground truth (path, Enter to skip)" + ) + + if workspace is None: + date = datetime.fromtimestamp(ports.clock.now()).strftime("%Y-%m-%d") + workspace = home / "schema-runs" / f"{_slug(source)}-{date}" + workspace.mkdir(parents=True, exist_ok=True) + + prompt = agents.seeded_prompt( + agent=agent, + intent=intent, + job_item_id=item_id, + source=source, + store_dir=jobs.item_dir(item_id), + workspace=workspace, + environment=environment if environment != DEFAULT_ENVIRONMENT else None, + existing_schema=existing_schema, + ground_truth=ground_truth, + ) + typer.echo( + f"Launching {agent.name} in {tilde(workspace)} — the schema loop " + "runs in that session; finish it to return here.", + err=True, + ) + try: + exit_code = ports.run_agent([*agent.command, prompt], cwd=workspace) + except OSError as error: + exit_with( + {"error": "agent_launch_failed", "agent": agent.name, + "message": str(error)}, + f"Could not launch {agent.name}: {error}", + as_json=as_json, + code=EXIT_FAILED, + ) + + schema_path = workspace / agents.DELIVERABLE + if not schema_path.is_file(): + exit_with( + { + "error": "no_deliverable", + "agent": agent.name, + "agent_exit_code": exit_code, + "workspace": str(workspace), + "message": "The agent session ended without " + "deliverable/schema.json.", + }, + f"The {agent.name} session ended (exit {exit_code}) without " + f"producing {tilde(schema_path)}; the workspace is kept — " + "re-run `ade schema build` to resume it.", + as_json=as_json, + code=EXIT_FAILED, + ) + + ref = items.short_id(jobs, item_id) + extract_hint = f"ade extract {ref} --schema {schema_path}" + emit( + { + "status": "schema_built", + "job_item_id": item_id, + "agent": agent.name, + "workspace": str(workspace), + "schema": str(schema_path), + "extract_hint": extract_hint, + # The skill's final iteration already ran this extraction, so + # the run below is a free cache hit — but it is the caller's + # to run, so the payload never claims it happened here. + "extracted": False, + }, + f"Schema built -> {tilde(schema_path)}" + f"\n workspace: {tilde(workspace)} (eval set, iteration log, " + "dispositions alongside)" + f"\n next: {extract_hint}", + as_json=as_json, + ) + if as_json: + return + if typer.confirm( + "Run that extraction now? (mostly a free cache hit)", + default=True, + err=True, + ): + code = ports.run_agent( + reexec_argv("extract", item_id, "--schema", str(schema_path)) + ) + raise typer.Exit(code=code) + + +@schema_app.command() +def build( + ctx: typer.Context, + job_id_token: str = typer.Argument( + ..., + metavar="JOB_ITEM_ID", + help="A completed parse job item id (or unambiguous prefix).", + ), + intent: str | None = typer.Option( + None, "--intent", + help="What to extract, in plain language (prompted interactively " + "when omitted; required with --json).", + ), + existing_schema: Path | None = typer.Option( + None, "--existing-schema", exists=True, dir_okay=False, readable=True, + help="A schema to improve: the skill runs it unchanged first as " + "the regression floor.", + ), + ground_truth: Path | None = typer.Option( + None, "--ground-truth", exists=True, dir_okay=False, readable=True, + help="Golden ground truth seeding the skill's eval set (JSON " + "keyed by document filename, or flat CSV).", + ), + workspace: Path | None = typer.Option( + None, "--workspace", + help="Skill workspace directory (default: " + "/schema-runs/-/; an existing one resumes).", + ), + agent_name: str | None = typer.Option( + None, "--agent", + help="Agent CLI to launch (claude, codex, or a config-declared " + "name); default: the configured or only available one.", + ), + as_json: bool = JSON_FLAG, +) -> None: + """Build an extraction schema for a parsed document by handing it to + an AI-agent session running the build-schema skill (measured + iteration: real extractions, reviewer verdicts, converged-or-blocked + stops). Ends by naming the delivered schema and offering the extract. + """ + ports: Ports = ctx.obj + home = ade_home() + jobs = store.JobStore(home) + # The agent session inherits this terminal — piped streams would feed + # the interactive session garbage (and its TUI would corrupt a piped + # stdout), so even --json requires a real terminal here. + if not ( + ports.stdin_is_tty() and ports.stdout_is_tty() and ports.stderr_is_tty() + ): + message = ( + "schema build launches an interactive agent session and needs " + "a real terminal on stdin/stdout/stderr." + ) + exit_with( + {"error": "no_terminal", "message": message}, + message, + as_json=as_json, + code=EXIT_USAGE, + ) + if as_json and intent is None: + message = "--json cannot prompt; pass --intent." + exit_with( + {"error": "intent_required", "message": message}, + message, + as_json=as_json, + code=EXIT_USAGE, + ) + item_id, meta = _parse_item_or_exit(jobs, job_id_token, as_json=as_json) + run_build( + ports, home, jobs, item_id, meta, + intent=intent, + existing_schema=existing_schema, + ground_truth=ground_truth, + workspace=workspace, + agent_name=agent_name, + as_json=as_json, + ) + + +_OFFER_LABELS = ( + "1) Build an extraction schema with an AI agent", + "2) Exit", +) + + +def offer_after_parse( + ports: Ports, home: Path, jobs: store.JobStore, item_id: str +) -> None: + """The post-parse doorway into ``run_build`` — only for a human at a + real terminal. Callers pass control only outside ``--json``/ + ``--id-only``; the gates here keep the offer away from pipes, agent + hosts, CI, and anyone who set ``schema_prompt: false``. Bare Enter + exits (the summary's ``next:`` line already printed), so the offer + never costs a keystroke of the old behavior.""" + if not ( + ports.stdin_is_tty() and ports.stdout_is_tty() and ports.stderr_is_tty() + ): + return + surf = surface.detect(os.environ, stdout_is_tty=ports.stdout_is_tty()) + if surf.host is not None or surf.term == "ci": + return + try: + config = load_config(home) + except Exception: + return # an unreadable config must never taint a completed parse + if config.get("schema_prompt") is False: + return + try: + index: int | None = None + if os.environ.get("TERM") != "dumb": + typer.echo( + "\nBuild an extraction schema from this parse? (↑/↓ and Enter)", + err=True, + ) + try: + index = term.select( + list(_OFFER_LABELS), default=1, getchar=ports.getchar + ) + except term.Unsupported: + pass # the widget erased itself; the typed fallback re-lists + else: + typer.echo("\nBuild an extraction schema from this parse?", err=True) + if index is None: + for label in _OFFER_LABELS: + typer.echo(f" {label}", err=True) + choice = typer.prompt("Choice", default="2", err=True).strip() + index = 0 if choice == "1" else 1 + if index != 0: + return + # A missing agent CLI is non-fatal on this path: the parse + # succeeded and the summary's next: line already serves — the + # handoff just isn't installed yet. + if not agents.detect(ports.which, config): + typer.echo(agents.install_message(), err=True) + return + meta = jobs.read_json(item_id, "meta.json") or {} + run_build( + ports, home, jobs, item_id, meta, + intent=None, + existing_schema=None, + ground_truth=None, + workspace=None, + agent_name=None, + as_json=False, + ) + except typer.Abort: + # Esc/Ctrl-C declines the optional add-on; the parse already + # succeeded and its exit code stays 0. + typer.echo("", err=True) + return diff --git a/src/ade_cli/update.py b/src/ade_cli/update.py index 79e530b..5194097 100644 --- a/src/ade_cli/update.py +++ b/src/ade_cli/update.py @@ -76,6 +76,15 @@ def is_frozen() -> bool: return bool(getattr(sys, "frozen", False)) +def reexec_argv(*command: str) -> list[str]: + """argv that re-runs this CLI: a PyInstaller binary (sys.frozen) takes + subcommands directly; a normal install goes through ``python -m ade_cli`` + (the only re-exec an importable package can promise).""" + if is_frozen(): + return [sys.executable, *command] + return [sys.executable, "-m", "ade_cli", *command] + + def install_mode() -> str: """How this CLI is installed: ``binary`` (the frozen standalone app — self-update replaces it in place) or ``python`` (uv/pipx/pip — an diff --git a/src/ade_cli/view.py b/src/ade_cli/view.py index 6fe4793..768eb6f 100644 --- a/src/ade_cli/view.py +++ b/src/ade_cli/view.py @@ -35,7 +35,6 @@ import json import os import subprocess -import sys from datetime import datetime, timezone from importlib import resources from pathlib import Path @@ -48,6 +47,7 @@ from .history import resolve_or_exit from .output import EXIT_FAILED, EXIT_USAGE, JSON_FLAG, emit, exit_with, tilde from .store import JobStore +from .update import reexec_argv from .parse import _parse_pages from .raster import CropError, render_source, source_drift_note @@ -809,15 +809,6 @@ def _latest_viewable(store: JobStore, records: list[dict]) -> str | None: return None -def _reexec_argv(*command: str) -> list[str]: - """argv that re-runs this CLI: a PyInstaller binary (sys.frozen) takes - subcommands directly; a normal install goes through ``python -m ade_cli`` - (the only re-exec an importable package can promise).""" - if getattr(sys, "frozen", False): - return [sys.executable, *command] - return [sys.executable, "-m", "ade_cli", *command] - - def _needs_chunks(store: JobStore, record: dict) -> bool: """Whether a parse item's page-imagery chunks are missing or stale — cheap (meta.json plus one first-line read per chunk file), so every @@ -876,7 +867,7 @@ def _spawn_builder(store: JobStore, records: list[dict]) -> bool: if not needs: return False subprocess.Popen( - _reexec_argv("view", "--sync-viewers"), + reexec_argv("view", "--sync-viewers"), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL, @@ -891,7 +882,7 @@ def _spawn_server(port: int) -> None: flag, fully disowned. The daemon settles the final port itself and records it in server.json; the caller watches for that.""" subprocess.Popen( - _reexec_argv("view", "--serve-daemon", str(port)), + reexec_argv("view", "--serve-daemon", str(port)), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL, diff --git a/tests/conftest.py b/tests/conftest.py index c9b73cb..cc71aa7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,6 +28,39 @@ def _no_getchar() -> str: raise AssertionError("unscripted raw key read") +def _which_none(name: str) -> str | None: + """Default PATH fake: no agent CLI exists unless a test says so.""" + return None + + +def _fake_which(names: dict[str, str]) -> Callable[[str], str | None]: + return lambda name: names.get(name) + + +def _no_run_agent(argv: list[str], cwd: Path | None = None) -> int: + raise AssertionError(f"unscripted subprocess run: {argv}") + + +class RecordedRuns: + """Attached-subprocess fake: records ``(argv, cwd)`` and plays back a + per-call script of side effects (e.g. writing the skill's deliverable + into the workspace) or plain exit codes. Running dry is a harness + failure, mirroring the transport and browser fakes.""" + + def __init__(self, script: list[int | Callable[[list[str], Path | None], int]]): + self.calls: list[tuple[list[str], Path | None]] = [] + self._script = list(script) + + def __call__(self, argv: list[str], cwd: Path | None = None) -> int: + self.calls.append((list(argv), cwd)) + if not self._script: + raise AssertionError(f"ran out of scripted subprocess runs: {argv}") + step = self._script.pop(0) + if callable(step): + return step(list(argv), cwd) + return step + + def _scripted_keys(keys: list[str | BaseException]) -> Callable[[], str]: """A raw-key reader that plays back a script — an exception entry is raised in place of a key (e.g. OSError for a raw-mode failure). Running @@ -124,6 +157,8 @@ def invoke( env: dict[str, str | None] | None = None, browser: Callable[[str], bool] | None = None, keys: list[str | BaseException] | None = None, + which: dict[str, str] | None = None, + run: Callable[..., int] | None = None, ): # Pin ADE_HOME to the temp store and shield the run from ambient # ADE_* vars and surface markers (the machine running the tests @@ -162,6 +197,8 @@ def invoke( clock=self.clock, browser=browser or _no_browser, getchar=_scripted_keys(keys) if keys is not None else _no_getchar, + which=_fake_which(which) if which is not None else _which_none, + run_agent=run or _no_run_agent, stdout_tty=self.stdout_tty, stderr_tty=self.stderr_tty, stdin_tty=self.stdin_tty, diff --git a/tests/test_parse.py b/tests/test_parse.py index bb53ab2..846f887 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -765,3 +765,131 @@ def test_keep_copy_on_a_cached_hit_attaches_late(cli): assert payload["kept_copy"] is True item_dir_ = cli.home / "jobs" / payload["job_item_id"] assert (item_dir_ / "document.pdf").read_bytes() == pdf + + +# ------------------------------------------------- the post-parse schema offer + + +OFFER = "Build an extraction schema" + + +def offer_tty(cli): + """The offer needs a real terminal on all three streams.""" + cli.stdin_tty = cli.stdout_tty = cli.stderr_tty = True + return cli + + +def seed_parse(cli, document): + cli.transport.respond(202, {"job_id": JOB_ID}) + cli.transport.respond(200, completed_job()) + + +def test_offer_fires_on_a_tty_and_bare_enter_declines(cli, document): + offer_tty(cli) + seed_parse(cli, document) + # Enter confirms the default option, which is Exit — the offer never + # costs a keystroke of the old behavior. keys are scripted: any extra + # read would fail the harness, proving the flow ends at the menu. + result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\r"]) + assert result.exit_code == 0, result.stdout + assert OFFER in result.stderr + assert "next:" in result.stdout # the plain hint still printed + + +def test_offer_esc_declines_without_failing_the_parse(cli, document): + offer_tty(cli) + seed_parse(cli, document) + result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\x1b"]) + assert result.exit_code == 0, result.stdout + + +def test_offer_fires_on_the_cached_path_too(cli, document): + offer_tty(cli) + seed_parse(cli, document) + assert cli.invoke( + "parse", "-d", str(document), env=AUTH_ENV, keys=["\r"] + ).exit_code == 0 + cached = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\r"]) + assert cached.exit_code == 0, cached.stdout + assert OFFER in cached.stderr + + +def test_offer_accept_without_an_agent_is_nonfatal(cli, document): + offer_tty(cli) + seed_parse(cli, document) + # "1" jumps the pointer, Enter confirms — accept; no agent CLI exists + # (the default which resolves nothing), so the offer teaches the + # install and the parse still succeeds. + result = cli.invoke( + "parse", "-d", str(document), env=AUTH_ENV, keys=["1", "\r"] + ) + assert result.exit_code == 0, result.stdout + assert "no AI agent CLI found" in result.stderr + + +def test_offer_accept_runs_the_handoff(cli, document): + from conftest import RecordedRuns + + offer_tty(cli) + seed_parse(cli, document) + + def deliver(argv, cwd): + (cwd / "deliverable").mkdir(parents=True) + (cwd / "deliverable" / "schema.json").write_text("{}") + return 0 + + runs = RecordedRuns([deliver]) + result = cli.invoke( + "parse", "-d", str(document), env=AUTH_ENV, + keys=["1", "\r"], + which={"claude": "/usr/local/bin/claude"}, + run=runs, + input="invoice totals\n\n\nn\n", # intent, skip, skip, no extract + ) + assert result.exit_code == 0, result.stdout + argv, cwd = runs.calls[0] + assert argv[0] == "claude" and "invoice totals" in argv[1] + assert "Schema built" in result.stdout + + +def test_offer_never_fires_in_machine_or_hosted_modes(cli, document): + offer_tty(cli) + seed_parse(cli, document) + # --json: stdout is payload-only. No keys are scripted — a menu read + # would fail the harness. + result = cli.invoke("parse", "-d", str(document), "--json", env=AUTH_ENV) + assert result.exit_code == 0 + assert OFFER not in result.stderr + + for env in ({"CLAUDECODE": "1"}, {"CI": "1"}): + cached = cli.invoke("parse", "-d", str(document), env={**AUTH_ENV, **env}) + assert cached.exit_code == 0, cached.stdout + assert OFFER not in cached.stderr + + +def test_offer_never_fires_without_a_full_tty(cli, document): + cli.stdin_tty = False + cli.stdout_tty = True + cli.stderr_tty = True + seed_parse(cli, document) + result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV) + assert result.exit_code == 0 + assert OFFER not in result.stderr + + +def test_offer_respects_the_config_opt_out(cli, document): + offer_tty(cli) + seed_parse(cli, document) + cli.home.mkdir(parents=True, exist_ok=True) + (cli.home / "config.json").write_text('{"schema_prompt": false}') + result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV) + assert result.exit_code == 0, result.stdout + assert OFFER not in result.stderr + + +def test_offer_id_only_stays_silent(cli, document): + offer_tty(cli) + seed_parse(cli, document) + result = cli.invoke("parse", "-d", str(document), "--id-only", env=AUTH_ENV) + assert result.exit_code == 0 + assert OFFER not in result.stderr diff --git a/tests/test_schema_build.py b/tests/test_schema_build.py new file mode 100644 index 0000000..d62ef91 --- /dev/null +++ b/tests/test_schema_build.py @@ -0,0 +1,304 @@ +"""``schema build`` — the handoff into the build-schema agent skill. + +The command's whole job is orchestration: find an agent CLI, seed it with +what ade already knows, account for the deliverable, offer the extract. +Every test drives the CLI seam with the subprocess and PATH ports faked — +the suite never launches a real agent (or any process at all). +""" + +import json +import re +import sys +from pathlib import Path + +import pytest + +from ade_cli import agents + +from conftest import RecordedRuns +from extract_fixtures import SCHEMA, completed_extract_job +from parse_fixtures import JOB_ID, completed_job + +KEY = "sk-test-0123456789abcd" +AUTH_ENV = {"ADE_API_KEY": KEY} +DOC_BYTES = b"%PDF-1.4 fake invoice bytes" + +CLAUDE = {"claude": "/usr/local/bin/claude"} +BOTH = {"claude": "/usr/local/bin/claude", "codex": "/usr/local/bin/codex"} + + +@pytest.fixture +def document(tmp_path): + path = tmp_path / "invoice.pdf" + path.write_bytes(DOC_BYTES) + return path + + +@pytest.fixture +def tty(cli): + """The command requires a real terminal on all three streams; the + handoff tests opt the harness into one.""" + cli.stdin_tty = cli.stdout_tty = cli.stderr_tty = True + return cli + + +def parse_doc(cli, document): + """Seed a completed parse job item; returns its job item id.""" + cli.transport.respond(202, {"job_id": JOB_ID}) + cli.transport.respond(200, completed_job()) + result = cli.invoke("parse", "-d", str(document), "--json", env=AUTH_ENV) + assert result.exit_code == 0, result.stdout + return json.loads(result.stdout)["job_item_id"] + + +def deliver_schema(argv, cwd): + """RecordedRuns side effect standing in for the skill: the agent + session 'ran' and left the five-artifact deliverable's schema.""" + (cwd / "deliverable").mkdir(parents=True, exist_ok=True) + (cwd / "deliverable" / "schema.json").write_text(json.dumps(SCHEMA)) + return 0 + + +# ---------------------------------------------------------------- agents.py + + +def prompt_for(agent, **overrides): + defaults = dict( + agent=agent, + intent="invoice totals and line items", + job_item_id="a" * 16, + source="/docs/invoice.pdf", + store_dir=Path("/home/u/.ade/jobs") / ("a" * 16), + workspace=Path("/home/u/.ade/schema-runs/invoice-2026-08-10"), + ) + defaults.update(overrides) + return agents.seeded_prompt(**defaults) + + +def test_seeded_prompt_carries_the_full_item_and_the_rules(): + prompt = prompt_for(agents.KNOWN_AGENTS[0]) + assert "a" * 16 in prompt # the full id, never a prefix + assert "never re-parse or pass --force" in prompt + assert "ade auth status --json" in prompt + # Targets are the skill's to negotiate — the prompt must say so, not + # carry numbers. + assert "accuracy targets" in prompt and "80" not in prompt + assert "deliverable" in prompt and "schema.json" in prompt + + +def test_seeded_prompt_skill_acquisition_differs_by_agent(): + claude, codex = agents.KNOWN_AGENTS + assert "landingai-ade plugin" in prompt_for(claude) + for prompt in (prompt_for(claude), prompt_for(codex)): + # Both carry the clone fallback (claude may lack the plugin too). + assert "git clone --depth 1 https://github.com/landing-ai/claude-skills" in prompt + assert "plugins/landingai-ade/skills/build-schema/SKILL.md" in prompt + + +def test_seeded_prompt_optional_lines_appear_only_when_given(tmp_path): + bare = prompt_for(agents.KNOWN_AGENTS[0]) + assert "ADE environment" not in bare + assert "Existing schema" not in bare and "ground truth" not in bare + full = prompt_for( + agents.KNOWN_AGENTS[0], + environment="staging", + existing_schema=tmp_path / "old.json", + ground_truth=tmp_path / "golden.csv", + ) + assert "ADE environment: staging" in full + assert "old.json" in full and "golden.csv" in full + + +def test_detect_orders_and_collapses(): + which = lambda name: BOTH.get(name) # noqa: E731 + assert [a.name for a in agents.detect(which, {})] == ["claude", "codex"] + assert [a.name for a in agents.detect(which, {"agent": {"default": "codex"}})] == [ + "codex" + ] + # A custom command wins outright when resolvable, degrades when not. + custom = {"agent": {"name": "aider", "command": ["aider", "--message"]}} + which_custom = lambda name: {"aider": "/bin/aider", **BOTH}.get(name) # noqa: E731 + assert [a.name for a in agents.detect(which_custom, custom)] == ["aider"] + assert [a.name for a in agents.detect(which, custom)] == ["claude", "codex"] + # Malformed config entries read as absent, never crash. + assert agents.detect(lambda name: None, {"agent": {"command": "aider"}}) == [] + + +# ---------------------------------------------------------- ade schema build + + +def test_build_requires_a_real_terminal(cli, document): + item_id = parse_doc(cli, document) + result = cli.invoke("schema", "build", item_id, "--json", which=CLAUDE) + assert result.exit_code == 2 + assert json.loads(result.stdout)["error"] == "no_terminal" + + +def test_build_without_an_agent_fails_with_install_instructions(tty, document): + item_id = parse_doc(tty, document) + result = tty.invoke("schema", "build", item_id, "--intent", "totals") + assert result.exit_code == 1 + assert "no AI agent CLI found" in result.stdout + assert "@anthropic-ai/claude-code" in result.stdout + assert "@openai/codex" in result.stdout + + +def test_build_launches_the_agent_and_offers_the_extract(tty, document): + item_id = parse_doc(tty, document) + runs = RecordedRuns([deliver_schema, 0]) # the session, then the extract + + result = tty.invoke( + "schema", "build", item_id, "--intent", "invoice totals", + which=CLAUDE, run=runs, input="\n\ny\n", # skip, skip, extract yes + ) + + assert result.exit_code == 0, result.stdout + session, extract = runs.calls + argv, cwd = session + assert argv[0] == "claude" and len(argv) == 2 + assert item_id in argv[1] and "invoice totals" in argv[1] + assert cwd is not None and re.fullmatch( + r"invoice-\d{4}-\d{2}-\d{2}", cwd.name + ), cwd + assert cwd.parent == tty.home / "schema-runs" + schema_path = cwd / "deliverable" / "schema.json" + assert str(schema_path) in result.stdout # the summary names the schema + # The extract is a re-exec of this CLI against the same item. + assert extract[0] == [ + sys.executable, "-m", "ade_cli", + "extract", item_id, "--schema", str(schema_path), + ] + + +def test_build_extract_declined_is_a_clean_stop(tty, document): + item_id = parse_doc(tty, document) + runs = RecordedRuns([deliver_schema]) + + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=CLAUDE, run=runs, input="\n\nn\n", + ) + + assert result.exit_code == 0, result.stdout + assert len(runs.calls) == 1 + assert "next:" in result.stdout and "ade extract" in result.stdout + + +def test_build_json_contract(tty, document): + item_id = parse_doc(tty, document) + # --json cannot prompt: intent is required up front. + missing = tty.invoke("schema", "build", item_id, "--json", which=CLAUDE) + assert missing.exit_code == 2 + assert json.loads(missing.stdout)["error"] == "intent_required" + + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=runs, + ) + assert result.exit_code == 0, result.stdout + payload = json.loads(result.stdout) + assert payload["status"] == "schema_built" + assert payload["job_item_id"] == item_id + assert payload["agent"] == "claude" + assert payload["schema"].endswith("schema.json") + assert payload["extracted"] is False + assert payload["extract_hint"].startswith("ade extract ") + assert len(runs.calls) == 1 # --json never auto-runs the extract + + +def test_build_no_deliverable_names_the_workspace(tty, document): + item_id = parse_doc(tty, document) + runs = RecordedRuns([3]) # the session ends without writing anything + + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=runs, + ) + + assert result.exit_code == 1 + payload = json.loads(result.stdout) + assert payload["error"] == "no_deliverable" + assert payload["agent_exit_code"] == 3 + assert tty.home / "schema-runs" in Path(payload["workspace"]).parents + + +def test_build_menu_remembers_the_choice(tty, document): + item_id = parse_doc(tty, document) + + first = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=BOTH, run=RecordedRuns([deliver_schema]), + keys=["2", "\r"], input="\n\nn\n", # pick codex on the arrow menu + ) + assert first.exit_code == 0, first.stdout + config = json.loads((tty.home / "config.json").read_text()) + assert config["agent"]["default"] == "codex" + + # The remembered default collapses detection: no menu (keys would be + # an unscripted read), straight to codex. + runs = RecordedRuns([deliver_schema]) + second = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=BOTH, run=runs, input="\n\nn\n", + ) + assert second.exit_code == 0, second.stdout + assert runs.calls[0][0][0] == "codex" + + +def test_build_agent_flag_beats_the_remembered_default(tty, document): + item_id = parse_doc(tty, document) + (tty.home).mkdir(parents=True, exist_ok=True) + (tty.home / "config.json").write_text( + json.dumps({"agent": {"default": "codex"}}) + ) + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", "--agent", "claude", + which=BOTH, run=runs, input="\n\nn\n", + ) + assert result.exit_code == 0, result.stdout + assert runs.calls[0][0][0] == "claude" + + # And an --agent that is not on PATH is a legible failure. + missing = tty.invoke( + "schema", "build", item_id, "--intent", "totals", "--agent", "claude", + which={"codex": "/usr/local/bin/codex"}, + ) + assert missing.exit_code == 1 + assert "not available" in missing.stdout + + +def test_build_custom_agent_command(tty, document): + item_id = parse_doc(tty, document) + (tty.home).mkdir(parents=True, exist_ok=True) + (tty.home / "config.json").write_text( + json.dumps({"agent": {"name": "aider", "command": ["aider", "--message"]}}) + ) + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which={"aider": "/bin/aider"}, run=runs, input="\n\nn\n", + ) + assert result.exit_code == 0, result.stdout + argv, _cwd = runs.calls[0] + assert argv[:2] == ["aider", "--message"] and item_id in argv[2] + + +def test_build_refuses_an_extract_item(tty, document, tmp_path): + parse_id = parse_doc(tty, document) + schema_file = tmp_path / "schema.json" + schema_file.write_text(json.dumps(SCHEMA)) + tty.transport.respond(202, {"job_id": "extract-0001"}) + tty.transport.respond(200, completed_extract_job()) + extracted = tty.invoke( + "extract", parse_id, "--schema", str(schema_file), "--json", env=AUTH_ENV + ) + assert extracted.exit_code == 0, extracted.stdout + extract_id = json.loads(extracted.stdout)["job_item_id"] + + result = tty.invoke( + "schema", "build", extract_id, "--json", "--intent", "totals", which=CLAUDE + ) + assert result.exit_code == 1 + assert json.loads(result.stdout)["error"] == "not_a_parse_item" diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index a63c1d2..e7eb221 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -56,6 +56,7 @@ def test_the_walked_tree_matches_the_expected_commands(): ("login",), ("logout",), ("parse",), + ("schema", "build"), ("update",), ("version",), ("view",), diff --git a/tests/test_update.py b/tests/test_update.py index a26b41e..a2434e5 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -446,3 +446,18 @@ def test_unrelated_http_errors_carry_no_update_hint(cli, tmp_path): payload = json.loads(result.stdout) assert payload["error"] == "http" assert "hint" not in payload + + +def test_reexec_argv_targets_this_cli(monkeypatch): + """The shared re-exec seam (viewer daemons, the schema-build extract + handoff): frozen binaries take subcommands directly, a normal install + goes through ``python -m ade_cli``.""" + import sys + + from ade_cli.update import reexec_argv + + assert reexec_argv("extract", "abc") == [ + sys.executable, "-m", "ade_cli", "extract", "abc", + ] + monkeypatch.setattr(sys, "frozen", True, raising=False) + assert reexec_argv("view", "--open") == [sys.executable, "view", "--open"] From c80a40a553a976b1fe23c1f901b87f185dc27500 Mon Sep 17 00:00:00 2001 From: Yuanwen Tian Date: Mon, 10 Aug 2026 14:16:39 +0800 Subject: [PATCH 2/5] test: cover the schema handoff's failure paths and the real subprocess seam Unit tests (offline) for the paths the first pass left uncovered: workspace slugging and reuse, --workspace, URL-sourced parses, the --existing-schema/--ground-truth flags, a mistyped optional path, non-default environments in the seeded prompt, an unlaunchable agent, extract exit-code propagation, both menu fallbacks (raw-mode failure and TERM=dumb), agent_ambiguous under --json, and unknown ids. Integration tests drive the one seam the offline suite fakes: a real agent executable found on PATH and spawned attached to a real pty, then ade's own re-exec running the delivered schema against production. The chained extraction reuses the direct extract test's schema and parse item, so it dedups to that item and the added coverage bills nothing. POSIX-only (pty); skipped on Windows. Fixes a bug these found: run_build read config.json unguarded, so an unreadable or corrupt file crashed the handoff -- offer_after_parse already guarded it. Both now go through one tolerant reader: the agent preference and the prompt opt-out are optional, so a bad config costs the preference, never the handoff or a completed parse. Co-Authored-By: Claude Fable 5 --- src/ade_cli/schema_build.py | 20 ++- tests/integration/test_production.py | 199 +++++++++++++++++++++--- tests/test_parse.py | 12 ++ tests/test_schema_build.py | 217 +++++++++++++++++++++++++++ 4 files changed, 422 insertions(+), 26 deletions(-) diff --git a/src/ade_cli/schema_build.py b/src/ade_cli/schema_build.py index a036ce0..c65dc20 100644 --- a/src/ade_cli/schema_build.py +++ b/src/ade_cli/schema_build.py @@ -55,6 +55,17 @@ def _slug(source: str) -> str: return slug or "document" +def _readable_config(home: Path) -> dict: + """config.json, or an empty one when it cannot be read. Everything + this module takes from it — the agent preference, the prompt opt-out + — is optional, so a corrupt or unreadable file must cost the + preference, never the handoff (and never a completed parse).""" + try: + return load_config(home) + except (OSError, ValueError): + return {} + + def _parse_item_or_exit( jobs: store.JobStore, token: str, *, as_json: bool ) -> tuple[str, dict]: @@ -146,7 +157,7 @@ def _remember_agent(home: Path, chosen: agents.Agent) -> None: """Persist the menu choice as ``agent.default`` — advisory: a config that cannot be written just means the menu asks again next time.""" try: - config = load_config(home) + config = _readable_config(home) entry = config.get("agent") config["agent"] = { **(entry if isinstance(entry, dict) else {}), @@ -196,7 +207,7 @@ def run_build( source = meta.get("source") or item_id environment = meta.get("environment", DEFAULT_ENVIRONMENT) - config = load_config(home) + config = _readable_config(home) if agent_name is not None: # An explicit --agent bypasses the remembered default: it names # any known or config-declared agent, needing only a PATH hit. @@ -427,10 +438,7 @@ def offer_after_parse( surf = surface.detect(os.environ, stdout_is_tty=ports.stdout_is_tty()) if surf.host is not None or surf.term == "ci": return - try: - config = load_config(home) - except Exception: - return # an unreadable config must never taint a completed parse + config = _readable_config(home) if config.get("schema_prompt") is False: return try: diff --git a/tests/integration/test_production.py b/tests/integration/test_production.py index 9bc838d..cf6c3e0 100644 --- a/tests/integration/test_production.py +++ b/tests/integration/test_production.py @@ -20,7 +20,9 @@ where only the trivially-quoted invoice number is asserted. Parse and extract bill real credits (one parse + one extract per run, -per OS). Everything else is served from the local store. +per OS). Everything else is served from the local store — including the +schema-build handoff's chained extraction, which reuses the same schema +and parse item as the direct extract test and so dedups to it. """ from __future__ import annotations @@ -50,12 +52,38 @@ ] INVOICE_NUMBER = "INV-2026-0806" +# One schema for both extract paths — the direct `extract` test and the +# one the schema-build handoff runs. Identical schema + identical parse +# item means the handoff's extraction dedups to the item the earlier test +# already paid for, so the added coverage bills nothing. +SCHEMA_JSON = json.dumps( + { + "type": "object", + "properties": { + "invoice_number": { + "type": "string", + "description": "The invoice number, verbatim.", + } + }, + "required": ["invoice_number"], + } +) + # Generous per-command ceiling: parse/extract poll server-side runs (the # CLI's own --wait default is 600s). A hang, not slowness, is the failure # this guards. COMMAND_TIMEOUT = 900.0 +def _base_env(home: Path) -> dict[str, str]: + """Every ADE_* variable is dropped so a run can't inherit a + developer's ADE_API_KEY/ADE_ENV/ADE_ENDPOINT — the stored credential + written by the login test is the only auth in play.""" + env = {k: v for k, v in os.environ.items() if not k.startswith("ADE_")} + env["ADE_HOME"] = str(home) + return env + + def run_ade( args: list[str], home: Path, @@ -63,22 +91,66 @@ def run_ade( stdin: str | None = None, ) -> subprocess.CompletedProcess[str]: """Run the real CLI out of process, homed at a temp store, targeting - production. Every ADE_* variable is dropped so the run can't inherit a - developer's ADE_API_KEY/ADE_ENV/ADE_ENDPOINT — the stored credential - written by the login test is the only auth in play.""" - env = {k: v for k, v in os.environ.items() if not k.startswith("ADE_")} - env["ADE_HOME"] = str(home) + production.""" return subprocess.run( [sys.executable, "-m", "ade_cli", *args], capture_output=True, text=True, encoding="utf-8", - env=env, + env=_base_env(home), input=stdin, timeout=COMMAND_TIMEOUT, ) +def run_ade_pty( + args: list[str], + home: Path, + *, + responses: str, + extra_env: dict[str, str] | None = None, +) -> tuple[int, str]: + """Run the real CLI attached to a real pty and answer its prompts. + + ``schema build`` refuses to run without a terminal on every stream — + it hands those streams to an interactive agent session — so the only + honest way to drive it is a genuine pty. ``TERM=dumb`` selects the + typed fallback over the arrow-key widget, which makes the exchange + line-oriented: canonical mode buffers ``responses`` until each prompt + reads its line, so nothing has to race the child. + """ + import pty + + env = _base_env(home) + env["TERM"] = "dumb" + env.update(extra_env or {}) + master, slave = pty.openpty() + try: + proc = subprocess.Popen( + [sys.executable, "-m", "ade_cli", *args], + stdin=slave, stdout=slave, stderr=slave, env=env, close_fds=True, + ) + os.close(slave) + slave = -1 + os.write(master, responses.encode()) + chunks: list[bytes] = [] + while True: + try: + data = os.read(master, 4096) + except OSError: # the child closed the pty: EOF on this platform + break + if not data: + break + chunks.append(data) + return proc.wait(timeout=COMMAND_TIMEOUT), b"".join(chunks).decode( + "utf-8", "replace" + ) + finally: + if slave != -1: + os.close(slave) + os.close(master) + + def payload_of(result: subprocess.CompletedProcess[str]) -> dict: assert result.returncode == 0, ( f"exit {result.returncode}\nstdout: {result.stdout}\nstderr: {result.stderr}" @@ -205,21 +277,9 @@ def test_crop_renders_every_element_to_png( def test_extract_pulls_the_invoice_number(parsed: dict, logged_in: Path) -> None: - schema = json.dumps( - { - "type": "object", - "properties": { - "invoice_number": { - "type": "string", - "description": "The invoice number, verbatim.", - } - }, - "required": ["invoice_number"], - } - ) payload = payload_of( run_ade( - ["extract", parsed["job_item_id"], "--schema", schema, "--json"], + ["extract", parsed["job_item_id"], "--schema", SCHEMA_JSON, "--json"], logged_in, ) ) @@ -230,6 +290,105 @@ def test_extract_pulls_the_invoice_number(parsed: dict, logged_in: Path) -> None assert payload["extraction"]["invoice_number"] == INVOICE_NUMBER +@pytest.mark.skipif( + sys.platform == "win32", + reason="pty is POSIX-only; the handoff needs a real terminal to drive", +) +def test_schema_build_hands_off_to_an_agent_and_chains_into_extract( + parsed: dict, logged_in: Path, tmp_path: Path +) -> None: + """The whole parse -> schema -> extract chain, out of process. + + The offline suite fakes the subprocess port, so this is the only test + that exercises the real seam: a real agent executable found on PATH, + spawned attached to a real terminal, writing the skill's + ``deliverable/schema.json`` — then ade's own re-exec running that + schema against production. The stand-in agent is a shell script (the + real skill loop needs a model and a human reviewer); everything ade + owns around it is real. + """ + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + prompt_capture = tmp_path / "seeded-prompt.txt" + agent = fake_bin / "claude" + # Records the prompt it was handed, then leaves the deliverable the + # skill would leave. `$1` is the seeded prompt; cwd is the workspace. + agent.write_text( + "#!/bin/sh\n" + f'printf "%s" "$1" > {prompt_capture}\n' + "mkdir -p deliverable\n" + f"cat > deliverable/schema.json <<'SCHEMA'\n{SCHEMA_JSON}\nSCHEMA\n" + ) + agent.chmod(0o755) + + workspace = tmp_path / "engagement" + code, output = run_ade_pty( + [ + "schema", "build", parsed["job_item_id"], + "--intent", "the invoice number", + "--workspace", str(workspace), + ], + logged_in, + # skip the baseline schema, skip the golden set, run the extract + responses="\n\ny\n", + extra_env={"PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}"}, + ) + assert code == 0, output + + # The agent was launched with everything the skill needs, and nothing + # it must negotiate itself. + seeded = prompt_capture.read_text(encoding="utf-8") + assert parsed["job_item_id"] in seeded # the full id, not a prefix + assert "the invoice number" in seeded + assert parsed["store_dir"] in seeded + assert "never re-parse or pass --force" in seeded + assert str(workspace) in seeded + + # The deliverable was found where the skill leaves it, and named. + schema_path = workspace / "deliverable" / "schema.json" + assert schema_path.is_file() + assert str(schema_path) in output + + # The chained extraction really ran against production: re-running it + # is a free cache hit that lets us assert the payload exactly. + payload = payload_of( + run_ade( + [ + "extract", parsed["job_item_id"], + "--schema", str(schema_path), "--json", + ], + logged_in, + ) + ) + assert payload["cached"] is True # the handoff's own run produced it + assert payload["parse_job_item_id"] == parsed["job_item_id"] + assert payload["extraction"]["invoice_number"] == INVOICE_NUMBER + + +@pytest.mark.skipif( + sys.platform == "win32", reason="pty is POSIX-only (see the test above)" +) +def test_schema_build_without_an_agent_says_how_to_install_one( + parsed: dict, logged_in: Path, tmp_path: Path +) -> None: + """The other half of the real seam: PATH lookup. With no agent CLI + installed the handoff must fail legibly rather than hang or traceback + — and must never reach the store.""" + empty_bin = tmp_path / "empty-bin" + empty_bin.mkdir() + code, output = run_ade_pty( + ["schema", "build", parsed["job_item_id"], "--intent", "the invoice number"], + logged_in, + responses="", + # A PATH with no agent on it (python still resolves: we invoke it + # by absolute path via sys.executable). + extra_env={"PATH": str(empty_bin)}, + ) + assert code == 1, output + assert "no AI agent CLI found" in output + assert "@anthropic-ai/claude-code" in output + + def test_logout_clears_the_credential(logged_in: Path) -> None: """Last in file order — every earlier test rides the stored login.""" payload = payload_of(run_ade(["logout", "--json"], logged_in)) diff --git a/tests/test_parse.py b/tests/test_parse.py index 846f887..8088d1f 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -893,3 +893,15 @@ def test_offer_id_only_stays_silent(cli, document): result = cli.invoke("parse", "-d", str(document), "--id-only", env=AUTH_ENV) assert result.exit_code == 0 assert OFFER not in result.stderr + + +def test_offer_survives_a_corrupt_config(cli, document): + """A hand-edited config.json with invalid JSON must never fail a + parse that already succeeded and billed.""" + offer_tty(cli) + seed_parse(cli, document) + cli.home.mkdir(parents=True, exist_ok=True) + (cli.home / "config.json").write_text("{not valid json") + result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\r"]) + assert result.exit_code == 0, result.stdout + assert OFFER in result.stderr diff --git a/tests/test_schema_build.py b/tests/test_schema_build.py index d62ef91..72aa7ae 100644 --- a/tests/test_schema_build.py +++ b/tests/test_schema_build.py @@ -302,3 +302,220 @@ def test_build_refuses_an_extract_item(tty, document, tmp_path): ) assert result.exit_code == 1 assert json.loads(result.stdout)["error"] == "not_a_parse_item" + + +# ------------------------------------------------- workspace & prompt details + + +def test_workspace_slug_comes_from_the_source_and_is_reused(tty, document): + """The default workspace names the engagement after the document, and + a second run lands in the same directory so the skill resumes rather + than starting a fresh loop.""" + item_id = parse_doc(tty, document) + first = RecordedRuns([deliver_schema]) + tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=first, + ) + second = RecordedRuns([deliver_schema]) + tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=second, + ) + assert first.calls[0][1] == second.calls[0][1] + assert first.calls[0][1].name.startswith("invoice-") + + +def test_explicit_workspace_is_honored(tty, document, tmp_path): + item_id = parse_doc(tty, document) + workspace = tmp_path / "engagement" + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + "--workspace", str(workspace), which=CLAUDE, run=runs, + ) + assert result.exit_code == 0, result.stdout + assert runs.calls[0][1] == workspace + assert json.loads(result.stdout)["workspace"] == str(workspace) + + +def test_url_sourced_parse_still_yields_a_usable_slug(tty): + """A URL source has no filesystem stem to borrow; the workspace name + must still be a clean directory (query strings and slashes stripped).""" + tty.transport.respond(202, {"job_id": JOB_ID}) + tty.transport.respond(200, completed_job()) + parsed = tty.invoke( + "parse", "--document-url", "https://example.com/docs/Q3%20Report.pdf?sig=abc", + "--json", env=AUTH_ENV, + ) + assert parsed.exit_code == 0, parsed.stdout + item_id = json.loads(parsed.stdout)["job_item_id"] + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=runs, + ) + assert result.exit_code == 0, result.stdout + name = runs.calls[0][1].name + assert re.fullmatch(r"[a-z0-9-]+-\d{4}-\d{2}-\d{2}", name), name + + +def test_flags_feed_the_prompt_without_prompting(tty, document, tmp_path): + """--existing-schema and --ground-truth skip their prompts and reach + the agent as the skill's baseline and golden set.""" + item_id = parse_doc(tty, document) + baseline = tmp_path / "baseline.json" + baseline.write_text(json.dumps(SCHEMA)) + golden = tmp_path / "golden.csv" + golden.write_text("doc_id,total\ninvoice.pdf,42\n") + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + "--existing-schema", str(baseline), "--ground-truth", str(golden), + which=CLAUDE, run=runs, input="n\n", # only the extract confirm remains + ) + assert result.exit_code == 0, result.stdout + prompt = runs.calls[0][0][1] + assert str(baseline) in prompt and str(golden) in prompt + + +def test_optional_path_typo_reprompts_instead_of_dropping_it(tty, document, tmp_path): + item_id = parse_doc(tty, document) + baseline = tmp_path / "baseline.json" + baseline.write_text(json.dumps(SCHEMA)) + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=CLAUDE, run=runs, + input=f"/no/such/schema.json\n{baseline}\n\nn\n", + ) + assert result.exit_code == 0, result.stdout + assert "No such file" in result.stderr + assert str(baseline) in runs.calls[0][0][1] + + +def test_prompt_names_a_non_default_environment(tty, document): + """The skill must run its extractions in the parse item's own + environment; production stays unstated (the beaten path).""" + tty.transport.respond(202, {"job_id": JOB_ID}) + tty.transport.respond(200, completed_job()) + staged = tty.invoke( + "parse", "-d", str(document), "--env", "staging", "--json", env=AUTH_ENV + ) + assert staged.exit_code == 0, staged.stdout + item_id = json.loads(staged.stdout)["job_item_id"] + runs = RecordedRuns([deliver_schema]) + tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=runs, + ) + assert "ADE environment: staging" in runs.calls[0][0][1] + + +# --------------------------------------------------------- failure & fallback + + +def test_agent_that_cannot_be_launched_fails_legibly(tty, document): + item_id = parse_doc(tty, document) + + def explode(argv, cwd=None): + raise OSError("Exec format error") + + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=explode, + ) + assert result.exit_code == 1 + payload = json.loads(result.stdout) + assert payload["error"] == "agent_launch_failed" + assert "Exec format error" in payload["message"] + + +def test_extract_exit_code_propagates(tty, document): + """The handoff ends as the extraction ended — a failed extract must + not be reported as a successful build.""" + item_id = parse_doc(tty, document) + runs = RecordedRuns([deliver_schema, 4]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=CLAUDE, run=runs, input="\n\ny\n", + ) + assert result.exit_code == 4 + + +def test_menu_falls_back_to_typed_input_when_raw_mode_fails(tty, document): + """term.Unsupported (a terminal that lies about raw mode) must reach + the numbered prompt, not abort the handoff.""" + item_id = parse_doc(tty, document) + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=BOTH, run=runs, keys=[OSError("no raw mode")], + input="2\n\n\nn\n", # typed agent choice, two skips, decline extract + ) + assert result.exit_code == 0, result.stdout + assert runs.calls[0][0][0] == "codex" + + +def test_dumb_terminal_uses_the_typed_menu(tty, document): + item_id = parse_doc(tty, document) + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=BOTH, run=runs, env={"TERM": "dumb"}, + input="2\n\n\nn\n", + ) + assert result.exit_code == 0, result.stdout + assert runs.calls[0][0][0] == "codex" + + +def test_unwritable_config_does_not_break_the_handoff(tty, document): + """Remembering the menu choice is advisory — a config.json that is a + directory (unwritable) must cost the memory, not the run.""" + item_id = parse_doc(tty, document) + (tty.home).mkdir(parents=True, exist_ok=True) + (tty.home / "config.json").mkdir() + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=BOTH, run=runs, keys=["1", "\r"], input="\n\nn\n", + ) + assert result.exit_code == 0, result.stdout + assert runs.calls[0][0][0] == "claude" + + +def test_unknown_id_is_the_shared_error_shape(tty): + result = tty.invoke( + "schema", "build", "ffffffff", "--json", "--intent", "totals", which=CLAUDE + ) + assert result.exit_code == 1 + assert json.loads(result.stdout)["error"] == "unknown_id" + + +def test_agent_ambiguous_under_json(tty, document): + """--json cannot show a menu, so several agents is a usage error that + names the flag that resolves it.""" + item_id = parse_doc(tty, document) + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", which=BOTH + ) + assert result.exit_code == 2 + payload = json.loads(result.stdout) + assert payload["error"] == "agent_ambiguous" + assert sorted(payload["agents"]) == ["claude", "codex"] + assert "--agent" in payload["message"] + + +def test_corrupt_config_costs_the_preference_not_the_handoff(tty, document): + """config.json is hand-editable; invalid JSON there must not take the + handoff down with it — the agent preference is the only thing lost.""" + item_id = parse_doc(tty, document) + (tty.home).mkdir(parents=True, exist_ok=True) + (tty.home / "config.json").write_text("{not valid json") + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=runs, + ) + assert result.exit_code == 0, result.stdout + assert json.loads(result.stdout)["agent"] == "claude" From 122692f84863dc4b82e4ec50d5a518af5053c21a Mon Sep 17 00:00:00 2001 From: Yuanwen Tian Date: Mon, 10 Aug 2026 14:26:14 +0800 Subject: [PATCH 3/5] =?UTF-8?q?feat:=20post-parse=20offer=20asks=20what=20?= =?UTF-8?q?next=20=E2=80=94=20schema,=20viewer,=20or=20exit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building a schema was the only door the offer opened, but the other thing a human does with a fresh parse is look at it, and a bare "Exit" taught nothing. The menu now offers three choices: build a schema, open the viewer, or exit. Exit stays last and default, so bare Enter still costs no keystroke of the pre-offer behavior -- but it is no longer a dead end. Exiting (and aborting with Esc, and finding no agent installed) restates both commands with a runnable short id, because by then the summary's next: line has scrolled behind a menu and the person who chose Exit is exactly the one who needs them back. The viewer choice rides the same subprocess-of-self re-exec as the extract handoff, so it inherits the terminal and the run ends as `ade view` ended. Co-Authored-By: Claude Fable 5 --- README.md | 6 +- docs/adr/0010-post-parse-agent-handoff.md | 21 ++++-- src/ade_cli/schema_build.py | 83 ++++++++++++++++------- tests/test_parse.py | 63 +++++++++++++++++ 4 files changed, 144 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index dd1d76a..c98952b 100644 --- a/README.md +++ b/README.md @@ -171,8 +171,10 @@ No schema yet? `ade schema build JOB_ITEM_ID` hands a parsed document to an AI-agent session running the [build-schema skill](https://github.com/landing-ai/claude-skills), which authors and validates the schema through real extractions before -delivering it (an interactive parse also offers this step when it -finishes). You describe what to extract; the session negotiates accuracy +delivering it. On an interactive terminal a finished parse also asks what +you want next — build a schema, open the viewer, or exit — and exiting +prints both commands so you can run them yourself later. +You describe what to extract; the session negotiates accuracy targets with you and iterates until the schema converges or is legibly blocked. It needs an agent CLI on your PATH — Claude Code (`claude`, ideally with the `landingai-ade` plugin) or Codex (`codex`); any other diff --git a/docs/adr/0010-post-parse-agent-handoff.md b/docs/adr/0010-post-parse-agent-handoff.md index bc40f66..eee2031 100644 --- a/docs/adr/0010-post-parse-agent-handoff.md +++ b/docs/adr/0010-post-parse-agent-handoff.md @@ -23,10 +23,23 @@ subprocess**, in two entry points sharing one body (`schema_build.py`): - **`ade schema build JOB_ITEM_ID`** — the standalone command, usable on any stored parse. - **A post-parse offer** — after a parse summary (fresh or cached hit) on - a real terminal, an arrow-key menu (ADR-0002's pattern) asks whether to - build a schema now. **Bare Enter exits**: the default preserves the old - behavior keystroke-for-keystroke, and the summary's `next:` line remains - the machine-mode teaching. + a real terminal, an arrow-key menu (ADR-0002's pattern) asks *what + next*, with three choices: build a schema, open the viewer, or exit. + Those are the two things a human actually does with a fresh parse, plus + the way out. + + **Exit is last and the default**, so bare Enter preserves the old + behavior keystroke-for-keystroke. It is not a dead end: exiting (and + aborting with Esc, and finding no agent installed) prints both commands + with a runnable short id, because by then the summary's `next:` line has + scrolled behind a menu, and the person who chose Exit is exactly the one + who needs them restated. The `next:` line itself remains the + machine-mode teaching. + + The viewer choice is the same subprocess-of-self re-exec the extract + handoff uses. A door that cannot open (no agent CLI installed) is + non-fatal — the parse already succeeded and billed; a door that opens + and then fails ends with that command's exit code. ade collects only what it already knows plus the extraction **intent** (and optional existing-schema / ground-truth files); per-field accuracy diff --git a/src/ade_cli/schema_build.py b/src/ade_cli/schema_build.py index c65dc20..40600f5 100644 --- a/src/ade_cli/schema_build.py +++ b/src/ade_cli/schema_build.py @@ -12,10 +12,12 @@ item, the user's extraction intent, and a workspace; when the session ends it looks for the skill's ``deliverable/schema.json`` and offers to run the extract. -- ``offer_after_parse`` is the post-parse doorway into the same flow, - heavily gated: a real terminal on all three streams, no ``--json`` or - ``--id-only``, no agent host or CI (an agent driving ``ade parse`` must - never meet a menu), and not silenced by ``schema_prompt: false``. +- ``offer_after_parse`` is the post-parse doorway — build a schema, open + the viewer, or exit — heavily gated: a real terminal on all three + streams, no ``--json`` or ``--id-only``, no agent host or CI (an agent + driving ``ade parse`` must never meet a menu), and not silenced by + ``schema_prompt: false``. Exit is the default and still teaches: it + restates both commands with a runnable id. ade itself stays network-free here: agents are found on PATH, the skill is acquired by the agent (the seeded prompt says how), and the extract @@ -416,21 +418,47 @@ def build( ) +# Exit last and default: the two doors are opt-in, and bare Enter must +# leave the pre-offer behavior untouched. _OFFER_LABELS = ( "1) Build an extraction schema with an AI agent", - "2) Exit", + "2) Open the viewer — grounded pages and markdown", + "3) Exit", ) +def _farewell(jobs: store.JobStore, item_id: str) -> str: + """What the Exit choice leaves behind: the two commands that reach the + doors just declined, spelled with a runnable id. The summary's + ``next:`` line scrolled past a menu ago — someone who chose Exit + deliberately is exactly who needs them restated.""" + ref = items.short_id(jobs, item_id) + rows = ( + (f"ade schema build {ref}", "build a schema with an AI agent"), + (f"ade view {ref} --open", "open the viewer"), + ) + width = max(len(command) for command, _ in rows) + return "\n".join( + [ + "\nWhenever you want them:", + *(f" {command:<{width}} {what}" for command, what in rows), + ] + ) + + def offer_after_parse( ports: Ports, home: Path, jobs: store.JobStore, item_id: str ) -> None: - """The post-parse doorway into ``run_build`` — only for a human at a - real terminal. Callers pass control only outside ``--json``/ - ``--id-only``; the gates here keep the offer away from pipes, agent - hosts, CI, and anyone who set ``schema_prompt: false``. Bare Enter - exits (the summary's ``next:`` line already printed), so the offer - never costs a keystroke of the old behavior.""" + """The post-parse doorway: build a schema, open the viewer, or exit — + only for a human at a real terminal. Callers pass control only outside + ``--json``/``--id-only``; the gates here keep the offer away from + pipes, agent hosts, CI, and anyone who set ``schema_prompt: false``. + + Exit is last and the default, so bare Enter costs no keystroke of the + old behavior — and it still teaches, restating both commands with a + runnable id. A door that cannot open (no agent CLI installed) is + non-fatal; a door that opens and then fails ends as that command + ended.""" if not ( ports.stdin_is_tty() and ports.stdout_is_tty() and ports.stderr_is_tty() ): @@ -441,33 +469,41 @@ def offer_after_parse( config = _readable_config(home) if config.get("schema_prompt") is False: return + exit_index = len(_OFFER_LABELS) - 1 try: index: int | None = None if os.environ.get("TERM") != "dumb": - typer.echo( - "\nBuild an extraction schema from this parse? (↑/↓ and Enter)", - err=True, - ) + typer.echo("\nWhat next? (↑/↓ and Enter)", err=True) try: index = term.select( - list(_OFFER_LABELS), default=1, getchar=ports.getchar + list(_OFFER_LABELS), default=exit_index, getchar=ports.getchar ) except term.Unsupported: pass # the widget erased itself; the typed fallback re-lists else: - typer.echo("\nBuild an extraction schema from this parse?", err=True) + typer.echo("\nWhat next?", err=True) if index is None: for label in _OFFER_LABELS: typer.echo(f" {label}", err=True) - choice = typer.prompt("Choice", default="2", err=True).strip() - index = 0 if choice == "1" else 1 - if index != 0: + choice = typer.prompt( + "Choice", default=str(exit_index + 1), err=True + ).strip() + index = int(choice) - 1 if choice in ("1", "2", "3") else exit_index + if index == exit_index: + typer.echo(_farewell(jobs, item_id), err=True) return - # A missing agent CLI is non-fatal on this path: the parse - # succeeded and the summary's next: line already serves — the - # handoff just isn't installed yet. + if index == 1: + # The viewer is a re-exec like the extract handoff: it inherits + # this terminal, and the run ends as `ade view` ended. + raise typer.Exit( + code=ports.run_agent(reexec_argv("view", item_id, "--open")) + ) + # A missing agent CLI is non-fatal: the parse succeeded and the + # summary's next: line already serves — the handoff just isn't + # installed yet. if not agents.detect(ports.which, config): typer.echo(agents.install_message(), err=True) + typer.echo(_farewell(jobs, item_id), err=True) return meta = jobs.read_json(item_id, "meta.json") or {} run_build( @@ -483,4 +519,5 @@ def offer_after_parse( # Esc/Ctrl-C declines the optional add-on; the parse already # succeeded and its exit code stays 0. typer.echo("", err=True) + typer.echo(_farewell(jobs, item_id), err=True) return diff --git a/tests/test_parse.py b/tests/test_parse.py index 8088d1f..778790b 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -793,14 +793,77 @@ def test_offer_fires_on_a_tty_and_bare_enter_declines(cli, document): result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\r"]) assert result.exit_code == 0, result.stdout assert OFFER in result.stderr + assert "Open the viewer" in result.stderr # all three doors are offered assert "next:" in result.stdout # the plain hint still printed +def test_exit_restates_both_commands(cli, document): + """Exiting is not a dead end: the menu scrolled the summary's next: + line away, so the choice that declines must hand back both commands + with an id that resolves.""" + offer_tty(cli) + seed_parse(cli, document) + result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\r"]) + assert result.exit_code == 0, result.stdout + assert "ade schema build " in result.stderr + assert "ade view " in result.stderr and "--open" in result.stderr + # The hinted id is runnable, not decorative. + ref = result.stderr.split("ade schema build ", 1)[1].split()[0] + found = cli.invoke("find", "--job", ref, "--json") + assert found.exit_code == 0, found.stdout + + +def test_offer_opens_the_viewer(cli, document): + """The second door: a re-exec of this CLI, so the viewer inherits the + terminal and the run ends as `ade view` ended.""" + import sys as _sys + + from conftest import RecordedRuns + + offer_tty(cli) + seed_parse(cli, document) + runs = RecordedRuns([0]) + result = cli.invoke( + "parse", "-d", str(document), env=AUTH_ENV, keys=["2", "\r"], run=runs + ) + assert result.exit_code == 0, result.stdout + argv, _cwd = runs.calls[0] + assert argv[:3] == [_sys.executable, "-m", "ade_cli"] + assert argv[3] == "view" and argv[-1] == "--open" + + +def test_viewer_failure_ends_the_run_as_view_ended(cli, document): + from conftest import RecordedRuns + + offer_tty(cli) + seed_parse(cli, document) + result = cli.invoke( + "parse", "-d", str(document), env=AUTH_ENV, keys=["2", "\r"], + run=RecordedRuns([3]), + ) + assert result.exit_code == 3 + + +def test_offer_typed_fallback_offers_all_three(cli, document): + """TERM=dumb (and any terminal whose raw mode fails) gets the same + three choices as a numbered prompt.""" + offer_tty(cli) + seed_parse(cli, document) + result = cli.invoke( + "parse", "-d", str(document), env={**AUTH_ENV, "TERM": "dumb"}, input="3\n" + ) + assert result.exit_code == 0, result.stdout + for label in ("1) Build an extraction schema", "2) Open the viewer", "3) Exit"): + assert label in result.stderr + assert "ade schema build " in result.stderr + + def test_offer_esc_declines_without_failing_the_parse(cli, document): offer_tty(cli) seed_parse(cli, document) result = cli.invoke("parse", "-d", str(document), env=AUTH_ENV, keys=["\x1b"]) assert result.exit_code == 0, result.stdout + assert "ade schema build " in result.stderr # aborting still teaches def test_offer_fires_on_the_cached_path_too(cli, document): From 4b8b10b9bb00a05cfcccf4f48fc8dfa9bc3b5386 Mon Sep 17 00:00:00 2001 From: Yuanwen Tian Date: Mon, 10 Aug 2026 14:31:20 +0800 Subject: [PATCH 4/5] docs: state that the schema loop bills credits per iteration `schema build` looks free -- it makes no API call itself -- but the session it launches does: the skill proves every schema version with a real extract run, so a converging loop costs roughly one extraction per iteration per document, not one at the end. That was only discoverable by reading the skill or the invoice. Now stated on every surface a user or agent meets before spending: `ade schema build --help`, the published result note in `ade help --json`, the `workflow` topic, and -- the moment it actually matters -- the launch notice printed just before the agent session starts. The final offered extraction is called out as a normally-free cache hit so the disclosure is not read as double-billing. Co-Authored-By: Claude Fable 5 --- docs/reference/help.json | 13 ++++++++----- src/ade_cli/help.py | 18 +++++++++++++----- src/ade_cli/schema_build.py | 13 ++++++++++++- tests/test_schema_build.py | 30 ++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/docs/reference/help.json b/docs/reference/help.json index a5cce4c..14fad67 100644 --- a/docs/reference/help.json +++ b/docs/reference/help.json @@ -892,7 +892,7 @@ { "name": "schema build", "usage": "ade schema build JOB_ITEM_ID [options] [--json]", - "summary": "Build an extraction schema for a parsed document by handing it to\nan AI-agent session running the build-schema skill (measured\niteration: real extractions, reviewer verdicts, converged-or-blocked\nstops). Ends by naming the delivered schema and offering the extract.", + "summary": "Build an extraction schema for a parsed document by handing it to\nan AI-agent session running the build-schema skill (measured\niteration: real extractions, reviewer verdicts, converged-or-blocked\nstops). Ends by naming the delivered schema and offering the extract.\n\nBills credits: the skill validates every schema version with a real\n`ade extract` run, so a converging loop costs one extraction per\niteration per document (the extraction offered at the end is normally\na free cache hit \u2014 the last iteration already ran it). The session\nreports what it spent; `ade history list` itemises every run.", "arguments": [ { "name": "JOB_ITEM_ID", @@ -970,7 +970,7 @@ "what": "always false \u2014 the extract run is offered interactively, never performed silently" } ], - "note": "Interactive terminals only: the command launches a foreground AI-agent session (claude, codex, or a config-declared CLI) seeded with the parse and your intent. Agents should invoke the build-schema skill directly instead of calling this." + "note": "Interactive terminals only: the command launches a foreground AI-agent session (claude, codex, or a config-declared CLI) seeded with the parse and your intent \u2014 that session runs the build-schema skill, which BILLS CREDITS: it validates every schema version with a real extract run, so a converging loop costs roughly one extraction per iteration per document. The extraction offered at the end is normally a free cache hit (the last iteration already ran it). Agents should invoke the build-schema skill directly instead of calling this." }, "band": "schema authoring \u2014 the agent handoff" }, @@ -1433,9 +1433,12 @@ " batch by find's own filters (--type figure, --all).", "4. schema build author the extract schema when you don't have", " one: hands the parse to an interactive AI-agent", - " session running the build-schema skill (interactive", - " terminals only \u2014 agents should run the skill", - " directly instead).", + " session running the build-schema skill, then offers", + " the extract it delivers. The skill bills credits \u2014", + " it proves each schema version with a real extract", + " run, so expect one extraction per iteration per", + " document (interactive terminals only \u2014 agents should", + " run the skill directly instead).", "5. extract ensure a schema extraction exists for a parse job item", " (or bring-your-own markdown). It becomes its own job", " item, referencing the parse \u2014 artifacts never copied.", diff --git a/src/ade_cli/help.py b/src/ade_cli/help.py index fe24f82..14bd531 100644 --- a/src/ade_cli/help.py +++ b/src/ade_cli/help.py @@ -93,9 +93,12 @@ " batch by find's own filters (--type figure, --all).", "4. schema build author the extract schema when you don't have", " one: hands the parse to an interactive AI-agent", - " session running the build-schema skill (interactive", - " terminals only — agents should run the skill", - " directly instead).", + " session running the build-schema skill, then offers", + " the extract it delivers. The skill bills credits —", + " it proves each schema version with a real extract", + " run, so expect one extraction per iteration per", + " document (interactive terminals only — agents should", + " run the skill directly instead).", "5. extract ensure a schema extraction exists for a parse job item", " (or bring-your-own markdown). It becomes its own job", " item, referencing the parse — artifacts never copied.", @@ -402,8 +405,13 @@ ], "note": "Interactive terminals only: the command launches a " "foreground AI-agent session (claude, codex, or a config-declared " - "CLI) seeded with the parse and your intent. Agents should invoke " - "the build-schema skill directly instead of calling this.", + "CLI) seeded with the parse and your intent — that session runs " + "the build-schema skill, which BILLS CREDITS: it validates every " + "schema version with a real extract run, so a converging loop " + "costs roughly one extraction per iteration per document. The " + "extraction offered at the end is normally a free cache hit (the " + "last iteration already ran it). Agents should invoke the " + "build-schema skill directly instead of calling this.", }, "version": { "shape": "object", diff --git a/src/ade_cli/schema_build.py b/src/ade_cli/schema_build.py index 40600f5..4f44106 100644 --- a/src/ade_cli/schema_build.py +++ b/src/ade_cli/schema_build.py @@ -271,9 +271,14 @@ def run_build( existing_schema=existing_schema, ground_truth=ground_truth, ) + # Said here, not only in --help: this is the last moment before the + # session starts spending. The skill proves every schema version with + # a real extract run, so the loop bills per iteration, not once. typer.echo( f"Launching {agent.name} in {tilde(workspace)} — the schema loop " - "runs in that session; finish it to return here.", + "runs in that session; finish it to return here." + "\nIt bills credits: each iteration runs a real extraction per " + "document (`ade history list` itemises them).", err=True, ) try: @@ -378,6 +383,12 @@ def build( an AI-agent session running the build-schema skill (measured iteration: real extractions, reviewer verdicts, converged-or-blocked stops). Ends by naming the delivered schema and offering the extract. + + Bills credits: the skill validates every schema version with a real + `ade extract` run, so a converging loop costs one extraction per + iteration per document (the extraction offered at the end is normally + a free cache hit — the last iteration already ran it). The session + reports what it spent; `ade history list` itemises every run. """ ports: Ports = ctx.obj home = ade_home() diff --git a/tests/test_schema_build.py b/tests/test_schema_build.py index 72aa7ae..40d2cac 100644 --- a/tests/test_schema_build.py +++ b/tests/test_schema_build.py @@ -519,3 +519,33 @@ def test_corrupt_config_costs_the_preference_not_the_handoff(tty, document): ) assert result.exit_code == 0, result.stdout assert json.loads(result.stdout)["agent"] == "claude" + + +def test_credit_cost_is_stated_before_the_session_starts(tty, document): + """The skill bills per iteration, not once — the user must learn that + before the agent launches, not from the invoice.""" + item_id = parse_doc(tty, document) + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--intent", "totals", + which=CLAUDE, run=runs, input="\n\nn\n", + ) + assert result.exit_code == 0, result.stdout + assert "bills credits" in result.stderr + assert "each iteration" in result.stderr + + +def test_help_warns_that_the_skill_bills_credits(cli): + """Both help surfaces carry the cost: the command's own --help and + the published result contract agents read.""" + scoped = cli.invoke("help", "schema build") + assert scoped.exit_code == 0 + assert "Bills credits" in scoped.stdout + + reference = json.loads(cli.invoke("help", "--json").stdout) + record = next( + c for c in reference["commands"] if c["name"] == "schema build" + ) + assert "BILLS CREDITS" in record["result"]["note"] + workflow = next(t for t in reference["topics"] if t["name"] == "workflow") + assert any("bills credits" in line for line in workflow["body"]) From 4884f6d3aafb87ade810090dfe8837c1d18bab11 Mon Sep 17 00:00:00 2001 From: Yuanwen Tian Date: Mon, 10 Aug 2026 15:41:37 +0800 Subject: [PATCH 5/5] fix: key the default schema workspace by parse item id, not filename alone Copilot review catch (#185): the default workspace was -, so two different documents both named invoice.pdf on the same day shared one workspace -- and the second run could find the first one's deliverable/schema.json and report schema_built for a schema its agent never produced. The item id (a content hash of source x content x params) now keys the default: --. Different documents can never collide however they are named; the same document re-parsed maps to the same id, so resuming an interrupted run still works. An explicit --workspace remains the caller's own choice. Co-Authored-By: Claude Fable 5 --- docs/adr/0010-post-parse-agent-handoff.md | 5 +++- docs/reference/help.json | 2 +- src/ade_cli/schema_build.py | 12 +++++++-- tests/test_schema_build.py | 30 +++++++++++++++++++++-- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/docs/adr/0010-post-parse-agent-handoff.md b/docs/adr/0010-post-parse-agent-handoff.md index eee2031..98858db 100644 --- a/docs/adr/0010-post-parse-agent-handoff.md +++ b/docs/adr/0010-post-parse-agent-handoff.md @@ -45,7 +45,10 @@ ade collects only what it already knows plus the extraction **intent** (and optional existing-schema / ground-truth files); per-field accuracy targets are deliberately *not* collected — the skill's own intake negotiates them in conversation. The seeded prompt carries the full parse -job item id, source, workspace (`/schema-runs/-/` +job item id, source, workspace (`/schema-runs/--/` +by default — the item id, a content hash, keys it so two documents that +share a filename never share a workspace or each other's deliverable, +while re-running the same document resumes by default), and skill-acquisition instructions. When the session ends, ade looks for `deliverable/schema.json`, names it, and offers to run the extract — a re-exec of this CLI (`update.reexec_argv`, shared with the diff --git a/docs/reference/help.json b/docs/reference/help.json index 14fad67..278ae2d 100644 --- a/docs/reference/help.json +++ b/docs/reference/help.json @@ -927,7 +927,7 @@ "metavar": "PATH", "required": false, "default": null, - "help": "Skill workspace directory (default: /schema-runs/-/; an existing one resumes)." + "help": "Skill workspace directory (default: /schema-runs/--/; an existing one resumes)." }, { "flags": "--agent", diff --git a/src/ade_cli/schema_build.py b/src/ade_cli/schema_build.py index 4f44106..1edf7f3 100644 --- a/src/ade_cli/schema_build.py +++ b/src/ade_cli/schema_build.py @@ -257,7 +257,14 @@ def run_build( if workspace is None: date = datetime.fromtimestamp(ports.clock.now()).strftime("%Y-%m-%d") - workspace = home / "schema-runs" / f"{_slug(source)}-{date}" + # The item id (a content hash) keys the default workspace, not just + # the filename: two different documents both named invoice.pdf must + # never share a workspace — a stale deliverable/schema.json from one + # would be reported as the other's. Same document re-parsed → same + # id → the run resumes, which is the reuse actually wanted. + workspace = ( + home / "schema-runs" / f"{_slug(source)}-{item_id[:8]}-{date}" + ) workspace.mkdir(parents=True, exist_ok=True) prompt = agents.seeded_prompt( @@ -370,7 +377,8 @@ def build( workspace: Path | None = typer.Option( None, "--workspace", help="Skill workspace directory (default: " - "/schema-runs/-/; an existing one resumes).", + "/schema-runs/--/; an existing " + "one resumes).", ), agent_name: str | None = typer.Option( None, "--agent", diff --git a/tests/test_schema_build.py b/tests/test_schema_build.py index 40d2cac..a3ba3d8 100644 --- a/tests/test_schema_build.py +++ b/tests/test_schema_build.py @@ -158,7 +158,7 @@ def test_build_launches_the_agent_and_offers_the_extract(tty, document): assert argv[0] == "claude" and len(argv) == 2 assert item_id in argv[1] and "invoice totals" in argv[1] assert cwd is not None and re.fullmatch( - r"invoice-\d{4}-\d{2}-\d{2}", cwd.name + rf"invoice-{item_id[:8]}-\d{{4}}-\d{{2}}-\d{{2}}", cwd.name ), cwd assert cwd.parent == tty.home / "schema-runs" schema_path = cwd / "deliverable" / "schema.json" @@ -326,6 +326,32 @@ def test_workspace_slug_comes_from_the_source_and_is_reused(tty, document): assert first.calls[0][1].name.startswith("invoice-") +def test_same_filename_different_documents_get_distinct_workspaces(tty, tmp_path): + """Filename stem + date alone would collide two different documents + both named invoice.pdf — and the second run would report the first + one's stale deliverable as its own. The item id (a content hash) in + the default workspace name keeps them apart.""" + workspaces = [] + for n, content in enumerate((b"%PDF-1.4 doc one", b"%PDF-1.4 doc two")): + doc_dir = tmp_path / f"client-{n}" + doc_dir.mkdir() + doc = doc_dir / "invoice.pdf" + doc.write_bytes(content) + tty.transport.respond(202, {"job_id": f"job-{n}"}) + tty.transport.respond(200, completed_job(job_id=f"job-{n}")) + parsed = tty.invoke("parse", "-d", str(doc), "--json", env=AUTH_ENV) + assert parsed.exit_code == 0, parsed.stdout + item_id = json.loads(parsed.stdout)["job_item_id"] + runs = RecordedRuns([deliver_schema]) + result = tty.invoke( + "schema", "build", item_id, "--json", "--intent", "totals", + which=CLAUDE, run=runs, + ) + assert result.exit_code == 0, result.stdout + workspaces.append(runs.calls[0][1]) + assert workspaces[0] != workspaces[1] + + def test_explicit_workspace_is_honored(tty, document, tmp_path): item_id = parse_doc(tty, document) workspace = tmp_path / "engagement" @@ -357,7 +383,7 @@ def test_url_sourced_parse_still_yields_a_usable_slug(tty): ) assert result.exit_code == 0, result.stdout name = runs.calls[0][1].name - assert re.fullmatch(r"[a-z0-9-]+-\d{4}-\d{2}-\d{2}", name), name + assert re.fullmatch(r"[a-z0-9-]+-[0-9a-f]{8}-\d{4}-\d{2}-\d{2}", name), name def test_flags_feed_the_prompt_without_prompting(tty, document, tmp_path):