fix(shell): render text-only A2A fallbacks#836
fix(shell): render text-only A2A fallbacks#836stablegenius49 wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful fallback mechanism to render text-only A2A responses, preventing a blank UI. The new convertA2APartsToMessages function correctly transforms text parts into A2UI components, and the logic is well-supported by new regression tests. I've provided one suggestion to refactor the new function to improve its maintainability by reducing some code duplication.
| if (textMessages.length === 1) { | ||
| const rootId = textFallbackComponentId(0); | ||
| return [ | ||
| { | ||
| beginRendering: { | ||
| root: rootId, | ||
| surfaceId: TEXT_FALLBACK_SURFACE_ID, | ||
| }, | ||
| }, | ||
| { | ||
| surfaceUpdate: { | ||
| surfaceId: TEXT_FALLBACK_SURFACE_ID, | ||
| components: [ | ||
| { | ||
| id: rootId, | ||
| component: { | ||
| Text: { | ||
| usageHint: "body" as const, | ||
| text: { literalString: textMessages[0] }, | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| ]; | ||
| } | ||
|
|
||
| const childIds = textMessages.map((_, index) => textFallbackComponentId(index)); | ||
|
|
||
| return [ | ||
| { | ||
| beginRendering: { | ||
| root: TEXT_FALLBACK_ROOT_ID, | ||
| surfaceId: TEXT_FALLBACK_SURFACE_ID, | ||
| }, | ||
| }, | ||
| { | ||
| surfaceUpdate: { | ||
| surfaceId: TEXT_FALLBACK_SURFACE_ID, | ||
| components: [ | ||
| { | ||
| id: TEXT_FALLBACK_ROOT_ID, | ||
| component: { | ||
| Column: { | ||
| children: { | ||
| explicitList: childIds, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| ...textMessages.map((text, index) => ({ | ||
| id: childIds[index], | ||
| component: { | ||
| Text: { | ||
| usageHint: "body" as const, | ||
| text: { literalString: text }, | ||
| }, | ||
| }, | ||
| })), | ||
| ], | ||
| }, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
The logic for handling single and multiple text messages is a bit repetitive. Both branches of the if statement construct and return a similar array of beginRendering and surfaceUpdate messages. You can reduce this duplication by first determining the rootId and components array based on the number of text messages, and then constructing the final message array once.
let rootId: string;
let components: v0_8.Types.ComponentInstance[];
if (textMessages.length === 1) {
rootId = textFallbackComponentId(0);
components = [
{
id: rootId,
component: {
Text: {
usageHint: "body" as const,
text: { literalString: textMessages[0] },
},
},
},
];
} else {
rootId = TEXT_FALLBACK_ROOT_ID;
const childIds = textMessages.map((_, index) =>
textFallbackComponentId(index)
);
components = [
{
id: TEXT_FALLBACK_ROOT_ID,
component: {
Column: {
children: {
explicitList: childIds,
},
},
},
},
...textMessages.map((text, index) => ({
id: childIds[index],
component: {
Text: {
usageHint: "body" as const,
text: { literalString: text },
},
},
})),
];
}
return [
{
beginRendering: {
root: rootId,
surfaceId: TEXT_FALLBACK_SURFACE_ID,
},
},
{
surfaceUpdate: {
surfaceId: TEXT_FALLBACK_SURFACE_ID,
components,
},
},
];
Summary
Fixes #585.
Why
The Lit shell client currently drops
textparts and only forwardsdataparts into the A2UI processor. When an agent returns a text-only fallback message, the UI ends up blank even though the backend sent a useful error/fallback string.Testing
npm installinrenderers/web_corenpm run build:tscinrenderers/web_corenpm installinrenderers/markdown/markdown-itnpm run buildinrenderers/markdown/markdown-itnpm installinsamples/client/lit/shellnpm run build:tscinsamples/client/lit/shellnode --test --enable-source-maps dist/tests/client.test.js