Skip to content
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

Update citations samples with title and nullable uri #15

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

andrewheard
Copy link
Collaborator

Updated the citations samples to include cases where the response has a title instead of a uri. This scenario was reported in firebase/firebase-ios-sdk#13518.

Copy link

Coverage Diff

GenerateContentResponse: 40
| candidates: 37
| | citationMetadata: 4
| | | citations: 4
| | | | title: 0 -> 2 ✅
Total Coverage: 42.74% -> 43.55% ✅

Legend:
✅ : total coverage increase
🔵 : files number increase, total coverage unaffected
🟡 : files number decrease, total coverage unaffected
❌ : total coverage decrease

@andrewheard andrewheard requested review from paulb777 and rlazo August 19, 2024 20:14
Copy link
Collaborator

@rlazo rlazo left a comment

Choose a reason for hiding this comment

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

I'll track fixing the Android test to correctly handle these cases. You can go ahead and merge the change.

@andrewheard
Copy link
Collaborator Author

cc: @hsubox76 I think the JS SDK already handles uri and title correctly. I think changing https://github.com/firebase/firebase-js-sdk/blob/f7c6dc4fe10ca3999b1499476ecf9b911c534d25/packages/vertexai/src/requests/stream-reader.test.ts#L213 from 2 to 3 will resolve the test failure when we tag the next major version.

@andrewheard andrewheard merged commit cff8d56 into main Aug 19, 2024
7 of 9 checks passed
@andrewheard andrewheard deleted the ah/citations-title branch August 19, 2024 20:43
@dlarocque
Copy link
Collaborator

JS Unit tests should be passing after firebase/firebase-js-sdk#8445

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.

3 participants