From 45ab3ad57f05efd02ac290db8c866e03071df21f Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:46:08 -0700 Subject: [PATCH] fix(delegate_task): return a schema-invalid child's raw text instead of failing the task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a child's final answer still missed its output_schema after the one bounded retry, the result entry flipped to status=failed with the error "Final answer does not satisfy the declared output_schema" — the completion line printed ✗ and orchestrators read a finished audit as a failure. Five audits of 413-4103 s were lost this way in the Sep 10-14 retrospective and the parent had to mine the live transcripts; in four of them the "violation" was a ```json fence around a valid array, which the candidate extractor sliced to its first..last object. Now: status stays completed, `summary` is the child's raw final text, `schema_valid: false` + `schema_errors` carry the verdict and a `schema_note` says the text is unvalidated; the sync completion line shows ⚠ with the reason. The extractor tries the earliest-opening bracket span and keeps the first that parses (fenced arrays validate). The OUTPUT CONTRACT the child sees now says "ONLY the JSON value — no prose, no code fence" and what a miss costs. One bounded retry is unchanged. --- tests/tools/test_delegate_output_schema.py | 52 +++++++------------ tools/AGENTS.md | 2 +- tools/delegate_tool.py | 4 +- tools/delegate_tool_child_run.py | 25 +++++---- tools/delegate_tool_dispatch.py | 6 ++- tools/delegation_output_schema.py | 26 ++++++---- .../docs/user-guide/features/delegation.md | 4 +- 7 files changed, 62 insertions(+), 57 deletions(-) diff --git a/tests/tools/test_delegate_output_schema.py b/tests/tools/test_delegate_output_schema.py index 56ea19d3a2..0d9d3d9d0e 100644 --- a/tests/tools/test_delegate_output_schema.py +++ b/tests/tools/test_delegate_output_schema.py @@ -263,43 +263,31 @@ class TestRunSingleChildSchemaValidation: assert len(child.calls) == 1 assert entry.get("schema_valid") is False - def test_schema_failure_reported_as_failed_not_completed(self): - """Regression: a final answer that still violates the declared - output contract after the bounded retry (here the classic empty - ``{}`` fallback) must be reported status="failed", not - "completed". Otherwise the batch report prints a ✓ and - orchestrators that read only status/icon accept an empty verdict - — schema_valid/schema_errors carry the detail, but status must - agree with them.""" - child = _StubChild(["not json at all", "{}"]) + def test_schema_failure_returns_raw_text_with_schema_valid_false(self): + """A final answer that still violates the contract after the bounded retry is NOT discarded: the + child did the work (audits of 400-4100 s were written off over a stray fence or one missing + field). The parent gets the raw text as ``summary`` with status "completed", ``schema_valid`` + false, ``schema_errors`` populated and a ``schema_note`` saying the text is unvalidated.""" + prose = 'Findings:\n```json\n{"town": "Oslo"}\n```\nlet me know.' + child = _StubChild([prose, prose]) child._delegate_output_schema = ADDRESS_SCHEMA entry = _run(child) + assert entry["status"] == "completed" + assert entry["summary"] == prose assert entry["schema_valid"] is False - assert entry["schema_errors"] - assert entry["status"] == "failed" - # the failed entry names the schema violation, not the generic - # "no response" error — the child DID respond, unusably - assert "output_schema" in entry.get("error", "") - # the invalid final text is still propagated for debugging - assert entry["summary"] == "{}" + assert entry["schema_errors"] and entry["schema_retries"] == 1 + assert "UNVALIDATED" in entry["schema_note"] + assert "error" not in entry + assert len(child.calls) == 2 # exactly one bounded retry - def test_schema_failure_without_retry_reported_as_failed(self): - """Same class, first-try path: retry turn raises, leaving the - original non-JSON answer in place — status must still be failed.""" - child = _StubChild(["nope"]) - child._delegate_output_schema = ADDRESS_SCHEMA - - original = child.run_conversation - - def flaky(user_message, task_id=None, **kw): - if child.calls: - raise RuntimeError("child died on retry") - return original(user_message, task_id=task_id, **kw) - - child.run_conversation = flaky + def test_prose_wrapped_array_answer_validates(self): + """A fenced JSON array with prose around it is the answer, not a violation: the extractor + used to slice it to its first..last object and reject every array-shaped result.""" + text = 'Verdicts below.\n```json\n[{"n": 1}, {"n": 2}]\n```\n' + child = _StubChild([text]) + child._delegate_output_schema = {"type": "array", "items": {"type": "object"}} entry = _run(child) - assert entry["schema_valid"] is False - assert entry["status"] == "failed" + assert entry["schema_valid"] is True and len(child.calls) == 1 def test_schema_valid_entry_still_completed(self): """Guard: schema_valid=True keeps status="completed" untouched.""" diff --git a/tools/AGENTS.md b/tools/AGENTS.md index 41c7775f6d..0f34ecf07b 100644 --- a/tools/AGENTS.md +++ b/tools/AGENTS.md @@ -96,7 +96,7 @@ subagent_auto_approve, inherit_mcp_toolsets, max_iterations`. **Child processes: processes are killed at its teardown and their notices are suppressed in the parent; `process_manage(action="handoff")` (children only) flips `ProcessSession.owner_task_id` to the parent under the registry lock (`process_registry.transfer_ownership`) so the completion routes and reaps by the new owner; un-handed leftovers land on -the result as `orphaned_processes`, exited-but-never-read notify processes as `unread_completions` (`_ChildRun.account_background_processes`, before `cleanup` kills them). **Durability:** background +the result as `orphaned_processes`, exited-but-never-read notify processes as `unread_completions` (`_ChildRun.account_background_processes`, before `cleanup` kills them). **Child kernels:** a child's `execute_code` kernels are keyed `::child::`, pinned against the `max_session_kernels` LRU cap while the child runs and disposed by `cleanup` (`code_kernel.shutdown_kernels_for_delegated_child`) — never let a finished child's kernel squat the cap. **output_schema:** a miss after the one retry keeps `status: completed` with the raw text in `summary` plus `schema_valid: false` / `schema_errors` / `schema_note` — never discard a child's result. **Durability:** background delegation is process-local; work that must survive restart uses `cronjob` or `terminal(background=True, notify_on_complete=True)`. API: `website/docs/developer-guide/subagent-lifecycle-api.md`. diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 272ae09718..3d5b11019f 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -641,8 +641,8 @@ DELEGATE_TASK_SCHEMA = { "object", "Optional JSON Schema this child's final answer must validate against (told to the " "child up front; parent validates with one bounded correction retry; result gains " - "schema_valid, plus schema_errors on failure). Keep it forgiving — require only " - "fields you will read.", + "schema_valid, plus schema_errors on failure — the child's raw text is still returned " + "as summary, never discarded). Keep it forgiving — require only fields you will read.", ), "images": _p( "array", diff --git a/tools/delegate_tool_child_run.py b/tools/delegate_tool_child_run.py index e21fa7c9c3..a9648e4e36 100644 --- a/tools/delegate_tool_child_run.py +++ b/tools/delegate_tool_child_run.py @@ -493,11 +493,13 @@ def _build_result_entry( status, exit_reason = "failed", "error" else: # exit_reason ("completed" vs "max_iterations") tells the parent HOW the task ended; completed=False with no - # failure = budget exhaustion. A declared schema still violated after the bounded retry makes the summary - # unusable under the contract, so status must not say completed (orchestrators reading only status/icon would - # accept an empty verdict). + # failure = budget exhaustion. A declared schema still violated after the bounded retry does NOT fail the + # run: the child's raw final text is the deliverable (audits of up to 68 min were written off as "failed" + # over a stray code fence or one missing field); ``schema_valid: false`` + ``schema_errors`` carry the + # contract verdict, and the summary is prefixed with a notice so a status-only reader cannot mistake it + # for validated output. exit_reason = "completed" if result.get("completed", False) else "max_iterations" - status = "completed" if schema.valid is not False and usable_summary else "failed" + status = "completed" if usable_summary else "failed" _cost = getattr(child, "session_estimated_cost_usd", 0.0) _cost_status = getattr(child, "session_cost_status", None) @@ -528,13 +530,7 @@ def _build_result_entry( entry["cost_usd"] = round(entry["_child_cost_usd"], 6) entry["cost_status"] = _cost_status if isinstance(_cost_status, str) and _cost_status else "unknown" if status == "failed": - if schema.valid is False and usable_summary: - # The child DID respond; name the contract violation instead of the generic "no response" error. - entry["error"] = ( - "Final answer does not satisfy the declared output_schema" + (" (after 1 retry)." if schema.retries else ".") - ) - else: - entry["error"] = result.get("error", "Subagent did not produce a response.") + entry["error"] = result.get("error", "Subagent did not produce a response.") # Classified reason from the child loop (e.g. "rate_limit", "billing") # lets the parent tell a quota wall from a task error without parsing prose. _failure_reason = result.get("failure_reason") @@ -549,6 +545,13 @@ def _build_result_entry( entry["schema_retries"] = schema.retries if not schema.valid and schema.errors: entry["schema_errors"] = schema.errors + if schema.valid is False and usable_summary: + entry["schema_note"] = ( + "Final answer does not satisfy the declared output_schema" + + (" (after 1 retry)" if schema.retries else "") + + "; `summary` is the child's raw, UNVALIDATED final text — extract what you need from it " + "yourself (see schema_errors) rather than re-running the task." + ) # A steer queued after the final assistant turn had no tool batch to land # in; name it so the parent sees it was MISSED rather than silently absorbed. diff --git a/tools/delegate_tool_dispatch.py b/tools/delegate_tool_dispatch.py index 7d52532698..f2043da500 100644 --- a/tools/delegate_tool_dispatch.py +++ b/tools/delegate_tool_dispatch.py @@ -91,8 +91,12 @@ def _report_child_done(parent_agent, spinner_ref, entry, tag, task_labels, n_tas label = task_labels[idx] if idx < len(task_labels) else f"Task {idx}" status = entry.get("status", "?") _slot = f"{tag} · {idx+1}/{n_tasks}" if tag else f"{idx+1}/{n_tasks}" - completion_line = f"{'✓' if status == 'completed' else '✗'} [{_slot}] {label} ({entry.get('duration_seconds', 0)}s)" + schema_invalid = entry.get("schema_valid") is False and status == "completed" + icon = "⚠" if schema_invalid else ("✓" if status == "completed" else "✗") + completion_line = f"{icon} [{_slot}] {label} ({entry.get('duration_seconds', 0)}s)" _err_line = _clean_error_text(entry.get("error"), max_chars=120) if status in SUBAGENT_FAILURE_STATUSES else "" + if schema_invalid: + _err_line = "output_schema not satisfied — raw text returned (schema_valid=false)" if _err_line: completion_line += f" — {_err_line}" _print_completion_line(parent_agent, spinner_ref, completion_line) diff --git a/tools/delegation_output_schema.py b/tools/delegation_output_schema.py index ea9833b1d8..e3f89929df 100644 --- a/tools/delegation_output_schema.py +++ b/tools/delegation_output_schema.py @@ -49,9 +49,10 @@ def append_output_contract(context: Optional[str], schema: Dict[str, Any]) -> st except (TypeError, ValueError): schema_text = str(schema) block = ("OUTPUT CONTRACT (machine-validated):\n" - "Your FINAL response must be a single JSON object that validates " - "against this JSON Schema. No prose before or after the JSON; a " - "```json code fence is acceptable but not required.\n" f"{schema_text}") + "Your FINAL response must be ONLY the JSON value that validates against this JSON " + "Schema — no prose before or after it, no code fence, no explanation. Anything else " + "costs a correction turn and, if it fails again, is handed to the caller unvalidated.\n" + f"{schema_text}") base = (context or "").rstrip() return f"{base}\n\n{block}" if base else block @@ -66,14 +67,21 @@ def extract_json_candidate(text: str) -> str: raw = raw.strip() if raw.lower().startswith("json\n"): raw = raw.split("\n", 1)[1] + # Try each bracket kind's outermost span, earliest opener first, and keep the first that parses: + # checking "{" before "[" unconditionally sliced a fenced array down to its first..last object and + # rejected every valid array answer. + spans = [] for opener, closer in (("{", "}"), ("[", "]")): - if raw.startswith(opener): - return raw - start = raw.find(opener) - end = raw.rfind(closer) + start, end = raw.find(opener), raw.rfind(closer) if start >= 0 and end > start: - return raw[start : end + 1] - return raw + spans.append((start, raw[start : end + 1])) + for _start, candidate in sorted(spans): + try: + json.loads(candidate) + return candidate + except ValueError: + continue + return spans[0][1] if spans else raw def validate_output(text: str, schema: Dict[str, Any]) -> Tuple[bool, List[str]]: diff --git a/website/docs/user-guide/features/delegation.md b/website/docs/user-guide/features/delegation.md index 51f7bb1121..ea8648c50c 100644 --- a/website/docs/user-guide/features/delegation.md +++ b/website/docs/user-guide/features/delegation.md @@ -54,7 +54,9 @@ delegate_task(tasks=[ ## Structured Output (`output_schema`) -Each task can carry an optional `output_schema`, a JSON Schema object the child's final answer must validate against. The child sees the schema up front as an output contract; when the answer comes back the parent validates it, and on failure sends the child exactly one bounded correction turn carrying the validation errors verbatim (the schema is not re-pasted). The task's result then gains `schema_valid` (true/false) and, on failure, `schema_errors`. +Each task can carry an optional `output_schema`, a JSON Schema object the child's final answer must validate against. The child sees the schema up front as an output contract ("return ONLY the JSON value — no prose, no code fence"); when the answer comes back the parent validates it, and on failure sends the child exactly one bounded correction turn carrying the validation errors verbatim (the schema is not re-pasted). The task's result then gains `schema_valid` (true/false) and, on failure, `schema_errors`. + +A contract miss after the retry does **not** discard the child's work: the result keeps `status: completed` with the child's raw final text in `summary`, `schema_valid: false`, the `schema_errors`, and a `schema_note` saying the text is unvalidated. The parent extracts what it needs from the raw text instead of re-running a task that may have taken an hour. Prose or a code fence around otherwise-valid JSON (object or array) is tolerated by the validator. ```python delegate_task(