Skip to content

fix(vertexai): update history getter to reflect google_generative_ai updates #13040

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

Merged
merged 5 commits into from
Aug 19, 2024

Conversation

Toglefritz
Copy link
Contributor

Problem Description

There are two ChatSession classes: one in the Flutterfire Vertex AI package and one in the google_generative_ai package. Each of these classes maintains a private _history list to store the content that has been sent to or received from the generative model. Both classes also provide a history getter that returns this list to the clients. However, an issue arose because the sendMessage method in the Flutterfire ChatSession class relies on the sendMessage method from the google_generative_ai.ChatSession class to update the chat history. Consequently, only the _history list in the google_generative_ai.ChatSession class was being updated, while the _history list in the Flutterfire ChatSession class remained outdated. This discrepancy caused the history getter in the Flutterfire ChatSession class to always return an empty list as the chat session history, leading to inconsistencies and potential errors in the application. Exactly the same issue can be observed with the sendMessageStream methods defined in each class.

The video below demonstrates this issue. In the example app, a Text widget has been added to the bottom of the screen to display the total length of the chat session history using the history getter from the Flutterfire ChatSession class. As shown in the video, the count remains at zero, even when messages are present and visible in the chat session. This indicates that the history getter in the Flutterfire ChatSession class is not being updated correctly with the latest messages, highlighting the need for the proposed fix.

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-07-04.at.12.57.58.mp4

Solution Description

To resolve this issue, the ChatSession class in the Flutterfire plugin was updated so that the history getter returns the history from the google_generative_ai package, which is the one being actively updated when new messages are added to the chat. This change ensures that the history returned to clients accurately reflects the current state of the chat session.

Here are the specific changes made to the ChatSession class:

  1. Removed the _history member: The private _history member and its initialization in the constructor were removed, as it was no longer necessary to maintain a separate history list in the Flutterfire ChatSession class.
  2. Updated the constructor: The constructor now accepts an initialHistory parameter, which is used to initialize the chat session in the startChat method call.
  3. Modified the history getter: The history getter was updated to fetch the history from the google_ai.ChatSession instance. This ensures that the history returned by the Flutterfire ChatSession class is always up-to-date and consistent with the state of the chat session.

The video below demonstrates the proposed solution for the ChatSession class in the Flutterfire package. In the example app, a Text widget at the bottom of the screen displays the total length of the chat session history using the updated history getter from the Flutterfire ChatSession class. As new messages are added to the chat, the Text widget correctly updates to reflect the current length of the chat history. This confirms that the history getter is now accurately reporting the chat history, validating the effectiveness of the proposed fix.

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-07-04.at.14.12.43.mp4

Alternate Solution Description

An alternate solution to the issue would involve updating the sendMessage and sendMessageStream methods in the Flutterfire ChatSession class to ensure that the _history list within the Flutterfire class is updated whenever these methods are called. This approach would maintain the original functionality of the history getter in the Flutterfire ChatSession class and keep the class constructor unchanged. However, it would result in maintaining two versions of the same chat history: one in the Flutterfire ChatSession class and one in the google_generative_ai.ChatSession class.

Related Issues

none found

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.yaml and changelogs is not required.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.
…_ai updates

- Removed the private _history member from the Flutterfire ChatSession class
- Updated the constructor to accept initialHistory parameter
- Modified the startChat method to use initialHistory
- Updated the history getter to return history from google_generative_ai package

This ensures that the history returned by the Flutterfire ChatSession class is always up-to-date and consistent with the state of the chat session.
- Clarified that the history is maintained by the google_generative_ai package
- Modified history getter comment to highlight that it returns the most current state of the chat session
@Lyokone Lyokone changed the title fix(chat-session): update history getter to reflect google_generative_ai updates Jul 26, 2024
@cynthiajoan cynthiajoan requested a review from natebosch August 15, 2024 00:18
@cynthiajoan
Copy link
Collaborator

@Toglefritz Thanks for the PR, can you fix the format so we can go ahead and merge it in? https://github.com/firebase/flutterfire/actions/runs/9799303587/job/27950531837?pr=13040

@Toglefritz Toglefritz changed the base branch from master to main August 15, 2024 22:13
@Toglefritz
Copy link
Contributor Author

Hello @cynthiajoan. Thank you for taking the time to review my PR. I fixed the formatting using Melos. Additionally, it looks like this PR was originally set to merge into the master branch. I updated the base to the main branch. Please let me know if this is contrary to your contribution process, or just change the base back. Thanks again!

@cynthiajoan cynthiajoan merged commit cc542d7 into firebase:main Aug 19, 2024
18 of 22 checks passed
@firebase firebase locked and limited conversation to collaborators Sep 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants