fix(decisioning): three Emma cross-cutting findings + matrix runner#341
Merged
fix(decisioning): three Emma cross-cutting findings + matrix runner#341
Conversation
… 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>
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>
bokelley
added a commit
that referenced
this pull request
May 1, 2026
ad-tech-protocol-expert called out three follow-ups from PR #341. This PR ships #2 + #3 with concrete fixes; #1 (MCP structuredContent migration) is documented as known-bridge — needs lowlevel Server.call_tool refactor. #3 — Truncation marker buyer-parseability. Pre-fix the MCP ToolError text-suffix path emitted bare '...' as the truncation marker, making buyer json.loads throw JSONDecodeError with no signal of why. Now emits a sentinel object: {_truncated: true, _reason: 'size' | 'non_serializable', _partial: '...'} — ALWAYS valid JSON. Buyers split on '\nDetails: ' prefix, json.loads the tail, and branch on the marker. Iterative fit replaces naive head-room subtract so quote/backslash escaping in _partial doesn't blow past the cap. #2 — caused_by.type Python-only debug breadcrumb. Documented in _internal_error_details that this field is a debug hint for the seller dev reading their own server logs, NOT a cross-language contract. JS sellers won't emit Python-typed exception names. Buyers wanting structured retry/fix/abandon semantics should read 'recovery' (terminal/correctable/transient), which IS the spec contract. #1 — MCP structuredContent migration. Documented as bridge in _to_mcp's docstring with the file:line citation of FastMCP's lossy _make_error_result. The protocol-correct fix needs lowlevel Server.call_tool registration so we can return CallToolResult directly with structuredContent=adcp_error + isError=True; deferred to a separate effort because it touches the FastMCP→ lowlevel boundary throughout serve.py. Tests: 1 updated (truncation now asserts parseable sentinel), 1 new (unserializable details emits sentinel, not malformed JSON). Test count: 2883 (was 2882). 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
Latest Emma matrix run (post-#341/#342) caught three small DX items all four backends agreed on. Fold into PR #342 since it's the same scope (protocol-polish follow-ups). 1. examples/hello_seller.py:222 — wire-field bug. Stub returned {deliveries: [...]} but the schema at schemas/cache/media-buy/get-media-buy-delivery-response.json requires media_buy_deliveries. A buyer copying this example would get INVALID_RESPONSE on call. Sales 9/10 P1. 2. examples/hello_seller.py docstring overclaims. Module docstring said Three platform methods (get_products, create_media_buy, get_media_buy_delivery) but the class actually implements all five required sales-non-guaranteed methods (per REQUIRED_METHODS_PER_SPECIALISM, validated at boot). Trim the claim to match reality. 3. serve() boot log line. Two of four matrix backends flagged silent-boot — adopters had to curl to confirm the listener was up. Now logs one INFO line at the bind boundary: MCP listening on http://host:port/mcp (transport=streamable-http) for MCP, A2A listening on http://0.0.0.0:port/ for A2A. Framework-controlled (uvicorn's startup log can be filtered/ quieted; this line always lands). Test count: 2883 (existing test_get_media_buy_delivery_returns_zeros updated to assert the spec field name; pins drift). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 1, 2026
* fix(server): protocol-polish follow-ups from PR #341 review ad-tech-protocol-expert called out three follow-ups from PR #341. This PR ships #2 + #3 with concrete fixes; #1 (MCP structuredContent migration) is documented as known-bridge — needs lowlevel Server.call_tool refactor. #3 — Truncation marker buyer-parseability. Pre-fix the MCP ToolError text-suffix path emitted bare '...' as the truncation marker, making buyer json.loads throw JSONDecodeError with no signal of why. Now emits a sentinel object: {_truncated: true, _reason: 'size' | 'non_serializable', _partial: '...'} — ALWAYS valid JSON. Buyers split on '\nDetails: ' prefix, json.loads the tail, and branch on the marker. Iterative fit replaces naive head-room subtract so quote/backslash escaping in _partial doesn't blow past the cap. #2 — caused_by.type Python-only debug breadcrumb. Documented in _internal_error_details that this field is a debug hint for the seller dev reading their own server logs, NOT a cross-language contract. JS sellers won't emit Python-typed exception names. Buyers wanting structured retry/fix/abandon semantics should read 'recovery' (terminal/correctable/transient), which IS the spec contract. #1 — MCP structuredContent migration. Documented as bridge in _to_mcp's docstring with the file:line citation of FastMCP's lossy _make_error_result. The protocol-correct fix needs lowlevel Server.call_tool registration so we can return CallToolResult directly with structuredContent=adcp_error + isError=True; deferred to a separate effort because it touches the FastMCP→ lowlevel boundary throughout serve.py. Tests: 1 updated (truncation now asserts parseable sentinel), 1 new (unserializable details emits sentinel, not malformed JSON). Test count: 2883 (was 2882). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: matrix-run findings (boot-log, hello_seller schema, docstring trim) Latest Emma matrix run (post-#341/#342) caught three small DX items all four backends agreed on. Fold into PR #342 since it's the same scope (protocol-polish follow-ups). 1. examples/hello_seller.py:222 — wire-field bug. Stub returned {deliveries: [...]} but the schema at schemas/cache/media-buy/get-media-buy-delivery-response.json requires media_buy_deliveries. A buyer copying this example would get INVALID_RESPONSE on call. Sales 9/10 P1. 2. examples/hello_seller.py docstring overclaims. Module docstring said Three platform methods (get_products, create_media_buy, get_media_buy_delivery) but the class actually implements all five required sales-non-guaranteed methods (per REQUIRED_METHODS_PER_SPECIALISM, validated at boot). Trim the claim to match reality. 3. serve() boot log line. Two of four matrix backends flagged silent-boot — adopters had to curl to confirm the listener was up. Now logs one INFO line at the bind boundary: MCP listening on http://host:port/mcp (transport=streamable-http) for MCP, A2A listening on http://0.0.0.0:port/ for A2A. Framework-controlled (uvicorn's startup log can be filtered/ quieted; this line always lands). Test count: 2883 (existing test_get_media_buy_delivery_returns_zeros updated to assert the spec field name; pins drift). 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
Post-#340 Emma matrix run produced clean reports from 3 of 4 backends (sales 8/10, signals 9.5/10, stability 8/10 — all up from 2/10, 8/10, 5/10 pre-fixes). All three working reports flagged the same three issues:
A1:
ADCPHandler.get_adcp_capabilitiesboot warning. Same TYPE_CHECKING bug we fixed forhandler.pyin PR #338, but inbase.py. The framework's own method warns on every cold boot about an annotation no adopter can fix. Move types to module scope. Same fix shape as #338.A2: Example field-name drift — three examples referenced fields that don't exist on the actual schemas:
hello_seller_signals.py:105—req.signal_id→req.signal_agent_segment_id(silentdep-unknownpre-fix)hello_seller_creative.pydocstring —req.brief/req.format_id→req.message/req.target_format_idhello_seller.py— added "Minimum valid buyer payloads" docstring with workedcreate_media_buyexampleA3: Narrowed ValidationError details on the wire (Stability AI P1). When a platform method raises
ValidationErrordirectly, the wire used to carry "see details for cause" with emptydetails. PR #340's narrowing was server-log only._internal_error_detailsnow special-cases ValidationError and populatesdetails.validation_errorswith narrowed field paths.Bonus:
scripts/run_emma_matrix.shships in the repo (with the--add-dir/--dangerously-skip-permissionsfixes that surfaced these findings).Test plan
test_invoke_validation_error_surfaces_narrowed_field_paths— pins the new wire shapetest_call_tool_invokes_handlerupdated: now sends minimum-valid GetProductsRequest payload (typed dispatch correctly rejects empty dict post-fix)🤖 Generated with Claude Code