Conversation
Generated by 🚫 Danger |
|
Please audit public API and change mutable stored properties to immutable lets if possible. |
FirebaseAI/Sources/Types/Public/Live/LiveGenerationConfig.swift
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Live API, a significant feature enabling real-time, bidirectional communication with the model. The changes are extensive, adding new public-facing classes like LiveGenerativeModel and LiveSession, along with a robust internal infrastructure to handle WebSocket communication, including a LiveSessionService actor and numerous data models for client/server messages.
Overall, the implementation is of very high quality. The separation of concerns is excellent, with clear distinctions between public API, internal services, and low-level networking. The use of modern Swift concurrency features like actors and async streams is well-executed. Error handling is also very thorough, with specific, descriptive error types for the new Live API.
I've identified one high-severity issue related to a hardcoded URL that would prevent testing against non-production environments, and a medium-severity issue regarding the WebSocket delegate setup. My feedback is focused on improving the robustness and correctness of the new networking layer.
ncooke3
left a comment
There was a problem hiding this comment.
Great work! Please do a round of testing before merging.
Adds support for the Live API through a new
LiveSessionstruct, which is facilitated through a newliveModelmethod onFirebaseAI. This includes support for the realtime api, as well as the incremental api.This PR also adds support for the
idfield inFunctionCallPart.Fixes #15147