Skip to content

Commit c2bd5bf

Browse files
committed
python: decode boolean-discriminated unions
The Python generator captured a union discriminator's JSON Schema `const` with `String()`, so a boolean const became the string "true"/"false" and the emitted dispatcher matched `case "true":`. A JSON boolean decodes to Python `True`, which never equals `"true"`, so every boolean-discriminated union fell through to `raise ValueError`. Two unions are affected. `sessions.list()` raised `ValueError: Unknown SessionListEntry isRemote: False` for any non-empty session list, and `QueuedCommandHandled.to_dict()` put the string `"true"` on the wire where the schema declares `{"type": "boolean", "const": true}`. Keep the const's JSON type through codegen and render it as a Python literal (`True`/`False`), annotating the discriminator `ClassVar` as `bool`. This mirrors how `go.ts` already models discriminator values. Regenerating changes six lines of `python/copilot/generated/rpc.py`; no other language changes.
1 parent aa4e707 commit c2bd5bf

3 files changed

Lines changed: 133 additions & 17 deletions

File tree

python/copilot/generated/rpc.py

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

python/test_rpc_generated.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
"""Tests for generated RPC method behavior."""
22

3+
import json
34
from unittest.mock import AsyncMock
45

56
import pytest
67

78
from copilot.rpc import (
89
CommandsApi,
910
CommandsInvokeRequest,
11+
CommandsRespondToQueuedCommandRequest,
12+
LocalSessionMetadataValue,
13+
QueuedCommandHandled,
14+
QueuedCommandNotHandled,
15+
RemoteControlStatusOff,
16+
RemoteControlStatusResult,
17+
RemoteSessionMetadataValue,
18+
SessionList,
1019
SlashCommandTextResult,
1120
)
1221

@@ -22,3 +31,77 @@ async def test_commands_invoke_deserializes_slash_command_result():
2231
assert isinstance(result, SlashCommandTextResult)
2332
assert result.text == "hello"
2433
assert result.markdown is True
34+
35+
36+
def test_remote_control_status_deserializes_string_discriminated_union():
37+
result = RemoteControlStatusResult.from_dict({"status": {"state": "off"}})
38+
39+
assert isinstance(result.status, RemoteControlStatusOff)
40+
assert result.status.state == "off"
41+
assert result.status.to_dict() == {"state": "off"}
42+
43+
44+
def test_session_list_deserializes_boolean_discriminated_entries():
45+
payload = {
46+
"sessions": [
47+
{
48+
"sessionId": "example-local",
49+
"startTime": "2026-07-26T10:00:00.000Z",
50+
"modifiedTime": "2026-07-26T10:05:00.000Z",
51+
"isRemote": False,
52+
},
53+
{
54+
"sessionId": "example-remote",
55+
"startTime": "2026-07-26T11:00:00.000Z",
56+
"modifiedTime": "2026-07-26T11:05:00.000Z",
57+
"isRemote": True,
58+
"remoteSessionIds": ["example-remote"],
59+
"repository": {"owner": "github", "name": "copilot-sdk", "branch": "main"},
60+
},
61+
]
62+
}
63+
64+
result = SessionList.from_dict(payload)
65+
66+
local, remote = result.sessions
67+
assert isinstance(local, LocalSessionMetadataValue)
68+
assert local.session_id == "example-local"
69+
assert local.is_remote is False
70+
assert isinstance(remote, RemoteSessionMetadataValue)
71+
assert remote.session_id == "example-remote"
72+
assert remote.is_remote is True
73+
assert remote.repository.owner == "github"
74+
75+
76+
@pytest.mark.parametrize(
77+
("handled", "expected_type"),
78+
[(True, QueuedCommandHandled), (False, QueuedCommandNotHandled)],
79+
)
80+
def test_queued_command_result_deserializes_boolean_discriminator(handled, expected_type):
81+
request = CommandsRespondToQueuedCommandRequest.from_dict(
82+
{"requestId": "example-request", "result": {"handled": handled}}
83+
)
84+
85+
assert isinstance(request.result, expected_type)
86+
87+
88+
@pytest.mark.parametrize(
89+
("variant", "expected_handled", "expected_json"),
90+
[
91+
(QueuedCommandHandled(), True, '{"handled": true}'),
92+
(QueuedCommandNotHandled(), False, '{"handled": false}'),
93+
],
94+
)
95+
def test_queued_command_result_serializes_boolean_discriminator(
96+
variant, expected_handled, expected_json
97+
):
98+
encoded = variant.to_dict()
99+
100+
assert encoded["handled"] is expected_handled
101+
assert json.dumps(encoded) == expected_json
102+
103+
request = CommandsRespondToQueuedCommandRequest(request_id="example-request", result=variant)
104+
round_tripped = CommandsRespondToQueuedCommandRequest.from_dict(request.to_dict())
105+
106+
assert request.to_dict()["result"]["handled"] is expected_handled
107+
assert isinstance(round_tripped.result, type(variant))

