feat(ai): expose recent firebase-js-sdk APIs in react-native-firebase#9028
feat(ai): expose recent firebase-js-sdk APIs in react-native-firebase#9028mikehardy wants to merge 15 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request improves API parity between react-native-firebase and the upstream firebase-js-sdk for the AI package. It introduces support for anyOf schema composition, exposes ThinkingLevel presets for model reasoning, and adds handling for server-side going away notices in live sessions. Additionally, it includes client-side validation for generation configurations to prevent common configuration errors. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
| export interface ObjectSchemaRequest extends Omit<SchemaRequest, 'type'> { | ||
| type: 'object'; |
There was a problem hiding this comment.
This was a curious one, here's the relevant output while steering an agent:
compare:types rebuilt successfully but found one undocumented shape difference: RN’s new ObjectSchemaRequest.type was emitted as SchemaType.OBJECT while the JS SDK emits the literal 'object'
So we have to use a type of object here if we want 100% fidelity with upstream types (which I believe we do...) but that implies we have to use a more complicated extends type so that we can extend SchemaRequest since that type expects the type to be SchemaType.OBJECT
I think that's okay, but it is worth a callout
There was a problem hiding this comment.
Code Review
This pull request introduces support for anyOf schemas and thinkingLevel configurations within the AI package, aligning the React Native Firebase implementation more closely with the JS SDK. Key changes include the addition of the AnyOfSchema class, the ThinkingLevel enum, and the LiveServerGoingAwayNotice response type for live sessions. The feedback identifies a potential issue where malformed duration strings could result in NaN values and suggests a more robust null check for thinkingLevel during validation to ensure future-proof code.
| if ( | ||
| // != allows for null and undefined. 0 is considered "set" by the model. | ||
| generationConfig?.thinkingConfig?.thinkingBudget != null && | ||
| generationConfig.thinkingConfig?.thinkingLevel |
There was a problem hiding this comment.
For consistency with the thinkingBudget check on the previous line and to follow defensive programming practices, it is recommended to use an explicit != null check for thinkingLevel. While the current enum values are truthy strings, this approach is more robust against future changes to the enum or unexpected values.
| generationConfig.thinkingConfig?.thinkingLevel | |
| generationConfig.thinkingConfig?.thinkingLevel != null |
| if (!duration || !duration.endsWith('s')) { | ||
| return 0; | ||
| } | ||
| return Number(duration.slice(0, -1)); |
There was a problem hiding this comment.
The Number() function can return NaN if the input string is not a valid number (e.g., if the server sends a malformed duration like "abc s"). It is safer to check for NaN and return a default value of 0 to ensure that timeLeft always contains a valid numeric value.
const val = Number(duration.slice(0, -1));
return Number.isNaN(val) ? 0 : val;|
maintainer note: compare-types did not surface the entry points for automatic function calling in the multiple various entry points, though it was detected during agent review - possibly augment compare-types so it surfaces them, red/green test would be to check any change aimed at this at commit point prior to the function-calling PRs after the first one |
|
@mikehardy - presuming one of the the automatic function calling methods is this: This isn't surfaced to end consumer (i.e. it's for internal use) so I think this is why it didn't surface in compare scripts? |
russellwheatley
left a comment
There was a problem hiding this comment.
Approved - I'll leave it to your discretion if you want to address the gemini feedback 👍
Description
This is a "converge towards API parity" PR for the AI package - there have been quite a few API elements added upstream in firebase-js-sdk and this PR should close the gap down a fair bit
Related issues
None logged
Release Summary
A series of feature conventional commits, a semver minor should issue
Checklist
AndroidiOSOther(macOS, web)e2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
Mostly compare:types and tests:jest analysis, periodic local runs of tests:macos:test but CI will also confirm
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter