From c52051eafc0b7698e66261c5cb4a01cd4b37cffb Mon Sep 17 00:00:00 2001 From: Jacob Simionato Date: Mon, 17 Nov 2025 10:25:47 +1030 Subject: [PATCH 1/7] Fix bug with surface creation and update notifications --- .../genui/lib/src/core/genui_manager.dart | 56 +++++++++++-------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/genui/lib/src/core/genui_manager.dart b/packages/genui/lib/src/core/genui_manager.dart index daabf84b9..ff41b9050 100644 --- a/packages/genui/lib/src/core/genui_manager.dart +++ b/packages/genui/lib/src/core/genui_manager.dart @@ -158,44 +158,46 @@ class GenUiManager implements GenUiHost { void handleMessage(A2uiMessage message) { switch (message) { case SurfaceUpdate(): - // No need for SurfaceAdded here because A2uiMessage will never generate - // those. We decide here if the surface is new or not, and generate a - // SurfaceAdded event if so. final String surfaceId = message.surfaceId; - final ValueNotifier notifier = getSurfaceNotifier( - surfaceId, - ); - final isNew = notifier.value == null; + final ValueNotifier notifier = + getSurfaceNotifier(surfaceId); + + // Caching logic remains the same UiDefinition uiDefinition = notifier.value ?? UiDefinition(surfaceId: surfaceId); - final Map newComponents = Map.of( - uiDefinition.components, - ); + final Map newComponents = + Map.of(uiDefinition.components); for (final Component component in message.components) { newComponents[component.id] = component; } uiDefinition = uiDefinition.copyWith(components: newComponents); notifier.value = uiDefinition; - if (isNew) { - genUiLogger.info('Adding surface $surfaceId'); - _surfaceUpdates.add(SurfaceAdded(surfaceId, uiDefinition)); - } else { + + // Notify UI ONLY if rendering has begun (i.e., rootComponentId is set) + if (uiDefinition.rootComponentId != null) { genUiLogger.info('Updating surface $surfaceId'); _surfaceUpdates.add(SurfaceUpdated(surfaceId, uiDefinition)); + } else { + genUiLogger.info( + 'Caching components for surface $surfaceId (pre-rendering)'); } case BeginRendering(): - dataModelForSurface(message.surfaceId); - final ValueNotifier notifier = getSurfaceNotifier( - message.surfaceId, - ); + final String surfaceId = message.surfaceId; + dataModelForSurface(surfaceId); + final ValueNotifier notifier = + getSurfaceNotifier(surfaceId); + + // Update the definition with the root component final UiDefinition uiDefinition = - notifier.value ?? UiDefinition(surfaceId: message.surfaceId); + notifier.value ?? UiDefinition(surfaceId: surfaceId); final UiDefinition newUiDefinition = uiDefinition.copyWith( rootComponentId: message.root, ); notifier.value = newUiDefinition; - genUiLogger.info('Started rendering ${message.surfaceId}'); - _surfaceUpdates.add(SurfaceUpdated(message.surfaceId, newUiDefinition)); + + // ALWAYS fire SurfaceAdded, as this is the signal to start rendering. + genUiLogger.info('Creating and rendering surface $surfaceId'); + _surfaceUpdates.add(SurfaceAdded(surfaceId, newUiDefinition)); case DataModelUpdate(): final String path = message.path ?? '/'; genUiLogger.info( @@ -205,13 +207,19 @@ class GenUiManager implements GenUiHost { ); final DataModel dataModel = dataModelForSurface(message.surfaceId); dataModel.update(DataPath(path), message.contents); + + // Notify UI of an update if the surface is already rendering + final notifier = getSurfaceNotifier(message.surfaceId); + final uiDefinition = notifier.value; + if (uiDefinition != null && uiDefinition.rootComponentId != null) { + _surfaceUpdates.add(SurfaceUpdated(message.surfaceId, uiDefinition)); + } case SurfaceDeletion(): final String surfaceId = message.surfaceId; if (_surfaces.containsKey(surfaceId)) { genUiLogger.info('Deleting surface $surfaceId'); - final ValueNotifier? notifier = _surfaces.remove( - surfaceId, - ); + final ValueNotifier? notifier = + _surfaces.remove(surfaceId); notifier?.dispose(); _dataModels.remove(surfaceId); _surfaceUpdates.add(SurfaceRemoved(surfaceId)); From 2624167e4c10a0fe57d55220dae3382f898d5310 Mon Sep 17 00:00:00 2001 From: Jacob Simionato Date: Wed, 19 Nov 2025 12:38:44 +1030 Subject: [PATCH 2/7] Apply automated fixes and formatting --- .../genui/lib/src/core/genui_manager.dart | 42 ++++++++++--------- .../genui/test/core/genui_manager_test.dart | 3 ++ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/genui/lib/src/core/genui_manager.dart b/packages/genui/lib/src/core/genui_manager.dart index ff41b9050..0974ab47c 100644 --- a/packages/genui/lib/src/core/genui_manager.dart +++ b/packages/genui/lib/src/core/genui_manager.dart @@ -159,33 +159,35 @@ class GenUiManager implements GenUiHost { switch (message) { case SurfaceUpdate(): final String surfaceId = message.surfaceId; - final ValueNotifier notifier = - getSurfaceNotifier(surfaceId); + final bool isNewSurface = !_surfaces.containsKey(surfaceId); + final ValueNotifier notifier = getSurfaceNotifier( + surfaceId, + ); - // Caching logic remains the same UiDefinition uiDefinition = notifier.value ?? UiDefinition(surfaceId: surfaceId); - final Map newComponents = - Map.of(uiDefinition.components); + final Map newComponents = Map.of( + uiDefinition.components, + ); for (final Component component in message.components) { newComponents[component.id] = component; } uiDefinition = uiDefinition.copyWith(components: newComponents); notifier.value = uiDefinition; - // Notify UI ONLY if rendering has begun (i.e., rootComponentId is set) - if (uiDefinition.rootComponentId != null) { + if (isNewSurface) { + genUiLogger.info('Adding new surface $surfaceId'); + _surfaceUpdates.add(SurfaceAdded(surfaceId, uiDefinition)); + } else { genUiLogger.info('Updating surface $surfaceId'); _surfaceUpdates.add(SurfaceUpdated(surfaceId, uiDefinition)); - } else { - genUiLogger.info( - 'Caching components for surface $surfaceId (pre-rendering)'); } case BeginRendering(): final String surfaceId = message.surfaceId; dataModelForSurface(surfaceId); - final ValueNotifier notifier = - getSurfaceNotifier(surfaceId); + final ValueNotifier notifier = getSurfaceNotifier( + surfaceId, + ); // Update the definition with the root component final UiDefinition uiDefinition = @@ -195,9 +197,8 @@ class GenUiManager implements GenUiHost { ); notifier.value = newUiDefinition; - // ALWAYS fire SurfaceAdded, as this is the signal to start rendering. - genUiLogger.info('Creating and rendering surface $surfaceId'); - _surfaceUpdates.add(SurfaceAdded(surfaceId, newUiDefinition)); + genUiLogger.info('Start rendering surface $surfaceId'); + _surfaceUpdates.add(SurfaceUpdated(surfaceId, newUiDefinition)); case DataModelUpdate(): final String path = message.path ?? '/'; genUiLogger.info( @@ -209,8 +210,10 @@ class GenUiManager implements GenUiHost { dataModel.update(DataPath(path), message.contents); // Notify UI of an update if the surface is already rendering - final notifier = getSurfaceNotifier(message.surfaceId); - final uiDefinition = notifier.value; + final ValueNotifier notifier = getSurfaceNotifier( + message.surfaceId, + ); + final UiDefinition? uiDefinition = notifier.value; if (uiDefinition != null && uiDefinition.rootComponentId != null) { _surfaceUpdates.add(SurfaceUpdated(message.surfaceId, uiDefinition)); } @@ -218,8 +221,9 @@ class GenUiManager implements GenUiHost { final String surfaceId = message.surfaceId; if (_surfaces.containsKey(surfaceId)) { genUiLogger.info('Deleting surface $surfaceId'); - final ValueNotifier? notifier = - _surfaces.remove(surfaceId); + final ValueNotifier? notifier = _surfaces.remove( + surfaceId, + ); notifier?.dispose(); _dataModels.remove(surfaceId); _surfaceUpdates.add(SurfaceRemoved(surfaceId)); diff --git a/packages/genui/test/core/genui_manager_test.dart b/packages/genui/test/core/genui_manager_test.dart index 2e8c078c3..da1807a21 100644 --- a/packages/genui/test/core/genui_manager_test.dart +++ b/packages/genui/test/core/genui_manager_test.dart @@ -80,6 +80,9 @@ void main() { manager.handleMessage( SurfaceUpdate(surfaceId: surfaceId, components: oldComponents), ); + manager.handleMessage( + const BeginRendering(surfaceId: surfaceId, root: 'root'), + ); final newComponents = [ const Component( From eae2d4ce85d06cd5765e74399b390e993f933329 Mon Sep 17 00:00:00 2001 From: Jacob Simionato Date: Wed, 19 Nov 2025 12:48:37 +1030 Subject: [PATCH 3/7] Fix(tests): Correct event stream handling in GenUiManager tests --- .../genui/lib/src/core/genui_manager.dart | 17 +++++---- .../genui/test/core/genui_manager_test.dart | 35 +++++++------------ packages/genui/test/ui_tools_test.dart | 5 ++- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/packages/genui/lib/src/core/genui_manager.dart b/packages/genui/lib/src/core/genui_manager.dart index 0974ab47c..a30250617 100644 --- a/packages/genui/lib/src/core/genui_manager.dart +++ b/packages/genui/lib/src/core/genui_manager.dart @@ -159,11 +159,11 @@ class GenUiManager implements GenUiHost { switch (message) { case SurfaceUpdate(): final String surfaceId = message.surfaceId; - final bool isNewSurface = !_surfaces.containsKey(surfaceId); final ValueNotifier notifier = getSurfaceNotifier( surfaceId, ); + // Caching logic remains the same UiDefinition uiDefinition = notifier.value ?? UiDefinition(surfaceId: surfaceId); final Map newComponents = Map.of( @@ -175,12 +175,14 @@ class GenUiManager implements GenUiHost { uiDefinition = uiDefinition.copyWith(components: newComponents); notifier.value = uiDefinition; - if (isNewSurface) { - genUiLogger.info('Adding new surface $surfaceId'); - _surfaceUpdates.add(SurfaceAdded(surfaceId, uiDefinition)); - } else { + // Notify UI ONLY if rendering has begun (i.e., rootComponentId is set) + if (uiDefinition.rootComponentId != null) { genUiLogger.info('Updating surface $surfaceId'); _surfaceUpdates.add(SurfaceUpdated(surfaceId, uiDefinition)); + } else { + genUiLogger.info( + 'Caching components for surface $surfaceId (pre-rendering)', + ); } case BeginRendering(): final String surfaceId = message.surfaceId; @@ -197,8 +199,9 @@ class GenUiManager implements GenUiHost { ); notifier.value = newUiDefinition; - genUiLogger.info('Start rendering surface $surfaceId'); - _surfaceUpdates.add(SurfaceUpdated(surfaceId, newUiDefinition)); + // ALWAYS fire SurfaceAdded, as this is the signal to start rendering. + genUiLogger.info('Creating and rendering surface $surfaceId'); + _surfaceUpdates.add(SurfaceAdded(surfaceId, newUiDefinition)); case DataModelUpdate(): final String path = message.path ?? '/'; genUiLogger.info( diff --git a/packages/genui/test/core/genui_manager_test.dart b/packages/genui/test/core/genui_manager_test.dart index da1807a21..6303d6758 100644 --- a/packages/genui/test/core/genui_manager_test.dart +++ b/packages/genui/test/core/genui_manager_test.dart @@ -41,24 +41,19 @@ void main() { ), ]; - final Future futureAdded = manager.surfaceUpdates.first; manager.handleMessage( SurfaceUpdate(surfaceId: surfaceId, components: components), ); - final GenUiUpdate addedUpdate = await futureAdded; - expect(addedUpdate, isA()); - expect(addedUpdate.surfaceId, surfaceId); - final Future futureUpdated = manager.surfaceUpdates.first; + final Future futureUpdate = manager.surfaceUpdates.first; manager.handleMessage( const BeginRendering(surfaceId: surfaceId, root: 'root'), ); - final GenUiUpdate updatedUpdate = await futureUpdated; + final GenUiUpdate update = await futureUpdate; - expect(updatedUpdate, isA()); - expect(updatedUpdate.surfaceId, surfaceId); - final UiDefinition definition = - (updatedUpdate as SurfaceUpdated).definition; + expect(update, isA()); + expect(update.surfaceId, surfaceId); + final UiDefinition definition = (update as SurfaceAdded).definition; expect(definition, isNotNull); expect(definition.rootComponentId, 'root'); expect(manager.surfaces[surfaceId]!.value, isNotNull); @@ -80,9 +75,6 @@ void main() { manager.handleMessage( SurfaceUpdate(surfaceId: surfaceId, components: oldComponents), ); - manager.handleMessage( - const BeginRendering(surfaceId: surfaceId, root: 'root'), - ); final newComponents = [ const Component( @@ -93,18 +85,17 @@ void main() { ), ]; - final Future futureUpdate = manager.surfaceUpdates.first; + expectLater( + manager.surfaceUpdates, + emitsInOrder([isA(), isA()]), + ); + + manager.handleMessage( + const BeginRendering(surfaceId: surfaceId, root: 'root'), + ); manager.handleMessage( SurfaceUpdate(surfaceId: surfaceId, components: newComponents), ); - final GenUiUpdate update = await futureUpdate; - - expect(update, isA()); - expect(update.surfaceId, surfaceId); - final UiDefinition updatedDefinition = - (update as SurfaceUpdated).definition; - expect(updatedDefinition.components['root'], newComponents[0]); - expect(manager.surfaces[surfaceId]!.value, updatedDefinition); }, ); diff --git a/packages/genui/test/ui_tools_test.dart b/packages/genui/test/ui_tools_test.dart index 754acd90e..52724b41c 100644 --- a/packages/genui/test/ui_tools_test.dart +++ b/packages/genui/test/ui_tools_test.dart @@ -64,6 +64,9 @@ void main() { ); await tool.invoke(args); + genUiManager.handleMessage( + const BeginRendering(surfaceId: 'testSurface', root: 'root'), + ); await future; }); @@ -99,7 +102,7 @@ void main() { final Future future = expectLater( genUiManager.surfaceUpdates, emits( - isA() + isA() .having((e) => e.surfaceId, surfaceIdKey, 'testSurface') .having( (e) => e.definition.rootComponentId, From 2a2890c850d9647258c8216fbc5f4e74868c4408 Mon Sep 17 00:00:00 2001 From: Jacob Simionato Date: Mon, 24 Nov 2025 15:35:25 +1030 Subject: [PATCH 4/7] Fix tests --- packages/genui/test/core/genui_manager_test.dart | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/genui/test/core/genui_manager_test.dart b/packages/genui/test/core/genui_manager_test.dart index 6303d6758..86e4a40c5 100644 --- a/packages/genui/test/core/genui_manager_test.dart +++ b/packages/genui/test/core/genui_manager_test.dart @@ -72,10 +72,6 @@ void main() { }, ), ]; - manager.handleMessage( - SurfaceUpdate(surfaceId: surfaceId, components: oldComponents), - ); - final newComponents = [ const Component( id: 'root', @@ -85,17 +81,22 @@ void main() { ), ]; - expectLater( + final expectation = expectLater( manager.surfaceUpdates, emitsInOrder([isA(), isA()]), ); + manager.handleMessage( + SurfaceUpdate(surfaceId: surfaceId, components: oldComponents), + ); manager.handleMessage( const BeginRendering(surfaceId: surfaceId, root: 'root'), ); manager.handleMessage( SurfaceUpdate(surfaceId: surfaceId, components: newComponents), ); + + await expectation; }, ); From 7fbcd4eca6e72219c74e5ee24b40585f48086eb9 Mon Sep 17 00:00:00 2001 From: Jacob Simionato Date: Mon, 24 Nov 2025 17:37:50 +1030 Subject: [PATCH 5/7] Fix style --- packages/genui/test/core/genui_manager_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/genui/test/core/genui_manager_test.dart b/packages/genui/test/core/genui_manager_test.dart index 86e4a40c5..7b139ae79 100644 --- a/packages/genui/test/core/genui_manager_test.dart +++ b/packages/genui/test/core/genui_manager_test.dart @@ -81,7 +81,7 @@ void main() { ), ]; - final expectation = expectLater( + final Future expectation = expectLater( manager.surfaceUpdates, emitsInOrder([isA(), isA()]), ); From ee8d132390e85807d70385b834b710a6b9e81412 Mon Sep 17 00:00:00 2001 From: jacobsimionato Date: Thu, 27 Nov 2025 15:01:01 +1030 Subject: [PATCH 6/7] Update packages/genui/lib/src/core/genui_manager.dart Co-authored-by: Andrew Kolos --- packages/genui/lib/src/core/genui_manager.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/genui/lib/src/core/genui_manager.dart b/packages/genui/lib/src/core/genui_manager.dart index a30250617..ab85941e4 100644 --- a/packages/genui/lib/src/core/genui_manager.dart +++ b/packages/genui/lib/src/core/genui_manager.dart @@ -163,7 +163,6 @@ class GenUiManager implements GenUiHost { surfaceId, ); - // Caching logic remains the same UiDefinition uiDefinition = notifier.value ?? UiDefinition(surfaceId: surfaceId); final Map newComponents = Map.of( From 340d6c532d48fa6d15b8791f57655412ca5f155c Mon Sep 17 00:00:00 2001 From: jacobsimionato Date: Thu, 27 Nov 2025 15:02:43 +1030 Subject: [PATCH 7/7] Update packages/genui/lib/src/core/genui_manager.dart Co-authored-by: Andrew Kolos --- packages/genui/lib/src/core/genui_manager.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/genui/lib/src/core/genui_manager.dart b/packages/genui/lib/src/core/genui_manager.dart index ab85941e4..a2b2d24fb 100644 --- a/packages/genui/lib/src/core/genui_manager.dart +++ b/packages/genui/lib/src/core/genui_manager.dart @@ -198,7 +198,6 @@ class GenUiManager implements GenUiHost { ); notifier.value = newUiDefinition; - // ALWAYS fire SurfaceAdded, as this is the signal to start rendering. genUiLogger.info('Creating and rendering surface $surfaceId'); _surfaceUpdates.add(SurfaceAdded(surfaceId, newUiDefinition)); case DataModelUpdate():