Update renderer guide to comprehensively cover Catalog API design and implementation process#788
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and valuable refactoring of the A2UI Renderer Guide. The new structure, with clear separation of concerns, implementation topologies, and a detailed agent implementation guide, greatly improves the clarity and usefulness of the documentation. The migration from RxJS to Preact Signals is also a positive step towards a more modern reactivity model. I've added a few minor suggestions to address small typos and restore some helpful explanatory text that was removed during the refactor.
Note: Security Review has been skipped due to the limited scope of the PR.
|
|
||
| * **Ideal Choice**: A library (like **Zod** in TypeScript or **Pydantic** in Python) that allows for programmatic definition of schemas and the ability to validate raw JSON data against those definitions. | ||
| * **Capabilities Generation**: The library should ideally support exporting these programmatic definitions to standard JSON Schema for the `getClientCapabilities` payload. | ||
| * **Fallback**: If no suitable programmatic library exists for the target language, raw **JSON Schema strings**, `Codable` structs, or `kotlinx.serialization` classes can be used instead. |
There was a problem hiding this comment.
Maybe also code generation?
There was a problem hiding this comment.
Well, the catalog agnostic part of the library can't refer to catalog-specific codegenned classes, so I think strings make sense in this case. Elsewhere, I think the doc already mentions codegen as an approach to universal bindings etc.
| * When an example is selected, it should pipe the messages into the `MessageProcessor` and render the surface. | ||
| * **Reactivity Test**: Add a mechanism to simulate delayed `updateDataModel` messages (e.g., waiting 2 seconds before sending data) to prove that the UI progressively renders and reacts to changes. | ||
|
|
||
| **STOP HERE. Ask the human user for approval of the architecture and demo application before proceeding to step 7.** |
There was a problem hiding this comment.
tiny nit: I'd remove "human", just because it might be an agent running this as a sub-agent. Or leave it. It probably doesn't matter.
There was a problem hiding this comment.
Agreed, let's remove "human". We can just say stop here and check the app works before moving on.
| * **Fallback**: If no suitable programmatic library exists for the target language, raw **JSON Schema strings**, `Codable` structs, or `kotlinx.serialization` classes can be used instead. | ||
|
|
||
| #### 2. Observable Library | ||
| A2UI relies on a standard observer pattern to reactively update the UI when data changes. The Data Layer and client-side functions must be able to return streams or reactive variables that hold an initial value and emit subsequent updates. |
There was a problem hiding this comment.
Is there any way we can ensure the API is of the form subscribe / unsubscribe and leave the particular mechanism as an implementation detail?
This might get into over-specifying and maybe it's not so important if we don't expect users to be using more than one client framework.
There is also some tension between a completely consistent API vs. leveraging idiomatic patterns and best practices for each language.
I think I'm talking myself out of suggesting any changes, but I'm going to leave this comment in case it spurs further thought from anyone else.
(This might be getting into where we provide one canonical implementation and encourage the use of agentic coding to "port" that implementation to other languages? At this point, we're probably going beyond the scope of this document, though...)
There was a problem hiding this comment.
I agree there is a tension here which is tricky to navigate. I think we're on the same page. I think there are two kinds of observation necessary here:
- Value updates (there is a current state e.g. data model value, which may change over time)
- Events with no current state (an action is triggered from a component)
Each codebase should choose some approach the two cases above and use it consistently, but we don't care how they do it.
| * **`ComponentModel`**: A specific component's raw configuration. | ||
| * **`DataModel`**: A dedicated store for application data. | ||
| * **`DataContext`**: A scoped window into the `DataModel`. | ||
| * **`DataContext`**: A scoped window into the `DataModel`. Used by functions and components to resolve dependencies and mutate state. |
There was a problem hiding this comment.
How is the appropriate "window" determined?
There was a problem hiding this comment.
The window is specifically a view of the data model that has some defined base path, so that relative path references are resolved against this. This is typically just the root /, except for cases where we are using the "template" pattern to build a ChildList of Components. When this occurs, the parent component will generally call some function to build each child, specifying the same component ID but a different base path for each child so that the same template can be rendered with different data.
Let's explain this.
There was a problem hiding this comment.
Even that template case assumes that all the data a child needs is contained in a narrow branch of the data model. If, for example, there's some other data that the child needs to either read or write, then this "window" seems like it is always just "/".
I like the idea of being able to limit what the component can access, but I just wonder if it's actually going to work out that way...
| * **Mutable:** State is updated in place. | ||
| * **Observable:** Each layer is responsible for making its direct properties observable via standard listener patterns, avoiding heavy reactive dependencies. | ||
| * **Encapsulated Composition:** Parent layers expose methods to add fully-formed child instances (e.g., `addSurface`, `addComponent`) rather than factory methods that take parameters. | ||
| These classes are designed to be "dumb containers" for data. They hold the state of the UI but contain minimal logic. They are organized hierarchically. |
There was a problem hiding this comment.
Can we be more descriptive of what "minimal logic" means?
There was a problem hiding this comment.
Let's avoid the use of the term "dumb" because it comes across as negative and judgemental.
These models classes are intended to represent specifically the information communicated in the A2UI protocol, while not implementing any parsing, decoding, validation, conversion or rendering logic. This should all be in other layers.
There was a problem hiding this comment.
That description makes it sound like there should be no logic.
I could see some logic living here -- e.g., if a component updates a string, having the model enforce that there's no leading or trailing spaces might make sense.
It might help if we called out specific examples of what kind of logic would be acceptable in the model, and then also specifically describe where other types of logic should live.
There was a problem hiding this comment.
What is the mapping for any other string, e.g., "foo"? Presumably this maps to false?
There was a problem hiding this comment.
Let's define this based on how it's currently implemented in web_core.
| myCustomCatalog = Catalog( | ||
| id="https://mycompany.com/catalogs/custom_catalog.json", | ||
| functions=basicCatalog.functions, | ||
| components=basicCatalog.components.append([MyCompanyLogoComponent()]) |
There was a problem hiding this comment.
This doesn't match the way the components member of Catalog is declared above. Should this be:
components = basicCatalog.components.update({'my-company-logo', MyCompanyLogoComponent()})
There was a problem hiding this comment.
I think that the name of a Component should be a property of the Component object. That way, when people assemble Catalogs from sets of Components that are implemented in SDKs, they use the same name by default. If they need to rename some component, to avoid a collision etc, they can use a more verbose API like textComponent.withName("alternativeText"). E.g. let's say GoogleMaps release some SDKs that implement GoogleMap and PlaceCard for web, iOS and Android, and also provide some schema to use for inference (we need to figure out how to represent an individual Component in this context). It'd be great if by default, it always uses the same name.
So I think it's okay to have a constructor that accepts a list of Components, but expose it as a map for efficient lookup. Though catalogs likely aren't big enough that it matters, so I'm also fine to just use arrays everywhere and do linear search as necessary.
Let's leave this the same for now, but I'm curious what @jgindin thinks about this.
There was a problem hiding this comment.
Ah, that makes sense, and I agree it is a much better approach. It just isn't clear from the documentation.
| @Component({ | ||
| template: `<ng-container *ngIf="props$ | async as props"> | ||
| <!-- Render specific component here --> | ||
| </ng-container>` |
There was a problem hiding this comment.
Bit of a nit, but we should use "modern" Angular. Something like:
@Component({
selector: 'app-angular-wrapper',
imports: [MatButtonModule],
template: `
@if (props(); as props) {
<button mat-button>{{ props.label }}</button>
}
`
})
export class AngularWrapper {
private binder = inject(BinderService);
private context = inject(...);
private bindingResource = resource({
loader: async () => {
const binding = this.binder.bind(this.context);
return {
instance: binding,
props: toSignal(binding.propsStream) // Convert Observable to Signal [cite: 18]
};
},
});
props = computed(() => this.bindingResource.value()?.props() ?? null);
constructor() {
inject(DestroyRef).onDestroy(() => {
this.bindingResource.value()?.instance.dispose();
});
}
}
There was a problem hiding this comment.
SGTM - let's do it! Thanks for the pointer.
This PR introduces a comprehensive refactor of the A2UI Renderer Guide to try to make it so you can vibe code based on just this design.
Renderer Guide Enhancements (v0.9)
ComponentApiuses a formalschemaproperty to reinforce the requirement for robust validation and capabilities advertising.