Skip to content

Commit 7ca801f

Browse files
stephentoubCopilot
andcommitted
Fix Python codegen so ReasoningSummary import is not shadowed
The 1.0.60 schema introduces OptionsUpdateReasoningSummary and SessionOpenOptionsReasoningSummary, structurally identical to the shared ReasoningSummary enum. quicktype's enum-merging path (independent of combineClasses) collapsed them into a single locally-defined `class ReasoningSummary(Enum)` that shadowed the import from session_events at the top of rpc.py. ModelSwitchToRequest.to_dict() then called `to_enum(ReasoningSummary, x)` against the local class while session.py constructed values via the imported one, so the isinstance check inside to_enum failed and the unit test TestSessionConfigForwarding.test_set_model_sends_correct_rpc broke. Add a small post-processing step that strips locally-emitted enum classes whose name and value set exactly match a name imported from .session_events. The existing aliases at the end of rpc.py (OptionsUpdateReasoningSummary = ReasoningSummary etc.) now resolve to the imported enum, restoring isinstance equality across the SDK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 367277d commit 7ca801f

2 files changed

Lines changed: 59 additions & 11 deletions

File tree

python/copilot/generated/rpc.py

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

scripts/codegen/python.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,60 @@ function removeRequiredAnyDefaultsForPython(
11771177
});
11781178
}
11791179

1180+
/**
1181+
* Remove locally-emitted Enum class definitions whose name already comes from
1182+
* a `.session_events` import.
1183+
*
1184+
* Quicktype's enum-merging path collapses structurally-identical enums (even
1185+
* with `combineClasses: false`, which only governs class merging). When the
1186+
* RPC schema gains sibling enums like `OptionsUpdateReasoningSummary` and
1187+
* `SessionOpenOptionsReasoningSummary` whose value set matches the shared
1188+
* `ReasoningSummary` enum, quicktype picks `ReasoningSummary` as the merged
1189+
* canonical name. That local class then shadows the import we add at the top
1190+
* of `rpc.py`, breaking `isinstance` checks against the canonical enum used
1191+
* elsewhere in the SDK.
1192+
*
1193+
* The fix: detect such shadowed enum definitions, verify the local values
1194+
* exactly match the imported enum's values in the session-events schema, and
1195+
* strip the local class so references resolve to the import.
1196+
*/
1197+
function removeShadowedSessionEventEnumsForPython(
1198+
code: string,
1199+
importedFromSessionEvents: Set<string>,
1200+
sessionEventsSchema: JSONSchema7 | undefined
1201+
): string {
1202+
if (importedFromSessionEvents.size === 0 || !sessionEventsSchema) return code;
1203+
const seDefs = collectDefinitionCollections(sessionEventsSchema as Record<string, unknown>);
1204+
const enumBlockRe =
1205+
/(?:^|\n)class\s+(\w+)\s*\(Enum\):\s*\r?\n([\s\S]*?)(?=\nclass\s+\w|\n@dataclass\b|\ndef\s+\w|$)/g;
1206+
return code
1207+
.replace(enumBlockRe, (match: string, className: string, body: string) => {
1208+
if (!importedFromSessionEvents.has(className)) return match;
1209+
const seDef = seDefs.definitions[className] ?? seDefs.$defs[className];
1210+
const seResolved = seDef ? resolveSchema(seDef, seDefs) ?? seDef : undefined;
1211+
if (
1212+
!seResolved?.enum ||
1213+
!Array.isArray(seResolved.enum) ||
1214+
!seResolved.enum.every((value) => typeof value === "string")
1215+
) {
1216+
return match;
1217+
}
1218+
const localValues = new Set<string>();
1219+
const valueRe = /^\s+\w+\s*=\s*"([^"]*)"/gm;
1220+
let vm: RegExpExecArray | null;
1221+
while ((vm = valueRe.exec(body)) !== null) {
1222+
localValues.add(vm[1]);
1223+
}
1224+
const seValues = new Set(seResolved.enum as string[]);
1225+
if (localValues.size !== seValues.size) return match;
1226+
for (const value of localValues) {
1227+
if (!seValues.has(value)) return match;
1228+
}
1229+
return "";
1230+
})
1231+
.replace(/\n{3,}/g, "\n\n");
1232+
}
1233+
11801234
function reorderPythonDataclassFields(code: string): string {
11811235
const fieldRe =
11821236
/^ \w+: (?:Any|bool|int|float|str|dict|list|ClassVar|[A-Z_]\w*|['"][A-Z_]\w*)(?:[^=]*)?(?: = .*)?$/;
@@ -2896,6 +2950,11 @@ async function generateRpc(schemaPath?: string, sessionEventsSchema?: JSONSchema
28962950
typesCode = collapsePlaceholderPythonDataclasses(typesCode, knownDefNames);
28972951
typesCode = postProcessExternalUnionAliasesForPython(typesCode, externalUnionAliases);
28982952
typesCode = postProcessExternalRefsForPython(typesCode, externalRefs.placeholderNames, externalEnumNames);
2953+
typesCode = removeShadowedSessionEventEnumsForPython(
2954+
typesCode,
2955+
externalRefs.imports.get(".session_events") ?? new Set<string>(),
2956+
sessionEventsSchema
2957+
);
28992958
const { code: typesCodeAfterUnions, unions: refBasedUnions } = postProcessRefBasedDiscriminatedUnionsForPython(
29002959
typesCode,
29012960
allDefinitions,

0 commit comments

Comments
 (0)