Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a major refactoring of the A2UI agent parser and streaming architecture, making the parser stateless and decoupling validation from translation. It replaces the legacy DirectJsonStreamProcessorImpl with a new DirectJsonResponseStream and a custom json-reader to fully align with the streaming conformance suite, using RendererState to track and validate surface updates incrementally. The feedback highlights several performance optimization opportunities in the newly added parsing code (blocks.ts and json-reader.ts), specifically recommending limiting the length of string slices when scanning for tags, words, or escape sequences to avoid unnecessary memory allocations.
2d7feb2 to
5e7bed1
Compare
5e7bed1 to
30e87e1
Compare
gspencergoog
left a comment
There was a problem hiding this comment.
This change refactors @a2ui/agent so Parser is stateless, moves incremental parsing into a per-response ResponseStream (openStream, feed, close), and binds A2uiRequestProcessor to an optional RendererState instead of an internal MessageProcessor. It also ports Python's Direct JSON block scanner (blocks.ts), tolerant JSON reader (json-reader.ts), and DirectJsonResponseStream (response-stream.ts), allowing all v0.9 and v1.0 Direct JSON streaming and unwrap conformance cases to pass.
🤖 Agent-generated
0f7ba28 to
d6bbd83
Compare
… SDKs Addresses the review of #3283, checking each rule against the Python (#3270), TypeScript (#3279) and Dart (#3277) pull requests: 1. `compile` takes `is_final`; a cut block is applied and its surfaces skip `check_topology` and are marked checked, and a cut Direct JSON block raises. `parse_response` errors carry `partial_results`. 2. Language-neutral wording for named arguments, the unsupported-stream error, and the callback form of `transaction` (returns the callback's result, must be synchronous). 3. A failing `feed` undoes only what that chunk applied; a failed stream marks the surfaces it emitted to as checked. 4. `RendererState.mark_checked`, and what `check_topology` does with duplicate, unknown, deleted and assumed ids. 5. The undo log records the state's own bookkeeping, copies the properties an in-place update replaces, keeps each component's `catalogId` and the v0.8 buffer when replaying a deleted surface, and treats a nested transaction as a position in the log. 6. How a v0.8 state buffers updates before `beginRendering`. 7. How `validate_payload` picks the catalog of an assumed surface, and that it accepts one message on its own. 8. The three linear-cost rules are requirements for every SDK. 9. `A2uiRequestProcessor.accepts_state`, used in the code example. 10. The conformance plan says what `parse_chunk` runs and that no suite covers RendererState, canvas turns or validate_payload yet. 11. A "Where the SDKs differ" list of the gaps the SDKs still have. Signed-off-by: Nan Yu <nanyu@google.com>
…te where the SDKs differ - a2ui_core: RendererState's atomic application undoes a rejected message from an undo log rather than rolling back to a copy. Requirement 4 now states only what a processor must do under a ValidationConfig that defers the graph checks, with no SDK status. - a2ui_core: the agent example shows the agent using RendererState (apply, then checkTopology) and, separately, the MessageProcessor calls RendererState makes inside, since an agent does not call MessageProcessor itself. - a2ui_agent: "Where the SDKs differ" assumes the pending Python (#3326, #3268-#3271) and TypeScript (#3327, #3278, #3279) pull requests. Both meet the three requirements of "Cost of a response"; the remaining entries are Dart's, plus Python being the only SDK with a v0.8 state. Signed-off-by: Nan Yu <nanyu@google.com>
e488ff2 to
76fb327
Compare
…ach chunk once The parser holds no state. parseResponse and openStream check a response against a RendererState, a new one per call unless the caller passes one (or binds one to A2uiRequestProcessor) for a canvas conversation. A2uiRequestProcessor gains createState and acceptsState, which compares the catalogs' ids and release lines. DirectJsonResponseStream reads each chunk once: 1. BlockScanner resumes the close-tag scan where the last chunk left it. 2. readPartialMessages reads through a PartialReadCache, ported from the Dart reader: a list or map that closed is kept by offset, and a list of messages goes on after the last message that closed. 3. The stream keeps the closed messages and applies each once. Only updateComponents and updateDataModel are emitted while they grow; any other message, callRendererFunction included, waits for its object to close. A growing updateComponents is applied for the components it gained, with ids repeated across chunks rejected. A growing updateDataModel is checked for the lists and maps its value gained, and applied again once it closes. 4. tests/timing/scaling_test.mjs runs conformance/agent/direct_json/ scaling.yaml and conformance/agent/express/scaling.yaml with `yarn test:timing`, using web_core's timing helper, and the ci-timing job runs it for @a2ui/agent. Streaming one updateComponents of 400 components in 40-character chunks drops from 7.0 s to 0.13 s (ratio 400/100 from 16.0 to 3.9). The Node restaurant finder sample and the eval TypeScript bridge use the new API. The conformance cases under conformance/agent/ are updated to the shared cases this SDK and the Python SDK (#3269) run: args.history is dropped, and the streaming cases this stream now passes leave the known failures. conformance/README.md says the TypeScript agent SDK runs the agent timing suites with `yarn test:timing`. Signed-off-by: Nan Yu <nanyu@google.com>
76fb327 to
20b4d6f
Compare
…until #3269 #3269 changes test_stream_update_before_create_surface_fails_v09 and test_stream_data_model_before_create_surface_fails_v09 to hold back a message for a surface that does not exist yet. The Python and Dart SDKs on main still emit it, so carrying the change here failed their CI. The cases stay as main has them, and the TypeScript harness lists them as known failures until #3269 lands. Signed-off-by: Nan Yu <nanyu@google.com>
…ases The shared conformance cases this PR carries from #3269 failed CI for the SDKs on main: - test_stream_update_before_create_surface_fails_v09 and test_stream_data_model_before_create_surface_fails_v09 now expect a message for a surface that does not exist yet to be held back, which the Python SDK on main does not do. They stay as main has them until #3269 lands, and the Dart harness skips them until then, since this stream already holds the message back. - With the updated cases, seven TypeScript streaming cases pass, so they leave the TypeScript harness's known failures, as #3279 does. Signed-off-by: Nan Yu <nanyu@google.com>
…ases The shared conformance cases this PR carries from #3269 failed CI for the SDKs on main: - test_stream_update_before_create_surface_fails_v09 and test_stream_data_model_before_create_surface_fails_v09 now expect a message for a surface that does not exist yet to be held back, which the Python SDK on main does not do. They stay as main has them until #3269 lands, and the Dart harness skips them until then, since this stream already holds the message back. - With the updated cases, seven TypeScript streaming cases pass, so they leave the TypeScript harness's known failures, as #3279 does. Signed-off-by: Nan Yu <nanyu@google.com>
…ases The shared conformance cases this PR carries from #3269 failed CI for the SDKs on main: - test_stream_update_before_create_surface_fails_v09 and test_stream_data_model_before_create_surface_fails_v09 now expect a message for a surface that does not exist yet to be held back, which the Python SDK on main does not do. They stay as main has them until #3269 lands, and the Dart harness skips them until then, since this stream already holds the message back. - With the updated cases, seven TypeScript streaming cases pass, so they leave the TypeScript harness's known failures, as #3279 does. Signed-off-by: Nan Yu <nanyu@google.com>
…d RendererState to A2uiRequestProcessor (#3277) * feat(dart/agent)!: make the Parser stateless; add ResponseStream; bind RendererState to A2uiRequestProcessor - A parser holds no state. Parser.parseChunk is removed: openStream returns a ResponseStream (DirectJsonResponseStream for direct JSON) whose feed and close read one response against a RendererState, applying each message as it is emitted and checking the trees a block touched when it closes. A failed stream keeps what it emitted and marks it checked (RendererState.markChecked, undone with a failed transaction like any other change). - Parser.compile only translates; parseResponse checks a response as a whole against a RendererState. A2uiRequestProcessor binds a RendererState and gains acceptsState. - DirectJsonResponseStream reads each chunk once: the close-tag scan and the read of the open block resume from the previous chunk (#3314's PartialReadCache), a closed message is applied once, a growing updateComponents applies only the components it gained (and the last one again if it grew) and rejects ids repeated across its chunks, and a growing updateDataModel is checked for the lists and maps its value gained and applied again once it closes. Only updateComponents and updateDataModel are emitted while they grow. - The shared conformance cases for the stateless parser and stream, as in #3269: compile cases assume the surfaces a block only updates, response parser cases create the surfaces they update, and the streaming cases hold back an update to a surface the response has not created. The Dart known gaps for those cases are removed. - The scaling conformance harness streams through openStream into a state that holds the surfaces a payload updates first, and times compile as parseResponse(payload, wrapped: false), as the Python harness does. Signed-off-by: Nan Yu <nanyu@google.com> * test(conformance): keep main's SDKs green with the shared streaming cases The shared conformance cases this PR carries from #3269 failed CI for the SDKs on main: - test_stream_update_before_create_surface_fails_v09 and test_stream_data_model_before_create_surface_fails_v09 now expect a message for a surface that does not exist yet to be held back, which the Python SDK on main does not do. They stay as main has them until #3269 lands, and the Dart harness skips them until then, since this stream already holds the message back. - With the updated cases, seven TypeScript streaming cases pass, so they leave the TypeScript harness's known failures, as #3279 does. Signed-off-by: Nan Yu <nanyu@google.com> --------- Signed-off-by: Nan Yu <nanyu@google.com>
…se-stream Signed-off-by: Nan Yu <nanyu@google.com>
… runs Signed-off-by: Nan Yu <nanyu@google.com>
Part of #3262 and #3263. Stacked on #3278, which is stacked on #3327: the core checks an
updateComponentsagainst the components it changes, andRendererStaterolls back with an undo log. A stream needs both to take time in proportion to the response.The TypeScript
DirectJsonParserforwardedparseChunkto a stream processor that held one response's buffers and its own copy of surface state, checked streamed messages only when avalidationConfigwas set, andA2uiRequestProcessorran every parsed message through a second, long-livedMessageProcessorwith the tree checks turned off. This PR mirrors the Python change in #3269 and #3270.Parser
Parseris stateless, and subclasses providecatalogs.compileonly translates.parseResponse(content, {wrapped?, state?})replaces the positionalwrappedargument. It applies every block inside a transaction on the given state, or a new one, and checks the trees once all blocks are applied. Breaking.openStream({wrapped?, state?})returns aResponseStreamwithstate,feedandclose.parseChunkandparseStreamare removed. Breaking.DirectJsonResponseStreamis a port of the Python stream, with the block reading inblocks.tsand the partial JSON reader injson-reader.ts. It applies each message as it is emitted, emits onlyupdateComponentsandupdateDataModelwhile they grow and holds every other message, such ascreateSurface,deleteSurfaceandcallRendererFunction, until its object closes, keeps no copy of surface state, and always validates. It reads each chunk once; see "Linear streaming".DirectJsonStreamProcessorImpl,streaming.tsandstreaming-types.tsare removed. Breaking.['text']inDirectJsonFormatandDirectJsonFormatFactory, and to none inDirectJsonParser, as in Python.literalStringwas v0.8-only.DirectJsonParser.unwrapfollows Python: a response without tags is text, an unterminated block hasisFinal: false, and tags may carry attributes.parseResponsechecks. A block without a root needs a state that holds its surface.Request processor
A2uiRequestProcessordrops the private_messageProcessorandMODEL_OUTPUT_VALIDATION, and gains a cachedparser,createState(), astategetter, a{state}constructor option, andopenStream.A2uiGenerator.createProcessortakes{state}too, and checks examples withvalidatePayload.parseResponseandopenStreamdefault to the bound state; with none, each call uses a new empty state. A state for other catalogs throwsA2uiCatalogError.acceptsStatecompares the catalogs' ids and release lines (catalogsKey), so a state built from the v0.9.1 spelling of a v0.9 catalog is accepted, asRendererStatetreats the two versions as one.Linear streaming
The stream follows requirement 3 of "Cost of a response" in the agent blueprint: a stream reads each chunk once.
BlockScannerresumes the close-tag scan where the last chunk left it.payloadStartandtrailingFenceStartfind the payload by offset instead of cutting a new string and running a trailing-fence regular expression over it.readPartialMessagesreads through aPartialReadCache, ported from the Dart reader in Fix performance and create conformance tests. #3314: a list or map that closed is kept by offset (closedOr), a list still open goes on after its last closed item, and a list of messages goes on after the last message that closed, which is not returned again.emitReady,applyMessage). A growingupdateComponentsis applied for the components it gained, and the last one again if it grew (applyAddedComponents); an id repeated across chunks of one message is rejected, as the core rejects one repeated within a batch. A growingupdateDataModelis applied when it first reads, checked for the lists and maps its value gains, and applied again once it closes (checkGrownValue).tests/timing/scaling_test.mjsrunsconformance/agent/direct_json/scaling.yamlandconformance/agent/express/scaling.yamlthrough web_core's timing helper (typescript/core/web_core/tests/timing/timing.mjs, imported by relative path), withyarn test:timing, whichyarn testleaves out. Theci-timingjob inweb_ci.ymlruns it for@a2ui/agent.compilecases runparseResponse(payload, {wrapped: false}), which compiles and checks the block, as compiling does in the other SDKs. The v0.9 healed case updates a surface an earlier response created, so each run starts on a state holding that surface with a root that lists the payload's components.Streaming in 40-character chunks, median of five runs:
updateComponentsof n components, beforeupdateComponentsof n components, nowBefore this change the direct JSON timing suite failed four of its stream cases with ratios of 13.6 to 14.5 (8 allowed), and streaming 400 components took 214 times as long as compiling them (10 allowed). Now every case passes: the stream cases have ratios of 3.1 to 3.9, and streaming takes 5.3 times as long as compiling.
Conformance, evaluation, sample and docs
parse_responsecases and drive streaming cases throughopenStream,feedandcloseagainst a new, empty state. Compile and round-trip cases check the compiled block on its own withvalidatePayload, as the Python runner does (compileCheckedinsuite-helpers.ts). All streaming and four direct JSON unwrap known failures pass and are removed. The legacyprocess_chunkaction and three legacyparse_fullcases contradictresponse_parser.yamland are marked superseded.conformance/agent/direct_json/andconformance/agent/express/are updated to the ones the Python SDK runs in feat(python/agent)!: make the Parser stateless; always validate; add ResponseStream #3269 (args.historyis dropped, and thecompileand streaming cases are brought in line with the stateless parser). Two cases,test_stream_update_before_create_surface_fails_v09andtest_stream_data_model_before_create_surface_fails_v09, stay at main's version here, because the Python and Dart SDKs on main still emit a message for a surface that does not exist yet; the TypeScript harness lists them as known failures until feat(python/agent)!: make the Parser stateless; always validate; add ResponseStream #3269 updates them. The rest is identical to feat(python/agent)!: make the Parser stateless; always validate; add ResponseStream #3269.ts_runner.mjsmirrors the Python bridge: withvalidateit callsparseResponse; otherwise it only unwraps and compiles.processor.openStream(), and drops its per-conversation cache of stream processors.KNOWN_GAPS.mdare updated, andconformance/README.mdsays which TypeScript packages run the timing suites.Testing
renderer-state-parsing.test.tsandresponse-stream.test.ts;streaming.test.tsis removed with the code it tested. The new stream cases cover acallRendererFunctionheld until it closes, a growingupdateComponentsemitted whole as it gains components, an id repeated across chunks, anupdateDataModelapplied again once it closes, and a block that stops emitting after a rejected closed message.renderer-state.test.tscoversmarkCheckedundone by a failed transaction, andrenderer-state-parsing.test.tsa v0.9.1 state accepted by a v0.9 processor.typescript/agents/a2ui_agent:yarn test897 passed, 1 expected fail (an unrelated catalog resolver gap);yarn typecheckclean;yarn lint0 errors;yarn format:checkclean;yarn test:timing10 of 10 passed.uv run pytest tests/test_bridges.py -k tsineval/: 40 passed. These tests skip unlesstypescript/a2ui_agent/distexists, becauseeval/tests/conftest.pystill checks the package's path from before it moved undertypescript/agents/.