Skip to content

Conversation

@jacobsimionato
Copy link
Collaborator

Introduces SurfaceUpdateMode to provide distinct create and update capabilities for UI surfaces. This change refactors SurfaceUpdateTool to be configurable and updates the AI content generators to provide separate surfaceCreate and surfaceUpdate tools based on the GenUiConfiguration.

This allows developers to control whether the AI can only create new UI surfaces, only update existing ones, or both, preventing unintended UI modifications.

Fixes #463

Copy link
Contributor

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

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 effectively introduces granular controls for UI surface creation and updates by adding a SurfaceUpdateMode. The refactoring of SurfaceUpdateTool and the updates to the AI content generators are well-executed and align with the goal of preventing unintended UI modifications. The code is clear and the changes are consistent across the different packages. I have two suggestions to improve maintainability and adopt more modern Dart syntax.

Comment on lines 196 to 214
final String surfaceIdDescription;
switch (updateMode) {
case SurfaceUpdateMode.create:
surfaceIdDescription =
'The unique identifier for the new UI surface to create. This '
'*must* be a new, unique identifier.';
break;
case SurfaceUpdateMode.update:
surfaceIdDescription =
'The unique identifier for the existing UI surface to update.';
break;
case SurfaceUpdateMode.both:
surfaceIdDescription =
'The unique identifier for the UI surface to create or '
'update. If you are adding a new surface this *must* be a '
'new, unique identified that has never been used for any '
'existing surfaces shown.',
),
'components': S.list(
description: 'A list of component definitions.',
minItems: 1,
items: S.object(
description:
'Represents a *single* component in a UI widget tree. '
'This component could be one of many supported types.',
properties: {
'id': S.string(),
'weight': S.integer(
description:
'Optional layout weight for use in Row/Column children.',
),
'component': S.object(
description:
'''A wrapper object that MUST contain exactly one key, which is the name of the component type (e.g., 'Text'). The value is an object containing the properties for that specific component.''',
properties: {
for (var entry
in ((catalog.definition as ObjectSchema)
.properties!['components']!
as ObjectSchema)
.properties!
.entries)
entry.key: entry.value,
},
),
},
required: ['id', 'component'],
'existing surfaces shown.';
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For improved conciseness and readability, you could refactor this switch statement into a switch expression, which is a feature available in modern Dart. This would make the assignment of surfaceIdDescription more direct and the code more idiomatic.

    final String surfaceIdDescription = switch (updateMode) {
      SurfaceUpdateMode.create =>
        'The unique identifier for the new UI surface to create. This '
        '*must* be a new, unique identifier.',
      SurfaceUpdateMode.update =>
        'The unique identifier for the existing UI surface to update.',
      SurfaceUpdateMode.both =>
        'The unique identifier for the UI surface to create or '
        'update. If you are adding a new surface this *must* be a '
        'new, unique identified that has never been used for any '
        'existing surfaces shown.',
    };

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.

FirebaseAiContentGenerator seems to ignore GenUiConfiguration allowUpdate false

1 participant