feat(a2a-response): typed-variant SSOT module + 100% unit coverage (#2967 phase 2) - #2977
Closed
HongmingWang-Rabbit wants to merge 1 commit into
Closed
feat(a2a-response): typed-variant SSOT module + 100% unit coverage (#2967 phase 2)#2977HongmingWang-Rabbit wants to merge 1 commit into
HongmingWang-Rabbit wants to merge 1 commit into
Conversation
…2967 phase 2) First of six PRs implementing the A2AResponse SSOT plan posted at #2967. This PR adds the module + its isolated unit tests; no caller sites change yet (Phase 3 will migrate them). ## What this is `workspace/a2a_response.py` exposes a typed discriminated union (Result | Error | Queued | Malformed) over the workspace-server's A2A response envelopes, plus a pure parser that produces it. The parser is total — never raises — so caller dispatch is variant-on the return value, not try/except over `KeyError`/`TypeError`. ## Why Pre-fix every caller reimplemented `if "result" in data ... elif "error" in data ...` and each had a different blind spot (#2967): - workspace/a2a_client.py:567 — JUST patched with inline poll-mode branch in #2972; works but duplicates the dispatch. - workspace/a2a_cli.py:123 — bare JSON-RPC parser, missing the poll-mode branch entirely; would error on poll-mode peers. - workspace/a2a_cli.py:175 — same shape for tasks/get; same blind spot. Three independent parsers, each diverging slightly. SSOT collapses them. Phase 3 swaps the call sites; this PR just ships the module in isolation so the typed model can be reviewed without cross-cutting caller diffs. ## Wire scope Unchanged. The on-the-wire envelopes the server emits stay identical: - `{"result": {"parts": [...]}}` (JSON-RPC success) - `{"error": {"code": N, "message": "..."}}` (JSON-RPC error) - `{"error": "<string>"}` (gateway error, no code) - `{"status": "queued", "delivery_mode": "poll", "method": "..."}` (poll-mode short-circuit per RFC #2339 PR 2) Pre-typed clients running their own JSON-RPC parser keep working. Only the way internal callers DISPATCH on these envelopes changes. ## Logging Per the issue's "comprehensive logging" requirement, every parse branch logs at structured levels: - Result → DEBUG with parts_len, text_len - Error → INFO with code, message_redacted - Queued → INFO with method, delivery_mode - Malformed → WARNING with raw snippet (operator attention — points at server bug or wire-shape drift) The `target` arg is purely for log traceability; the parser is pure and the return value never depends on it. ## Tests — 65 cases - TestA2AResult (8): text part, empty parts, missing parts key, non-dict result, non-list parts, image-only parts, non-string text, result-takes- precedence-over-error. - TestA2AError (8): code+message, code-only, message-only, code=0 (discriminating against truthy-check shortcut), non-int code, gateway- string error, non-dict-non-string error value, message strip. - TestA2AQueued (6): canonical envelope, other method (sendStream), missing delivery_mode → Malformed, non-poll delivery_mode → Malformed, status="dispatched" → Malformed, missing method defaults to message/send. - TestA2AMalformed (8): empty dict, unrelated dict, non-dict roots (string, list, None, int), snippet cap behavior. - TestFuzz (3): 35 known adversarial parametrize cases + 200 random JSON-shaped payloads (RNG-seeded for reproducibility) + 10MB string edge case. - TestIssue2967ProductionRepro: pins the parser against the EXACT envelope captured from reno-stars on 2026-05-05. ## Discrimination — verified Replaced parse_a2a_response body with `return A2AMalformed(raw_snippet="forced")`, re-ran the suite. Result: 24 of 65 tests FAILED (every test that asserts a specific non-Malformed variant). Restored real parser → all 65 pass. This proves the test suite would catch a regression that always falls through to the catch-all — the failure mode that #2967 actually exhibited in production. Per memory feedback_assert_exact_not_substring. Coverage: a2a_response.py reports "1 file skipped due to complete coverage" — every line + branch hit by the unit suite. ## Out of scope (future PRs) - Phase 3 (#86): swap a2a_client.send_a2a_message + a2a_cli.py to use the typed dispatch. Replaces #2972's inline branch. - Phase 4 (#87): branch-coverage check + hypothesis-style adversarial property test on the parser. - Phase 5 (#88): real-Postgres integration test of A→B(poll) round-trip. - Phase 6 (#89): real-subprocess E2E in e2e-api.yml. - Phase 7 (#90): ship + verify on reno-stars. - Go-side typed constructors (server emit sites): deferred to a Phase 2b follow-up. The Python parser is what fixes the visible symptom; Go emit sites already produce the right wire shape. Refs #2967.
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
May 6, 2026 00:24
HongmingWang-Rabbit
enabled auto-merge
May 6, 2026 00:25
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import json | ||
| import random | ||
| import string |
Contributor
Author
|
Superseded by #2979. #2979 covers everything in #2977 (typed model + 100% coverage on a2a_response.py) AND additionally lands Phase 3 (caller migration in send_a2a_message + a2a_tools_delegation auto-fallback) AND starts Phase 6 (e2e poll-mode test). Both PRs stage workspace/a2a_response.py at the same path, so only one can land — going with the more comprehensive one. Phase 2 module work isn't lost — #2979's a2a_response.py is feature-equivalent to mine. The 6-PR plan from the #2967 design comment collapses to fewer PRs because #2979 bundled what I'd planned to ship as Phases 2+3+(start of)6 separately. Tasks 85 + 86 will close on #2979 merge. |
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
First of six PRs implementing the A2AResponse SSOT plan from #2967 design comment. This PR ships the typed-variant module + its isolated unit tests; no caller sites change yet.
workspace/a2a_response.pyexposes a typed discriminated union (Result | Error | Queued | Malformed) over the server's A2A response envelopes, plus a pure parser that produces it. Pre-fix three independent parsers in the codebase each had a different blind spot — Phase 3 will migrate them to this single dispatch.Wire shapes unchanged. Pre-typed clients keep working; only internal caller DISPATCH centralizes here.
Logging
Comprehensive structured logging at every branch:
Tests — 65, all 100% module coverage
Discrimination verified: Replaced
parse_a2a_responsebody with always-Malformed, re-ran suite — 24 of 65 tests FAILED (every test that pins a specific non-Malformed variant). Restored real parser → all 65 pass. Per memoryfeedback_assert_exact_not_substring.Coverage report:
1 file skipped due to complete coverage— every line hit.Out of scope (future PRs)
🤖 Generated with Claude Code