Skip to content

Migrate genui internals to a2ui_core#956

Closed
andrewkolos wants to merge 16 commits into
flutter:mainfrom
andrewkolos:migrate-genui-to-a2ui-core
Closed

Migrate genui internals to a2ui_core#956
andrewkolos wants to merge 16 commits into
flutter:mainfrom
andrewkolos:migrate-genui-to-a2ui-core

Conversation

@andrewkolos

@andrewkolos andrewkolos commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Context: #811

This is the first of a series of changes RE #811. In this PR, I focussed on trying to keep the diff size down. To this end, this PR preserves the current API surface of package:genui by creating adapters that connect to package:a2ui_core. This diff is still very large, but this approach did manage to cut the diff size to one third 🤷 .

Pre-launch Checklist

  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I read the Contributors Guide.
  • I have added sample code updates to the changelog.
  • I updated/added relevant documentation (doc comments with ///).
  • If my PR is a fork PR, I've checked that [e2e tests] passed.

If you need help, consider asking for advice on the #hackers-devrel channel on Discord.

…e JSON

Four substrate-level changes the genui facade migration depends on or
benefits from. Bundled because they're all isolated to a2ui_core and
each is small enough to review at the same sitting.

1. messages.dart: parse-time envelope validation.
   - Reject envelopes missing the `version` field or carrying a version
     other than 'v0.9' (was: accepted anything).
   - Reject envelopes that contain more than one action key
     (createSurface/updateComponents/updateDataModel/deleteSurface) —
     the v0.9 schema models these as `oneOf`, but the old code silently
     dispatched on whichever check matched first.
   - UpdateDataModelMessage gains a `hasValue` field plus a
     `.removeKey` named constructor. The wire distinction between
     `"value": null` (set to null) and an omitted `value` key (remove
     the key / sparse-clear a list index) now round-trips losslessly.
     Runtime DataModel.set behavior is unchanged; the distinction only
     matters for observers/relayers that inspect the message.

2. data_model.dart: defensively copy incoming Map/List values.
   Wraps every container the caller hands to DataModel (constructor
   `initialData` and `set()` writes) with a deep mutable copy so later
   nested writes don't blow up when callers passed `const` literals or
   otherwise-unmodifiable views. Test added covering the
   "set('/') of a const map, then set('/nested/count')" path.

3. data_path.dart: drop RFC 6901 `~0`/`~1` escape interpretation.
   The web_core reference renderer never implemented escaping despite
   the v0.9 spec citing RFC 6901; a2ui_core was the only implementation
   diverging. Aligns Dart with web_core. Affects only keys containing
   literal '~' or '/' — extremely rare in data model usage.

4. reactivity.dart: add `effect` to the re-exported symbols.
   preact_signals provides `effect()` as a one-shot wrapper for
   `Effect(fn)()`; exporting it lets consumers avoid the
   `Effect()..call()` ceremony at the call site.
… facade)

Wraps a2ui_core.DataModel inside genui's existing DataModel/InMemoryDataModel
public API. Adds a2ui_core to the genui pubspec. Preserves the existing
GenUI surface (DataPath, ValueListenable subscriptions, getValue<T>,
update(DataPath, value), bindExternalState, DataModelTypeException) so
no consumer rename is required.

Substantive pieces inside this commit:

1. data_model.dart: InMemoryDataModel becomes a compat facade.
   - Constructor `InMemoryDataModel()` constructs a fresh core.DataModel
     and owns it. `InMemoryDataModel.wrap(core.DataModel)` is @internal
     and lets the controller share a single live model with the surface.
   - update(DataPath, value) -> core.set(path.toString(), value).
   - subscribe<T>(DataPath) returns a _SignalNotifier — a ChangeNotifier
     backed by a preact_signals effect over core.watch<Object?>. The
     effect forces a notification on every source change (including
     `==`-equal values) so in-place Map/List mutations propagate.
   - getValue<T>(DataPath) preserves the typed-read throw via
     DataModelTypeException so legacy callers keep their type check.
   - bindExternalState is preserved as both instance method and a
     top-level helper.
   - DataContext keeps its existing public surface (subscribe,
     subscribeStream, getValue, update, nested, resolvePath, resolve,
     evaluateConditionStream). Internally it routes through the
     wrapped DataModel.

