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

google gemini support function call #5581

Merged

Conversation

lloydzhou
Copy link
Member

@lloydzhou lloydzhou commented Oct 3, 2024

πŸ’» ε˜ζ›΄η±»εž‹ | Change Type

  • feat
  • fix
  • refactor
  • perf
  • style
  • test
  • docs
  • ci
  • chore
  • build

πŸ”€ ε˜ζ›΄θ―΄ζ˜Ž | Description of Change

πŸ“ θ‘₯充俑息 | Additional Information

Summary by CodeRabbit

  • New Features

    • Enhanced streaming capabilities for chat interactions, allowing for more dynamic responses.
    • New methods added to the API for usage tracking and model retrieval.
  • Improvements

    • Improved logic for displaying plugins based on service provider and model type.
    • Enhanced handling of request options in the fetch function.
    • Improved clarity and functionality of the stream method for managing tool calls.
  • Bug Fixes

    • Refined error handling in the streaming process to prevent unnecessary processing.
  • Refactor

    • Streamlined internal logic for managing tool calls and responses in the chat functionality.

Copy link

vercel bot commented Oct 3, 2024

@lloydzhou is attempting to deploy a commit to the NextChat Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Oct 3, 2024

Walkthrough

The changes in this pull request primarily focus on enhancing the GeminiProApi class in the google.ts file to support improved streaming capabilities. Key modifications include the restructuring of the chat method, the introduction of new methods for usage and models, and updates to utility functions like showPlugins and fetch. The changes aim to streamline how tool calls and responses are managed, particularly in the context of Google services.

Changes

File Path Change Summary
app/client/platforms/google.ts - Restructured chat method for streaming responses.
- Added methods: usage() and models() in GeminiProApi.
app/utils.ts - Updated showPlugins to include conditions for Google provider.
- Enhanced fetch function to better handle options.
app/utils/chat.ts - Improved stream function logic with new tool_call_id property.
- Refined error handling in onmessage event.

Possibly related PRs

  • hotfix for using google api in appΒ #5141: This PR modifies the GeminiProApi class in google.ts, which is directly related to the changes made in the main PR that also focuses on the GeminiProApi class, specifically enhancing chat functionality and API integration.
  • Hotfix: default configΒ #4988: This PR also involves changes to the GeminiProApi class in google.ts, addressing the construction of baseUrl, which may interact with the chat functionality improvements in the main PR.

Suggested labels

enhancement, help wanted

Suggested reviewers

  • Dogtiti
  • Leizhenpeng

Poem

In the land of code where rabbits play,
Streaming flows in a new, bright way.
With tools and calls, we hop along,
Crafting responses, where we belong.
So let’s cheer for changes, big and small,
Together we’ll code, and have a ball! πŸ‡βœ¨


πŸ“œ Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

πŸ“₯ Commits

Files that changed from the base of the PR and between 450766a and 4ae34ea.

πŸ“’ Files selected for processing (3)
  • app/client/platforms/google.ts (2 hunks)
  • app/utils.ts (1 hunks)
  • app/utils/chat.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/utils.ts
  • app/utils/chat.ts
🧰 Additional context used
πŸ”‡ Additional comments (5)
app/client/platforms/google.ts (5)

10-17: Imports added correctly

The new imports from @/app/store and @/app/utils/chat are appropriate and necessary for the updated functionality involving plugins and streaming.


27-27: Importing 'nanoid' is appropriate

The addition of the nanoid import is correct for generating unique identifiers within the code.


185-190: Proper retrieval of tools and functions

The extraction of tools and funcs using usePluginStore and useChatStore is correctly implemented, ensuring that the necessary plugins are obtained based on the current session's mask configuration.


199-215: Ensure robust error handling for JSON parsing

Consider adding a try-catch block around JSON.parse(text) to handle potential parsing errors gracefully. This will prevent runtime exceptions if text contains invalid JSON and enhance the robustness of the application.


220-256: Define appropriate types to eliminate '@ts-ignore'

The use of @ts-ignore suppresses TypeScript errors but may hide underlying issues and affect code maintainability. By defining proper types for toolCallMessage and toolCallResult, you can address the type errors and remove the need for @ts-ignore, enhancing type safety.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❀️ Share
πŸͺ§ Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

github-actions bot commented Oct 3, 2024

Your build has completed!

Preview deployment

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (5)
app/utils.ts (1)

Line range hint 291-315: Approved with suggestions for improvement

The changes to the fetch function improve its flexibility and consistency. However, I have a few suggestions:

  1. Consider separating the Tauri-specific logic into a separate function for better maintainability.
  2. The options?.responseType == "text" check might be more robust as a strict equality check (===).
  3. Consider using TypeScript's type system more effectively to avoid type assertions.

Here's a suggested refactor:

