Conversation
…tended variant Stability AI Emma backend test (verdict 5/10) flagged that constructing a CreativeManifest whose assets value matched one variant (e.g. ImageAsset) but was missing fields required by THAT variant produced a 60-line pydantic ValidationError listing every variant of the asset content union (13+ variants, 26 errors). The user's actual mistake (one missing field on the variant they picked) was buried. adcp.types.error_narrowing.narrow_union_errors post-processes ValidationError.errors() to keep only errors from the closest-fit variant. Strategy: 1. Discriminator match — variants with no literal_error / union_tag_not_found had their discriminator value match the user's input. If exactly one such variant exists, surface only its errors. Stability case: ImageAsset. 2. Fewest-errors fallback — when no clear winner, pick the variant with fewest non-literal errors as a closest-fit guess. 3. Tie → pass through all errors so the adopter can disambiguate. Wired into create_tool_caller's INVALID_REQUEST projection so wire-side validation errors get the narrowing automatically. Adopters can also call narrow_union_errors manually on a caught ValidationError. Before/after: BEFORE: 26 errors (every variant of the asset union) AFTER: 2 errors (assets.hero.ImageAsset.width / .height) Tests: 6 new. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two reviewers (code-reviewer, python-expert) flagged five P0/P1s:
P0 (python-expert): pydantic upper bound. The narrowing heuristic
depends on pydantic-2 ValidationError.errors() internals (err["type"]
literals, CamelCase variant names interleaved in err["loc"]).
Pydantic 3 makes no API guarantee on these. Pin <3.
P1 (code-reviewer): missing union_tag_invalid in mismatch types.
Sibling of union_tag_not_found that pydantic-2 emits when a tag is
found but invalid. Without it, a variant with that error stays in
the candidate pool and may falsely win.
P1 (python-expert): _split_at_variant used FIRST CamelCase segment.
Nested unions (Union[Outer[Union[A, B]], C]) emit loc like
("field", "Outer", "inner", "A", "subfield"); splitting at "Outer"
collapsed A and B into one bucket. Switch to LAST variant segment
(innermost wins).
P1 (code-reviewer): narrowing call was unguarded. A bug in the
heuristic would 500 the wire path. Wrap in try/except in the
INVALID_REQUEST projection; fall back to unfiltered errors with
WARNING log.
P1 (python-expert): defensive copy of returned dicts. Caller
mutation could leak back into pydantic's internal error list.
``dict(err)`` per-error at the boundary; cheap insurance.
P2 cleanup:
- Lift import out of hot path to module top.
- DoS guard: cap input at 500 errors (narrowing is UX, not
correctness; an attacker shouldn't get to amplify CPU through
bucketing).
- Document residual literal_error overloading edge case.
Tests: 4 new (union_tag_invalid mismatch handling, nested-union
innermost-variant resolution, defensive-copy guard, DoS cap).
Deferred to follow-up (per python-expert): schema-level fix —
teach codegen to emit Annotated[Union[...], Field(discriminator=...)]
so pydantic narrows correctly without our post-processor.
Test count: 2878 passed (was 2874 — +4 hardening tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
bokelley
added a commit
that referenced
this pull request
May 1, 2026
…341) * fix(decisioning): three Emma cross-cutting findings (post-#340 matrix run) The post-#340 Emma matrix run (sales 8/10, signals 9.5/10, stability 8/10) surfaced three items every working backend flagged: A1 — base.py TYPE_CHECKING leak. ``ADCPHandler.get_adcp_capabilities`` (and every other framework baseline method) had its Request type imported under ``if TYPE_CHECKING:``. The dispatcher's ``_resolve_params_pydantic_model`` calls ``get_type_hints`` at boot; forward refs failed, ``NameError`` got swallowed, every adopter saw ``"typed params annotation failed to resolve … unresolved name: GetAdcpCapabilitiesRequest"`` on cold boot. The remediation text told adopters to import a model — but they CAN'T, it's not their method. Same bug class as PR #338's handler.py fix; we missed base.py because the framework methods inherit ``not_supported`` defaults so the wire-failure mode was invisible until #338's resolver-warning bump made it console noise. A2 — example field-name drift. Three examples referenced fields that don't exist on the actual schemas: * ``hello_seller_signals.py:105`` read ``req.signal_id``; ``ActivateSignalRequest`` carries ``signal_agent_segment_id``. Pre- fix: silent ``dep-unknown`` deployments. Now correctly reads the spec field AND iterates buyer-supplied ``destinations``. * ``hello_seller_creative.py`` docstring said ``req.brief`` / ``req.format_id``; ``BuildCreativeRequest`` actually carries ``message`` / ``target_format_id``. * ``hello_seller.py`` lacked a buyer-side request example. Adopters copy-pasted seller stub and got INVALID_REQUEST. Added a "Minimum valid buyer payloads" docstring section with a worked ``create_media_buy`` payload. A3 — narrowed ValidationError details on the wire. When a platform method raises ``pydantic.ValidationError`` directly (typically the seller's code constructed an invalid response), the wire used to say "see details for cause" with empty details. PR #340's narrowing was server-log only. ``_internal_error_details`` now special-cases ValidationError and populates ``details.validation_errors`` with the narrowed field-path list (filtered through ``narrow_union_errors``). Defensive: a narrowing bug never 500s the wire — fall back silently to ``caused_by`` only. Plus: ``scripts/run_emma_matrix.sh`` (the runner that surfaced these findings) ships in the repo for future re-validation. Tests: 1 new dispatch test pinning ValidationError surfacing. ``test_call_tool_invokes_handler`` updated to send a minimum-valid ``GetProductsRequest`` payload now that typed dispatch correctly rejects empty dicts. Test count: 2879 passed (was 2878). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(server): surface AdcpError details on MCP wire (AudioStack Emma P0) Decisioning AdcpError isn't a subclass of adcp.exceptions.ADCPError, so 'except ADCPError' in serve.py didn't catch it — propagated to FastMCP's default handler and details was silently dropped on the MCP wire (A2A buyers got it via data.details, MCP buyers got nothing). Three-part fix: 1. translate.py:translate_error — recognize decisioning AdcpError (lazy import) and extract code/message/suggestion/recovery/details/ field. ADCPTaskError branch also lifts errors[0].details so create_tool_caller's INVALID_REQUEST projection (with #341's narrowed validation_errors) reaches MCP buyers. 2. translate.py:_to_mcp — accept details and serialise as a '\nDetails: <json>' suffix in the ToolError text payload. Truncated to 8 KB. 3. serve.py:_call_handler — follow-on except branch catches decisioning AdcpError (lazy import + isinstance) and routes through translate_error. Tests: 3 new pinning caused_by + validation_errors surfacing and the 8 KB truncation. Test count: 2882 passed (was 2879). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stability AI Emma backend test (verdict 5/10) flagged: constructing a
CreativeManifestwith one missing field on a discriminated-union asset produced a 60-lineValidationErrorcovering every variant (13+ asset types, 26 errors). The user's actual mistake — one missing field on ImageAsset — was buried.Adtech-product-expert called this "next-PR-worthy" specifically: "every creative adopter hits it the moment they wire a real generation API."
What changed
adcp/types/error_narrowing.py— new utilitynarrow_union_errors(errors)that post-processesValidationError.errors()to keep only the closest-fit variant's errors:literal_error/union_tag_not_foundmatched the user's discriminator value. Surface only their errors.adcp/server/mcp_tools.py— wired intocreate_tool_caller'sINVALID_REQUESTprojection so wire-side validation errors get narrowed automatically.Before / after
Test plan
🤖 Generated with Claude Code