From 29a8072293733f44306e62c918799b01729c8f55 Mon Sep 17 00:00:00 2001 From: syf2211 Date: Thu, 30 Jul 2026 12:08:45 +0000 Subject: [PATCH] fix(python): preserve boolean const discriminators in RPC codegen Boolean JSON schema const values were coerced to strings during Python codegen, breaking match dispatchers for SessionListEntry and QueuedCommandResult. Preserve bool consts and emit True/False patterns and ClassVar[bool] defaults, matching the Go generator behavior. Fixes #2122 --- python/copilot/generated/rpc.py | 12 +++---- python/test_rpc_generated.py | 51 ++++++++++++++++++++++++++++ scripts/codegen/python.ts | 60 +++++++++++++++++++++++++++------ 3 files changed, 106 insertions(+), 17 deletions(-) diff --git a/python/copilot/generated/rpc.py b/python/copilot/generated/rpc.py index 500a16eb76..84f21db7e5 100644 --- a/python/copilot/generated/rpc.py +++ b/python/copilot/generated/rpc.py @@ -7391,7 +7391,7 @@ class QueuedCommandHandled: """Queued-command response indicating the host executed the command, with an optional flag to stop queue processing. """ - handled: ClassVar[str] = "true" + handled: ClassVar[bool] = True """The host actually executed the queued command.""" stop_processing_queue: bool | None = None @@ -7418,7 +7418,7 @@ class QueuedCommandNotHandled: """Queued-command response indicating the host did not execute the command and the queue may continue. """ - handled: ClassVar[str] = "false" + handled: ClassVar[bool] = False """The host did not execute the queued command. Unblocks the queue without claiming the command was processed (e.g. when the handler threw before completing). """ @@ -31183,8 +31183,8 @@ def _load_QueuedCommandResult(obj: Any) -> "QueuedCommandResult": assert isinstance(obj, dict) kind = obj.get("handled") match kind: - case "true": return QueuedCommandHandled.from_dict(obj) - case "false": return QueuedCommandNotHandled.from_dict(obj) + case True: return QueuedCommandHandled.from_dict(obj) + case False: return QueuedCommandNotHandled.from_dict(obj) case _: raise ValueError(f"Unknown QueuedCommandResult handled: {kind!r}") # State of the runtime-managed remote-control singleton. @@ -31207,8 +31207,8 @@ def _load_SessionListEntry(obj: Any) -> "SessionListEntry": assert isinstance(obj, dict) kind = obj.get("isRemote") match kind: - case "false": return LocalSessionMetadataValue.from_dict(obj) - case "true": return RemoteSessionMetadataValue.from_dict(obj) + case False: return LocalSessionMetadataValue.from_dict(obj) + case True: return RemoteSessionMetadataValue.from_dict(obj) case _: raise ValueError(f"Unknown SessionListEntry isRemote: {kind!r}") # Open a session by creating, resuming, attaching, connecting to a remote, or handing off. diff --git a/python/test_rpc_generated.py b/python/test_rpc_generated.py index 8b9423c137..2f3f780603 100644 --- a/python/test_rpc_generated.py +++ b/python/test_rpc_generated.py @@ -4,9 +4,15 @@ import pytest +from copilot.generated.rpc import _load_SessionListEntry from copilot.rpc import ( CommandsApi, CommandsInvokeRequest, + LocalSessionMetadataValue, + QueuedCommandHandled, + QueuedCommandNotHandled, + RemoteSessionMetadataValue, + SessionList, SlashCommandTextResult, ) @@ -22,3 +28,48 @@ async def test_commands_invoke_deserializes_slash_command_result(): assert isinstance(result, SlashCommandTextResult) assert result.text == "hello" assert result.markdown is True + + +def test_session_list_entry_decodes_boolean_is_remote_discriminator(): + local_payload = { + "sessionId": "example-local", + "startTime": "2026-07-26T10:00:00.000Z", + "modifiedTime": "2026-07-26T10:05:00.000Z", + "isRemote": False, + } + remote_payload = { + "sessionId": "example-remote", + "startTime": "2026-07-26T10:00:00.000Z", + "modifiedTime": "2026-07-26T10:05:00.000Z", + "isRemote": True, + "remoteSessionIds": ["rs-1"], + "repository": { + "owner": "github", + "name": "copilot-sdk", + "branch": "main", + }, + } + + local_entry = _load_SessionListEntry(local_payload) + remote_entry = _load_SessionListEntry(remote_payload) + + assert isinstance(local_entry, LocalSessionMetadataValue) + assert local_entry.session_id == "example-local" + assert isinstance(remote_entry, RemoteSessionMetadataValue) + assert remote_entry.session_id == "example-remote" + + session_list = SessionList.from_dict({"sessions": [local_payload, remote_payload]}) + assert len(session_list.sessions) == 2 + assert isinstance(session_list.sessions[0], LocalSessionMetadataValue) + assert isinstance(session_list.sessions[1], RemoteSessionMetadataValue) + + +def test_queued_command_result_round_trips_boolean_handled_discriminator(): + handled = QueuedCommandHandled(stop_processing_queue=True) + not_handled = QueuedCommandNotHandled() + + assert handled.to_dict() == {"handled": True, "stopProcessingQueue": True} + assert not_handled.to_dict() == {"handled": False} + + assert QueuedCommandHandled.from_dict({"handled": True, "stopProcessingQueue": True}) == handled + assert QueuedCommandNotHandled.from_dict({"handled": False}) == not_handled diff --git a/scripts/codegen/python.ts b/scripts/codegen/python.ts index 5b343122b6..b6ce16ad86 100644 --- a/scripts/codegen/python.ts +++ b/scripts/codegen/python.ts @@ -272,6 +272,33 @@ function postProcessExternalUnionAliasesForPython(code: string, aliases: Map; + dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>; } function postProcessRefBasedDiscriminatedUnionsForPython( code: string, @@ -304,7 +331,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython( aliasName: string; variantNames: string[]; discriminatorProp: string; - dispatch: Array<{ value: string; typeName: string }>; + dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }>; description: string | undefined; } const unions: UnionInfo[] = []; @@ -333,8 +360,12 @@ function postProcessRefBasedDiscriminatedUnionsForPython( const discProp = (resolvedVariants[i].properties as Record)[ discriminator.property ]; + const discValue = pyDiscriminatorConstValue(discProp); + if (discValue === undefined) { + throw new Error(`Missing discriminator const on ${variantRefNames[i]}`); + } return { - value: String(discProp.const), + value: discValue, typeName: toPascalCase(variantRefNames[i]), }; }); @@ -387,7 +418,7 @@ function postProcessRefBasedDiscriminatedUnionsForPython( for (const union of unions) { const actualAliasName = resolveActualName(union.aliasName); const actualVariantNames: string[] = []; - const actualDispatch: Array<{ value: string; typeName: string }> = []; + const actualDispatch: Array<{ value: PyDiscriminatorValue; typeName: string }> = []; let allResolved = true; for (let i = 0; i < union.variantNames.length; i++) { const actual = resolveActualName(union.variantNames[i]); @@ -450,7 +481,9 @@ function postProcessRefBasedDiscriminatedUnionsForPython( dispatcherLines.push(` kind = obj.get(${JSON.stringify(union.discriminatorProp)})`); dispatcherLines.push(` match kind:`); for (const m of actualDispatch) { - dispatcherLines.push(` case ${JSON.stringify(m.value)}: return ${m.typeName}.from_dict(obj)`); + dispatcherLines.push( + ` case ${pyDiscriminatorMatchPattern(m.value)}: return ${m.typeName}.from_dict(obj)` + ); } dispatcherLines.push( ` case _: raise ValueError(f"Unknown ${actualAliasName} ${union.discriminatorProp}: {kind!r}")` @@ -500,7 +533,7 @@ function postProcessDiscriminatorDefaultsForPython( unions: ResolvedRefBasedUnion[] ): string { // Build variant lookup: variant class name → { prop, value }. - const variantInfo = new Map(); + const variantInfo = new Map(); for (const union of unions) { for (const d of union.dispatch) { // First-wins; multiple unions referencing the same variant share a @@ -571,9 +604,10 @@ function postProcessDiscriminatorDefaultsForPython( continue; } const fieldIndent = (block[fieldIdx].match(/^(\s+)/) ?? ["", ""])[1]; - const literal = JSON.stringify(info.value); + const literal = pyDiscriminatorLiteral(info.value); + const classVarType = pyDiscriminatorClassVarType(info.value); // Replace the field with a class-level constant. - block[fieldIdx] = `${fieldIndent}${info.prop}: ClassVar[str] = ${literal}`; + block[fieldIdx] = `${fieldIndent}${info.prop}: ClassVar[${classVarType}] = ${literal}`; usedClassVar = true; // Drop any field-trailing docstring lines that immediately followed the @@ -1590,7 +1624,7 @@ function tryEmitPyRefBasedDiscriminatedUnion( if (!discriminator) return undefined; const variantTypeNames: string[] = []; - const dispatch: Array<{ value: string; typeName: string }> = []; + const dispatch: Array<{ value: PyDiscriminatorValue; typeName: string }> = []; for (let i = 0; i < variants.length; i++) { const variantTypeName = toPascalCase(variantRefNames[i]); const variantSchema = resolveObjectSchema(variants[i], ctx.definitions); @@ -1599,7 +1633,11 @@ function tryEmitPyRefBasedDiscriminatedUnion( } variantTypeNames.push(variantTypeName); const discProp = resolvedVariants[i].properties?.[discriminator.property] as JSONSchema7; - dispatch.push({ value: String(discProp.const), typeName: variantTypeName }); + const discValue = pyDiscriminatorConstValue(discProp); + if (discValue === undefined) { + throw new Error(`Missing discriminator const on ${variantRefNames[i]}`); + } + dispatch.push({ value: discValue, typeName: variantTypeName }); } if (!ctx.aliasesByName.has(aliasName)) { @@ -1627,7 +1665,7 @@ function tryEmitPyRefBasedDiscriminatedUnion( lines.push(` match kind:`); for (const m of dispatch) { lines.push( - ` case ${JSON.stringify(m.value)}: return ${m.typeName}.from_dict(obj)` + ` case ${pyDiscriminatorMatchPattern(m.value)}: return ${m.typeName}.from_dict(obj)` ); } lines.push(