-
Notifications
You must be signed in to change notification settings - Fork 46
Support incremental surface updates #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ import 'ui_tools.dart'; | |
|
||
/// A sealed class representing an update to the UI managed by [GenUiManager]. | ||
/// | ||
/// This class has three subclasses: [SurfaceAdded], [SurfaceUpdated], and | ||
/// This class has three subclasses: [SurfaceAdded], [SurfaceChanged], and | ||
/// [SurfaceRemoved]. | ||
sealed class GenUiUpdate { | ||
/// Creates a [GenUiUpdate] for the given [surfaceId]. | ||
|
@@ -42,10 +42,10 @@ class SurfaceAdded extends GenUiUpdate { | |
} | ||
|
||
/// Fired when an existing surface is modified. | ||
class SurfaceUpdated extends GenUiUpdate { | ||
/// Creates a [SurfaceUpdated] event for the given [surfaceId] and | ||
class SurfaceChanged extends GenUiUpdate { | ||
/// Creates a [SurfaceChanged] event for the given [surfaceId] and | ||
/// [definition]. | ||
const SurfaceUpdated(super.surfaceId, this.definition); | ||
const SurfaceChanged(super.surfaceId, this.definition); | ||
|
||
/// The new definition of the surface. | ||
final UiDefinition definition; | ||
|
@@ -119,7 +119,7 @@ class GenUiManager implements GenUiHost { | |
if (event is! UiActionEvent) throw ArgumentError('Unexpected event type'); | ||
final stateValue = valueStore.forSurface(event.surfaceId); | ||
final eventString = | ||
'Action: ${jsonEncode(event.value)}\n' | ||
'Action: ${jsonEncode(event.value)}\\n' | ||
'Current state: ${jsonEncode(stateValue)}'; | ||
_onSubmit.add(UserMessage([TextPart(eventString)])); | ||
} | ||
|
@@ -164,19 +164,58 @@ class GenUiManager implements GenUiHost { | |
/// If a surface with the given ID does not exist, a new one is created. | ||
/// Otherwise, the existing surface is updated. | ||
void addOrUpdateSurface(String surfaceId, JsonMap definition) { | ||
final uiDefinition = UiDefinition.fromMap({ | ||
'surfaceId': surfaceId, | ||
...definition, | ||
}); | ||
final action = definition['action'] as String? ?? 'replace'; | ||
final notifier = surface(surfaceId); // Gets or creates the notifier. | ||
final isNew = notifier.value == null; | ||
notifier.value = uiDefinition; | ||
|
||
if (isNew) { | ||
final uiDefinition = UiDefinition.fromMap({ | ||
'surfaceId': surfaceId, | ||
...definition, | ||
}); | ||
notifier.value = uiDefinition; | ||
genUiLogger.info('Adding surface $surfaceId'); | ||
_surfaceUpdates.add(SurfaceAdded(surfaceId, uiDefinition)); | ||
} else { | ||
genUiLogger.info('Updating surface $surfaceId'); | ||
_surfaceUpdates.add(SurfaceUpdated(surfaceId, uiDefinition)); | ||
switch (action) { | ||
case 'replace': | ||
final uiDefinition = UiDefinition.fromMap({ | ||
'surfaceId': surfaceId, | ||
...definition, | ||
}); | ||
notifier.value = uiDefinition; | ||
genUiLogger.info('Replacing surface $surfaceId'); | ||
_surfaceUpdates.add(SurfaceChanged(surfaceId, uiDefinition)); | ||
case 'update': | ||
final currentDefinition = notifier.value!; | ||
assert( | ||
definition['root'] == currentDefinition.root, | ||
'The root widget ID must be the same for update actions.', | ||
); | ||
final newWidgetsList = definition['widgets'] as List<Object?>; | ||
final newWidgetsMap = <String, JsonMap>{}; | ||
for (final widget in newWidgetsList) { | ||
final typedWidget = widget as JsonMap; | ||
newWidgetsMap[typedWidget['id'] as String] = typedWidget; | ||
} | ||
Comment on lines
+195
to
+200
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code in this block uses several direct casts ( For example: final newWidgetsList = definition['widgets'];
if (newWidgetsList is! List) {
// Handle error, e.g., throw an ArgumentError or log a warning.
return;
}
for (final widget in newWidgetsList) {
if (widget is! JsonMap) {
// Handle error
continue;
}
final id = widget['id'];
if (id is! String) {
// Handle error
continue;
}
newWidgetsMap[id] = widget;
} |
||
|
||
final updatedWidgets = { | ||
...currentDefinition.widgets, | ||
...newWidgetsMap, | ||
}; | ||
|
||
// TODO(andrewkolb): Prune orphaned widgets. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
final uiDefinition = UiDefinition.fromMap({ | ||
'surfaceId': surfaceId, | ||
'root': currentDefinition.root, | ||
'widgets': updatedWidgets.values.toList(), | ||
}); | ||
notifier.value = uiDefinition; | ||
genUiLogger.info('Updating surface $surfaceId'); | ||
_surfaceUpdates.add(SurfaceChanged(surfaceId, uiDefinition)); | ||
default: | ||
throw ArgumentError('Invalid action: $action'); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,12 +30,23 @@ class AddOrUpdateSurfaceTool extends AiTool<JsonMap> { | |
'action': S.string( | ||
description: | ||
'The action to perform. You must choose from the available ' | ||
'actions. If you choose the `add` action, you must choose a ' | ||
'new unique surfaceId. If you choose the `update` action, ' | ||
'you must choose an existing surfaceId.', | ||
'actions.\n' | ||
'- `add`: Creates a new surface. You must choose a new, ' | ||
'unique `surfaceId`.\n' | ||
'- `update`: Updates an existing surface by adding or ' | ||
'replacing individual widgets. This is efficient for small ' | ||
'changes, as it preserves the rest of the widget tree. The ' | ||
'`root` widget ID must be the same as the original ' | ||
'surface.\n' | ||
'- `replace`: Replaces the entire content of an existing ' | ||
'surface. This is for when the entire UI needs to be ' | ||
'changed.', | ||
enumValues: [ | ||
if (configuration.actions.allowCreate) 'add', | ||
if (configuration.actions.allowUpdate) 'update', | ||
if (configuration.actions.allowUpdate) ...[ | ||
'update', | ||
'replace', | ||
], | ||
], | ||
), | ||
'surfaceId': S.string( | ||
|
@@ -50,7 +61,9 @@ class AddOrUpdateSurfaceTool extends AiTool<JsonMap> { | |
'root': S.string( | ||
description: | ||
'The ID of the root widget. This ID must correspond to ' | ||
'the ID of one of the widgets in the `widgets` list.', | ||
'the ID of one of the widgets in the `widgets` list. ' | ||
'For `update` actions, this must be the same as the ' | ||
'original surface.', | ||
), | ||
'widgets': S.list( | ||
items: catalog.schema, | ||
|
@@ -78,6 +91,8 @@ class AddOrUpdateSurfaceTool extends AiTool<JsonMap> { | |
Future<JsonMap> invoke(JsonMap args) async { | ||
final surfaceId = args['surfaceId'] as String; | ||
final definition = args['definition'] as JsonMap; | ||
final action = args['action'] as String; | ||
definition['action'] = action; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line mutates the For example: final newDefinition = {
...definition,
'action': action,
};
onAddOrUpdate(surfaceId, newDefinition); |
||
onAddOrUpdate(surfaceId, definition); | ||
return {'surfaceId': surfaceId, 'status': 'SUCCESS'}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change from
\n
to\\n
appears to be a mistake. The double backslash escapes the newline character, which means a literal\n
string will be sent to the model instead of a newline. This is unlikely to be interpreted as a line break by the AI. The original single backslash\n
was correct for creating a newline character within the string.