Repository navigation
fix(images): label transcoded image parts with their real media type on the OpenAI wire - #1778
Conversation
…on the OpenAI wire Resolves the review finding left open on the merged #1738: with imageOptions.outputFormat set to "jpeg", real JPEG bytes reached the OpenAI chat-completions wire inside a data URI labelled image/png. Two independent drops caused it, and each one alone reproduces the bug: - core MessageBuilder rebuilt every image content part from `type` and `image` only, discarding the resolved media type on both buildMessages() and buildMessagesForStream(). The upstream builder emits it as `mimeType` while ImagePart declares `mediaType`, so both are read and the canonical `mediaType` is carried forward. - convertContentForOpenAI never passed the part's media type to imageDataToURL, so every raw-base64 image fell back to its hardcoded image/png default. The vision suite now asserts the wire label as well as the decoded bytes, on generate() and on stream(). stream() serializes to the wire at its own call site, so it gets its own label test: stripping the media type at only that site now fails that one test (it passed all four before) while every generate() test stays green.
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: juspay/neurolink/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughImage conversion now preserves resolved media types in both message-building paths. OpenAI serialization uses the media type when labeling image data URIs. Tests check the JPEG label in generate and stream requests. ChangesImage media type handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The image-label change is mergeable after normal checks; the reported test-skip concern is not reachable in these tests. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
src/lib/core/modules/MessageBuilder.tsParsing error: Unable to parse the specified 'tsconfig' file. Ensure it's correct and has valid syntax. error TS5012: Cannot read file '/.svelte-kit/tsconfig.json': ENOENT: no such file or directory, open '/.svelte-kit/tsconfig.json'. src/lib/providers/openaiChatCompletionsClient.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). test/continuous-test-suite-vision-image-format.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🎉 This PR is included in version 12.21.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Resolves the review finding left open on the merged #1738: with
imageOptions.outputFormat: "jpeg", real JPEG bytes reached the OpenAI chat-completions wire inside a data URI labelledimage/png.Two independent drops caused it, and each one alone reproduces the bug:
src/lib/core/modules/MessageBuilder.tsrebuilt every image content part fromtypeandimageonly, discarding the resolved media type in bothbuildMessages()andbuildMessagesForStream().src/lib/providers/openaiChatCompletionsClient.ts:convertContentForOpenAInever passed the part's media type toimageDataToURL, so every raw-base64 image fell back to its hardcodedimage/pngdefault.Tests
test/continuous-test-suite-vision-image-format.tsnow asserts the wire label as well as the decoded bytes, on bothgenerate()andstream(). It is offline (loopback mock chat server), so it runs without credentials.stream()serializes to the wire at its own call site (doStream), so it gets its own label test. Stripping the media type at only that site now fails exactly that test with✗and exit 1, while everygenerate()test stays green. Before this change the same mutant passed all four tests.pnpm run check0 errors,pnpm run lint0 errors,docs:apiregenerated with no diff (neither file is in the public typedoc output).Where the other deferred review findings went
This PR was planned to consolidate several deferred findings. Most turned out to live in code that exists only on still-open PRs, so a
release-based PR cannot fix them:releaseStdioClientTransport.close()already escalates SIGTERM→SIGKILL; a PID fallback could signal a recycled PID after auto-restartlitellmon a machine with no proxy runningofficeOptionstype exist only on that branchbyCodepoint, empty-heading anchors (#1763)Summary by CodeRabbit