-
Notifications
You must be signed in to change notification settings - Fork 959
Firebase AI Hybrid Inference Implementation #9029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1Affected Products
Test Logs |
constructor( | ||
apiSettings: ApiSettings, | ||
public model: string, | ||
private chromeAdapter?: ChromeAdapter, | ||
public params?: StartChatParams, | ||
public requestOptions?: RequestOptions | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change for users who pass params
or requestOptions
. Could we make this a non-breaking change by moving chromeAdapter
to be the last parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users aren't supposed to instantiate ChatSession directly, they are supposed to call startChat
, which doesn't have ordered params and actually doesn't allow passing requestOptions at all. I wanted to actually stop exporting ChatSession
for that reason, but that would be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our documentation doesn't state that users shouldn't use the ChatSession
constructor so I feel we should try our best to avoid breaking it outside of a breaking release.
I think we should continue to export the ChatSession
class, but in the next breaking release maybe we should make the constructor internal if we don't want users to create it.
This PR contains implementation & docs for hybrid inference.
API Proposal (internal doc): https://docs.google.com/document/d/1gBlTkMABfjsCre9g5jMfN2DrMV6isql43mFg58V9PkQ/edit?tab=t.0#heading=h.jsemgbpc5x7
Notes
ChromeAdapter
is an internal class the user should not call but it's passed to the constructor ofChatSession
which is publicly exported, so it needs to be exported. I created aChromeAdapter
interface that reflects the public methods of that class so it can be used as a type, without exporting the whole class.Documentation
This also contains some fixes to documentation in other sections of the AI SDK (missing beta flag on an Imagen symbol, fixed some links).
The hybrid inference feature is experimental and we don't have automated doc tooling to handle that case (currently
@beta
tags become labeled "(Public Preview)") so for now I have manually put "(EXPERIMENTAL)" on all the new features.