-
Notifications
You must be signed in to change notification settings - Fork 743
Clarify broadcast goes just to the server that created the surface, r… #503
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
Conversation
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.
Code Review
This pull request clarifies the behavior of client-to-server data synchronization by renaming broadcastDataModel to syncDataModel and updating the documentation to specify that data is sent only to the server that created the surface. The changes are applied consistently across the protocol documentation and relevant JSON schemas. My review includes a couple of minor suggestions to improve documentation formatting and maintain consistency in the JSON files.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| - `catalogId` (string, required): A string that uniquely identifies the catalog (components and functions) used for this surface. It is recommended to prefix this with an internet domain that you own, to avoid conflicts (e.g., `https://mycompany.com/1.0/somecatalog`). | ||
| - `theme` (object, optional): A JSON object containing theme parameters (e.g., `primaryColor`) defined in the catalog's theme schema. | ||
| - `broadcastDataModel` (boolean, optional): If true, the client will append the full data model of this surface to the metadata of every A2A message (like 'action') sent to the server. Defaults to false. | ||
| - `syncDataModel` (boolean, optional): If true, the client will append the full data model of this surface to the metadata of every A2A message sent to the server that created the surface. This ensures the surface owner receives the full current state of the UI alongside the user's action or query. Defaults to false. |
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.
Sync implies two-way exchange to make the server and client identical. This is a broadcast because it's only one-way.
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.
I was afraid broadcast implies 1:n, when it just goes back to the server who created the surface
what about attachDataModel?
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.
Yeah, that's fair. attachDataModel works. What about sendDataModel?
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.
went with attachDataModelToRequests, this should be quite accurate, I was afraid send implied separate message, please review, thanks
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.
I think attachDataModel worked better, attachDataModelToRequests feels a little long (and a little redundant).
Sorry, I'll stop bikeshedding now.
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.
done
| ``` | ||
|
|
||
| ### Client to Server Updates (Broadcasting) | ||
| ### Client to Server Updates (State Synchronization) |
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.
It isn't synchronizing, so maybe just remove the parenthetical?
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.
done
…enamed to sync
Description
Clarify client->server data broadcast goes only to the server that created the surface