Port Angular Restaurant Sample to 0.9 and Stabilize Tests#1189
Port Angular Restaurant Sample to 0.9 and Stabilize Tests#1189josemontespg wants to merge 7 commits intogoogle:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the Angular renderer and the restaurant finder sample to version 0.9. Key updates include the implementation of reactive property updates and weight support in ComponentHostComponent, the addition of stable change detection in ListComponent, and a specialized path resolution logic in DataContext. The restaurant finder agent was also updated with a monkey patch for ADK compatibility and refined LLM instructions. Feedback focuses on ensuring the weight signal is updated during property changes, improving type safety by replacing any with specific models, addressing the impact of the new path resolution logic on global state access, and correcting the action handling in the client to ensure the backend receives the expected event format.
| private resolvePath(path: string): string { | ||
| if (path.startsWith('/')) { | ||
| return path; | ||
| } | ||
| if (path === '' || path === '.') { | ||
| // In A2UI v0.9, all paths generated by the prompt-first LLM must begin with a leading `/` | ||
| // to satisfy RFC 6901 JSON Pointer validation. | ||
| // Thus, paths like `/imageUrl` are effectively relative to this DataContext. | ||
| let relPath = path.startsWith('/') ? path.substring(1) : path; |
There was a problem hiding this comment.
This change to resolvePath effectively disables absolute path resolution (paths starting with /) by treating them as relative to the current DataContext. While the comment explains this is to accommodate LLM behavior in v0.9, it breaks standard JSON Pointer expectations and prevents components from accessing global state outside their nested scope. This might lead to issues if a component genuinely needs to reference data at the root of the model.
There was a problem hiding this comment.
I'm not sure about this one. @jacobsimionato is the assumption in code here correct?
jacobsimionato
left a comment
There was a problem hiding this comment.
Wow, you worked through a lot of issues to get this running. Thank you so much for going on the journey - looks great and will really help our users figure out how to use A2UI!
Let's sync about the relative/absolute path thing tomorrow!
| * Track-by function to ensure stable change detection for list items. | ||
| * Uses the full resolved path (`basePath/id`) to uniquely identify items. | ||
| */ | ||
| readonly trackBy = (index: number, item: Child) => `${item.basePath}/${item.id}`; |
There was a problem hiding this comment.
Nice!! This is where having a proper framework expert is important.
|
|
||
| @HostBinding('style.flex') | ||
| get flexStyle() { | ||
| const w = this.weight(); |
There was a problem hiding this comment.
Weight is property that's defined in the basic catalog specification (https://github.com/google/A2UI/blob/main/specification/v0_9/json/basic_catalog.json#L1253), but not in the core A2UI specification, so handling of it should live in the basic catalog implementation rather than in the core renderer codebase.
I know it might be awkward to implement, but it's okay to have a shared utility that is used by every Component in the catalog. It's just important that we don't bake any catalog-specific stuff into the core, so that developers have a blank canvas to build their own catalogs on.
| console.warn(`Component ${id} not found in surface ${this.surfaceId()}`); | ||
| console.warn(`Component ${id} not found in surface ${this.surfaceId()}. Waiting for it...`); | ||
|
|
||
| const sub = surface.componentsModel.onCreated.subscribe((comp) => { |
There was a problem hiding this comment.
Oh nice - I assume this fixes streaming.
| // component to react when a new prop is added after creation. | ||
| const onPropsUpdateSub = componentModel.onUpdated.subscribe(() => { | ||
| this.props = this.binder.bind(this.context!); | ||
| this.weight.set(componentModel.properties['weight'] || null); |
There was a problem hiding this comment.
Again, let's remove any references to weight here.
| return path; | ||
| } | ||
| if (path === '' || path === '.') { | ||
| // In A2UI v0.9, all paths generated by the prompt-first LLM must begin with a leading `/` |
There was a problem hiding this comment.
I ran into this issue too with other renderers. We made a mistake in the specification. We do want to have absolutely and relative path support, which doesn't follow the RFC exactly.
I merged #1084 last week which updates the spec to be clear on this, but I found out yesterday that there is a bug in the streaming parser in Python that adds the trailing slash back even if the LLM does not include it.
I just mailed #1179 to resolve this. Can you try removing this logic from your change, but patching my Python change instead? The agent should send the relative path without the leading slash, and the client should interpret it correctly.
| original_inject = instructions_utils.inject_session_state | ||
|
|
||
|
|
||
| async def custom_inject_session_state(template: str, context: Any) -> str: |
There was a problem hiding this comment.
Oh yeah, we keep hitting this. Nice!
| "formatString": { | ||
| "type": "object", | ||
| "description": "Performs string interpolation of data model values and other functions in the catalog functions list and returns the resulting string. The value string can contain interpolated expressions in the `${expression}` format. Supported expression types include: JSON Pointer paths to the data model (e.g., `${/absolute/path}` or `${relative/path}`), and client-side function calls (e.g., `${now()}`). Function arguments must be named (e.g., `${formatDate(value:${/currentDate}, format:'MM-dd')}`). To include a literal `${` sequence, escape it as `\\${`.", | ||
| "description": "Performs string interpolation of data model values and other functions in the catalog functions list and returns the resulting string. The value string can contain interpolated expressions in the `${<expression>}` format. Supported expression types include: JSON Pointer paths to the data model (e.g., `${/absolute/path}` or `${relative/path}`), and client-side function calls (e.g., `${now()}`). Function arguments must be named (e.g., `${formatDate(value:${/currentDate}, format:'MM-dd')}`). To include a literal `${` sequence, escape it as `\\${`.", |
There was a problem hiding this comment.
Is this necessary? I imagine it was another attempt to work around the ADK string interpolation issue?
This PR ports the Angular restaurant sample app to version 0.9.
Credits to @gspencergoog for the first port attempt in #1101, I reused most of his work to create this PR.
Key Changes
ComponentHostComponentto handle property updates reactively using Angular Signals.renderers/angularto use the realComponentBinderservice and formalComponentModelinstances instead of object-literal mocks.data-context.Fixes #701
Screenshots