Skip to content

Conversation

@techraed
Copy link
Member

@techraed techraed commented Dec 8, 2025

TODO:

  • proper errors handling
  • check_asyncness to be method in concrete exposure
  • consts map to be outside of wasm module
  • clean-up macro impl

Notice

The branch is created from st-header-type-idlv2

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @techraed, 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 introduces a significant overhaul to the binary protocol handling within Sails by implementing a new program macro. The core change shifts from byte-slice based message parsing to a structured SailsMessage type, which encapsulates both header and payload. This enables a more robust and efficient routing mechanism, leveraging compile-time generated interface and entry IDs for service dispatch. The refactoring also includes improvements to how service metadata and asynchronous handling are managed, leading to a more streamlined and type-safe approach for defining and interacting with services.

Highlights

  • Binary Protocol Macro Implementation: A new program macro for Sails has been implemented to handle binary protocol messages, enabling a more structured and efficient routing mechanism for service calls.
  • Structured SailsMessage: Introduced a SailsMessage struct that encapsulates message headers and payloads, facilitating structured decoding and routing based on interface and entry IDs.
  • Compile-Time Interface ID Generation: The system now generates INTERFACE_IDS and BASE_SERVICES_IDS at compile time, enhancing performance and type safety in service dispatch.
  • Refactored Service Dispatch: The handle() function in programs has been updated to use a match statement on route_id for dispatching service calls, moving away from previous byte-slice matching.
  • Enhanced Exposure Trait: The Exposure::check_asyncness method now accepts interface_id and entry_id parameters, allowing for more precise asynchronous handling checks.
  • Lexicographical Sorting for Base Services: Base services are now sorted lexicographically when generating metadata and dispatch logic, ensuring consistent ordering and predictability.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new binary protocol for Sails, moving from string-based routing to a more efficient and deterministic InterfaceId and entry_id system. Key changes include the addition of AnyServiceIds and ServiceMeta::BASE_SERVICES_IDS for compile-time service identification, and a significant refactoring of the program and service macros to support this new routing. The SailsMessage struct is introduced to encapsulate message headers and payloads, and new routing module provides const fn utilities for compile-time service counting and interface ID generation. The changes align well with the INTERFACE_ID_SPEC.md for deterministic ID generation, including lexicographical sorting of functions and base services.

The refactoring efforts, such as collecting data in a single pass to avoid borrow issues and applying modifications outside iteration loops, are good practices. The PR also addresses several items from its own TODO list, such as proper error handling (though some improvements can still be made), moving constants outside the WASM module, and cleaning up macro implementations.

Comment on lines +293 to +296
let is_async = <#service_type as #sails_path::gstd::services::Service>::Exposure::check_asyncness(interface_id, entry_id)
.unwrap_or_else(|| {
gstd::unknown_input_panic("Unknown call", &[])
});
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The unwrap_or_else for check_asyncness currently provides a generic panic message. For better debugging and clarity, it would be more helpful to include the specific interface_id and entry_id that led to the "Unknown call" panic.

This would provide more context if a service call fails to be routed correctly.

                        let is_async = <#service_type as #sails_path::gstd::services::Service>::Exposure::check_asyncness(interface_id, entry_id)
                            .unwrap_or_else(|| {
                                gstd::unknown_input_panic(
                                    &format!("Unknown call for interface_id: {:?}, entry_id: {}", interface_id, entry_id),
                                    &[],
                                )
                            });


match route_id {
#(#route_dispatches)*
_ => gstd::unknown_input_panic("Unknown route_id", &[route_id])
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The unknown_input_panic for an Unknown route_id could be more informative. Including the interface_id along with the route_id in the panic message would provide additional context for debugging routing issues.

                    _ => gstd::unknown_input_panic(
                        &format!("Unknown route_id: {} for interface_id: {:?}", route_id, interface_id),
                        &[route_id],
                    )

Base automatically changed from st-header-type-idlv2 to master-idl-v2 December 16, 2025 12:36
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.

2 participants