Skip to content

Invoke FirebaseProvider's onFunctionCalls callback - #199

Open
shoemoney wants to merge 1 commit into
flutter:mainfrom
shoemoney:fix/firebase-provider-onfunctioncalls
Open

Invoke FirebaseProvider's onFunctionCalls callback#199
shoemoney wants to merge 1 commit into
flutter:mainfrom
shoemoney:fix/firebase-provider-onfunctioncalls

Conversation

@shoemoney

Copy link
Copy Markdown

FirebaseProvider's constructor accepts an onFunctionCalls parameter but never stores or invokes it, so the callback silently never fires. The parameter arrived in #99, which wired it into the former GeminiProvider and VertexProvider; when those were consolidated into FirebaseProvider, the parameter survived but the wiring did not.

This change stores the callback and invokes it once per turn with the function calls collected from that turn, matching the #99 behavior. It adds a regression test that drives a stubbed streaming response (a function-call turn followed by a text turn) through the provider via firebase_ai's createModelWithClient test hook, asserting the callback fires with the collected calls and stays silent on text-only turns. Two dev dependencies (firebase_core, firebase_core_platform_interface) were added for the Firebase core test mocks.

flutter test (8 tests), dart analyze --fatal-infos, dart format, dart fix --dry-run, and dart doc --dry-run all pass.

The onFunctionCalls constructor parameter added in flutter#99 was accepted but
never stored or invoked after GeminiProvider and VertexProvider were
consolidated into FirebaseProvider. Store the callback and invoke it
once per turn with the function calls collected from that turn, and add
a regression test that drives a stubbed streaming response through the
provider.
GenerationConfig? chatGenerationConfig,
Future<Map<String, Object?>?> Function(FunctionCall)? onFunctionCall,
}) : _model = model,
_onFunctionCalls = onFunctionCalls,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

While we're at it, should we also rename the callback to willCallFunctions? I'm not a fan of the on prefix, as it doesn't explain when exactly the callback is called relative to the anchor event. Otherwise, the bug fix looks good.

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.

2 participants