Feat: port UiWidget event rendering metadata from adk-python - #581
Open
AmaadMartin wants to merge 4 commits into
Open
Feat: port UiWidget event rendering metadata from adk-python#581AmaadMartin wants to merge 4 commits into
AmaadMartin wants to merge 4 commits into
Conversation
added 4 commits
August 3, 2026 09:39
Adds the UiWidget data model, EventActions.renderUiWidgets, the Context.renderUiWidget() accessor with its duplicate-id guard, widget aggregation in mergeEventActions, and payload preservation across the snake_case/camelCase event transform.
…ervation Adds a renderUiWidgets suite to event_actions_test, a new context_test mirroring the adk-python TestContextAddUiWidget cases, merge aggregation cases in functions_test, and payload round-trip cases in event_test.
An all-snake_case payload is a no-op under the camelCase-to-snake_case transform, so the fixture could not detect a missing preserve key in that direction. The payload now also carries MCP's own camelCase inputSchema, which makes each direction fail if its preserve key is dropped.
The readRenderUiWidgets helper existed to read a snake_case key that nothing can produce. mergeEventActions has one production caller, mergeParallelFunctionResponseEvents, which maps event.actions over in-process events typed as EventActions; the only snake_case wire boundary runs transformToCamelCaseEvent before an Event exists. The adk-python fallback it mirrored is dead there too: EventActions sets alias_generator=to_camel and the merge dumps with by_alias=True, so the dump key is always renderUiWidgets and the second pop only ever returns None. Verified against pydantic 2.13.4. Inlining the concatenation also removes both unchecked casts. Drops the three tests that only covered the deleted branch, and adds one for the empty-list guard that keeps the field undefined rather than [].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A — no existing issue; this is a cross-language parity port.
Problem: ADK Python lets an agent attach rendering metadata to an event so a UI host can render a rich interactive widget (e.g. an MCP App iframe) instead of plain text. TypeScript has none of this —
UiWidget/uiWidget/renderUiWidgetsreturns zero hits acrosscore/srcanddev/src, andEventActionshas no widget field. A TypeScript agent therefore cannot emit the widget metadata that the same UI host already understands from a Python agent.Solution: Port the core mechanism, on the same wire shape:
core/src/events/ui_widget.ts(new) —UiWidget(id,provider,payload), exported as a type fromcore/src/common.ts.core/src/events/event_actions.ts—EventActions.renderUiWidgets?: UiWidget[], andmergeEventActionsnow concatenates widgets across sources so widgets emitted by several tools in one turn all survive a parallel-function-response merge.core/src/agents/context.ts—Context.renderUiWidget(uiWidget), which appends to the current event's actions and throws on a duplicate widget id.core/src/events/event.ts— the widgetpayloadis added to both preserve lists so the DB session service's snake/camel event transform leaves provider-defined payload keys untouched.Why this shape. Every claim below was read out of the adk-python sources, not inferred:
EventActionsin adk-js is a plain interface plus acreateEventActions()factory, soUiWidgetfollows suit. Python'sUiWidgetsetsalias_generator=to_camel, but that is a no-op for its own fields —id,providerandpayloadare single words whose camelCase alias equals the field name. The only place camelCase matters is the containing field,render_ui_widgets⇄renderUiWidgets, so no alias machinery is ported. NocreateUiWidget()factory either — a single default does not justify one.payloadis required. This matches the repo's existing translation of aField(default_factory=dict)field (EventActions.stateDeltais likewise required), every known provider needs payload data, and it keeps readers free of optional chaining.undefined, not[]— mirroring Python'sOptional[list[UiWidget]] = None.createEventActions()does not add an empty array.render_ui_widgetskey, mirroring Python'sactions_dict.pop('renderUiWidgets', None) or actions_dict.pop('render_ui_widgets', None). That fallback has been removed as unreachable dead code, and the reasoning is worth recording because the Python line is misleading:EventActionsdeclaresalias_generator=alias_generators.to_camel(events/event_actions.py:81-85) and the merge dumps withmodel_dump(exclude_none=True, by_alias=True), so the dump key is alwaysrenderUiWidgets. Reproduced on pydantic 2.13.4: for both a populated and an empty list the dump keys are['stateDelta', 'renderUiWidgets'], and'render_ui_widgets' in dumpisFalse. The secondpopcan only ever returnNone— an artifact of pydantic's aliasing layer, not intended behaviour.mergeEventActionshas exactly one production caller,core/src/agents/functions.ts:560insidemergeParallelFunctionResponseEvents, which mapsevent.actionsover events built in-process by the tool-call loop — typedEventActions, camelCase by construction. The only snake_case wire boundary iscore/src/sessions/db/schema.ts:32,35, which runstransformToCamelCaseEventbefore the object is ever visible as anEvent.source as Record<string, unknown>andsnakeCased as UiWidget[]), so the merge branch now reads exactly like the four dictionary branches above it.Object.assign(result, target)copiestarget's array by reference, so pushing in place would mutate the caller'starget. A test pins this.Errorwith the exact Python message —UI widget with ID '<id>' already exists in the current event actions.— matching theValueErrorinContext.render_ui_widget. PlainErrormatches every other throw incontext.ts; a single throw site does not warrant a new error class or code enum. As in Python, the list is initialised before the duplicate check, so a rejected call still leavesrenderUiWidgetsdefined as[]if it was previously unset.Parity-vs-local-convention conflicts. Local TS convention won for things that never leave the process (interface over pydantic model, no alias generator, module layout). Parity won for everything observable across the boundary: the wire field name
render_ui_widgets, the camelCase output key on a merge, theundefined/Nonedefault, the widget ordering, and the verbatim error message.Deliberately NOT ported (verified — the Python side does not do these either, so doing them here would be a divergence, not parity):
VertexAiSessionService— adk-python's outboundconfig['actions']allowlist (sessions/vertex_ai_session_service.py, ~line 404) contains onlyskip_summarization,state_delta,artifact_delta,transfer_agent,escalate,requested_auth_configs. Widgets are dropped there, socore/src/sessions/vertex_ai_session_service.tsis untouched — adding a reader for a field the writer never sends is dead config.core/src/a2a/event_converter_utils.tsdeliberately restricts peer-settable action fields to anescalate-only allowlist. Not extended.core/src/events/structured_events.ts— an adk-js-only abstraction with no Python analogue.dev/src/integration/test_types.tsFilteredEventActions— a deliberately filtered recording-comparison subset; the new field is optional and absent by default.MCPToolwidget emission is out of scope and queued separately.core/src/tools/mcp/mcp_tool.tsis 79 lines with nometa/_metahandling at all, so detectingmeta.ui.resourceUriand callingrenderUiWidgetin_run_async_implis a distinct piece of work that depends on this one. Accordinglytest_mcp_tool.py::test_run_async_impl_adds_ui_widgetis not mirrored here; the other two Python test modules are.Collision check. Ran
gh pr list --repo AmaadMartin/adk-js --state open --limit 1000(480 open PRs) and grepped titles/branches forwidget— zero hits; no open PR implements this. Inspected the file lists of the plausibly adjacent ones: #570 (removes the unusedtargetparam frommergeEventActions), #209 (event_actions.tslong-running-tool fix), #79 / #62 (consolidatemergeParallelFunctionResponseEventsintoEvent), #503 (VertexAiSessionServiceround-trip). All are file-level overlaps on different fields/functions, none touch UI widgets, so this branches frommainrather than stacking. Note #570 would remove thetargetparameter my no-mutation test exercises; if it lands first, that one assertion moves to the two-source form.Follow-up queued, not fixed here. While testing the no-mutation property I confirmed a pre-existing bug in the same function:
Object.assign(result, target)aliasestarget's dictionary fields, soObject.assign(result.stateDelta, source.stateDelta)mutates the caller'starget. Probe:mergeEventActions([createEventActions({stateDelta: {added: 2}})], createEventActions({stateDelta: {base: 1}}))leaves the target at{base: 1, added: 2}. It is independent of this change, so it is queued as its own task rather than widened into this diff. The newrenderUiWidgetspath deliberately avoids the bug via the spread-copy.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
No existing test was modified or deleted. The widget cases in
event_actions_test.tsare a newdescribeblock appended after the existing suites, which remain byte-identical; the additions tofunctions_test.tsandevent_test.tsare newitblocks inside existing describes;core/test/agents/context_test.tsis a new file (none existed).core/test/events/event_actions_test.ts— 6 new cases: defaultundefined; override passthrough; multi-source concatenation in order; no widgets leaves the fieldundefined; an empty source list also leaves itundefinedrather than[]; and no mutation of source or target arrays.core/test/agents/context_test.ts(new) — mirrorstest_context.py::TestContextAddUiWidget: append + object identity, call ordering, and duplicate-id rejection asserting the exact message and that the array still holds only the first widget. Fixture is a realLlmAgent/createSession/InvocationContext/Contextchain, no cast mock literals.core/test/agents/functions_test.ts— mirrorstest_functions_simple.py::test_merge_parallel_function_response_events_merges_ui_widgets: 1 widget + 2 widgets merge to 3 in order, and no-widget events leave the fieldundefined.core/test/events/event_test.ts— payload round-trip in both directions.Proof each test can fail (mutation testing). Every new behaviour was run against mutated source and confirmed to FAIL:
result.renderUiWidgets = source.renderUiWidgets)concatenates widgets from multiple sources in source order;does not mutate …;should aggregate UI widgets from every merged event(3 failures)expected [ 'widget_2', 'widget_3' ] to deeply equal [ 'widget_1', 'widget_2', 'widget_3' ]Context.renderUiWidgetrejects a duplicate widget id and leaves the list unchangedexpected [Function] to throw an error'actions.renderUiWidgets.payload'removed fromPRESERVE_KEYS_CAMEL_CASEpreserves UI widget payload keys during conversion to snake_caseexpected { resource_uri: 'ui://app', …(2) } to deeply equal { resource_uri: 'ui://app', …(2) }(the nestedinputSchemabecameinput_schema)'actions.render_ui_widgets.payload'removed fromPRESERVE_KEYS_SNAKE_CASEpreserves UI widget payload keys during conversion to camelCaseexpected { resourceUri: 'ui://app', …(2) } to deeply equal { resource_uri: 'ui://app', …(2) }pushdoes not mutate the widget arrays of the sources or the targetexpected [ … ] not to be [ … ] // Object.is equalityif (source.renderUiWidgets))leaves renderUiWidgets undefined when a source has an empty listexpected [] to be undefinedMutation 3a is worth calling out: the payload fixture originally used only snake_case keys, and it survived that mutation — camelCase→snake_case is a no-op on already-snake keys, so the test could not detect a missing preserve key in that direction. The fixture now also carries MCP's own camelCase
inputSchema(the real spelling in a raw MCP tool definition), which makes each direction fail if its own preserve key is dropped. That fix is its own commit.Coverage.
core/src/events/event_actions.tsis 100% statements / branches / functions / lines.Context.renderUiWidget(lines 197-206) is fully covered — the uncovered ranges reported forcontext.tsall end at line 185 and are pre-existing methods not exercised by this targeted run.core/src/events/ui_widget.tsreports 0% because it declares only aninterface, which TypeScript erases at compile time; this matches 13 other type-only modules already incore/src(examples/example.ts,memory/memory_entry.ts,agents/transcription_entry.ts, …), and no coverage suppression was added for it.Manual End-to-End (E2E) Tests:
No E2E test is included, and this is deliberate rather than an omission: there is no runtime producer of widgets in adk-js until the follow-up
MCPTooltask lands, and this change adds no network, process, or I/O boundary — it is a data field plus two pure functions. An "E2E" test here could only drive fakes, which per the repo's own convention makes it a unit test, so it lives incore/test/under a plain descriptive name instead. The merge path is additionally exercised end-to-end by the existinghandleFunctionCallListsuite.To exercise the feature manually once a producer exists, call
context.renderUiWidget({id: context.functionCallId!, provider: 'mcp', payload: {resource_uri: 'ui://my-app/checkout', tool, tool_args: args}})from a tool or callback and readevent.actions.renderUiWidgetsoff the emitted event.Commands run locally on the pushed commit:
docs:checkmatters here specifically:Context.renderUiWidgetis public API whose signature referencesUiWidget, so exportingUiWidgetfromcommon.tsis mandatory — typedoc runs with--treatWarningsAsErrorsand fails otherwise.No new dependencies;
package.jsonandpackage-lock.jsonare untouched. No@ts-expect-error,@ts-ignore,eslint-disable,any, or coverage suppression was added anywhere in this diff.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.
CI status on this PR
run-testsandrun-tests (ubuntu-latest)pass.run-tests (macos-latest)andrun-tests (windows-latest)fail, and both failures are pre-existing platform flakes, not this change — verified by finding the identical failures on unrelated branches of this fork within the same hour:tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration > should discover apps vs agents across directories and standalone files(timeout, 43s) — 1 failed / 224 passedfeat/agent-to-mcp-server, job 91750596043, same test, also 1 failed / 224 passedcore/test/code_executors/unsafe_local_code_executor_test.ts > UnsafeLocalCodeExecutor > should execute shell code and return stdout(timeout at 5s)fix/dev-workspace-undeclared-dependencies, job 91746127625, same test, 1 failed / 224 passed; also onfeat/application-integration-toolset-part1, job 91755089946Neither test touches
core/src/events/,core/src/agents/context.ts, orcore/src/common.ts. The failed jobs were re-run once and reproduced the same two flakes. On the re-run the windows job was additionally cancelled by the matrix's fail-fast when macos failed first, so its result there is "cancelled", not a test failure. All four test files added or extended by this PR pass on every runner, including macos and windows.Revision after complexity review
The reviewer flagged the snake_case
render_ui_widgetsfallback inmergeEventActionsas unreachable. That was correct, and I verified it independently on both sides before removing it (evidence in the "Only the camelCase key is read on merge" bullet above): the adk-pythonpopit mirrored is dead there too, becauseby_alias=Trueon analias_generator=to_camelmodel always emitsrenderUiWidgets. My original justification for the branch came from the task spec, which asserted the empty-camelCase list "falls through to the snake_case key" — true of theorshort-circuit, but irrelevant, since the snake key is never in the dump at all. I should have checked that against pydantic rather than taking it from the spec.Removed exactly the five items the review bounded:
RENDER_UI_WIDGETS_SNAKE_CASE_KEY,readRenderUiWidgets, the doc sentence about both spellings, thesourceWithSnakeCaseWidgetstest helper, the now-unusedEventActionstest import, and the three tests that only covered the deleted branch. Nothing else in any test file changed; the four surviving cases in that describe block and every case incontext_test.ts,functions_test.tsandevent_test.tsare untouched. Both unchecked casts went with it, so this diff now contains zero casts.One addition beyond the removal: re-running the mutation suite against the reshaped branch showed that weakening
if (source.renderUiWidgets?.length)to a bare truthiness check survived — no test distinguished an empty source list from an absent one. That guard is reachable with a well-typed input (createEventActions({renderUiWidgets: []})) and is what upholds the documented "undefined, never[]" invariant, so I added the one case that pins it (mutation 5 in the table).core/src/events/event_actions.tsremains at 100% statements / branches / functions / lines, and the targeted suite is 94 passing.CI status on the reviewed commit (276f480) — final
run-testsrun-tests (ubuntu-latest)run-tests (macos-latest)app_loaderflake seen on the earlier commit did not recur)run-tests (windows-latest)Windows was re-run once and failed both times, but on a different set of tests each time, which is itself the signature of an environmental problem rather than a code defect:
core/test/code_executors/unsafe_local_code_executor_test.ts > UnsafeLocalCodeExecutor > should execute shell code and return stdout, timing out at 5012ms. 1 failed / 2692 passed. The same single test fails on unrelated branches of this fork:fix/dev-workspace-undeclared-dependencies(job 91746127625) andfeat/application-integration-toolset-part1(job 91755089946).tests/integration/a2a/basic/a2a_agent_test.ts > A2A: Remote Agent Basicfailing attests/integration/test_case_utils.ts:341withCLI exited prematurely with code 1, root cause[ADK CLI] Error starting API server: listen EACCES: permission denied ::1:49859— the runner could not bind a port. 1 failed / 2691 passed (the a2a file contributes a setup failure, not a test assertion failure).Neither test touches
core/src/events/,core/src/agents/context.ts, orcore/src/common.ts. All four test files added or extended by this PR pass on every runner, including Windows. Reported honestly as not green rather than green, since a test job did fail and I could not fix it — but the failures are not attributable to this change.