Skip to content

fix(parse): run the request transform once in parse() - #3616

Open
rasmusfaber wants to merge 1 commit into
openai:mainfrom
rasmusfaber:fix/parse-run-transform-once
Open

rasmusfaber wants to merge 1 commit into
openai:mainfrom
rasmusfaber:fix/parse-run-transform-once

Conversation

@rasmusfaber

Copy link
Copy Markdown
  • I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

For concurrent usage with large payloads, systems can use a disproportionately long time just in the _transform_recursive() method. A more optimal implementation (compile the per-type transform plan once, execute without runtime typing introspection) could speed up performance of transform up to 370x. You would probably want to do that yourself, however, and it might conflict with the migration to Castiron.

There is a tiny, trivial 2x improvement to be had, however:

The hand-written parse() helpers pass the CreateParams union to the request transform, while the generated create() methods dispatch to the concrete Streaming/NonStreaming type. For union annotations, _transform_recursive walks the body once per union member, so parse() requests pay exactly 2x the transform cost of create() requests (measured 1.99-2.01x).

Pass the concrete params type instead, mirroring create().

The transformed body is unchanged, we only skip an unnecessary second walk in _transform_recursive(). Output is verified by differential comparison on large bodies for both APIs.

This is the same fix anthropic-sdk-python shipped for Messages.stream() in v0.122.0 (12c744c8, "run the request transform once in messages.stream()").

Additional context & links

@rasmusfaber
rasmusfaber requested a review from a team as a code owner August 14, 2026 09:18

@ting-hong-shieh ting-hong-shieh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated at 67ed3cad against main/d9029e3a.

Focused checks:

  • .venv/bin/pytest -q -n 0 tests/lib/chat/test_completions.py tests/lib/responses/test_responses.py — 24 passed
  • Ruff lint and format checks on both changed files — passed
  • Mypy on both changed files — passed

I also ran a same-input transform snapshot for Chat non-streaming and Responses with stream omitted, false, and true. The union and concrete transforms produced identical canonical output hashes in all four cases. Recursive transform calls dropped from 187 to 94 for Chat and from 211/215 to 105/108/106 for the three Responses cases.

main is still the PR base, so there is no base drift. GitHub currently reports all three workflows as action_required with no jobs created, so CI has not run yet. I did not call the live API; request serialization was validated locally.

No blocking issues found.

@jnohclee-rgb

Copy link
Copy Markdown

AI-assisted independent offline corroboration at 67ed3cadc136920980c5ee7a6f1f4a2784ea19a8 vs parent d9029e3ada3c008b4631d78a425743445730892a. Actual sync and async chat.completions.parse()/responses.parse() requests were intercepted by MockTransport; counting wrappers delegated to the real sync/async TypedDict transform functions.

All four non-streaming requests visit both NonStreaming and Streaming root schemas on the parent, and only NonStreaming on this head. Serialized top-level key order is unchanged for these fictional fixtures (Chat: messages/model/stream; Responses: input/model). This is direct evidence of eliminated duplicate root-schema traversal, not a latency or total recursive-work benchmark.

Suggested regression boundary: count/delegate to the actual transform for both client modes, assert the selected schema, and inspect the serialized request. The Responses stream=True selection also deserves a separate test; it was not exercised here. Non-idempotent/nested conversions, broad payload equality and final response parsing are not established by these minimal fixtures.

Scope: four constructed calls, fictional key and payloads, MockTransport deliberate400 stops before response parsing. No network/provider requests, timing claim or full SDK suite. Python3.14/Pydantic2.13.5 with existing shared dependencies and archived sources, sandbox network denied. Counters cover root dicts containing model; they do not count every recursive conversion.

Standalone reproducer
import asyncio,json,re,httpx
from openai import OpenAI,AsyncOpenAI
rows=[]
import importlib
m=importlib.import_module('openai._utils._transform')
original=m._transform_typeddict
visits=[]
def counted(data,expected_type):
 if isinstance(data,dict) and 'model' in data:visits.append(expected_type.__name__)
 return original(data,expected_type)
m._transform_typeddict=counted
async_original=m._async_transform_typeddict
async def async_counted(data,expected_type):
 if isinstance(data,dict) and 'model' in data:visits.append(expected_type.__name__)
 return await async_original(data,expected_type)
m._async_transform_typeddict=async_counted
def capture(req):
 body=req.read();kind=req.headers.get('content-type','')
 order=list(json.loads(body)) if 'application/json' in kind else re.findall(r'Content-Disposition: form-data; name="([^"]+)"',body.decode())
 rows.append({'path':req.url.path,'order':order,'root_transform_visits':list(visits)});visits.clear();return httpx.Response(400,json={'error':{'message':'fictional stop','type':'fictional'}},request=req)
client=OpenAI(api_key='fictional',http_client=httpx.Client(transport=httpx.MockTransport(capture)),max_retries=0)
for call in [lambda:client.chat.completions.parse(model='fictional',messages=[{'role':'user','content':'fictional'}]),lambda:client.responses.parse(model='fictional',input='fictional')]:
 try:call()
 except Exception:pass
client.close()
async def main():
 async def handle(req):await req.aread();return capture(req)
 client=AsyncOpenAI(api_key='fictional',http_client=httpx.AsyncClient(transport=httpx.MockTransport(handle)),max_retries=0)
 for call in [lambda:client.chat.completions.parse(model='fictional',messages=[{'role':'user','content':'fictional'}]),lambda:client.responses.parse(model='fictional',input='fictional')]:
  try:await call()
  except Exception:pass
 await client.close()
asyncio.run(main());assert len(rows)==4;print(json.dumps(rows))

@rasmusfaber
rasmusfaber force-pushed the fix/parse-run-transform-once branch from 67ed3ca to cce25a1 Compare October 25, 2026 21:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T21:45:57.185656Z cce25a1 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants