Repository navigation
Implement agent 1.0. - #3242
Implement agent 1.0.#3242
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the A2UI protocol v1.0 across the Dart SDK, including full support for compiling, decompiling, and validating v1.0 messages in both direct JSON and Express formats, as well as enabling multi-catalog resolution and standalone renderer function calls. While the implementation is comprehensive and well-tested, the reviewer correctly identified several critical syntax errors in both the compiler and test files where invalid null-aware map insertion syntax (e.g., 'catalogId': ?catalogId) was used. These syntax errors will prevent compilation and must be resolved using Dart's collection if elements.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Polina Cherkasova <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Polina Cherkasova <[email protected]>
gspencergoog
left a comment
There was a problem hiding this comment.
This pull request implements A2UI protocol v1.0 across dart/a2ui_agent and adds agent-side callRendererFunction validation in dart/a2ui_core. Both the Direct JSON and Express inference formats now support v1.0 message shapes, per-component and per-call catalogId resolution across multiple catalogs, standalone function calls compiled to callRendererFunction, catalog instructions and function descriptions in Express prompts, and v1.0 component buffering during streaming. The change also removes v1.0 skips across the conformance and e2e test suites, updates conformance/agent/multi_catalog_formats.yaml to use common_types.json and Text with percent, and lowers the video_player constraint in dart/a2ui_flutter/pubspec.yaml to match the package's declared Flutter 3.38 floor.
The review panel verified two high-severity validation edge cases, three medium-severity issues (an Express ordering bug when a standalone call precedes default-surface assignments, documentation drift around allowedCallers in streaming, and per-chunk PayloadChecker replay overhead), and one low-severity changelog ambiguity.
🤖 Agent-generated
| if (arg case MapNode( | ||
| entries: {'catalogId': LiteralNode(value: final String catalogId)}, | ||
| ) when arg.entries.length == 1) { | ||
| return ([...check.args]..removeAt(index), catalogId); | ||
| } |
There was a problem hiding this comment.
_checkArguments only strips {catalogId: ...} when its value is a String literal. A non-string catalogId stays in written as a positional argument instead of failing as a bad catalogId. Checks with a non-string catalogId either fail with a misleading argument-count error or pass {catalogId: ...} as a positional parameter.
| if (arg case MapNode( | |
| entries: {'catalogId': LiteralNode(value: final String catalogId)}, | |
| ) when arg.entries.length == 1) { | |
| return ([...check.args]..removeAt(index), catalogId); | |
| } | |
| if (arg case MapNode( | |
| entries: {'catalogId': final ExpressNode node}, | |
| ) when arg.entries.length == 1) { | |
| if (node case LiteralNode(value: final String catalogId)) { | |
| return ([...check.args]..removeAt(index), catalogId); | |
| } | |
| throw A2uiValidationError( | |
| "Argument 'catalogId' of ?${check.name}(...) must be a string " | |
| 'literal.', | |
| ); | |
| } |
Repro test
test('check rejects non-string catalogId directly', () {
final catalog = CatalogDefinition.fromJson({
'catalogId': 'basic',
'a2uiVersion': 'v1.0',
'components': {
'TextField': {
'type': 'object',
'properties': {
'component': {'const': 'TextField'},
'label': {'type': 'string'},
'value': {'type': 'string'},
'checks': {'type': 'array'},
},
'required': ['component', 'label'],
},
},
'functions': {
'required': {
'parameters': {
'type': 'object',
'properties': {'value': {}},
'required': ['value'],
},
'returnType': 'boolean',
},
},
});
final compiler = ExpressCompiler([catalog]);
expect(
() => compiler.compile(
'root = TextField("Name", \$/name, checks=?required({catalogId: 123}))',
),
throwsA(
isA<A2uiValidationError>().having(
(e) => e.message,
'message',
contains('catalogId'),
),
),
);
});🤖 Agent-generated
There was a problem hiding this comment.
Fixed as suggested in _checkArguments. ?required({catalogId: 3}) now fails with "Argument 'catalogId' of ?required(...) must be a string literal.", the same message a call's catalogId keyword argument gives. Test.
🤖 Agent-generated
| ); | ||
| case ExpressionStatement(value: final CallNode call) | ||
| when _v1 && !_declaresComponent(call): | ||
| operations.add(_StandaloneCall(call, current)); |
There was a problem hiding this comment.
When a standalone function call appears before any surface(...) statement, _operations passes current (null) to _StandaloneCall. Subsequent assignments on default_surface create a separate _Surface instance. A standalone function call at the top of an implicit default surface block fails to resolve variables assigned later in that block.
case ExpressionStatement(value: final CallNode call)
when _v1 && !_declaresComponent(call):
+ current ??= _Surface(defaultSurfaceId, _surfaceHelper(null));
operations.add(_StandaloneCall(call, current));
case ExpressionStatement(value: CallNode(:final String name)):
@@ -188,4 +189,4 @@
case AssignmentStatement():
- if (current == null) {
- current = _Surface(defaultSurfaceId, _surfaceHelper(null));
+ current ??= _Surface(defaultSurfaceId, _surfaceHelper(null));
+ if (!operations.contains(current)) {
operations.add(current);
}Repro test
test('standalone call resolves variable assigned later on default surface', () {
final catalog = CatalogDefinition.fromJson({
'catalogId': 'basic',
'a2uiVersion': 'v1.0',
'components': {
'Text': {
'type': 'object',
'properties': {
'component': {'const': 'Text'},
'text': {'type': 'string'},
},
'required': ['component', 'text'],
},
},
'functions': {
'openUrl': {
'parameters': {
'type': 'object',
'properties': {
'url': {'type': 'string'},
},
'required': ['url'],
},
'returnType': 'void',
},
},
});
final compiler = ExpressCompiler([catalog]);
final messages = compiler.compile('''
openUrl(target)
target = "https://example.com"
root = Text("Hi")
''');
expect(messages, hasLength(2));
});🤖 Agent-generated
There was a problem hiding this comment.
Fixed as suggested in _operations. A standalone call before any surface(...) line now shares the default surface with the assignments after it, as it already did after an explicit surface(...) line. The default surface is still created only on its first assignment, so a block of calls alone emits no empty createSurface. Test.
🤖 Agent-generated
| break; | ||
| // The block is not whole yet, so a message for a surface it creates later | ||
| // waits for compile, and so do the checks of a whole surface. | ||
| final checker = PayloadChecker(reader.catalogs, reader.version); |
There was a problem hiding this comment.
_emitReady constructs a new PayloadChecker and MessageProcessor.forAgent on every chunk and replays all messages read so far. Disabling checker.add cut 16-byte chunk streaming of a 6.4 KB payload from 5.1 s to 1.8 s. Token-sized streaming chunks multiply validator initialization and message replay cost, taking seconds of CPU where parseResponse takes 20 ms.
Reuse validator instances across chunks or validate only messages that changed since the previous chunk.
Repro test
final String response = block([
{'version': 'v1.0', 'createSurface': {'surfaceId': 's', 'catalogId': basic.id}},
{'version': 'v1.0', 'updateComponents': {'surfaceId': 's', 'components': [
{'id': 'root', 'component': 'Column', 'children': [for (var i = 0; i < 100; i++) 'c$i']},
for (var i = 0; i < 100; i++) {'id': 'c$i', 'component': 'Text', 'text': 'Item number $i'},
]}},
]);
final Parser parser =
const DirectJsonFormatFactory().createFormat([basic]).createParser();
final sw = Stopwatch()..start();
for (var i = 0; i < response.length; i += 16) {
parser.parseChunk(response.substring(
i, i + 16 > response.length ? response.length : i + 16));
}
print('stream=${sw.elapsedMilliseconds} ms');🤖 Agent-generated
There was a problem hiding this comment.
TL;DR: Before, every chunk read the whole block again and replayed all its messages through a new MessageProcessor, so the cost of streaming grew with the square of the payload. Now each chunk reads and checks only what it adds, and a2ui_core no longer walks the whole surface graph on every update.
- Your repro (6.4 KB in 16-byte chunks):
- before: ~ 1.36 s
- now: ~ 0.1 s
- Larger payloads now: 105 KB in 0.29 s and 214 KB in 0.64 s. Doubling the payload about doubles the time.
Details below.
I measured your repro (6.4 KB, 16-byte chunks): 1.36 s in total. Replaying the messages through checker.add took 860 ms and reading them again 393 ms, while constructing PayloadChecker took 6 ms, so reusing validator instances alone would have saved little.
parseChunk now reads and checks only what each chunk adds:
_StreamedCheckskeeps onePayloadChecker, and the surfaces read so far, for the whole block. A message is read and checked once, when it closes. A growingupdateComponentsis checked only for its new components, with duplicate ids checked across the batches.readPartialMessagestakes aPartialReadCache, so a list or map that closed is not read again, and a list still open is read on from its last item that closed. The block is read in place rather than copied, and the scan for the close tag and the check for a trailing markdown fence no longer read the whole block.- In
a2ui_core,MessageProcessorwalked the whole surface graph on everyupdateComponents.SurfaceComponentsModel.checkComponentsUpdatenow keeps a bound on each component's chain of references and walks only what an update reaches when the bound shows the surface has no cycle and no chain past the depth limit. Otherwise it walks the whole surface as before, so the same updates fail with the same errors.
Your repro now takes about 0.1 s, and the time grows with the payload rather than with its square: 1600 components (105 KB) take 0.29 s and 3200 components (214 KB) 0.64 s. Tests cover a repeated id and a cycle that a later chunk closes, which must still fail on that chunk. json_reader_test.dart reads every start of several payloads with and without the cache, and component_graph_bound_test.dart checks the bound's verdict against the full walk on random batches.
I filed #3285 for performance tests in CI that would catch a regression like this one.
🤖 Agent-generated
Co-authored-by: Greg Spencer <[email protected]> Signed-off-by: Polina Cherkasova <[email protected]>
Co-authored-by: Greg Spencer <[email protected]> Signed-off-by: Polina Cherkasova <[email protected]>
Co-authored-by: Greg Spencer <[email protected]> Signed-off-by: Polina Cherkasova <[email protected]>
|
Regarding the "Decisions to review" section in the PR description, the protocol choices look solid. Two follow-up questions on Decision 1 (
|
@gspencergoog Both are fixed in 19278e4.
Other cleanups in the same commit, none of which changes behavior:
🤖 Agent-generated |
Prepares the first publish of a2ui_core since 0.2.2 and of a2ui_agent since 0.0.1-wip004. Neither 0.3.0 nor 0.0.1-wip005 reached pub.dev when their pubspecs were bumped (a2ui-project#3192, a2ui-project#2890), so this folds each package's `## Unreleased` section into that version's section instead of opening a third one. While folding, the entries were checked against lib/ and corrected: - a2ui_core: `PayloadValidator.protocolVersion`, `commonTypesSchema`, `checkVersion`, `forVersion` and `MessageProcessor.commonTypesSchema` were described as removed but still exist. `DataContext.isV10`, `A2uiCompileError`, `Catalog.protocolVersion` as a `String?` and the flattened `ComponentApi.schema` were never in a published version, so they are described in their final form rather than as renames or removals. `DataContext.dispose` does not exist. The new `intl` and `logging` dependencies are listed. Notes about tests and tooling are dropped and overlong lines are wrapped. - a2ui_agent: payload checks go through `MessageProcessor.forAgent`, not `Catalog.withFunctions`. Entries that revised unpublished wip005 entries are merged into them, including the v1.0 entries from a2ui-project#3242. No code changes. The versions in pubspec.yaml were already set.
…n catalog_transformer.yaml - Allow ComponentPruningTransformer and FunctionPruningTransformer to accept null for allowedComponents and allowedFunctions, treating null the same as [] (#3242). - Add conformance test runner for conformance/agent/catalog_transformer.yaml.
…n catalog_transformer.yaml (#3330) * fix(ts_agent): support null allowlists in pruning transformers and run catalog_transformer.yaml - Allow ComponentPruningTransformer and FunctionPruningTransformer to accept null for allowedComponents and allowedFunctions, treating null the same as [] (#3242). - Add conformance test runner for conformance/agent/catalog_transformer.yaml. * test(ts_agent): default testCase.args and use optional chaining in catalog-transformer.test.ts
Contributes to: #2356
Contributes to: #3100
Summary
#3233 added the protocol v1.0 API to the Dart agent SDK, with stubs that threw
UnimplementedError. This PR implements them, soa2ui_agentreads and writes v1.0 in both inference formats (direct JSON and Express). The shared conformance suites under conformance/agent now run as written in v1.0, as well as lowered to v0.9, and the Dart harnesses no longer skip any case as a known gap.It also makes streaming direct JSON cost time in proportion to the payload:
parseChunkused to read and check the whole block again on every chunk, which took 1.36 s for a 6.4 KB payload in 16-byte chunks and now takes about 0.1 s. See Performance.Changes in a2ui_core
MessageProcessor.forAgentchecks acallRendererFunctioninstead of throwing (processor.dart). The surface must exist, the catalog the call names must be held and target the message's version, and that catalog must declare the function, let the agent call it, and accept its arguments. A function withrequiresUserActivationis accepted only whenprocessMessagesis givenisUserActivated. Each of these throwsA2uiValidationError, since a renderer answers each withINVALID_FUNCTION_CALL.PayloadValidator.validateFunctionCall(validator.dart) checks a call written on its own, such as thecallFunctionof acallRendererFunction. The call, and each call nested in its arguments, is checked against the catalog itscatalogIdnames, and may hold only the fieldsFunctionCallallows. AcatalogIdthat is not a string is rejected.forAgentand the agent'sMessageReaderboth use it.PayloadValidator.validateFunctionArgsis public (validator.dart), so the agent can check a call nested in a component against the catalog the call names rather than the component's.checkDuplicateComponentIdsis exported (component_graph.dart), with anearlierargument for the ids of earlier batches, so the agent can check one message's components in several batches as they stream.MessageProcessorwhosevalidationConfigallows dangling references, and orphan components or a missing root, checks anupdateComponentswithout walking the whole surface (component_model.dart). The surface's component model keeps a bound on each component's chain of references (_GraphBound), and walks only the components an update reaches when the bound shows there is no cycle and no chain past the depth limit. Otherwise it walks the whole surface as before. The bound holds only while nothing has changed the model since the processor applied the batch it was found for.allowedParentsorallowedChildren(processor.dart), rather than building every component's JSON for each update.Changes in a2ui_agent
Direct JSON
MessageReader(message_reader.dart) checks each v1.0 component and function call against the catalog it names, else its surface's catalog. For a surface the payload does not create, a component that names no catalog is checked against the first catalog that declares it. The parser reads with catalogs linked bylinkCatalogs, so a cross-catalog$reffrom #3230 resolves.PayloadChecker(payload_checker.dart) applies the messages for the surfaces a payload creates throughMessageProcessor.forAgent.compile(parser.dart) then rejects a message that comes before its surface'screateSurface, and a created surface with no root, a reference to a missing component, a component the root does not reach, or a cycle. A surface the payload only updates may hold components from earlier responses, so those checks skip it, as in the Python SDK.catalogIdmay come last, and checks what has arrived, so a cycle is never emitted. AnupdateComponentswhose closed components fail the check raises on the chunk that closes them. A string cut between the two halves of an escaped surrogate pair is cut before the pair rather than healed with U+FFFD (json_reader.dart)._StreamedChecks(parser.dart) keeps onePayloadChecker, and the surfaces read so far, for the whole block. A message is read and checked once, when it closes. A growingupdateComponentsis checked only for its new components, and for its last one again when components are not held back, with duplicate ids checked across the batches. A growingupdateDataModelis checked again whole when it changes.readPartialMessagestakes aPartialReadCache(json_reader.dart): a list or map that closed is not read again, and a list still open is read on from its last item that closed (L308-L371). It reads the block in place between two offsets rather than a copy._BlockScanner), and a trailing markdown fence is found by reading back from the end (_trailingFenceStart) rather than with a regular expression that tried every offset of the block.Express
createSurfacecarries the components and data model, and bindings and calls use@pathand@call. A component, function call or check names its catalog with acatalogIdargument, which a name declared by several catalogs needs, and which must be a string literal (L766-L788). A function call written as a statement of its own compiles to acallRendererFunction(L433-L483), with the idcall_<n>or the one afunctionCallIdargument gives, and may name the variables of the surface it is written in, the default surface included (L171-L176). Two calls of a block may not share an id (L103-L124), and an inline component may not take an id another component has (L972-L986). The output goes through the sameMessageReaderas direct JSON.catalogIdonly where the compiler could not tell the catalog from the name alone, andfunctionCallIdonly when the id is notcall_<n>. A call that belongs to no active catalog throwsA2uiValidationErrorrather than being written as a map the compiler rejects. It writes a single check without brackets, a multiline string with backslashes as a raw triple-quoted string, and anupdateDataModelbelow the root as assignments at its path, asconformance/agent/express/decompiler.yamlexpects.callRendererFunctionis allowed, it lists the functions the agent may call. It also carries each catalog'sinstructions, with their JSON examples rewritten in Express, and each function's description (L331-L355).Request processor
validatePayloads(validation.dart) throwsA2uiValidationErrorfor a v1.0 component or call that names a catalog that is not active, then runs the payload throughMessageProcessor.forAgent. It skips a component'smetadata, as the processor does.Changes in a2ui_flutter
video_playeris^2.11.1rather than^2.14.1. Version 2.14.1 needs Dart 3.12 and Flutter 3.44, above the package's declared Dart 3.10 and Flutter 3.38, and 2.11.1 is the oldest release thatgenui ^0.8.0, another package of the workspace, accepts.Performance
Streaming a payload of one
createSurfaceand oneupdateComponentswith aColumnand NTextchildren, in 16-byte chunks, on a MacBook:compileBefore, each chunk read the whole block again, replayed every message through a new
MessageProcessor, and ran a regular expression over the block. Doubling the payload now about doubles the time; most of the 0.10 s for 100 components is the first run's warm-up. #3285 proposes performance tests in CI that would catch a regression like this one.Decisions to review
allowedCallersis enforced: a response may call only a function markedagentOnlyorrendererOrAgent. The default isrendererOnly, so the v1.0 test catalogs setrendererOrAgenton the functions the tests call.parseResponseenforces it; a parser'scompileandparseChunkcheck only a call's arguments, as the README says.bufferIncompleteComponentsnow applies only to catalogs before v1.0.updateComponentsandupdateDataModelis emitted once, after its object closes, and the messages after it wait until then (parser.dart). Before, acreateSurfaceorcallRendererFunctionwas emitted while it grew, so a renderer could run a call twice, the first time with partial arguments.compilechecks the surfaces a payload creates whole. The basic catalog example31_incremental-dashboard.jsonleaves its loading placeholders unreachable, so specification_round_trip_test.dart compiles its later updates as payloads of their own, as later responses would send them.functionCallId. The Python compiler gives both the same id.Required check failed, without the trailing period, as in Python, TypeScript and the suite.updateDataModelbelow the root decompiles to assignments that compile to an update of the root, as the suite expects, which replaces the rest of the data model. An update of a list item, such as/restaurants/3in00_incremental.json, still throws, since the assignment would turn the list into a map.Chart.caption, which custom_catalog_v1_0.json types as a plain string. A renderer would reject the call there, and so does this SDK. The cases now place the call inText.text, aDynamicString, and test the same catalog resolution. The inline catalogs of two cases also referencedcommon.json, which after feat(agent,core): support cross-catalog $ref resolution across peer catalogs #3230 is an unknown document thata2ui_corerejects. They now referencecommon_types.json. The Python and TypeScript SDKs pass the edited suite.Testing
flutter test:a2ui_agent1021 passed and 70 skipped,a2ui_core1798 passed and 5 skipped,a2ui_flutter309 passed, the last also withvideo_player2.11.1, anda2ui_cli35 passed. The skips are v0.8 cases and cases v0.9 cannot state.A2uiRequestProcessor. processor_for_agent_test.dart covers thecallRendererFunctioncheck. The v1.0 group of specification_round_trip_test.dart round-trips every v1.0 basic catalog example through direct JSON, and all but00_incremental.jsonthrough Express.PartialReadCacheand expects the same messages. component_graph_bound_test.dart applies random batches to two component models, one that keeps the bound and one that walks the whole graph every time, and expects the same verdict and error for every batch; it fails when the bound skips a step.flutter analyze --fatal-infosanddart formatare clean for the four packages, and Prettier 3.8.4 is clean for the changed Markdown and YAML.multi_catalog_formats.yamlpass (34 cases), and so do the TypeScripta2ui_agenttests.Outside this PR
a2ui_agentusesa2ui_coreAPI that is not released yet. Publishing it needs ana2ui_corerelease and a higher constraint than^0.3.0.a2ui_coredeclares Dart 3.5, but it depends onjson_schema_builder ^0.1.7, and every release from 0.1.5 needs Dart 3.10, so the package resolves on Dart 3.10 and later only, asa2ui_agentanda2ui_flutterdo. a2ui_core declares Dart 3.5 but resolves only on Dart 3.10 and later #3289 tracks the fix.🤖 Agent-generated