feat: Support component deletion via component: null #528
Conversation
Update server_to_client.json schema to allow component: null. Update protocol documentation. Add component_deletion.json test cases. Update evaluation framework to support and test this feature.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature to support component deletion using a component: null tombstone. The implementation is solid, with corresponding updates to the protocol documentation, JSON schema, and evaluation logic. I have a couple of suggestions to improve the clarity and consistency of the documentation. Additionally, please remember to fill out the Pre-Review Checklist in the pull request description as required by the repository's contribution guidelines.
| ### `updateComponents` | ||
|
|
||
| This message provides a list of UI components to be added to or updated within a specific surface. The components are provided as a flat list, and their relationships are defined by ID references in an adjacency list. This message may only be sent to a surface that has already been created. Note that components may reference children or data bindings that do not yet exist; clients should handle this gracefully by rendering placeholders (progressive rendering). | ||
| This message provides a list of UI components to be added, updated, or removed within a specific surface. The components are provided as a flat list, and their relationships are defined by ID references in an adjacency list. This message may only be sent to a surface that has already been created. |
There was a problem hiding this comment.
The previous version of this paragraph included an important note about progressive rendering: "Note that components may reference children or data bindings that do not yet exist; clients should handle this gracefully by rendering placeholders (progressive rendering)."
While this concept is mentioned elsewhere in the documentation, it is highly relevant in the updateComponents section. Its removal could cause confusion for developers implementing a client. I recommend re-adding a concise version of this note to ensure client implementation details are clear in this context.
There was a problem hiding this comment.
Yes, we should keep the part describing progressive rendering.
| { | ||
| "id": "user_title", | ||
| "component": "Text", | ||
| "text": "Software Engineer" | ||
| "id": "old_component_id", | ||
| "component": null | ||
| } |
There was a problem hiding this comment.
The updated example is a bit confusing. The root component (defined on lines 172-174) references a component with id: "user_title" in its children array. However, this change removes the user_title component and replaces it with a deletion request for old_component_id. This leaves a dangling reference in root and it's not clear what old_component_id refers to.
To make the example clearer and self-consistent, I recommend showing the deletion of an existing component from the example, like user_title. You would also need to update the root component to remove user_title from its children array for a fully consistent state.
Here is a suggestion to make the intent clearer, though the root component would still need to be updated separately.
| { | |
| "id": "user_title", | |
| "component": "Text", | |
| "text": "Software Engineer" | |
| "id": "old_component_id", | |
| "component": null | |
| } | |
| "id": "user_title", | |
| "component": null |
There was a problem hiding this comment.
This is a valid issue don't remove user_title: it's referenced.
|
This addresses #235 |
| "component": "Text", | ||
| "text": "Software Engineer" | ||
| "id": "old_component_id", | ||
| "component": null |
There was a problem hiding this comment.
why wouldn't it work to just exclude component?
{
"id": "old_component_id"
}
updateDataModel overwrites instead of merging, and it treats null as remove key, so it should have the same effect, no?
|
How do we expect this to be used? Will we be prompting the LLM to do garbage collection? Wouldn't it be more effective to just have a message from the LLM to the client that it believes all relevant components are being referenced, and the client is free to do garbage collection? I have doubts that the LLM will be able (or have the information to) keep track of all component references. |
| ### `updateComponents` | ||
|
|
||
| This message provides a list of UI components to be added to or updated within a specific surface. The components are provided as a flat list, and their relationships are defined by ID references in an adjacency list. This message may only be sent to a surface that has already been created. Note that components may reference children or data bindings that do not yet exist; clients should handle this gracefully by rendering placeholders (progressive rendering). | ||
| This message provides a list of UI components to be added, updated, or removed within a specific surface. The components are provided as a flat list, and their relationships are defined by ID references in an adjacency list. This message may only be sent to a surface that has already been created. |
There was a problem hiding this comment.
Yes, we should keep the part describing progressive rendering.
|
|
||
| - `surfaceId` (string, required): The unique identifier for the UI surface to be updated. This is typically a name with meaning (e.g. "user_profile_card"), and it has to be unique within the context of the GenUI session. | ||
| - `components` (array, required): A list of component objects. The components are provided as a flat list, and their relationships are defined by ID references in an adjacency list. | ||
| - `components` (array, required): A list of component objects (for updates) or tombstone objects (for deletions). |
There was a problem hiding this comment.
What about the adjacency list description? I think we should keep that.
| { | ||
| "id": "user_title", | ||
| "component": "Text", | ||
| "text": "Software Engineer" | ||
| "id": "old_component_id", | ||
| "component": null | ||
| } |
There was a problem hiding this comment.
This is a valid issue don't remove user_title: it's referenced.
3 participants
Description
This PR introduces a mechanism to explicitly delete components from a surface within the
updateComponentsmessage.Changes
server_to_client.jsonschema forupdateComponentshas been modified. Thecomponentslist now accepts a 'tombstone' object in addition to standard component definitions.idand thecomponentproperty set explicitly tonull.a2ui_protocol.mdto document this new behavior.component_deletion.jsonto the test suite and updated the evaluation framework to support this feature.Rationale
Previously, removing a component typically required re-sending the entire parent structure or using
deleteSurfaceand recreating it, which is inefficient and can disrupt the user experience. This change allows for granular updates where specific components can be removed while others are added or updated in the same atomic message.Sample Code
To delete a component with ID
loading-spinnerand simultaneously add a newstatus-textcomponent:{ "updateComponents": { "surfaceId": "main", "components": [ { "id": "loading-spinner", "component": null }, { "id": "status-text", "component": "Text", "text": "Loaded Successfully" } ] } }