Refactor SDK logic and improve logging of LLM inferences #72
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces some great refactoring for clarity, improves logging for LLM inferences, and fixes a critical bug related to parsing model output. My review highlights a syntax error in the new logging code that needs to be fixed and suggests re-adding a removed test to maintain test coverage for the token tracking functionality.
|
It is very rare that widget names in Flutter contain the word 'Widget'. Instead they contain some word that means something visual, like 'Button' or 'View'. How about:
|
|
gspencergoog
left a comment
gspencergoog
left a comment
There was a problem hiding this comment.
I'm going to approve this and submit so we can get the bug fix. We can refine the naming in future PRs.
| List<Tool>? tools, | ||
| ToolConfig? toolConfig, | ||
| }) { | ||
| return GenerativeModelWrapper( |
There was a problem hiding this comment.
I named that so that it could grow into the interface to any generative model. I know it is still specific to Firebase AI Logic right now, but eventually it shouldn't be.
3 participants
These were some first steps I made towards redoing the event logic before I had to stop working today. They're a bit scattered!
GenerativeModelWrappertoFirebaseAiGenerativeModelseeing as this is a Firebase AI logic-specific implementation of an interface.DynamicUitoSurfaceWidgetseeing as this widget is specifically for displaying an individual surface.ai_client.dartthat was causingcapturedResultto be pulled incorrectly resulting in an infinite loop of inferences.GenerativeModelInterfaceto remove unused parameters.