Repository navigation
fix(firebase_ai): parse promptTokensDetails and empty CountTokensResponse on GoogleAI - #18916
cynthiajoan wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates parseCountTokensResponse in the developer API to parse promptTokensDetails and handle empty JSON objects, updates documentation for LiveSession.receive(), and adds corresponding unit tests. Feedback highlights a potential parsing failure if totalTokens is omitted while promptTokensDetails is present, and recommends avoiding the ! operator in tests to comply with the repository's null safety style guide.
| if (jsonObject case {'totalTokens': final int totalTokens}) { | ||
| return CountTokensResponse(totalTokens); | ||
| final promptTokensDetails = switch (jsonObject) { | ||
| {'promptTokensDetails': final List<Object?> details} => | ||
| parseUsageMetadata({ | ||
| 'promptTokensDetails': details, | ||
| }).promptTokensDetails, | ||
| _ => null, | ||
| }; | ||
| return CountTokensResponse( | ||
| totalTokens, | ||
| promptTokensDetails: promptTokensDetails, | ||
| ); | ||
| } |
There was a problem hiding this comment.
If totalTokens is omitted (e.g., because it is 0 and omitted in protobuf JSON) but promptTokensDetails is present, the pattern match jsonObject case {'totalTokens': final int totalTokens} will fail, causing the method to throw an unhandledFormat exception. We should make the parsing more robust by checking if either totalTokens or promptTokensDetails is present, and defaulting totalTokens to 0 if it is missing.
final hasTotalTokens = jsonObject.containsKey('totalTokens');
final hasPromptTokensDetails = jsonObject.containsKey('promptTokensDetails');
if (hasTotalTokens || hasPromptTokensDetails) {
final totalTokens = jsonObject['totalTokens'] as int? ?? 0;
final promptTokensDetails = switch (jsonObject) {
{'promptTokensDetails': final List<Object?> details} =>
parseUsageMetadata({
'promptTokensDetails': details,
}).promptTokensDetails,
_ => null,
};
return CountTokensResponse(
totalTokens,
promptTokensDetails: promptTokensDetails,
);
}| expect(details, isNotNull); | ||
| expect(details, hasLength(2)); | ||
| expect(details![0].modality, ContentModality.text); | ||
| expect(details[0].tokenCount, 100); | ||
| expect(details[1].modality, ContentModality.audio); | ||
| expect(details[1].tokenCount, 23); |
There was a problem hiding this comment.
Avoid using the ! operator as per the repository style guide (Rule 23). Instead, use flow analysis with a null check (if (details != null)) to safely access the list elements.
expect(details, isNotNull);
if (details != null) {
expect(details, hasLength(2));
expect(details[0].modality, ContentModality.text);
expect(details[0].tokenCount, 100);
expect(details[1].modality, ContentModality.audio);
expect(details[1].tokenCount, 23);
}References
- Null Safety: NO ! operator. Use ? and flow analysis (e.g. if (x != null)). (link)
Description
DeveloperSerialization.parseCountTokensResponse(lib/src/developer/api.dart) droppedpromptTokensDetailsreturned by the Gemini Developer API (googleAIbackend), so per-modality token counts such asTEXTandAUDIOwere alwaysnull, and it threwUnhandled format for CountTokensResponse: {}when the server omittedtotalTokens(protobuf JSON omits zero values).parseCountTokensResponsenow returnsCountTokensResponse(0)for an empty object and parsespromptTokensDetailswith the existingparseUsageMetadatahelper, matching theagentPlatformserializer. TheLiveSession.receive()doc comment is also corrected: the stream stays open across turns until the session is closed.Verification
bugfixd07113e3a+ only the new tests):flutter test --no-pub test/developer_api_test.dart→ exit 1, 25 passed, 2 failed; new tests failing:parses promptTokensDetails when present,defaults totalTokens to 0 on empty JSON object:3ca476f21):flutter test --no-pub test/developer_api_test.dart→ exit 0, 27 passed (4s).Related Issues
N/A (
AI-COUNT-TOKENS-1)Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
Prepared with the Firebase SDK dev-loop agent (Google-internal, built on Jetski) and reviewed by @cynthiajoan before opening. Evidence: red/green captured by
verify-redgreen(verdictverified) at based07113e3a/ head3ca476f21.