-
Notifications
You must be signed in to change notification settings - Fork 298
Investigate baml image serialization issue #2513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: canary
Are you sure you want to change the base?
Investigate baml image serialization issue #2513
Conversation
Co-authored-by: aaron <[email protected]>
Cursor Agent can help with this pull request. Just |
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🌿 Preview your docs: https://boundary-preview-9c688c75-547d-4ee1-a56d-96e2f8cdeb6a.docs.buildwithfern.com |
return arg; | ||
}); | ||
// Transform any BAML media inputs (even when nested) to their JSON representation | ||
const transformedInput = input.map(serializeBamlMediaDeep) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Circular Reference Handling in Serialization
The serializeBamlMediaDeep
function recursively traverses objects without tracking visited objects. This can lead to infinite recursion and a stack overflow if the input contains circular references.
Additional Locations (1)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This commit refactors the media parsing logic in the jinja runtime to be more robust and tolerant of surrounding characters. It also updates the react hooks generator to only serialize BamlImage and BamlAudio types, as other media types are not supported by the current implementation. Co-authored-by: aaron <[email protected]>
🌿 Preview your docs: https://boundary-preview-4b89712a-ea2b-49a9-9c66-07854b7694d5.docs.buildwithfern.com |
Pull Request Template
Thanks for taking the time to fill out this pull request!
Issue Reference
Please link to any related issues
Changes
Please describe the changes proposed in this pull request
This PR introduces a deep serialization mechanism for BAML media types (Image, Audio, Pdf, Video) within React hooks. Previously, only top-level BAML media instances were serialized to their JSON representation before being sent to server actions. This change adds a
serializeBamlMediaDeep
helper function that recursively traverses input objects and arrays, ensuring that any nested BAML media instances are correctly converted to JSON.The
mutate
function in theuseBamlAction
hook now uses this deep serialization, preventing nested media from being stripped by Next.js server action serialization.Testing
Please describe how you tested these changes
Screenshots
If applicable, add screenshots to help explain your changes
[Add screenshots here...]
PR Checklist
Please ensure you've completed these items
Additional Notes
Add any other context about the PR here
Root Cause: When a
BamlImage
(or other BAML media) was nested inside a dynamic class or array as an input parameter to a BAML function called via a React hook, it was not being properly serialized. TheuseBamlAction
hook only performed a shallow transformation of top-level arguments. Consequently, Next.js server action serialization would strip the private fields of theBamlImage
object, effectively turning it into an empty object{}
before it reached the server, causing it to "disappear."Fix: The
serializeBamlMediaDeep
helper ensures that all BAML media instances, regardless of their nesting level within objects or arrays, are converted to their JSON representation ({ url }
or{ base64, media_type }
) before the server action call. This allows the server-side argument coercer to correctly recognize and process the media.The Go file changes are unrelated cleanups (removal of unused imports) and do not directly pertain to the BAML media serialization fix.