fix(grpc): make history_length optional #1071
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @yarolegovich, 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 enhances the flexibility of history management within the a2a system by making the history_length field optional in key gRPC message definitions: SendMessageConfiguration and GetTaskRequest. This update allows clients to precisely control whether history is fetched or persisted, differentiating between a zero value and an unset state. The change ensures consistency across various communication transports, including gRPC and JSON-RPC, and is fully backward-compatible.
Highlights
- gRPC Field Optionality: The
history_lengthfield in bothSendMessageConfigurationandGetTaskRequestgRPC messages has been updated from a requiredint32to anoptional int32. This allows for explicit differentiation between a zero value (e.g., unlimited history) and an unset value (e.g., no history desired). - Consistency Across Transports: This change aligns the gRPC specification with existing
types.tsand JSON-RPC schemas, wherehistory_lengthis already optional. This ensuresa2aservers can provide consistent behavior regarding history handling, regardless of the communication transport used. - Backward Compatibility: The modification is backward-compatible, meaning existing clients will continue to function without issues, while new clients can leverage the enhanced control offered by the optional field.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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
-
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. ↩
There was a problem hiding this comment.
Code Review
This pull request makes the history_length field optional in SendMessageConfiguration and GetTaskRequest to align with the JSON-RPC specification and provide more flexibility. The change is logical and well-justified. My review includes suggestions to improve the comments for these fields to clearly document the behavior when the field is unset, which is important for API consumers.
specification/grpc/a2a.proto
Outdated
| // The maximum number of messages to include in the history. if 0, the | ||
| // history will be unlimited. | ||
| int32 history_length = 3; | ||
| // The maximum number of messages to include in the history. |
There was a problem hiding this comment.
The comment for history_length has become less informative. The previous version specified that a value of 0 means unlimited history. With the field now being optional, it's crucial to document the behavior for both unset and 0 values to avoid ambiguity for developers using this proto.
I'd suggest clarifying the semantics in the comment.
| // The maximum number of messages to include in the history. | |
| // The maximum number of messages to include in the history. If unset, no history is persisted; if 0, history is unlimited. |
specification/grpc/a2a.proto
Outdated
| // The resource name of the task. | ||
| // Format: tasks/{task_id} | ||
| string name = 1 [(google.api.field_behavior) = REQUIRED]; | ||
| // The number of most recent messages from the task's history to retrieve. |
There was a problem hiding this comment.
Now that history_length is optional, it would be helpful to clarify in the comment what happens when the field is not set. This improves clarity for developers using the API.
| // The number of most recent messages from the task's history to retrieve. | |
| // The number of most recent messages from the task's history to retrieve. If not set, no history is retrieved. |
holtskinner
added
the
TSC Review
Description
Both fields are optional in types.ts and jsonrpc schema.
For both cases it might be useful to differentiate 0 and unset. For example:
The change is backward-compatible and makes it possible for a2a servers to provide consistent behavior regardless of the selected transport.
Fixes #1072