Fix Python from_dict() round-trip for optional fields with schema defaults

nodejs/test/python-codegen.test.ts

@@ -71,11 +71,11 @@ describe("python session event codegen", () => {
         );
         expect(code).toContain("def to_timedelta_int(x: timedelta) -> int:");
         expect(code).toContain(
-            'action = from_union([from_none, lambda x: parse_enum(SessionSyntheticDataAction, x)], obj.get("action", "store"))'
-        );
-        expect(code).toContain(
-            'summary = from_union([from_none, from_str], obj.get("summary", ""))'
+            'action = from_union([from_none, lambda x: parse_enum(SessionSyntheticDataAction, x)], obj.get("action"))'
         );
+        expect(code).toContain('summary = from_union([from_none, from_str], obj.get("summary"))');
+        expect(code).not.toContain('obj.get("action", "store")');
+        expect(code).not.toContain('obj.get("summary", "")');
         expect(code).toContain("uri: str");
         expect(code).toContain("pattern: str");
         expect(code).toContain("payload: str");

python/test_event_forward_compatibility.py

@@ -17,7 +17,10 @@
     ElicitationCompletedAction,
     ElicitationRequestedMode,
     ElicitationRequestedSchema,
+    PermissionPromptRequest,
+    PermissionPromptRequestKind,
     PermissionRequest,
+    PermissionRequestKind,
     PermissionRequestMemoryAction,
     SessionEventType,
     SessionTaskCompleteData,
@@ -137,10 +140,49 @@ def test_data_shim_preserves_raw_mapping_values(self):
         constructed = Data(arguments={"tool_call_id": "call-1"})
         assert constructed.to_dict() == {"arguments": {"tool_call_id": "call-1"}}
 
-    def test_schema_defaults_are_applied_for_missing_optional_fields(self):
-        """Generated event models should honor primitive schema defaults during parsing."""
+    def test_missing_optional_fields_remain_none_after_parsing(self):
+        """Generated event models should leave missing optional fields as None.
+
+        Regression test for github/copilot-sdk issues #1139, #1140, and #1141:
+        the Python codegen previously baked JSON Schema `default` values into
+        ``obj.get(key, default)`` for optional fields, so ``from_dict()`` returned
+        the schema default instead of ``None`` and broke ``from_dict(to_dict(x))``
+        round-trips for instances where the field was ``None``.
+        """
+        # #1141: PermissionRequest.action defaults to None when missing.
         request = PermissionRequest.from_dict({"kind": "memory", "fact": "remember this"})
-        assert request.action == PermissionRequestMemoryAction.STORE
+        assert request.action is None
+        assert PermissionRequestMemoryAction.STORE.value == "store"  # sanity
+
+        # #1140: PermissionPromptRequest.action defaults to None when missing.
+        prompt_request = PermissionPromptRequest.from_dict({"kind": "memory"})
+        assert prompt_request.action is None
 
+        # #1139: SessionTaskCompleteData.summary defaults to None when missing.
         task_complete = SessionTaskCompleteData.from_dict({"success": True})
-        assert task_complete.summary == ""
+        assert task_complete.summary is None
+
+        # Explicit JSON null should also map to None.
+        task_complete_null = SessionTaskCompleteData.from_dict({"success": True, "summary": None})
+        assert task_complete_null.summary is None
+
+    def test_optional_fields_round_trip_none(self):
+        """``from_dict(to_dict(x))`` should equal ``x`` when optional fields are None.
+
+        Regression test for github/copilot-sdk issues #1139, #1140, and #1141.
+        """
+        # #1139: SessionTaskCompleteData round-trip with summary=None.
+        task = SessionTaskCompleteData(success=None, summary=None)
+        assert SessionTaskCompleteData.from_dict(task.to_dict()) == task
+
+        # #1140: PermissionPromptRequest round-trip with action=None.
+        prompt = PermissionPromptRequest(kind=PermissionPromptRequestKind.MEMORY)
+        assert prompt.action is None
+        assert "action" not in prompt.to_dict()
+        assert PermissionPromptRequest.from_dict(prompt.to_dict()) == prompt
+
+        # #1141: PermissionRequest round-trip with action=None.
+        permission = PermissionRequest(kind=PermissionRequestKind.MEMORY)
+        assert permission.action is None
+        assert "action" not in permission.to_dict()
+        assert PermissionRequest.from_dict(permission.to_dict()) == permission

scripts/codegen/python.ts

@@ -865,22 +865,6 @@ function isPyBase64StringSchema(schema: JSONSchema7): boolean {
     return schema.format === "byte" || (schema as Record<string, unknown>).contentEncoding === "base64";
 }
 
-function toPythonLiteral(value: unknown): string | undefined {
-    if (typeof value === "string") {
-        return JSON.stringify(value);
-    }
-    if (typeof value === "number") {
-        return Number.isFinite(value) ? String(value) : undefined;
-    }
-    if (typeof value === "boolean") {
-        return value ? "True" : "False";
-    }
-    if (value === null) {
-        return "None";
-    }
-    return undefined;
-}
-
 function extractPyEventVariants(schema: JSONSchema7): PyEventVariant[] {
     const definitionCollections = collectDefinitionCollections(schema as Record<string, unknown>);
     return getSessionEventVariantSchemas(schema, definitionCollections)
@@ -1430,9 +1414,6 @@ function emitPyClass(
             fieldName: toSnakeCase(propName),
             isRequired,
             resolved,
-            defaultLiteral: isRequired ? undefined : toPythonLiteral(
-                propSchema.default ?? resolveSchema(propSchema, ctx.definitions)?.default
-            ),
         };
     });
 
@@ -1474,9 +1455,7 @@ function emitPyClass(
     lines.push(`    def from_dict(obj: Any) -> "${typeName}":`);
     lines.push(`        assert isinstance(obj, dict)`);
     for (const field of fieldInfos) {
-        const sourceExpr = field.defaultLiteral
-            ? `obj.get(${JSON.stringify(field.jsonName)}, ${field.defaultLiteral})`
-            : `obj.get(${JSON.stringify(field.jsonName)})`;
+        const sourceExpr = `obj.get(${JSON.stringify(field.jsonName)})`;
         lines.push(
             `        ${field.fieldName} = ${field.resolved.fromExpr(sourceExpr)}`
         );
@@ -1592,9 +1571,6 @@ function emitPyFlatDiscriminatedUnion(
             fieldName: toSnakeCase(propName),
             isRequired: requiredInAll,
             resolved,
-            defaultLiteral: requiredInAll ? undefined : toPythonLiteral(
-                propSchema.default ?? resolveSchema(propSchema, ctx.definitions)?.default
-            ),
         };
     });
 
@@ -1620,9 +1596,7 @@ function emitPyFlatDiscriminatedUnion(
     lines.push(`    def from_dict(obj: Any) -> "${typeName}":`);
     lines.push(`        assert isinstance(obj, dict)`);
     for (const field of fieldInfos) {
-        const sourceExpr = field.defaultLiteral
-            ? `obj.get(${JSON.stringify(field.jsonName)}, ${field.defaultLiteral})`
-            : `obj.get(${JSON.stringify(field.jsonName)})`;
+        const sourceExpr = `obj.get(${JSON.stringify(field.jsonName)})`;
         lines.push(
             `        ${field.fieldName} = ${field.resolved.fromExpr(sourceExpr)}`
         );