scripts/codegen/python.ts

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,39 @@ function postProcessExternalUnionAliasesForPython(code: string, aliases: Map<str
272272
return code.replace(/\n{3,}/g, "\n\n");
273273
}
274274

275+
/**
276+
* Literal value of a union discriminator. The API schema discriminates on
277+
* string `const`s (`kind: "text"`) and on boolean ones
278+
* (`SessionListEntry.isRemote`, `QueuedCommandResult.handled`), so the JSON
279+
* type has to survive codegen: coercing `true` to `"true"` emits a dispatcher
280+
* arm that the decoded Python `True` can never match. Mirrors
281+
* `GoDiscriminatorValue` in `go.ts`.
282+
*/
283+
type PyDiscriminatorValue = string | boolean;
284+
285+
/**
286+
* Capture a schema `const` as a discriminator value, keeping booleans as
287+
* booleans and stringifying everything else.
288+
*/
289+
function pyDiscriminatorValue(constValue: unknown): PyDiscriminatorValue {
290+
return typeof constValue === "boolean" ? constValue : String(constValue);
291+
}
292+
293+
/**
294+
* Render a discriminator value as a Python literal. Booleans need Python's
295+
* `True` / `False` spelling, since the JSON `true` would parse as a capture
296+
* pattern in a `match` arm rather than as a literal.
297+
*/
298+
function pyDiscriminatorValueExpr(value: PyDiscriminatorValue): string {
299+
if (typeof value === "boolean") return value ? "True" : "False";
300+
return JSON.stringify(value);
301+
}
302+
303+
/** Python type of a discriminator constant, for its `ClassVar` annotation. */
304+
function pyDiscriminatorValueType(value: PyDiscriminatorValue): string {
305+
return typeof value === "boolean" ? "bool" : "str";
306+
}
307+
275308
/**
276309
* Replace flat-merged dataclasses emitted by quicktype for $ref-based
277310
* discriminated unions with proper Python unions: a `Name = VariantA | ...`
@@ -293,7 +326,7 @@ function postProcessExternalUnionAliasesForPython(code: string, aliases: Map<str
293326
interface ResolvedRefBasedUnion {
294327
aliasName: string;
295328
discriminatorProp: string;
296-
dispatch: Array<{ value: string; typeName: string }>;
329+
dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>;
297330
}
298331
function postProcessRefBasedDiscriminatedUnionsForPython(
299332
code: string,
@@ -304,7 +337,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
304337
aliasName: string;
305338
variantNames: string[];
306339
discriminatorProp: string;
307-
dispatch: Array<{ value: string; typeName: string }>;
340+
dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>;
308341
description: string | undefined;
309342
}
310343
const unions: UnionInfo[] = [];
@@ -334,7 +367,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
334367
discriminator.property
335368
];
336369
return {
337-
value: String(discProp.const),
370+
value: pyDiscriminatorValue(discProp.const),
338371
typeName: toPascalCase(variantRefNames[i]),
339372
};
340373
});
@@ -387,7 +420,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
387420
for (const union of unions) {
388421
const actualAliasName = resolveActualName(union.aliasName);
389422
const actualVariantNames: string[] = [];
390-
const actualDispatch: Array<{ value: string; typeName: string }> = [];
423+
const actualDispatch: Array<{ value: PyDiscriminatorValue; typeName: string }> = [];
391424
let allResolved = true;
392425
for (let i = 0; i < union.variantNames.length; i++) {
393426
const actual = resolveActualName(union.variantNames[i]);
@@ -450,7 +483,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython(
450483
dispatcherLines.push(` kind = obj.get(${JSON.stringify(union.discriminatorProp)})`);
451484
dispatcherLines.push(` match kind:`);
452485
for (const m of actualDispatch) {
453-
dispatcherLines.push(` case ${JSON.stringify(m.value)}: return ${m.typeName}.from_dict(obj)`);
486+
dispatcherLines.push(` case ${pyDiscriminatorValueExpr(m.value)}: return ${m.typeName}.from_dict(obj)`);
454487
}
455488
dispatcherLines.push(
456489
` case _: raise ValueError(f"Unknown ${actualAliasName} ${union.discriminatorProp}: {kind!r}")`
@@ -500,7 +533,7 @@ function postProcessDiscriminatorDefaultsForPython(
500533
unions: ResolvedRefBasedUnion[]
501534
): string {
502535
// Build variant lookup: variant class name → { prop, value }.
503-
const variantInfo = new Map<string, { prop: string; value: string }>();
536+
const variantInfo = new Map<string, { prop: string; value: PyDiscriminatorValue }>();
504537
for (const union of unions) {
505538
for (const d of union.dispatch) {
506539
// First-wins; multiple unions referencing the same variant share a
@@ -571,9 +604,9 @@ function postProcessDiscriminatorDefaultsForPython(
571604
continue;
572605
}
573606
const fieldIndent = (block[fieldIdx].match(/^(\s+)/) ?? ["", ""])[1];
574-
const literal = JSON.stringify(info.value);
607+
const literal = pyDiscriminatorValueExpr(info.value);
575608
// Replace the field with a class-level constant.
576-
block[fieldIdx] = `${fieldIndent}${info.prop}: ClassVar[str] = ${literal}`;
609+
block[fieldIdx] = `${fieldIndent}${info.prop}: ClassVar[${pyDiscriminatorValueType(info.value)}] = ${literal}`;
577610
usedClassVar = true;
578611

579612
// Drop any field-trailing docstring lines that immediately followed the
@@ -1590,7 +1623,7 @@ function tryEmitPyRefBasedDiscriminatedUnion(
15901623
if (!discriminator) return undefined;
15911624

15921625
const variantTypeNames: string[] = [];
1593-
const dispatch: Array<{ value: string; typeName: string }> = [];
1626+
const dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }> = [];
15941627
for (let i = 0; i < variants.length; i++) {
15951628
const variantTypeName = toPascalCase(variantRefNames[i]);
15961629
const variantSchema = resolveObjectSchema(variants[i], ctx.definitions);
@@ -1599,7 +1632,7 @@ function tryEmitPyRefBasedDiscriminatedUnion(
15991632
}
16001633
variantTypeNames.push(variantTypeName);
16011634
const discProp = resolvedVariants[i].properties?.[discriminator.property] as JSONSchema7;
1602-
dispatch.push({ value: String(discProp.const), typeName: variantTypeName });
1635+
dispatch.push({ value: pyDiscriminatorValue(discProp.const), typeName: variantTypeName });
16031636
}
16041637

16051638
if (!ctx.aliasesByName.has(aliasName)) {
@@ -1627,7 +1660,7 @@ function tryEmitPyRefBasedDiscriminatedUnion(
16271660
lines.push(` match kind:`);
16281661
for (const m of dispatch) {
16291662
lines.push(
1630-
` case ${JSON.stringify(m.value)}: return ${m.typeName}.from_dict(obj)`
1663+
` case ${pyDiscriminatorValueExpr(m.value)}: return ${m.typeName}.from_dict(obj)`
16311664
);
16321665
}
16331666
lines.push(

0 commit comments

Comments
 (0)