Skip to content

Commit 4ad8fc0

Browse files
committed
Address review: pin spec-invalid hybrid divergences, de-bias benchmark ordering
The discriminator does not reproduce smart-union scoring on spec-invalid hybrids: {method, error} now classifies as a call instead of JSONRPCError, and {method: <non-str>, result} is rejected instead of falling through to JSONRPCResponse. No fixed key-priority order can mirror field-count scoring on these shapes, and the key-presence rule is the safer choice (a malformed frame can no longer masquerade as the error response to a pending request), so pin the divergence with tests and correct the docstring's parity claim to spec-valid messages. Also alternate adapter run order each benchmark round so warm-up and CPU-state effects are not attributed to either adapter; speedups are unchanged (1.6-2.2x).
1 parent 2e506e7 commit 4ad8fc0

3 files changed

Lines changed: 60 additions & 15 deletions

File tree

scripts/bench_jsonrpc_codec.py

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,22 @@
5151
REPEATS = 5
5252

5353

54-
def bench(fn: Callable[[], Any], iterations: int) -> float:
55-
"""Return best-of-REPEATS per-call time in microseconds."""
56-
best = float("inf")
57-
for _ in range(REPEATS):
58-
start = time.perf_counter()
59-
for _ in range(iterations):
60-
fn()
61-
best = min(best, time.perf_counter() - start)
62-
return best / iterations * 1e6
54+
def bench_pair(old_fn: Callable[[], Any], new_fn: Callable[[], Any], iterations: int) -> tuple[float, float]:
55+
"""Return best per-call time in microseconds for each adapter over 2*REPEATS rounds.
56+
57+
Run order alternates each round so warm-up and CPU-state effects are not
58+
attributed to either adapter.
59+
"""
60+
fns = (old_fn, new_fn)
61+
best = [float("inf"), float("inf")]
62+
for round_index in range(2 * REPEATS):
63+
order = (0, 1) if round_index % 2 == 0 else (1, 0)
64+
for key in order:
65+
start = time.perf_counter()
66+
for _ in range(iterations):
67+
fns[key]()
68+
best[key] = min(best[key], time.perf_counter() - start)
69+
return best[0] / iterations * 1e6, best[1] / iterations * 1e6
6370

6471

6572
def _decode_validate_json(adapter: TypeAdapter[Any], body: bytes) -> Any:
@@ -88,8 +95,11 @@ def main() -> None:
8895
print(f"{'payload':<20} {'path':<14} {'smart union':>12} {'discriminator':>14} {'speedup':>8}")
8996
for label, body, iterations in payloads:
9097
for path_label, decode in decoders:
91-
old = bench(lambda: decode(bare_union_adapter, body), iterations)
92-
new = bench(lambda: decode(jsonrpc_message_adapter, body), iterations)
98+
old, new = bench_pair(
99+
lambda: decode(bare_union_adapter, body),
100+
lambda: decode(jsonrpc_message_adapter, body),
101+
iterations,
102+
)
93103
print(f"{label:<20} {path_label:<14} {old:>10.2f}us {new:>12.2f}us {old / new:>7.2f}x")
94104

95105

src/mcp-types/mcp_types/jsonrpc.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,14 @@ def _discriminate_jsonrpc_message(value: Any) -> str | None:
127127
"""Tag a wire object by key presence per JSON-RPC 2.0.
128128
129129
Selects exactly one union branch to validate instead of letting smart-union
130-
mode score all four on every message. Mirrors the smart-union outcomes
131-
exactly, including the rule that a ``method`` member with a missing, null,
132-
or non-int/str ``id`` classifies as a notification, and that ``error`` wins
133-
over ``result`` when both are present.
130+
mode score all four on every message. Classification matches the previous
131+
smart-union outcome for every spec-valid message: a ``method`` member with
132+
a missing, null, or non-int/str ``id`` classifies as a notification
133+
(mirroring ``RequestId``), and ``error`` wins over ``result`` when both are
134+
present. For spec-invalid hybrids that combine ``method`` with ``result``/
135+
``error`` members, the ``method`` key deterministically makes the message a
136+
call; smart-union scoring previously preferred whichever branch matched
137+
more fields, which let a malformed frame classify as an error response.
134138
"""
135139
if isinstance(value, dict):
136140
wire = cast("dict[str, Any]", value)

tests/types/test_jsonrpc_discriminator.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,37 @@ def test_error_wins_over_result() -> None:
6767
assert type(message) is JSONRPCError
6868

6969

70+
@pytest.mark.parametrize(
71+
("request_id", "expected_type"),
72+
[
73+
(1, JSONRPCRequest),
74+
(None, JSONRPCNotification),
75+
],
76+
)
77+
def test_method_wins_over_error(request_id: object, expected_type: type) -> None:
78+
"""A spec-invalid {method, error} hybrid classifies as a call, deliberately diverging from smart union.
79+
80+
Smart-union scoring preferred `JSONRPCError` here (more matching fields),
81+
which let a malformed frame masquerade as an error response to a pending
82+
request. The `method` key now deterministically makes the message a call.
83+
"""
84+
wire: dict[str, Any] = {"jsonrpc": "2.0", "id": request_id, "method": "m", "error": {"code": -1, "message": "x"}}
85+
message = jsonrpc_message_adapter.validate_python(wire, by_name=False)
86+
assert type(message) is expected_type
87+
88+
89+
def test_non_string_method_with_result_is_rejected() -> None:
90+
"""A `method` key selects the call arm even when its value is invalid, deliberately diverging from smart union.
91+
92+
Smart union previously fell through to `JSONRPCResponse` for
93+
{method: 123, result: {}}; the call arm now rejects the non-string method.
94+
"""
95+
wire: dict[str, Any] = {"jsonrpc": "2.0", "id": 1, "method": 123, "result": {}}
96+
with pytest.raises(ValidationError) as exc_info:
97+
jsonrpc_message_adapter.validate_python(wire, by_name=False)
98+
assert exc_info.value.errors()[0]["loc"] == ("request", "method")
99+
100+
70101
@pytest.mark.parametrize(
71102
"unclassifiable",
72103
[

0 commit comments

Comments
 (0)