Skip to content

Conversation

@jacobsimionato
Copy link
Collaborator

This is a simple move and restructure of SDK logic out of the example app folder.

  • Move everything except Firebase config and main.dart
  • Put internal classes in lib/src
  • Rename "widgets" folder to "core_widgets"
  • Remove a little dead code and reformat some files

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 refactors the project by moving core SDK logic from the generic_chat example into the flutter_genui package. This includes renaming WidgetTreeLlmAdapter to ConversationManager, moving several files, and removing some dead code. The changes are well-structured and align with the PR's goal. My main feedback is to improve test coverage for the newly packaged logic, as per the repository's style guide, to ensure its stability and maintainability.

import 'src/event_debouncer.dart';
import 'src/ui_models.dart';

class ConversationManager {
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

According to the repository's style guide, code should be tested.1 As ConversationManager is a core component being moved into the package, this would be a great time to add unit tests for it. Creating a conversation_manager_test.dart with tests for methods like sendUserPrompt and _handleEvents (perhaps by making it package-private for testing) using a mock LlmConnection would significantly improve the package's quality and maintainability.

Style Guide References

Footnotes

  1. The style guide states that code should be tested. (link)

@jacobsimionato jacobsimionato merged commit 8349a4e into main Jul 30, 2025
1 check passed
@jacobsimionato jacobsimionato deleted the split_sdk_2 branch September 28, 2025 21:51
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