Repository navigation
fix: WASM tool schema advertisement for exported typed schemas - #1348
hanakannzashi wants to merge 2 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the interaction between the agent and WASM tools by refining how tool schemas are advertised to the Language Model. It ensures that tools with small, well-defined schemas are presented accurately, allowing the agent to make precise calls, while larger schemas still provide necessary hints for discovery. This change directly addresses and fixes a critical regression in the Brave Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where WASM tool schemas were not being advertised properly to the LLM, leading to incorrect tool calls. The approach of advertising small schemas directly while falling back to a permissive schema for large ones is sound. The new test cases provide good coverage for this new logic. I have one suggestion to improve the clarity of a test name.
|
Same PR #1352 |
henrypark133
left a comment
There was a problem hiding this comment.
Review: WASM tool schema advertisement — overlaps with #1352
Thanks for the contribution! Unfortunately this PR overlaps almost entirely with #1352 (G7CNF), which implements the same behavior (advertise typed WASM schemas directly to the LLM when small enough) and has already been approved.
Both PRs modify the same lines in src/tools/wasm/wrapper.rs and src/tools/wasm/loader.rs. If #1352 lands first, this will create merge conflicts.
One thing your PR gets right that #1352 doesn't: the 8KB size limit (MAX_ADVERTISED_SCHEMA_BYTES = 8 * 1024) is more appropriate than #1352's 64KB. These schemas get serialized into every LLM call alongside all other tools, so a tighter budget is better for context management. We've asked #1352 to adopt the 8KB limit.
I'd recommend closing this PR in favor of #1352 to avoid the conflict. Thank you for the work — it clearly identified the right fix.
Minor note: the branch name fix/brave-search doesn't match the PR content, and there's an unrelated channels-src/whatsapp/Cargo.lock version bump included.
There was a problem hiding this comment.
Pull request overview
Fixes WASM tool schema advertisement so the LLM sees exported typed schemas for small/reasonable-size components (instead of always {}), while retaining the permissive advertisement + tool_info discovery path for large schemas. Also updates WASM loader warnings to reflect that WASM-exported metadata/schema can be used when sidecar fields are missing.
Changes:
- Advertise small typed WASM-exported schemas directly; keep permissive fallback for large schemas.
- Update/extend unit tests to cover small-vs-large exported schema advertisement behavior.
- Adjust WASM loader warning messages about missing
description/parametersin capabilities sidecars.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/tools/wasm/wrapper.rs | Adds size/typed checks to decide whether to advertise exported schemas; updates tests for small vs large exported schemas. |
| src/tools/wasm/loader.rs | Updates warning text for missing capabilities fields to indicate WASM-exported metadata/schema usage. |
| channels-src/whatsapp/Cargo.lock | Bumps the whatsapp-channel package version in the lockfile (aligned with Cargo.toml). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn should_advertise_discovery(schema: &serde_json::Value) -> bool { | ||
| Self::typed_property_count(schema) > 0 | ||
| && schema.to_string().len() <= Self::MAX_ADVERTISED_SCHEMA_BYTES | ||
| } | ||
|
|
||
| fn new(discovery: serde_json::Value) -> Self { | ||
| let advertised = if Self::should_advertise_discovery(&discovery) { | ||
| discovery.clone() | ||
| } else { | ||
| Self::permissive_schema() | ||
| }; |
| Self::typed_property_count(schema) > 0 | ||
| && schema.to_string().len() <= Self::MAX_ADVERTISED_SCHEMA_BYTES |
| path = %cap_path.display(), | ||
| "Capabilities file missing \"parameters\" field; \ | ||
| tool will accept any JSON object (permissive fallback)" | ||
| using exported WASM schema when available" |
Summary
{}schema.tool_infohint when the exported schema is too large.descriptionorparametersare missing from the capabilities sidecar.web-searchbehavior where the agent previously saw only{}and repeatedly called the tool without the requiredqueryfield.Change Type
Linked Issue
#1303
Validation
cargo fmtcargo clippy --all --benches --tests --examples --all-featurescargo check -q,cargo test -q test_advertised_schema_stays_permissive_until_sidecar_override --lib,cargo test -q test_large_exported_schema_stays_permissive_for_advertising --libweb-searchregression end-to-end; after the fix the agent successfully called the Brave WASM tool with real search parameters and completed a search, instead of repeatedly calling it with{}and failing on missingquerySecurity Impact
None.
Database Impact
None.
Blast Radius
Touches WASM tool schema advertisement and loader warning behavior. Regressions could affect how WASM tools are exposed to the LLM, especially tools with large exported schemas that should still stay compact.
Rollback Plan
Revert the WASM schema advertisement change in
src/tools/wasm/wrapper.rsand the warning text updates insrc/tools/wasm/loader.rsto restore the previous behavior.Review track: B