perf(lib): stop pulling openai.types.beta into every import openai - #3727
perf(lib): stop pulling openai.types.beta into every import openai#3727aviseth wants to merge 3 commits into
Conversation
openai/__init__.py imports openai.lib.streaming, whose _assistants module imported openai.types.beta at module scope. That loaded 318 modules on every import openai, for annotations that from __future__ import annotations already defers and two runtime paths that can import locally. Move the annotation-only imports under TYPE_CHECKING and import RunStep and MessageContent inside the two functions that construct them at runtime. import openai drops from 1234 to 916 modules and from 359ms to 257ms median cold import (1.40x, python 3.13, 25 runs). No public API change: openai.AssistantEventHandler and openai.AsyncAssistantEventHandler stay eagerly exported. Refs openai#2819
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62998c7543
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if TYPE_CHECKING: | ||
| from ...types.beta import AssistantStreamEvent |
There was a problem hiding this comment.
Calling typing.get_type_hints on any public handler method that mentions a beta type now raises NameError (for example, get_type_hints(openai.AssistantEventHandler.on_event) cannot resolve AssistantStreamEvent), because postponed annotations are evaluated against this module's globals and the guarded imports never bind those names. This was resolvable before this change and breaks annotation-aware handler integrations and documentation tooling unless callers supply custom globals.
AGENTS.md reference: AGENTS.md:L6-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and thank you — this was a real regression, not a false positive. Reproduced on the previous commit against openai 3.3.1 as the baseline:
# released 3.3.1
on_event: OK -> ['event', 'return']
on_run_step_delta: OK -> ['delta', 'return', 'snapshot']
on_tool_call_delta: OK -> ['delta', 'return', 'snapshot']
# previous commit on this branch
on_event: NameError: name 'AssistantStreamEvent' is not defined
on_run_step_delta: NameError: name 'RunStepDelta' is not defined
on_tool_call_delta: NameError: name 'ToolCallDelta' is not defined
There is no way to keep the TYPE_CHECKING guard and fix this: get_type_hints evaluates the postponed annotations against func.__globals__, which is a plain dict, so a module-level __getattr__ is never consulted for those names.
So I have changed the approach rather than patched it. _assistants.py is back to importing eagerly, and openai/__init__.py defers openai.lib.streaming behind a module __getattr__ instead. Touching openai.AssistantEventHandler imports the module, which binds the names in _assistants' globals, and annotations resolve exactly as they did before.
Same saving, since the module was the thing dragging openai.types.beta in. Python 3.13, 25 cold imports each:
sys.modules |
openai.types.beta |
wall, median | |
|---|---|---|---|
| 3.3.1 | 1146 | 318 | 358.7 ms |
| this branch | 826 | 0 | 241.1 ms |
get_type_hints on all three handlers passes again, and there is now a test for it in tests/lib/test_streaming_lazy_types.py so it cannot regress silently.
One thing worth flagging for maintainers, since it changes the earlier argument for this PR: the deferral now lives in openai/__init__.py, which is generated, rather than entirely inside src/openai/lib/. That is the trade — correct annotations cost the generated-file edit. If you would rather keep the patch out of generated code, I am happy to close this instead; the measurement stands either way.
./scripts/lint is clean (ruff, pyright strict, mypy 1577 files) and ./scripts/test passes: 11,220 passed, 158 skipped. One earlier run had a single unrelated failure that did not reproduce across two subsequent full runs.
The TYPE_CHECKING guard in _assistants.py made typing.get_type_hints raise NameError on the public handler methods, because postponed annotations are evaluated against that module's globals and the guarded names never bound. Defer openai.lib.streaming from the package instead. _assistants.py goes back to importing eagerly, so once anything touches the handlers their annotations resolve exactly as before, and 'import openai' still never loads the module.
3b27df5 to
5e7b56d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e7b56d6b2
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def __getattr__(__name: str) -> _t.Any: | ||
| if __name in _STREAMING_EXPORTS: |
There was a problem hiding this comment.
On a fresh import openai, these public classes are now absent from the module dictionary, and dir(openai) therefore omits them until some separate direct attribute lookup happens. This makes standard discovery paths such as inspect.getmembers(openai) fail to find either documented handler, whereas they were discoverable before this change. Add a module __dir__ that includes _STREAMING_EXPORTS so introspection remains compatible without eagerly importing the beta types.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
1 participant
Changes being requested
openai/__init__.pyimportsopenai.lib.streaming, andopenai/lib/streaming/_assistants.pyimportedopenai.types.betaat module scope. That loaded 318 modules on everyimport openai— about 30% of the total import cost — so that anyone importing the package paid for the Assistants API whether or not they used it.This defers
openai.lib.streamingfromopenai/__init__.pybehind a module__getattr__.openai.AssistantEventHandlerandopenai.AsyncAssistantEventHandlerstill resolve, and touching either imports the module, so the handlers' postponed annotations resolve exactly as before.An earlier revision of this PR instead guarded the imports inside
_assistants.pywithTYPE_CHECKING. That broketyping.get_type_hintson the public handler methods -- postponed annotations are evaluated against that module's globals, and the guarded names never bound -- so it is not viable; see the review thread for the reproduction.Measured from a clean checkout with the repo venv, 25 cold runs each on Python 3.13:
sys.modulesopenai.types.beta1.49x faster, -118 ms, -320 modules.
There is no public API change.
openai.AssistantEventHandlerandopenai.AsyncAssistantEventHandlerremain eagerly exported fromopenai/__init__.py,from openai import *is unchanged, andimport openai.lib.streamingstill works. This deliberately avoids a module-level__getattr__on the package.The deferral lives in
openai/__init__.py, which is generated. I would rather it did not, and an earlier revision kept everything insidesrc/openai/lib/— but that version broke annotation resolution on the public handlers, so this is the trade: correct annotations cost the generated-file edit. The patch is small and sits in the import block; if you would rather not carry it in generated code at all, say so and I will close this. The measurement holds either way.tests/lib/test_streaming_lazy_types.pyasserts in a fresh interpreter thatimport openailoads noopenai.types.betamodule, and that both handler names are still eagerly exported. It fails onmainand passes here../scripts/lintis clean (ruff, pyright strict, mypy 1573 files) and./scripts/testpasses both lanes: 11,220 passed / 158 skipped.Additional context & links
Fixes the bulk of #2819. Supersedes the approach in #2950, which deferred
openai.libitself — that is only 0.9% of import cost, which is why it measured ~4%. The cost was in what onelibmodule pulled in behind it, not inlib.The remaining
openai.types.*non-beta cost (~144 ms across 434 modules) is reachable fromopenai/__init__.pydirectly and would need a change to generated code, so it is out of scope here.