2. widget_utilities.dart: Bound* widgets rewritten on preact_signals
   internally, but the constructor parameters and builder signature are
   unchanged. Two non-obvious wrinkles preserved from this branch's
   earlier exploration:
   - BoundValue uses a signal+listener bridge (not core.computed)
     because computed gates downstream notifications on `==` equality,
     which for Maps/Lists is identity-based — so a2ui_core's bubble
     notifications on in-place mutations would be silently dropped.
     The bridge force-mirrors the source value on every change.
   - _SignalBuilder is a small private StatefulWidget that subscribes
     to a ReadonlySignal via preact_signals' effect(). signals_flutter
     is NOT compatible (different signals package).

3. client_function.dart: ExecutionContext interface picks up the
   compat-facade signatures (`Object` for path args so callers can
   pass either DataPath or String). format_string.dart drops a couple
   of redundant DataPath constructors at call sites.

4. test/widgets/widget_utilities_test.dart: regression for the
   BoundObject/BoundList in-place mutation case — without the bridge
   fix, these would silently miss bubble-notifications.

Test/dev_tools/example fallout from this commit is minimal because
the public API shape is unchanged.
…n facade

Surfaces internally become live core.SurfaceModel instances managed by
core.SurfaceGroupModel, with the existing SurfaceDefinition / Component
public API preserved as a snapshot facade. GenUI's own renderer reads
the live model for granular per-component rebuilds; external code that
implements SurfaceContext keeps using the legacy snapshot API.

Substantive pieces:

1. surface_registry.dart owns the live core.SurfaceModel for each
   active surface and exposes both:
   - watchSurface(id): ValueListenable<core.SurfaceModel?> (live)
   - watchDefinition(id): ValueListenable<SurfaceDefinition?> (snapshot)
   The latter materializes a SurfaceDefinition.fromCore lazily when
   listeners actually read it, and re-materializes on per-component
   updates. removeSurface only nulls the notifier values — the
   notifiers stay alive across delete so re-create-with-same-id finds
   widgets still attached.

2. interfaces/surface_context.dart exposes the legacy
   `definition: ValueListenable<SurfaceDefinition?>` plus a new
   @internal LiveSurfaceContext extension that adds
   `surface: ValueListenable<core.SurfaceModel?>`. External
   SurfaceContext implementations only need to provide the legacy
   `definition` snapshot path; GenUI's own controller provides both.

