Repository navigation
Conversation
6ad2fec to
ac63c61
Compare
ac63c61 to
1fc4841
Compare
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
This looks right. The wire bytes are little-endian, so <f4 makes the NumPy path explicit and byteswap() gives the stdlib path the same behaviour on big-endian hosts. Using struct.pack("<3f", ...) also makes the fixture independent of the test machine.
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 36599625795 --repo openai/openai-python \
--name castiron-custom-code-36599625795-1 --dir /tmp/castiron-custom-code-36599625795-1
git apply --stat /tmp/castiron-custom-code-36599625795-1/custom-code.patch
cat /tmp/castiron-custom-code-36599625795-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin b19c2161b1eac80fbf1f6f67a64a50af99c53356 1fc4841bdcfb1a5d21810f47d355945170486b3a
python3 scripts/castiron/custom_code_report.py report \
--base b19c2161b1eac80fbf1f6f67a64a50af99c53356 \
--head 1fc4841bdcfb1a5d21810f47d355945170486b3a --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-1fc4841bdcfb
cat /tmp/castiron-custom-code-1fc4841bdcfb/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Re-reviewed current head 1fc4841 after the refreshed generated-baseline validation. The wire format is explicitly little-endian in both paths: NumPy uses <f4, while the stdlib array path byteswaps only on big-endian hosts. The fixture is machine-independent via struct.pack("<3f", ...), and the big-endian regression exercises the byteswap branch. Castiron reports no new custom-code files or changed existing customizations, and there are no unresolved review threads. No blocker from my review.
|
AI-assisted independent numeric check at Unlike a test double that returns expected VALUES after observing
Little-endian fixture construction: Both simulated branches on the parent produced This adds byte-derived numerical evidence for the big-endian branch alongside the existing call-order regression test. It is a simulation on a little-endian machine, not execution on big-endian hardware, a live API test, or a full-suite result. All vectors and metadata were fictional. |
This branch has not been deployed
3 participants
Root cause
The base64 embedding representation contains little-endian float32 bytes, but the handwritten response parser used native byte order in both its NumPy and stdlib paths. On big-endian Python platforms, that silently produces incorrect vector values. The existing fixture was also native-endian, so it followed the test host instead of the wire format and could not expose the defect.
Fix
<f4dtype.array("f")path on big-endian hosts.struct.pack("<3f", ...)so the test represents the wire format on every host.The branch is rebased onto current
mainand preserves #3757: NumPy availability is still checked once per response, while each encoded vector is decoded only once.Validation
On the current head with Python 3.12:
.venv/bin/pytest -q -n 0 tests/lib/test_embeddings.py— 60 passed.venv/bin/ruff check src/openai/lib/_parsing/_embeddings.py tests/lib/test_embeddings.py— passed.venv/bin/ruff format --check src/openai/lib/_parsing/_embeddings.py tests/lib/test_embeddings.py— passedgit diff --check— passed