function tauriFetch(url: string, options?: Record<string, unknown>): Promise<any> {
  const payload = options?.body || options?.data;
  return window.__TAURI__.fetch(url, {
    ...options,
    body: payload && {
      type: "Text",
      payload,
    },
    timeout: ((options?.timeout as number) || REQUEST_TIMEOUT_MS) / 1000,
    responseType: options?.responseType === "text" ? ResponseType.Text : ResponseType.JSON,
  });
}

export function fetch(url: string, options?: Record<string, unknown>): Promise<any> {
  if (window.__TAURI__) {
    return tauriFetch(url, options);
  }
  return window.fetch(url, options);
}

This refactor separates the Tauri-specific logic, uses strict equality, and reduces the need for type assertions.

app/utils/chat.ts (4)

Line range hint 22-34: Prevent potential infinite loop in 'compressImage' function

In the compressImage function, the do...while loop may result in an infinite loop if the image cannot be compressed below maxSize, especially when both quality and dimensions reach their minimum thresholds. Consider adding a maximum number of iterations or additional checks to prevent a potential infinite loop.

Apply this diff to implement a maximum iteration count:

 let iteration = 0;
 do {
   // existing code...
   iteration++;
-} while (dataUrl.length > maxSize);
+} while (dataUrl.length > maxSize && iteration < 10);
+if (iteration >= 10) {
+  console.warn('Image compression stopped after reaching maximum iterations');
+}

Line range hint 162-164: Avoid suppressing TypeScript errors with @ts-ignore

The use of @ts-ignore suppresses TypeScript errors, which may hide potential issues and reduce type safety. To improve code quality, consider properly typing funcs and tool so that TypeScript can correctly type-check this code without the need for @ts-ignore.

You can define appropriate TypeScript interfaces for funcs and tool to ensure type correctness.


Line range hint 119-130: Possible race condition in finish function leading to multiple chatApi calls

In the finish function, when !finished and !running && runTools.length > 0, the function calls chatApi after a setTimeout, but running is set to false before calling chatApi. This could lead to multiple concurrent chatApi calls if finish is invoked again before chatApi completes. Consider ensuring running remains true until chatApi has completed its execution to prevent concurrent calls.

Apply this diff to adjust the running flag appropriately:

-            running = false;
             chatApi(chatPath, headers, requestPayload, tools); // call fetchEventSource
+            running = false;

Line range hint 116-127: Optimize animateResponseText for smoother updates

The animateResponseText function uses requestAnimationFrame for text updates, but when remainText.length is large, it might cause UI lag due to heavy processing within each frame. Consider throttling updates or processing smaller chunks per frame to ensure smoother UI performance.

πŸ“œ Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

πŸ“₯ Commits

Files that changed from the base of the PR and between 05e6e4b and 450766a.

πŸ“’ Files selected for processing (3)
  • app/client/platforms/google.ts (2 hunks)
  • app/utils.ts (1 hunks)
  • app/utils/chat.ts (1 hunks)
πŸ”‡ Additional comments (4)
app/utils.ts (2)

Line range hint 1-391: Summary of changes and suggestions

  1. The addition of Google provider support in the showPlugins function is approved, but clarification is needed on why vision models are excluded for Google.
  2. The fetch function improvements are approved, but a refactor is suggested for better separation of concerns between browser and Tauri environments.

Overall, these changes seem to support Google Gemini integration as mentioned in the PR title. However, more context in the PR description would have been helpful for a more comprehensive review.


287-289: LGTM, but clarification needed on vision model exclusion

The addition of Google provider support is consistent with the function's structure. However, could you please clarify why vision models are excluded for Google? This information would be helpful for maintainers and could potentially be added as a comment.

To ensure consistency, let's check for any other occurrences of Google provider handling:

app/client/platforms/google.ts (1)

184-188: Verify initialization of 'mask' and 'plugin' properties to prevent undefined behavior

In lines 184-188, the code accesses useChatStore.getState().currentSession().mask?.plugin || [] to retrieve plugins. Ensure that currentSession(), mask, and plugin are properly initialized to avoid potential runtime errors if any of these properties are undefined or null.

app/utils/chat.ts (1)

243-243: Ensure tool.id is defined and unique

The addition of tool_call_id: tool.id assumes that each tool object has an id property that is defined and unique. Please verify that tool.id is always initialized before this point to prevent undefined values or collisions.

Run the following script to search for assignments to tool.id in the codebase:

app/client/platforms/google.ts Show resolved Hide resolved
app/client/platforms/google.ts Show resolved Hide resolved
app/client/platforms/google.ts Show resolved Hide resolved
@lloydzhou lloydzhou requested a review from Dogtiti October 3, 2024 12:59
@lloydzhou
Copy link
Member Author

image

@Issues-translate-bot

This comment was marked as duplicate.

@lloydzhou lloydzhou merged commit c5074f0 into ChatGPTNextWeb:main Oct 10, 2024
1 of 2 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 24, 2024
10 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 13, 2024
10 tasks
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