From ec91a93de3f59b489c947d1bb8c07258dd0e83b2 Mon Sep 17 00:00:00 2001 From: Zhichao Date: Wed, 19 Aug 2026 12:36:06 +0800 Subject: [PATCH] fix: extract --json always carries the extraction inline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documented contract is that `ade extract --json` puts the whole result on stdout: the payload's `extraction` key is the schema-shaped result itself, not a pointer to `/extract.json`. Reported (#189) as a fresh-vs-cached shape divergence, with downstream tooling breaking one step later on a payload with no `extraction` key. Both paths already go through one `emit_summary`, and the payload's keys are identical on a fresh completion and a cached hit — checked across every input form (parse item, `extract -d` reuse, `extract -d` parse-first, `--markdown`, `--markdown-url`) and a forced re-extract. The new parity tests pin that, so no refactor can split the two paths again; the only key that legitimately differs is the parse provenance of the invocation (`parsed_first` on a fresh `extract -d` that had to parse, `reused_parse` on its re-run). The one way a successful run could still reach stdout with no result in it was a completed body whose `result` carried no `extraction`: the summary read it with `.get`, so the run reported `status: extracted`, billed, wrote an `extract.json` with no result in it, and printed `extraction: null`. That now fails whole in the consume-before-write block, the same posture as any other unreadable completion — the ticket is marked unreadable with the reason and no artifact is written. `ade help extract --json` now says outright that `extraction` is inline on every successful run, cached hits included. Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference/help.json | 2 +- src/ade_cli/extract.py | 12 ++++ src/ade_cli/help.py | 3 +- tests/test_extract.py | 123 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 2 deletions(-) diff --git a/docs/reference/help.json b/docs/reference/help.json index 9a62397..6aa46f1 100644 --- a/docs/reference/help.json +++ b/docs/reference/help.json @@ -835,7 +835,7 @@ }, { "key": "extraction", - "what": "THE RESULT: the schema-shaped object, verbatim" + "what": "THE RESULT: the schema-shaped object, verbatim \u2014 inline on every successful run, cached hits included" }, { "key": "fields", diff --git a/src/ade_cli/extract.py b/src/ade_cli/extract.py index 37894c3..4b3cda5 100644 --- a/src/ade_cli/extract.py +++ b/src/ade_cli/extract.py @@ -757,6 +757,18 @@ def post(ticket: dict) -> httpx.Response: billing["service_tier"], meta.get("model_version") or meta.get("version"), ) + if not isinstance(data["extraction"], (dict, list)): + # The result itself (#189): `extraction` is what the run was + # for, and stdout carries it whole on every path — so a + # completed body that carries no schema-shaped result is not a + # success this CLI can serve. Failing here keeps it out of the + # store and off stdout, instead of reporting `extraction: null` + # beside a billed run and a payload no consumer can read. A + # missing key raises KeyError into the same handler. + raise TypeError( + "extraction is " + f"{type(data['extraction']).__name__}, not an object or array" + ) if markdown_url is not None and not isinstance(data.get("markdown"), str): # The URL form's markdown.md materializes from the response's # echo — the CLI never had a local file. A missing or non-string diff --git a/src/ade_cli/help.py b/src/ade_cli/help.py index 54a797e..5f180c2 100644 --- a/src/ade_cli/help.py +++ b/src/ade_cli/help.py @@ -213,7 +213,8 @@ ("version", "resolved extract model version"), ("credits", "credits billed (0 on a cached hit)"), ("tier", "service tier the run was billed at"), - ("extraction", "THE RESULT: the schema-shaped object, verbatim"), + ("extraction", "THE RESULT: the schema-shaped object, verbatim — " + "inline on every successful run, cached hits included"), ("fields", "number of leaf fields"), ("ungroundable", "field paths whose non-empty value has no box"), ("empty_fields", "field paths with no value (nothing to ground)"), diff --git a/tests/test_extract.py b/tests/test_extract.py index 09ad4cd..4162117 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -830,6 +830,129 @@ def test_extract_json_carries_the_contract_fields(cli, document, schema_file): assert payload["extraction"] == stored["extraction"] +# --- payload shape parity (#189): one summary, both paths --- + +# How this invocation got its parse is a property of the invocation, not of +# the payload's shape: a fresh `extract -d` that had to parse first reports +# `parsed_first`, and its cached re-run reports the `reused_parse` it now +# finds. Every other key is the same on both paths. +PARSE_PROVENANCE = {"parsed_first", "reused_parse"} + + +def assert_same_shape(fresh, cached): + """Fresh and cached `--json` payloads are the same payload — same keys, + same values, `cached` apart. The extraction rides inline on both: the + documented contract is that stdout carries the whole result, so a fresh + run may never reduce to a summary that sends the caller to disk (#189). + """ + assert set(fresh) - PARSE_PROVENANCE == set(cached) - PARSE_PROVENANCE + assert {k for k in set(fresh) & set(cached) if fresh[k] != cached[k]} == {"cached"} + assert fresh["cached"] is False and cached["cached"] is True + assert isinstance(fresh["extraction"], dict) and fresh["extraction"] + + +def extract_twice(cli, *args, result=None): + """One invocation run fresh, then re-run once the item is cached.""" + fresh = complete_extract(cli, *args, result=result) + again = cli.invoke("extract", *args, "--json", env=AUTH_ENV) + assert again.exit_code == 0, again.stdout + return fresh, json.loads(again.stdout) + + +def test_fresh_and_cached_payloads_are_the_same_shape(cli, document, schema_file): + parse_id = parse_doc(cli, document) + + fresh, cached = extract_twice(cli, parse_id, "--schema", str(schema_file)) + + assert_same_shape(fresh, cached) + (extract_dir,) = extract_item_dirs(cli) + stored = json.loads((extract_dir / "extract.json").read_text()) + assert fresh["extraction"] == cached["extraction"] == stored["extraction"] + + +def test_fresh_and_cached_markdown_payloads_are_the_same_shape( + cli, tmp_path, schema_file +): + # Bring-your-own markdown: spans-only evidence, no parse referenced — + # the degraded join must not change which keys the payload carries. + notes = tmp_path / "notes.md" + notes.write_text(MARKDOWN) + + fresh, cached = extract_twice(cli, "--markdown", str(notes), "--schema", str(schema_file)) + + assert_same_shape(fresh, cached) + assert fresh["evidence"]["kind"] == cached["evidence"]["kind"] == "spans_only" + + +def test_fresh_and_cached_parse_first_payloads_are_the_same_shape( + cli, document, schema_file +): + # `extract -d` with nothing to reuse runs two billable jobs; the cached + # re-run reuses the parse it minted. Only the parse-provenance key + # differs — the extraction is inline on both. + cli.transport.respond(202, {"job_id": JOB_ID}) + cli.transport.respond(200, completed_job()) + + fresh, cached = extract_twice(cli, "-d", str(document), "--schema", str(schema_file)) + + assert_same_shape(fresh, cached) + assert "parsed_first" in fresh and "reused_parse" not in fresh + assert "reused_parse" in cached and "parsed_first" not in cached + + +def test_a_forced_re_extract_payload_is_the_same_shape_as_its_cached_re_run( + cli, document, schema_file +): + parse_id = parse_doc(cli, document) + complete_extract(cli, parse_id, "--schema", str(schema_file)) + + forced = complete_extract( + cli, parse_id, "--schema", str(schema_file), "--force", job_id="extract-0002" + ) + again = cli.invoke( + "extract", parse_id, "--schema", str(schema_file), "--json", env=AUTH_ENV + ) + + assert again.exit_code == 0 + assert_same_shape(forced, json.loads(again.stdout)) + + +@pytest.mark.parametrize( + "mangle", + [ + # A completed body that carries no result to serve: the key gone + # (a contract rename), or present and empty (a null result). + lambda result: result.pop("extraction"), + lambda result: result.update(extraction=None), + ], +) +def test_a_result_without_an_extraction_fails_whole_before_any_write( + cli, document, schema_file, mangle +): + parse_id = parse_doc(cli, document) + drifted = extract_result() + mangle(drifted) + cli.transport.respond(202, {"job_id": "extract-0001"}) + cli.transport.respond(200, completed_extract_job(drifted)) + + result = cli.invoke( + "extract", parse_id, "--schema", str(schema_file), "--json", env=AUTH_ENV + ) + + # Never a success whose payload has no result in it: rejected whole, + # named, and recoverable — the same posture as any other unreadable + # completion (#189). + assert result.exit_code == 1 + payload = json.loads(result.stdout) + assert payload["error"] == "unsupported_result_schema" + (extract_dir,) = extract_item_dirs(cli) + assert not (extract_dir / "extract.json").exists() + assert not (extract_dir / "meta.json").exists() + ticket = json.loads((extract_dir / "job.json").read_text()) + assert ticket["state"] == "unreadable" + assert "extraction" in ticket["reason"] + + # --- partial extraction (#118): a reduced result is labeled, never silent --- VIOLATION = (