Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the UI update logic by removing redundant surface updates in the SurfaceController and updating the prompt builder to instruct the AI against modifying existing surfaces. Feedback includes a security concern regarding the lack of backend enforcement for the 'no-update' policy, a suggestion to improve the robustness of surface update assertions in tests, and a recommendation to add a comment explaining the removal of the surface update trigger to clarify how reactivity is handled.
| updated.length == created.length, | ||
| 'In chat setup surfaces should not be updated after initial creation', | ||
| ); |
There was a problem hiding this comment.
The check updated.length == created.length only verifies the total number of events. It doesn't ensure that each specific surface was updated exactly once. For example, if one surface is updated twice and another is never updated, this check would still pass. A more robust check would verify the update count per surface ID.
updated.length == created.length,
'In chat setup surfaces should not be updated after initial creation',
);
for (final id in created) {
final updateCount = updated.where((u) => u == id).length;
reporter.expect(updateCount == 1, 'Surface $id should be updated exactly once');
}Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
3a382b5 to
734ef06
Compare
Description
This fixes the
packages/genui/examples/evaltests, since they are failing. They also aren't being run by CI, presumably because they make actual backend calls.