fix: avoid repeated numpy checks for embeddings - #3757
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 34 mixed files remain; 0 existing customizations changed. Compared 34 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 33193573074 --repo openai/openai-python \
--name castiron-custom-code-33193573074-1 --dir /tmp/castiron-custom-code-33193573074-1
git apply --stat /tmp/castiron-custom-code-33193573074-1/custom-code.patch
cat /tmp/castiron-custom-code-33193573074-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 9917c6e28e66e90e1227b3d223c06a8c5441515a 7138d3e5aa609d385c7a4594d55cfdb7b690867e
python3 scripts/castiron/custom_code_report.py report \
--base 9917c6e28e66e90e1227b3d223c06a8c5441515a \
--head 7138d3e5aa609d385c7a4594d55cfdb7b690867e --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-7138d3e5aa60
cat /tmp/castiron-custom-code-7138d3e5aa60/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the exact current head. NumPy availability is now resolved once only when encoded vectors are present, while mixed/already-decoded vectors retain the existing behavior and focused tests cover both paths. No blocking findings. (GitHub does not permit this account to formally approve its own PR.)
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed 7138d3e5aa609d385c7a4594d55cfdb7b690867e; no substantive issues found. NumPy availability is resolved once per response only when encoded vectors are present, while explicit formats, mixed/already-decoded vectors, decoder error behavior, and the shared sync/async response paths retain their existing semantics.
Changes being requested
Avoid calling has_numpy() once per returned embedding when the default embeddings decoder handles a bulk base64 response. The parser now checks whether any encoded vectors need decoding, resolves NumPy availability once per response, and reuses that result for every encoded vector.
Add focused regressions that verify multiple encoded vectors trigger one availability check and responses containing only already-decoded vectors do not inspect NumPy.
Additional context & links
Closes #3753
Parser-only benchmark for 1,500 vectors x 1,536 dimensions with NumPy unavailable: 86.0 ms -> 20.5 ms median across 30 alternating runs, a 76.1% reduction and 4.18x speedup. With NumPy installed, performance was effectively unchanged/slightly improved: 24.0 ms -> 23.5 ms.
Validation: