feat(python): replace runtime SSE reflection with compile-time discriminant context#14183
feat(python): replace runtime SSE reflection with compile-time discriminant context#14183
Conversation
…minant context Use the IR's discriminatorContext field to determine at code generation time whether SSE endpoints use protocol-level or data-level discrimination, instead of reflecting on Pydantic union types at runtime. Protocol-level endpoints now call parse_sse_protocol and data-level endpoints call parse_sse_data, eliminating type introspection overhead critical for LLM inference workflows.
🌱 Seed Test SelectorSelect languages to run seed tests for:
How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR. |
jsklan
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Co-Authored-By: bot_apk <apk@cognition.ai>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| envelope: Dict[str, Any] = {"event": sse.event} | ||
| if sse.data is not None: | ||
| try: | ||
| parsed_data = json.loads(sse.data) | ||
| except (json.JSONDecodeError, ValueError): | ||
| parsed_data = None | ||
|
|
There was a problem hiding this comment.
🚩 parse_sse_protocol drops SSE id/retry fields compared to old asdict(sse)
The old parse_sse_obj used asdict(sse) which included ALL fields of the ServerSentEvent dataclass (typically event, data, id, retry). The new parse_sse_protocol only constructs {"event": sse.event, "data": ...}, dropping id and retry. This is a semantic change for protocol-level discrimination. If any union variant model has an id field that should come from the SSE protocol-level id, it would now be missing. The generated seed output (seed/python-sdk/server-sent-events-openapi/with-wire-tests/src/seed/raw_client.py) suggests the current variant models only use event and data, so this appears intentional, but future variants that include id would silently lose that data.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…s for new SSE parse functions Co-Authored-By: bot_apk <apk@cognition.ai>
…ads("null") from parse failures
Co-Authored-By: bot_apk <apk@cognition.ai>
|
Fixed in f5c00ad — all 4 |
…sting mypy bug) Co-Authored-By: judah <jsklan.development@gmail.com>
| except (json.JSONDecodeError, ValueError): | ||
| parsed_data = None | ||
|
|
||
| if parsed_data is not None: |
There was a problem hiding this comment.
🔴 parse_sse_protocol uses None as sentinel instead of unique object, mishandling JSON null
In parse_sse_protocol, parsed_data = None is used as the failure sentinel for json.loads, but json.loads("null") legitimately returns None. When sse.data is the string "null", json.loads succeeds and returns Python None, but parsed_data is not None evaluates to False, so the code falls through to envelope["data"] = sse.data (the raw string "null") instead of envelope["data"] = None (the parsed JSON null). This means JSON null data values are silently treated as unparseable strings.
All four source-of-truth template files correctly use _PARSE_FAILED = object() as a unique sentinel (e.g., generators/python/core_utilities/shared/pydantic_utilities.py:168), but this generated output diverges from the template, suggesting it was not regenerated after the template fix.
Prompt for agents
Regenerate the seed output for server-sent-events-openapi by running: pnpm seed test --generator python-sdk --fixture server-sent-events-openapi
The generated file seed/python-sdk/server-sent-events-openapi/with-wire-tests/src/seed/core/pydantic_utilities.py should match the template at generators/python/core_utilities/shared/pydantic_utilities.py, which correctly uses _PARSE_FAILED = object() as a unique sentinel in parse_sse_protocol (lines 168-176). The current generated output incorrectly uses parsed_data = None as the sentinel on lines 174-177, which breaks when sse.data is the JSON string "null".
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Description
Replace runtime reflection-based SSE discriminant detection in the Python SDK generator with compile-time knowledge from the IR's
discriminatorContextfield. This eliminates per-event type introspection overhead that is critical for high-throughput SSE workloads like LLM inference.The previous implementation used
_get_discriminator_and_variants,_find_variant_by_discriminator,_get_field_annotation, and_is_string_typeto reflect on Pydantic union types at runtime on every SSE event. The TypeScript and Go generators already solve this correctly by readingdiscriminatorContextfrom the IR at generation time.Changes Made
pydantic_utilities.pyx4 variants): Addedparse_sse_data(data-level) andparse_sse_protocol(protocol-level) functions. Deletedparse_sse_objand all reflection helpers (_get_discriminator_and_variants,_find_variant_by_discriminator,_get_field_annotation,_is_string_type)endpoint_response_code_writer.py): Resolves SSE payload'sUnionTypeDeclaration.discriminator_contextfrom IR at generation time. Emitsparse_sse_protocolfor protocol-context endpoints andparse_sse_datafor data-context endpointscore_utilities.py): Updatedget_construct_sseto acceptis_protocolparameter and reference the new functionsparse_sse_protocolstring-variant fallback: Attemptsjson.loadsfirst; if Pydantic validation fails (e.g. variant expectsstrbut data was valid JSON like'{"status": "processing"}'), falls back to the raw string so string-typed variants work correctly_PARSE_FAILED = object()instead ofNoneso thatjson.loads("null")returning PythonNoneis correctly distinguished from actual parse failuresUpdates since last revision
json.loads("null")sentinel bug flagged by Devin Review: all 4pydantic_utilities.pyvariants now use_PARSE_FAILED = object()instead ofNoneas the parse-failure sentinelserver-sent-event-examplesseed output and hand-written test assertions to match new log messagesTesting
pnpm seed test --generator python-sdk --fixture server-sent-events-openapi --skip-scripts— 1/1 passedpnpm seed test --generator python-sdk --fixture exhaustive --skip-scripts— 28/28 passedpnpm seed:local test --generator python-sdk --fixture server-sent-event-examples— 1/1 passed (build + test scripts)streamProtocolNoCollision,streamProtocolCollision,streamProtocolWithFlatSchema) callparse_sse_protocolstreamDataContext,streamNoContext,streamDataContextWithEnvelopeSchema,streamOasSpecNative) callparse_sse_datapydantic_utilities.pyHuman Review Checklist
pydantic_utilities.pyvariants consistently use_PARSE_FAILED = object()sentinel (the cumulative diff truncation makes it hard to confirm thewith_pydantic_v1_on_v2/with_aliasescopy was updated)except Exceptionfallback inparse_sse_protocol— it intentionally catches validation errors to fall back to raw strings for string-typed variants, but could mask other issuesserver-sent-event-examplesandserver-sent-events-openapiLink to Devin session: https://app.devin.ai/sessions/cf621d7834184fbab7e24ace259f8c77