Skip to content

Conversation

@khj809
Copy link
Contributor

@khj809 khj809 commented Oct 7, 2025

This PR adds WebAssembly (WASM) bindings for the core components in the value module, as outlined in #210.
While complete parity between the generated type definitions for WASM and Node.js is not possible due to binding library limitations, this implementation aims to make them as consistent as possible.

Implementations

  • Similar to other language bindings, Value can be converted to and from any primitive JavaScript value, and is typed as any.
  • Most structs and enums use tsify to take advantage of serde-wasm-bindgen’s type generation support.
  • WASM bindings can be generated using: wasm-pack build --out-dir ./bindings/web/src --out-name ailoy-web --no-pack. Note that this command creates a .gitignore file that must be manually removed afterward. (There’s an ongoing request to disable this behavior: wasm-pack#1408, though it hasn’t been addressed by maintainers.)

Changes

  • The internal representation of Bytes was changed from Vec<u8> to serde_bytes::ByteBuf, which is natively supported by tsify and serde-wasm-bindgen.
  • Added "constEnum": false in the NAPI configuration so that simple string enums are generated as string literals for both WASM and Node.js.
  • Updated the FinishReason and Grammar enums from tuple variants to struct variants to ensure their generated types align across WASM and Node.js targets.
  • Removed the obsolete header.d.ts in nodejs binding.

@khj809 khj809 self-assigned this Oct 7, 2025
@khj809 khj809 linked an issue Oct 7, 2025 that may be closed by this pull request
Base automatically changed from feature/model-manager-cli to main October 13, 2025 05:02
@khj809 khj809 marked this pull request as ready for review October 13, 2025 14:03
@khj809 khj809 requested review from grf53 and jhlee525 October 13, 2025 14:03
@khj809
Copy link
Contributor Author

khj809 commented Oct 14, 2025

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link

QodoAI-Agent commented Oct 14, 2025

PR Reviewer Guide 🔍

(Review updated until commit 26a168c)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

API Consistency

The renaming from f/args to function/arguments and enum tagging/rename_all changes alter the external JSON/TS shapes. Validate all downstream marshal/unmarshal paths and language bindings (Python/Node/WASM) are updated consistently to prevent runtime decoding issues.

pub enum Part {
    Text {
        text: String,
    },
    Function {
        id: Option<String>,
        function: PartFunction,
    },
    Value {
        value: Value,
    },
    Image {
        image: PartImage,
    },
}

impl Part {
    pub fn text(v: impl Into<String>) -> Self {
        Self::Text { text: v.into() }
    }

    pub fn image_binary(
        height: u32,
        width: u32,
        colorspace: impl TryInto<PartImageColorspace>,
        data: impl IntoIterator<Item = u8>,
    ) -> anyhow::Result<Self> {
        let colorspace: PartImageColorspace = colorspace
            .try_into()
            .ok()
            .context("Colorspace parsing failed")?;
        let data = data.into_iter().collect::<Vec<_>>();
        let nbytes = data.len() as u32 / height / width / colorspace.channel();
        if !(nbytes == 1 || nbytes == 2 || nbytes == 3 || nbytes == 4) {
            panic!("Invalid data length");
        }
        Ok(Self::Image {
            image: PartImage::Binary {
                height: height as u32,
                width: width as u32,
                colorspace,
                data: super::bytes::Bytes(data.into()),
Role Mapping

Gemini marshaling now maps Role to "model"/"user" with panic for other roles. Confirm no code paths construct system/tool messages for Gemini; otherwise this will panic at runtime.

    );
}

// Role
let role: String = if msg.role == Role::Assistant {
    "model".into()
} else if msg.role == Role::User {
    "user".into()
} else {
    panic!("Gemini accepts \"model\" and \"user\" role only.")
};

// Collecting contents
let mut parts = Vec::<Value>::new();
if let Some(thinking) = &msg.thinking
    && !thinking.is_empty()
    && include_thinking
{
    parts.push(to_value!({"text": thinking, "thought": true}));
}
parts.extend(msg.contents.iter().map(part_to_value));
parts.extend(
    msg.tool_calls
        .clone()
        .unwrap_or(vec![])
        .iter()
        .map(part_to_value),
);

// Final message object with role and collected parts
to_value!({"role": role, "parts": parts})
Image Buffer Handling

In image conversions, buf.to_vec() is used for from_raw; ensure this extra allocation is intended and not a regression vs previous clone() semantics. Consider zero-copy if possible.

Self::Image {
    image:
        PartImage::Binary {
            height,
            width,
            colorspace,
            data: super::bytes::Bytes(buf),
        },
} => {
    let (h, w) = (*height as u32, *width as u32);
    let nbytes = buf.len() as u32 / h / w / colorspace.channel();
    match (colorspace, nbytes) {
        // Grayscale 8-bit
        (&PartImageColorspace::Grayscale, 1) => {
            let buf = image::GrayImage::from_raw(w, h, buf.to_vec())?;
            Some(image::DynamicImage::ImageLuma8(buf))
        }
        // Grayscale 16-bit
        (&PartImageColorspace::Grayscale, 2) => {
            let buf = image::ImageBuffer::<image::Luma<u16>, _>::from_raw(
                w,
                h,
                bytes_to_u16_ne(buf)?,
            )?;
            Some(image::DynamicImage::ImageLuma16(buf))
        }
        // RGB 8-bit
        (&PartImageColorspace::RGB, 1) => {
            let buf = image::RgbImage::from_raw(w, h, buf.to_vec())?;
            Some(image::DynamicImage::ImageRgb8(buf))
        }
        // RGBA 8-bit
        (&PartImageColorspace::RGBA, 1) => {
            let buf = image::RgbaImage::from_raw(w, h, buf.to_vec())?;
            Some(image::DynamicImage::ImageRgba8(buf))
        }

@khj809 khj809 requested a review from grf53 October 14, 2025 11:37
Copy link
Contributor

@jhlee525 jhlee525 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work 👍 Thank you

@khj809
Copy link
Contributor Author

khj809 commented Oct 15, 2025

@jhlee525 @grf53 As I mentioned earlier, I changed some fields to optional (thinking and tool_calls in Message, thinking_effort and grammar in InferenceConfig), and fixed every broken tests accordingly. PTAL.

@khj809 khj809 requested a review from jhlee525 October 15, 2025 15:14
@QodoAI-Agent
Copy link

Persistent review updated to latest commit 26a168c

Copy link
Contributor

@grf53 grf53 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@khj809 khj809 merged commit cfd3399 into main Oct 16, 2025
@khj809 khj809 deleted the feature/wasm-binding branch October 16, 2025 03:43
@QodoAI-Agent
Copy link

Persistent review updated to latest commit 26a168c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WASM binding: mod value

5 participants