3. widgets/surface.dart has two render paths:
   - _buildLiveWidget: used when the context is LiveSurfaceContext.
     Wraps each rendered component in a _ComponentBuilder that
     subscribes to that component's onUpdated, so an UpdateComponents
     for one component rebuilds only its subtree (flutter#811 §3.5).
   - _buildWidgetFromDefinition: used when the context only provides
     the legacy snapshot path. Rebuilds from the snapshot on every
     definition change (pre-flutter#811 behavior).
   This dual path is the cost of the compat-facade strategy. Custom
   SurfaceContext implementations stay on the legacy path; granular
   reactivity is opt-in by implementing LiveSurfaceContext.

4. ui_models.dart: SurfaceDefinition gains a .fromCore(SurfaceModel)
   factory and keeps its validate(Schema), copyWith, asContextDescription
   methods. SurfaceUpdate.{SurfaceAdded,ComponentsUpdated} carries
   both the live `surface` and a lazy `definition` snapshot —
   lifecycle-only listeners pay nothing.

5. engine/data_model_store.dart is kept as a compat facade. Its
   lookup callback now redirects to InMemoryDataModel.wrap of the live
   surface.dataModel for active surfaces, falling back to standalone
   models when no live surface exists. Carries an explicit dartdoc
   marking it as compatibility-only.

6. model/parts/ui.dart's UiPart.create accepts either SurfaceDefinition
   or a raw JsonMap as `definition`. Parsing always re-materializes a
   SurfaceDefinition snapshot so consumers don't see the wire-shape
   difference.

7. Legacy SurfaceDefinition.validate(Schema) is preserved.

Tests covering surface lifecycle live in commit 4 with their
controller/registry counterparts.
…ssor

SurfaceController becomes a thin Flutter-side wrapper around
core.MessageProcessor. The substrate owns canonical state mutation
(create/update/delete surfaces and their components/data models);
this class adds the Flutter-specific concerns: pre-create message
buffering, schema validation, a SurfaceUpdate stream the facade
subscribes to, and the GenUI-named A2uiMessage facade.

Substantive pieces:

1. engine/surface_controller.dart: rewrite.
   - Public `handleMessage(A2uiMessage)` accepts the genui-facade
     message type. It converts to core via `message.toCoreMessage()`
     and delegates to a private `_handleCoreMessage(core.A2uiMessage)`.
     The buffered/flushed path uses the private method directly so
     no double-conversion happens.
   - Holds a core.MessageProcessor built from each catalog's
     `coreCatalog` adapter; subscribes to its
     groupModel.onSurfaceCreated/onSurfaceDeleted and forwards them
     to the SurfaceRegistry.
   - Pre-create buffering: UpdateComponents/UpdateDataModel that
     arrive before their createSurface are held in a per-surfaceId
     queue with `pendingUpdateTimeout` cleanup, then flushed when
     the surface is created.
   - Lenient unknown-catalogId behavior is preserved (registers an
     empty stub core.Catalog so the substrate's "not found" check
     passes; the genui Surface widget surfaces FallbackWidget at
     render time as before).
   - Schema validation runs after each UpdateComponents via the new
     shared schema_validation helper (item 5 below). Mutation is not
     rolled back on failure — the error is reported and the caller
     decides.

2. model/a2ui_message.dart: GenUI message facade classes.
   `A2uiMessage`, `CreateSurface`, `UpdateComponents`,
   `UpdateDataModel`, `DeleteSurface` each wrap the corresponding
   core message. `.fromJson(JsonMap)` parses via core then converts;
   `.fromCore(core.A2uiMessage)` adapts an existing core message;
   `.toCoreMessage()` converts back. `UpdateDataModel` preserves
   the value-omitted vs value-null wire distinction via a
   `hasValue` flag + `.removeKey` named constructor matching
   a2ui_core's new shape.

3. model/catalog.dart: `coreCatalog` extension exposes a
   core.Catalog<core.ComponentApi> view of a genui Catalog so the
   substrate has real component metadata for the processor.
   Functions and theme are not adapted yet — Node Layer (#1282)
   will redo this boundary; until then catalog widgets resolve
   functions through the genui DataContext, not the core one.

4. model/catalog_item.dart: CatalogItemContext keeps both the
   legacy public unnamed constructor (which builds a stand-alone
   substrate context internally) AND an @internal fromCore
   constructor used by the live render path. `withOverrides({...})`
   is the rebind-callbacks-while-preserving-substrate-context
   pattern Catalog.buildWidget uses.

5. model/schema_validation.dart (new): shared validator extracted
   from both SurfaceDefinition.validate and the controller's
   post-mutation validation. Takes an iterable of
   `({id, type, json})` records plus the catalog Schema; both
   call sites adapt their representation into the iterable.

6. transport/a2ui_parser_transformer.dart: routes through
   core.A2uiMessage.fromJson then wraps the result in the facade
   A2uiMessage. Envelope detection (the `_looksLikeA2uiEnvelope`
   helper) treats `version` as an envelope marker, so a payload
   like {"version":"v0.9","unknownAction":{}} is surfaced as a
   validation error rather than falling through to TextEvent.

7. New regression tests:
   - data_model_edge_cases_test.dart: covers the
     mutable-copy-of-const-Map case the substrate fix in commit 1
     introduced.
   - surface_controller_test.dart: store-facade test, duplicate
     createSurface, pre-create dataModel access.
   - a2ui_message_test.dart: facade round-trip cases.
   - core_widgets_test.dart: minor accommodation for the new
     contextFor(...)/dataModel path.
Short guide for consumers. The substrate moved into a2ui_core but
the existing GenUI public API names are preserved as compat facades,
so most catalog widget authors and example apps need no source
changes. The guide:

1. Lists what stays source-compatible: CreateSurface, DataPath,
   DataModel, InMemoryDataModel, SurfaceDefinition, Component,
   SurfaceContext.definition, SurfaceController.store / DataModelStore,
   ActionDelegate.handleEvent's existing signature,
   CatalogItemContext public fields, UiPart.create(definition:
   SurfaceDefinition(...)).

2. Documents what changed internally: SurfaceController delegates
   to MessageProcessor; InMemoryDataModel wraps core.DataModel;
   the Surface widget uses live core models for granular rebuilds
   when its context is GenUI's own controller context, and falls
   back to the legacy snapshot path for custom SurfaceContexts.

3. Lists residual behavior changes to watch for: stricter DataModel
   writes, defensively-copied stored containers, signal-backed
   reactivity inside Bound*, stricter envelope validation, duplicate
   createSurface as an error, RFC 6901 escapes no longer interpreted.

4. Notes that the GenUI-named compat types are the current public
   API, not short-lived shims. A future PR may add a2ui_core-shaped
   aliases for cross-language parity, but the existing names will
   not be removed without a separate deprecation cycle.
…akage

Pre-create writes to `SurfaceContext.dataModel` were lost when the live
surface arrived (the fallback `InMemoryDataModel` stayed shadowed by the
new live wrapper). `DataModelStore` now exposes `attachLive(id, model)`,
called from `_onCoreSurfaceCreated`, which snapshots any fallback data
into the live core model and disposes the fallback.
`_ControllerContext.dataModel` is routed through the store so post-create
re-fetches see the migrated data.

Marks `SurfaceRegistry`, `SurfaceAdded.surface`, and
`ComponentsUpdated.surface` `@internal` so the public consumer surface
stays GenUI-typed. Updates the migration guide to call out the small set
of integrator-facing APIs that still expose `a2ui_core.SurfaceModel`.

Adds tests covering pre-create dataModel access, fallback->live data
migration on createSurface, and facade-level UpdateDataModel
toCoreMessage/fromCore round-trip preservation of the explicit-null vs
omitted distinction.

Adds CHANGELOG entries to both packages.
- _onCoreSurfaceCreated: migrate fallback data via store.attachLive
  BEFORE registry.addSurface notifies listeners, so a synchronous
  ValueListener callback that calls contextFor.dataModel sees the
  populated live model rather than racing the migration.
- DataModelStore.getDataModel: check the _liveDataModels cache before
  invoking the lookup callback. Avoids redundant wrap calls and avoids
  the race where a getDataModel call cached one wrapper just before
  attachLive cached another.
- DataModelStore.attachLive: drop the null-snapshot guard so a pre-create
  `update(root, null)` is preserved across createSurface. Adds a test.
- Revert the genui.dart hide of SurfaceRegistry. SurfaceRegistry existed
  as a public type pre-migration; hiding it would be an unrelated source
  break. The new core-typed `surface` fields on SurfaceAdded /
  SurfaceUpdated remain `@internal`.
- Migration guide: add an explicit Scope section listing the deferred
  follow-ups (catalog widget bodies, action dispatch, sendDataModel,
  GenericBinder, typed-props), the flutter#938 dependency for the
  hasValue/remove runtime split, and the A2UI#1499 RFC 6901 tracking.
`SurfaceRegistry.watchSurface` and `getSurface` are reverted to their
pre-migration return types (`SurfaceDefinition`-typed), and `SurfaceAdded`
/ `SurfaceUpdated` regain their `definition: SurfaceDefinition` field.
Existing consumers of those names see no signature change.

Live core access moves to new `@internal` siblings: `watchLiveSurface`,
`getLiveSurface`, and the existing `.surface` field on the registry
events. The migration guide is updated to reflect the actual set of
internal-marked APIs.

Internal callers (the lookup-callback in `DataModelStore`,
`_findCatalogForSurface`, and `LiveSurfaceContext.surface`) move to the
new live methods.
`SurfaceAdded` and `SurfaceUpdated` regain their pre-migration public
constructor signature `(surfaceId, SurfaceDefinition)`. The live core
surface is moved to an `@internal SurfaceAdded.fromCore` /
`SurfaceUpdated.fromCore` named constructor used by `SurfaceRegistry`;
the `surface` field becomes nullable + `@internal` (null when constructed
via the public ctor, populated when emitted by the registry).

The controller's surfaceUpdates mapper unwraps `surface!` since registry-
emitted events always use `.fromCore`.

Adds a regression test exercising `watchSurface` / `getSurface` against
the `SurfaceDefinition` snapshot.
Same pattern as the registry-event compat fix, applied to the user-
facing `SurfaceAdded` and `ComponentsUpdated` events in
`packages/genui/lib/src/model/ui_models.dart`:

- Public ctor takes `(surfaceId, SurfaceDefinition)` (pre-migration shape),
  leaves `surface` null.
- `@internal SurfaceAdded.fromCore` / `ComponentsUpdated.fromCore` populate
  both the snapshot and the live `core.SurfaceModel`.
- `surface` field is now `core.SurfaceModel?` and `@internal`.
- The surfaceUpdates mapper in SurfaceController uses `.fromCore`.

Marks `SurfaceRegistry.addSurface` and `notifyUpdated` `@internal` (new
in this branch; only SurfaceController calls them). The pre-migration
`SurfaceRegistry.updateSurface(SurfaceDefinition)` is intentionally not
restored; the live-model registry can't honor the definition-only push
path without diverging from the substrate state. The removal is now
called out in the migration guide.

Adds a regression test for the public SurfaceUpdate ctors.
… updateSurface removal

- Adds `const` to public `SurfaceAdded` and `ComponentsUpdated`
  constructors to match the pre-migration shape (and the existing
  `SurfaceRemoved` ctor).
- Promotes the `SurfaceRegistry.updateSurface(...)` removal to a
  BREAKING bullet in the CHANGELOG. It is a pre-migration public API
  removal; preserving it would require diverging the registry from the
  live core surface model.
Per reviewer nit: "replacements" implied external users should reach for
them, which is the opposite of the @internal intent. Reads now as
"internal lifecycle hooks."
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Package publishing

Package Version Status Publish tag (post-merge)
package:a2ui_core 0.0.1-wip002 ready to publish a2ui_core-v0.0.1-wip002
package:genai_primitives 0.2.4-wip001 ready to publish genai_primitives-v0.2.4-wip001
package:genui 0.9.2 (error) pubspec version (0.9.2) and changelog (0.10.0) don't agree
package:genui_a2a 0.9.0 already published at pub.dev
package:json_schema_builder 0.1.4 ready to publish json_schema_builder-v0.1.4

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request migrates the runtime substrate of package:genui onto the shared package:a2ui_core implementation, preserving the existing GenUI API as a compatibility facade. Key changes include wrapping core.MessageProcessor in SurfaceController, adapting InMemoryDataModel to wrap a2ui_core.DataModel, and updating BoundValue widgets to use preact_signals internally. Feedback on the changes highlights a critical runtime correctness issue where checking event is! UserActionEvent always evaluates to false due to extension type erasure, suggesting a key-presence check instead. Additionally, it is recommended to reject empty surface IDs across all message types to prevent inefficient buffering.

Note: Security Review did not run due to the size of the PR.

Comment on lines 296 to 297
void handleUiEvent(UiEvent event) {
if (event is! UserActionEvent) return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

At runtime, Dart extension types are completely erased to their representation type (Map<String, Object?>). Therefore, the type check event is! UserActionEvent will always evaluate to false because any UiEvent is represented as a Map at runtime. This causes non-action events to be incorrectly processed as user actions.

To fix this, check for the presence of the 'name' key in the underlying map to distinguish UserActionEvent from other UiEvents. This is a critical correctness issue under the Effective Dart guidelines for extension types.

Suggested change
void handleUiEvent(UiEvent event) {
if (event is! UserActionEvent) return;
void handleUiEvent(UiEvent event) {
if (!event.toMap().containsKey('name')) return;

Comment on lines +110 to +124
void _handleCoreMessage(core.A2uiMessage coreMessage) {
// Reject empty surfaceId before delegating; the substrate would otherwise
// create a surface with id "".
if (coreMessage is core.CreateSurfaceMessage &&
coreMessage.surfaceId.isEmpty) {
reportError(
A2uiValidationException(
'Surface ID cannot be empty',
surfaceId: '',
path: 'surfaceId',
),
StackTrace.current,
);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of only checking for an empty surfaceId on CreateSurfaceMessage, we should reject empty surfaceIds for all message types. Otherwise, messages like UpdateComponentsMessage or UpdateDataModelMessage with an empty surfaceId will be buffered indefinitely under the key "" until they time out, which is inefficient.

We can use the existing _surfaceIdOf helper to check the surfaceId of any incoming message. This aligns with the Effective Dart guidelines for robust error handling and input validation.

  void _handleCoreMessage(core.A2uiMessage coreMessage) {
    final String? surfaceId = _surfaceIdOf(coreMessage);
    if (surfaceId != null && surfaceId.isEmpty) {
      reportError(
        A2uiValidationException(
          'Surface ID cannot be empty',
          surfaceId: '',
          path: 'surfaceId',
        ),
        StackTrace.current,
      );
      return;
    }

schema_validation.dart imported ui_models.dart only to throw
A2uiValidationException, while ui_models.dart imported schema_validation.dart
for SurfaceDefinition.validate. That cycle failed layerlens --fail-on-cycles.

A2uiValidationException is a cross-cutting error type thrown across the model,
engine, and transport layers, and ui_models.dart never used the one it defined.
Move it to a primitives leaf so schema_validation no longer depends on the model
layer and the graph becomes an acyclic DAG.

primitives.dart re-exports it, so package:genui consumers are unchanged.
handleUiEvent guarded on `event is! UserActionEvent`, but extension types
erase to their representation at runtime, so the check was always false and
every UiEvent was submitted to the AI as an action. Add a data-based
discriminator (UiEvent.isUserAction, keyed on the action `name`) and use it
in handleUiEvent and Surface._dispatchEvent, which had the same erased check.

Empty surfaceId was rejected only for CreateSurface; UpdateComponents,
UpdateDataModel, and DeleteSurface with an empty id buffered under "" until
timeout. Reject an empty surfaceId on any message that carries one.

Regression tests cover both.
Add a widget test for live per-component rebuilds (the migration's headline
behavior) and a test for the new duplicate-createSurface error. The migration
nets a deletion of well-tested bespoke code, which lowered packages/genui
aggregate line coverage, so re-mark the high-water baseline from 79.71% to its
post-migration value (79.38%).
@andrewkolos andrewkolos closed this Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant