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

Feature/realtime chat #5786

Merged
merged 27 commits into from
Nov 11, 2024
Merged

Conversation

Dogtiti
Copy link
Member

@Dogtiti Dogtiti commented Nov 7, 2024

💻 变更类型 | Change Type

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

🔀 变更说明 | Description of Change

📝 补充信息 | Additional Information

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced real-time chat functionality with audio message support.
    • Added configuration options for real-time chat settings, including provider selection and API key input.
    • Enhanced chat interface with improved layout and responsiveness for input actions and audio messages.
    • Implemented a visual representation of audio frequency data with the new VoicePrint component.
    • Added a new AudioRecorderProcessor for managing audio recording functionality.
    • Enhanced localization support for real-time chat features in both English and Chinese.
    • Updated README to reflect new features and improvements.
  • Bug Fixes

    • Adjusted styles for better readability and alignment in chat components.
  • Chores

    • Added new dependencies for real-time audio capabilities.

These updates enhance user experience by providing new features and improving existing functionalities within the chat application.

@Dogtiti Dogtiti requested a review from lloydzhou November 7, 2024 01:48
Copy link

vercel bot commented Nov 7, 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 Nov 7, 2024

Walkthrough

The changes in this pull request introduce enhancements to the chat application, focusing on the integration of real-time audio chat features. Key modifications include the addition of new components and styles for real-time chat functionality, updates to existing chat components, and localization support for new features. The PR also introduces a new audio handler class for managing audio recording and playback, along with updates to configuration settings and localization files to support these features.

Changes

File Change Summary
app/components/chat.module.scss Updated styles for .chat-input-actions, added .chat-input-actions-end, and introduced .chat-message-audio. Adjusted media queries for responsiveness.
app/components/chat.tsx Added RealtimeChat component and HeadphoneIcon. Updated ChatActions props to manage side panel visibility.
app/components/realtime-chat/index.ts Re-exported all entities from ./realtime-chat.
app/components/realtime-chat/realtime-chat.module.scss Created styles for .realtime-chat, .circle-mic, .bottom-icons, and added animation styles.
app/components/realtime-chat/realtime-chat.tsx Introduced RealtimeChat component for real-time audio chat, managing connection and audio input.
app/components/realtime-chat/realtime-config.tsx Added RealtimeConfigList component for configuring real-time chat settings.
app/components/settings.tsx Integrated RealtimeConfigList into the Settings component for managing real-time configurations.
app/lib/audio.ts Added AudioHandler class for audio recording and playback management.
app/locales/cn.ts Added localization keys for real-time chat features under the Realtime section.
app/locales/en.ts Added localization keys for real-time chat features under the Realtime section.
app/store/chat.ts Updated ChatMessage type to include optional audio_url property.
app/store/config.ts Introduced realtimeConfig in DEFAULT_CONFIG and added RealtimeConfig type for real-time settings.
app/utils/chat.ts Commented out a console log in the uploadImage function.
package.json Added rt-client dependency for real-time audio capabilities.
public/audio-processor.js Introduced AudioRecorderProcessor class for audio recording within an audio processing context.
app/components/voice-print/index.ts Re-exported all entities from voice-print.
app/components/voice-print/voice-print.module.scss Added styles for .voice-print class.
app/components/voice-print/voice-print.tsx Introduced VoicePrint component for visualizing audio frequency data.

Possibly related PRs

  • style: improve classname by clsx #5782: This PR improves class name management using clsx, enhancing the readability and maintainability of the code, which aligns with the CSS changes in the main PR.

Suggested labels

enhancement, planned

Suggested reviewers

  • lloydzhou

Poem

🐰 In the chat where voices blend,
New features hop, around the bend.
With audio waves and styles so bright,
Our chat's alive, a joyful sight!
So raise your ears, let laughter flow,
In real-time chats, we steal the show! 🎤✨


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.

@Dogtiti Dogtiti requested review from lloydzhou and removed request for lloydzhou November 7, 2024 04:04
@Dogtiti Dogtiti requested review from lloydzhou and removed request for lloydzhou November 7, 2024 04:22
Copy link
Contributor

github-actions bot commented Nov 7, 2024

Your build has completed!

Preview deployment

@lloydzhou lloydzhou requested review from lloydzhou and removed request for lloydzhou November 7, 2024 08:58
@lloydzhou lloydzhou requested a review from ElricLiu November 7, 2024 09:23
@lloydzhou lloydzhou requested review from ElricLiu and removed request for ElricLiu November 7, 2024 09:41
@Dogtiti Dogtiti marked this pull request as ready for review November 7, 2024 13:34
@Dogtiti Dogtiti requested review from lloydzhou and removed request for lloydzhou and ElricLiu November 7, 2024 13:36
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: 15

🧹 Outside diff range and nitpick comments (26)
public/audio-processor.js (2)

1-1: Consider enabling TypeScript type checking.

The @ts-nocheck directive disables all TypeScript checks for this file. Given that this is handling critical audio processing functionality, type safety could help prevent runtime errors.

Consider removing the directive and properly typing the code:

- // @ts-nocheck
+ /// <reference lib="webworker" />
+ declare const AudioWorkletProcessor: {
+   prototype: AudioWorkletProcessor;
+   new (): AudioWorkletProcessor;
+ };
+ declare function registerProcessor(
+   name: string,
+   processorCtor: typeof AudioWorkletProcessor
+ ): void;

48-48: Consider adding processor name as a constant.

The processor name is hardcoded in the registration call, which could lead to inconsistencies if referenced elsewhere.

Consider this improvement:

+const PROCESSOR_NAME = "audio-recorder-processor";
-registerProcessor("audio-recorder-processor", AudioRecorderProcessor);
+registerProcessor(PROCESSOR_NAME, AudioRecorderProcessor);
package.json (1)

55-56: Architectural decision documentation needed.

The introduction of Azure's real-time audio SDK represents a significant architectural decision that requires clarification:

  1. Why was this specific SDK chosen?
  2. What are the Azure service requirements and associated costs?
  3. Are there any runtime dependencies or configuration needs?

Please update the PR description to include:

  1. Architectural decision record (ADR)
  2. Azure configuration requirements
  3. Local development setup instructions
  4. Cost implications, if any
app/components/realtime-chat/realtime-config.tsx (4)

10-14: Consider improving constants management.

The hardcoded arrays for providers, models, and voices should be moved to a dedicated constants file for better maintainability. Additionally, the model name suggests a date-based version which might need periodic updates.

Consider:

  1. Moving these constants to app/constant.ts
  2. Adding a mechanism to fetch available models dynamically if the API supports it
  3. Creating an enum or type for the voice options

16-19: Add component documentation and explicit return type.

The component interface would benefit from JSDoc documentation explaining the purpose and usage of the component, along with an explicit return type.

+/**
+ * Configuration component for real-time chat settings
+ * @param props.realtimeConfig - Current real-time configuration
+ * @param props.updateConfig - Callback to update configuration
+ */
 export function RealtimeConfigList(props: {
   realtimeConfig: RealtimeConfig;
   updateConfig: (updater: (config: RealtimeConfig) => void) => void;
-}) {
+}): JSX.Element {

62-70: Improve accessibility for the enable/disable checkbox.

The checkbox lacks proper ARIA labels for accessibility.

 <input
   type="checkbox"
+  aria-label={Locale.Settings.Realtime.Enable.Title}
   checked={props.realtimeConfig.enable}
   onChange={(e) =>
     props.updateConfig((config) => {
-      config.enable = e.currentTarget.checked
+      return { ...config, enable: e.currentTarget.checked };
     })
   }
 />
🧰 Tools
🪛 Biome

[error] 67-67: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


1-173: Consider implementing a configuration validation layer.

The component would benefit from a dedicated configuration validation layer that ensures all settings are valid before they're applied. This could be implemented using a schema validation library like Zod or Yup.

Key recommendations:

  1. Create a validation schema for the configuration
  2. Add error states and user feedback for invalid inputs
  3. Implement a configuration service to handle updates and validations
  4. Consider adding a "Test Configuration" button to verify settings before saving

Would you like assistance in implementing any of these improvements?

🧰 Tools
🪛 Biome

[error] 33-33: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 48-48: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 67-67: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 85-85: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 104-104: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 126-126: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 140-140: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 164-164: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/store/config.ts (1)

104-104: Review high temperature setting.

The temperature value of 0.9 is quite high for real-time chat. High temperature values can lead to more creative but potentially inconsistent responses.

Consider lowering the temperature to around 0.5-0.7 for a better balance between creativity and consistency in real-time conversations.

app/components/chat.module.scss (3)

452-469: Refactor audio message styling for better maintainability.

Consider these improvements:

  1. Extract hardcoded height to a CSS variable
  2. Translate Chinese comment to English
  3. Consider extending .chat-message-item to reduce property duplication
 .chat-message-audio {
   display: flex;
   align-items: center;
   justify-content: space-between;
-  border-radius: 10px;
-  background-color: rgba(0, 0, 0, 0.05);
-  border: var(--border-in-light);
-  position: relative;
-  transition: all ease 0.3s;
-  margin-top: 10px;
-  font-size: 14px;
-  user-select: text;
-  word-break: break-word;
-  box-sizing: border-box;
+  @extend .chat-message-item;
   audio {
-    height: 30px; /* 调整高度 */
+    height: var(--audio-height, 30px); /* Adjust height */
   }
 }

500-506: Improve readability of image width calculations.

The calculations are correct but could be more maintainable. Consider extracting common calculations into CSS custom properties.

 @media only screen and (max-width: 600px) {
+  --base-mobile-width: calc(100vw / 3 * 2);
-  $calc-image-width: calc(100vw / 3 * 2 / var(--image-count));
+  $calc-image-width: calc(var(--base-mobile-width) / var(--image-count));

   .chat-message-item-image-multi {
     width: $calc-image-width;
     height: $calc-image-width;
   }

   .chat-message-item-image {
-    max-width: calc(100vw / 3 * 2);
+    max-width: var(--base-mobile-width);
   }
 }

 @media screen and (min-width: 600px) {
+  --content-width: calc(var(--window-width) - var(--sidebar-width));
+  --max-content-width: calc(1200px - var(--sidebar-width));
+  --base-width: calc(var(--content-width) / 3 * 2);
+  --max-width: calc(var(--max-content-width) / 3 * 2);

-  $max-image-width: calc(
-    calc(1200px - var(--sidebar-width)) / 3 * 2 / var(--image-count)
-  );
+  $max-image-width: calc(var(--max-width) / var(--image-count));
-  $image-width: calc(
-    calc(var(--window-width) - var(--sidebar-width)) / 3 * 2 /
-      var(--image-count)
-  );
+  $image-width: calc(var(--base-width) / var(--image-count));

Also applies to: 508-508, 513-519, 529-529


658-659: Remove empty focus state block.

This empty block serves no purpose and should be removed.

-.chat-input:focus {
-}
app/locales/cn.ts (1)

565-597: LGTM with suggestions for reducing redundancy.

The new Realtime section follows consistent translation patterns and structure. However, consider consolidating redundant settings:

  1. Provider and Model settings might overlap with existing global settings
  2. Temperature setting might be redundant with the global temperature setting

Consider refactoring to reuse existing settings:

 Realtime: {
   Enable: {
     Title: "实时聊天",
     SubTitle: "开启实时聊天功能",
   },
-  Provider: {
-    Title: "模型服务商",
-    SubTitle: "切换不同的服务商",
-  },
-  Model: {
-    Title: "模型",
-    SubTitle: "选择一个模型",
-  },
   ApiKey: {
     Title: "API Key",
     SubTitle: "API Key",
     Placeholder: "API Key",
   },
   Azure: {
     Endpoint: {
       Title: "接口地址",
       SubTitle: "接口地址",
     },
     Deployment: {
       Title: "部署名称",
       SubTitle: "部署名称",
     },
   },
-  Temperature: {
-    Title: "随机性 (temperature)",
-    SubTitle: "值越大,回复越随机",
-  },
 },
app/locales/en.ts (3)

573-605: LGTM! Well-structured localization for real-time chat settings.

The new Realtime section follows consistent patterns and provides comprehensive coverage of real-time chat configuration options.

Consider enhancing the following descriptions for better clarity:

       Azure: {
         Endpoint: {
           Title: "Endpoint",
-          SubTitle: "Endpoint",
+          SubTitle: "Azure endpoint URL for real-time chat service",
         },
         Deployment: {
           Title: "Deployment Name",
-          SubTitle: "Deployment Name",
+          SubTitle: "Name of your Azure OpenAI deployment for real-time models",
         },
       },
       Temperature: {
         Title: "Randomness (temperature)",
-        SubTitle: "Higher values result in more random responses",
+        SubTitle: "Controls response creativity (0.0 = focused, 1.0 = creative)",
       },

574-577: Consider adding more context about real-time chat capabilities.

The enable section could provide more information about what real-time chat offers to users.

     Enable: {
       Title: "Realtime Chat",
-      SubTitle: "Enable realtime chat feature",
+      SubTitle: "Enable real-time voice chat and streaming responses",
     },

586-590: Add API key format guidance.

The API key placeholder could include format hints to help users.

     ApiKey: {
       Title: "API Key",
       SubTitle: "API Key",
-      Placeholder: "API Key",
+      Placeholder: "Enter your provider-specific API key (e.g., sk-...)",
     },
app/components/settings.tsx (1)

1806-1812: Refactor the update logic for better readability.

The nested function calls with assignment in expression could be simplified for better maintainability.

Consider this refactoring:

-            updateConfig={(updater) => {
-              const realtimeConfig = { ...config.realtimeConfig };
-              updater(realtimeConfig);
-              config.update(
-                (config) => (config.realtimeConfig = realtimeConfig),
-              );
-            }}
+            updateConfig={(updater) => {
+              const realtimeConfig = { ...config.realtimeConfig };
+              updater(realtimeConfig);
+              config.update((config) => {
+                config.realtimeConfig = realtimeConfig;
+                return config;
+              });
+            }}

This change:

  1. Makes the assignment more explicit
  2. Clearly shows the return value
  3. Improves code readability
🧰 Tools
🪛 Biome

[error] 1810-1810: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/components/chat.tsx (4)

466-466: Consider improving the real-time chat button implementation.

While the implementation is functional, consider these improvements:

  1. The "Realtime Chat" text is hardcoded - should use the localization system like other buttons
  2. The button's visibility depends on config.realtimeConfig.enable but there's no fallback UI or message when disabled
- text={"Realtime Chat"}
+ text={Locale.Chat.Actions.RealtimeChat}

Also applies to: 796-802


2045-2047: Implement voice chat functionality.

The onStartVoice callback is currently just logging to console. This appears to be incomplete implementation of the voice chat feature.

Would you like me to help implement the voice chat functionality or create a GitHub issue to track this?


594-602: Simplify unnecessary Fragment usage.

The Fragment wrapper is redundant since it contains only conditional rendering.

- <>
-   {theme === Theme.Auto ? (
-     <AutoIcon />
-   ) : theme === Theme.Light ? (
-     <LightIcon />
-   ) : theme === Theme.Dark ? (
-     <DarkIcon />
-   ) : null}
- </>
+ {theme === Theme.Auto ? (
+   <AutoIcon />
+ ) : theme === Theme.Light ? (
+   <LightIcon />
+ ) : theme === Theme.Dark ? (
+   <DarkIcon />
+ ) : null}
🧰 Tools
🪛 Biome

[error] 594-602: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)


1676-1678: Improve code clarity by avoiding assignment in expression.

The config update could be clearer by avoiding assignment within the expression.

- config.update(
-   (config) => (config.tightBorder = !config.tightBorder),
- );
+ config.update((config) => {
+   config.tightBorder = !config.tightBorder;
+   return config;
+ });
🧰 Tools
🪛 Biome

[error] 1676-1678: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/lib/audio.ts (3)

178-184: Release resources and handle errors in the close method

Ensure that all resources are properly released in the close method. Additionally, consider wrapping resource cleanup in try-catch blocks to handle any potential errors during disconnection or stopping of tracks.

Here's how you might update the close method:

 async close() {
   this.recordBuffer = [];
-  this.workletNode?.disconnect();
-  this.source?.disconnect();
-  this.stream?.getTracks().forEach((track) => track.stop());
+  try {
+    this.workletNode?.disconnect();
+    this.source?.disconnect();
+    this.stream?.getTracks().forEach((track) => track.stop());
+    await this.context.close();
+  } catch (error) {
+    console.error("Error closing audio resources:", error);
+  }
 }

65-65: Handle potential race conditions when starting recording

Before sending the START_RECORDING command at line 65, ensure that the AudioWorkletNode is fully initialized to prevent any race conditions that could cause the recording to fail.

Consider adding an event listener or a readiness check before starting the recording:

 await this.workletNode.port.postMessage({ command: "START_RECORDING" });
+// Optionally, wait for confirmation from the worker if necessary

28-36: Improve error handling for user media permissions

When requesting microphone access, consider handling cases where the user denies permission or where the media devices are not available. Provide user-friendly error messages to enhance the user experience.

Enhance the error handling in the try-catch block:

 try {
   this.stream = await navigator.mediaDevices.getUserMedia({
     audio: {
       channelCount: 1,
       sampleRate: this.sampleRate,
       echoCancellation: true,
       noiseSuppression: true,
     },
   });
 } catch (error) {
   console.error("Error accessing microphone:", error);
+  // Provide a user-friendly message or handle the error appropriately
   throw error;
 }
app/components/realtime-chat/realtime-chat.tsx (3)

179-180: Clarify the use of concat() without arguments

Using session.messages = session.messages.concat(); creates a shallow copy of the array, which may be intended to trigger state updates due to reference changes. However, this usage can be unclear to future maintainers.

Consider adding a comment to explain the intention or use a more explicit method:

- session.messages = session.messages.concat();
+ // Trigger state update by cloning the array
+ session.messages = [...session.messages];

This enhances readability and makes the purpose of the code clearer.

Also applies to: 188-189, 215-216


89-100: Avoid using type assertions with any

Type assertions to any are used in the following code:

role: role as any,
type: (role === "assistant" ? "text" : "input_text") as any,

Define appropriate types or interfaces instead of using any to enhance type safety and catch potential bugs at compile time.


295-326: Enhance accessibility for recording status indicator

The visual pulsating effect indicating the recording status might not be sufficient for users with visual impairments.

Consider adding ARIA attributes or alternative text to improve accessibility:

<div
  className={clsx(styles["circle-mic"], {
    [styles["pulse"]]: isRecording,
  })}
+ aria-live="polite"
+ aria-label={isRecording ? "Recording in progress" : "Not recording"}
>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between fbb9385 and 48a1e8a.

⛔ Files ignored due to path filters (6)
  • app/icons/close-24.svg is excluded by !**/*.svg
  • app/icons/headphone.svg is excluded by !**/*.svg
  • app/icons/power.svg is excluded by !**/*.svg
  • app/icons/voice-off.svg is excluded by !**/*.svg
  • app/icons/voice.svg is excluded by !**/*.svg
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (15)
  • app/components/chat.module.scss (9 hunks)
  • app/components/chat.tsx (6 hunks)
  • app/components/realtime-chat/index.ts (1 hunks)
  • app/components/realtime-chat/realtime-chat.module.scss (1 hunks)
  • app/components/realtime-chat/realtime-chat.tsx (1 hunks)
  • app/components/realtime-chat/realtime-config.tsx (1 hunks)
  • app/components/settings.tsx (2 hunks)
  • app/lib/audio.ts (1 hunks)
  • app/locales/cn.ts (1 hunks)
  • app/locales/en.ts (1 hunks)
  • app/store/chat.ts (1 hunks)
  • app/store/config.ts (2 hunks)
  • app/utils/chat.ts (1 hunks)
  • package.json (1 hunks)
  • public/audio-processor.js (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • app/components/realtime-chat/index.ts
  • app/components/realtime-chat/realtime-chat.module.scss
  • app/utils/chat.ts
🧰 Additional context used
🪛 Biome
app/components/chat.tsx

[error] 594-602: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment

(lint/complexity/noUselessFragments)


[error] 1676-1678: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/components/realtime-chat/realtime-config.tsx

[error] 33-33: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 48-48: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 67-67: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 85-85: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 104-104: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 126-126: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 140-140: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 164-164: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/components/settings.tsx

[error] 1810-1810: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

🪛 eslint
app/components/chat.tsx

[error] 1597-1597: React Hook "useState" is called in function "_Chat" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".

(react-hooks/rules-of-hooks)

🔇 Additional comments (20)
package.json (1)

55-56: Verify the impact on the build process.

The addition of a binary dependency might affect the build process across different platforms.

✅ Verification successful

Let me gather more information about the build configuration and dependencies.


Let me check the Next.js configuration since we found next.config.mjs in the repository.


Let me check how the rt-client is being used in the codebase to understand its integration.


No build process modifications needed for the rt-client dependency

After analyzing the codebase, I can confirm that:

  1. The rt-client package is properly integrated as a client-side dependency and used in the realtime chat feature
  2. The Next.js build configuration already handles external dependencies appropriately
  3. The package is consumed as a standard npm package and doesn't require any special platform-specific build configurations
  4. The build process (both for web and standalone modes) has proper webpack configuration to handle external dependencies

The binary dependency is properly packaged and distributed through GitHub releases, and the current build setup is sufficient to handle it without any modifications.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential build script modifications needed for the new dependency

# Test 1: Check if build scripts handle binary dependencies
rg -l "\.tgz" "package.json" "next.config.*" "webpack.config.*"

# Test 2: Check for platform-specific handling
rg -l "process.platform" "next.config.*" "webpack.config.*"

# Test 3: Verify if there are any existing binary dependencies
rg '"resolved":.*\.tgz' package-lock.json || echo "No package-lock.json found"

Length of output: 523


Script:

#!/bin/bash
# Check for build-related configurations and dependencies

# Look for any build configuration files
fd -e config.js -e config.mjs -e config.cjs

# Check package.json for build scripts and dependencies
cat package.json

# Look for any platform-specific code in the src directory
rg -l "process.platform" 

Length of output: 3266


Script:

#!/bin/bash
# Check Next.js configuration and build setup

# Check next.config.mjs content
cat next.config.mjs

# Look for any build-related files in the project
fd -t f -e ts -e js -e mjs build

# Check if rt-client is used in the codebase
rg -l "rt-client"

Length of output: 2928


Script:

#!/bin/bash
# Check rt-client usage and build configuration

# Check the content of files using rt-client
cat app/store/config.ts
cat app/components/realtime-chat/realtime-config.tsx
cat app/components/realtime-chat/realtime-chat.tsx

# Check build-related files
cat app/config/build.ts
cat app/masks/build.ts

Length of output: 24041

app/components/realtime-chat/realtime-config.tsx (1)

155-167: 🛠️ Refactor suggestion

Review temperature range constraints and improve state updates.

The temperature range is arbitrarily limited to 0.6-1.0. Additionally, the state updates should follow React's immutability principles.

 <InputRange
   aria={Locale.Settings.Temperature.Title}
   value={props.realtimeConfig?.temperature?.toFixed(1)}
-  min="0.6"
-  max="1"
+  min="0"
+  max="2"
   step="0.1"
   onChange={(e) => {
     props.updateConfig((config) => {
-      config.temperature = e.currentTarget.valueAsNumber
+      return { ...config, temperature: e.currentTarget.valueAsNumber };
     });
   }}
 />
🧰 Tools
🪛 Biome

[error] 164-164: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/store/config.ts (4)

18-18: LGTM: Clean type import.

The type-only import from rt-client is properly defined and follows TypeScript best practices.


113-113: LGTM: Clean type export.

The RealtimeConfig type export follows the established pattern and maintains consistency with other config type exports.


98-99: 🛠️ Refactor suggestion

Review hardcoded model name with future date.

The model name gpt-4o-realtime-preview-2024-10-01 contains a future date. This could cause issues if the model naming convention changes or if the model becomes unavailable.

Let's check if this model name is used consistently:

Consider:

  1. Moving model names to a constants file
  2. Using a more stable model identifier without dates

105-105: 🛠️ Refactor suggestion

Verify voice type compatibility.

The voice value "alloy" is being type cast to Voice without validation.

Let's check the available voice options:

Consider:

  1. Adding runtime validation for voice values
  2. Creating a const enum or union type for valid voices
  3. Adding a validator similar to existing TTSConfigValidator
app/components/chat.module.scss (4)

48-55: LGTM! Well-structured flexbox layout.

The changes improve the spacing and alignment of chat input actions using flexbox best practices.


238-243: LGTM! Improved gradient definition readability.

The gradient definition is now more maintainable with proper formatting.


382-382: LGTM! Proper user message alignment.

Correctly aligns user messages to the end of the container, following standard chat UI patterns.


728-753: LGTM! Well-implemented chat layout with animated side panel.

The implementation follows best practices:

  • Proper use of flexbox for layout
  • Hardware-accelerated transform for smooth animations
  • Correct z-index management
app/store/chat.ts (1)

55-55: Consider enhancing audio message integration.

While the type addition is good, consider these architectural improvements:

  1. Add audio-specific message handlers in the chat store
  2. Implement audio message cleanup in clearAllData
  3. Consider audio message persistence strategy
  4. Add audio status tracking (playing, paused, etc.)

Let's verify the audio integration points:

app/components/settings.tsx (2)

88-88: LGTM!

The import statement follows React conventions and maintains proper module organization.


1803-1814: LGTM! Component integration is consistent.

The RealtimeConfigList component is well-integrated following the same patterns as other config components in the file.

🧰 Tools
🪛 Biome

[error] 1810-1810: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

app/components/chat.tsx (2)

49-49: LGTM: New imports for real-time chat functionality.

The additions of HeadphoneIcon and RealtimeChat imports align well with the PR's objective of implementing real-time chat features.

Also applies to: 124-124


2034-2051: LGTM: Well-structured side panel implementation.

The side panel implementation is well done with:

  • Proper mobile responsiveness using conditional classes
  • Clean integration with the existing chat layout
  • Proper panel show/hide functionality
app/lib/audio.ts (4)

51-51: Verify the conversion from Float32 to Int16

The conversion logic at line 51 might introduce quantization errors. Ensure that the conversion from Float32Array to Int16Array accurately represents the audio data without distortion.

Consider reviewing the conversion formula or using built-in functions for audio data conversion. If necessary, perform tests to confirm the audio quality remains intact after conversion.


104-104: Confirm the Int16 to Float32 conversion logic

At line 104, the conversion from Int16Array to Float32Array divides by either 0x8000 or 0x7fff. Verify that this correctly maps the full range of Int16 values to the normalized Float32 range between -1 and 1.

You may want to use a consistent divisor, such as 32768, for both positive and negative values to ensure linear mapping. Testing with known values can help confirm the accuracy of the conversion.


19-19: Verify the path to the audio worklet module

Ensure that the audio worklet module at "/audio-processor.js" is correctly located and accessible. An incorrect path can lead to failure in loading the module, causing the recording functionality to break.

You can run the following script to check if the file exists:

✅ Verification successful

Audio worklet module path and implementation verified

The audio worklet module exists at the correct location and contains a valid AudioWorkletProcessor implementation. The path /audio-processor.js is correct as it will be served from the public directory, and the file contains the necessary processor registration with the name "audio-recorder-processor".

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that /audio-processor.js exists in the public directory.

# Test: Check for the existence of the file. Expect: File should be found.
if [[ -f "public/audio-processor.js" ]]; then
  echo "audio-processor.js exists in the public directory."
else
  echo "audio-processor.js not found in the public directory."
fi

Length of output: 147


Script:

#!/bin/bash
# Let's verify the content of the audio processor file to ensure it's a valid worklet
cat public/audio-processor.js

# Also check if there are any other references to this file in the codebase
rg -l "audio-processor.js"

Length of output: 1245


175-175: ⚠️ Potential issue

Ensure correct data concatenation when creating Int16Array

When slicing recordBuffer, which is an array of numbers, it's important to create a new Int16Array from the sliced data correctly. If recordBuffer remains as an array of number[] after previous fixes, you can directly create an Int16Array from the sliced portion.

Update the code to correctly create the Int16Array:

-return new Int16Array(this.recordBuffer.slice(startIndex, endIndex)),
+return new Int16Array(this.recordBuffer.slice(startIndex, endIndex));

Likely invalid or redundant comment.

app/components/realtime-chat/realtime-chat.tsx (1)

252-277: Handle unmounting of component with proper cleanup

In the useEffect hook, the cleanup function should ensure that any ongoing processes are properly terminated to prevent memory leaks or unexpected behavior.

Please verify that all event listeners, streams, and connections are appropriately closed when the component unmounts. Consider adding any necessary cleanup code.

public/audio-processor.js Show resolved Hide resolved
public/audio-processor.js Show resolved Hide resolved
public/audio-processor.js Show resolved Hide resolved
public/audio-processor.js Show resolved Hide resolved
package.json Show resolved Hide resolved
app/lib/audio.ts Show resolved Hide resolved
app/lib/audio.ts Show resolved Hide resolved
app/lib/audio.ts Show resolved Hide resolved
app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
@lloydzhou lloydzhou requested a review from ElricLiu November 8, 2024 05:25
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: 7

🧹 Outside diff range and nitpick comments (5)
app/lib/audio.ts (2)

1-24: Consider improving audio quality and encapsulation

A few suggestions for the initialization:

  1. The sample rate of 24000Hz might be too low for high-quality audio. Consider using 44100Hz or 48000Hz for better quality.
  2. The analyser property should be private with a public getter method to maintain encapsulation.
  3. Add error handling in the constructor for AudioContext initialization.
-  public analyser: AnalyserNode;
+  private analyser: AnalyserNode;
+  
+  public getAnalyser(): AnalyserNode {
+    return this.analyser;
+  }

193-199: Ensure complete cleanup of all resources

The close method should reset all state variables, including playback-related properties.

 async close() {
   this.recordBuffer = [];
+  this.playBuffer = [];
+  this.playbackQueue = [];
+  this.isPlaying = false;
+  this.nextPlayTime = 0;
   this.workletNode?.disconnect();
   this.source?.disconnect();
   this.stream?.getTracks().forEach((track) => track.stop());
   await this.context.close();
 }
app/components/realtime-chat/realtime-chat.tsx (3)

24-28: Add JSDoc documentation for the RealtimeChatProps interface.

Adding documentation will help other developers understand the purpose of each callback prop and when they are triggered.

+/**
+ * Props for the RealtimeChat component
+ * @property {() => void} onClose - Callback triggered when the chat is closed
+ * @property {() => void} onStartVoice - Callback triggered when voice recording starts
+ * @property {() => void} onPausedVoice - Callback triggered when voice recording is paused
+ */
 interface RealtimeChatProps {
   onClose?: () => void;
   onStartVoice?: () => void;
   onPausedVoice?: () => void;
 }

42-43: Consider making modality and VAD configurable via props or app config.

The modality and VAD settings are currently hardcoded. Consider making these configurable to improve component flexibility.

-  const [modality, setModality] = useState("audio");
-  const [useVAD, setUseVAD] = useState(true);
+  const [modality, setModality] = useState(config.realtimeConfig.modality ?? "audio");
+  const [useVAD, setUseVAD] = useState(config.realtimeConfig.useVAD ?? true);

156-158: Optimize message updates to reduce unnecessary renders.

The current implementation calls concat() multiple times to trigger updates. Consider batching these updates to improve performance.

-chatStore.updateTargetSession(session, (session) => {
-  session.messages = session.messages.concat();
-});
+// Batch updates using a single state update
+const batchedUpdate = (updates: Partial<typeof botMessage>) => {
+  Object.assign(botMessage, updates);
+  chatStore.updateTargetSession(session, (session) => {
+    session.messages = [...session.messages];
+  });
+};

Also applies to: 179-181, 188-190

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 48a1e8a and 50e6310.

📒 Files selected for processing (2)
  • app/components/realtime-chat/realtime-chat.tsx (1 hunks)
  • app/lib/audio.ts (1 hunks)
🔇 Additional comments (1)
app/lib/audio.ts (1)

1-200: Verify proper cleanup of sensitive audio data

Let's ensure that all instances of AudioHandler properly clean up sensitive audio data when the component unmounts.

✅ Verification successful

AudioHandler cleanup is properly implemented

The code review concern about cleanup of sensitive audio data is addressed. The implementation shows:

  1. Proper cleanup in the component unmount:

    • Stops recording if active
    • Calls close() on the AudioHandler instance which:
      • Clears record buffers
      • Disconnects audio nodes
      • Stops media tracks
      • Closes AudioContext
  2. The AudioHandler class itself implements comprehensive cleanup in its close() method:

    • Clears sensitive data from recordBuffer
    • Properly disconnects and stops all audio processing
    • Closes the AudioContext
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for AudioHandler usage to verify cleanup
echo "Checking AudioHandler cleanup in components..."
rg -A 10 "new AudioHandler" | rg -B 10 "useEffect.*return.*close"

# Check for proper error handling of getUserMedia
echo "Checking getUserMedia error handling..."
rg "getUserMedia.*catch"

Length of output: 286


Script:

#!/bin/bash
# Let's try a different approach to find AudioHandler usage and cleanup patterns

# Find files containing AudioHandler
echo "Files containing AudioHandler:"
rg -l "AudioHandler"

# Look for cleanup patterns in React components
echo "\nChecking component cleanup patterns:"
rg -A 5 "useEffect.*audio" 

# Check error handling in audio.ts
echo "\nChecking error handling in audio.ts:"
rg -A 5 "catch|throw" app/lib/audio.ts

# Look for any audio-related cleanup
echo "\nChecking audio cleanup patterns:"
rg -A 5 "\.close\(\)|cleanup|dispose" 

Length of output: 2663


Script:

#!/bin/bash
# Let's examine the AudioHandler usage in realtime-chat.tsx to verify cleanup
echo "Checking AudioHandler usage in realtime-chat.tsx:"
rg -B 10 -A 10 "AudioHandler" app/components/realtime-chat/realtime-chat.tsx

# Look for useEffect cleanup in realtime-chat.tsx
echo "\nChecking cleanup in useEffect:"
rg -B 5 -A 10 "useEffect.*audio.*return" app/components/realtime-chat/realtime-chat.tsx

Length of output: 3226


Script:

#!/bin/bash
# Let's check for cleanup in component unmount
echo "Checking component unmount cleanup:"
rg -B 15 -A 5 "return \(\) =>" app/components/realtime-chat/realtime-chat.tsx

# Check for any other useEffect cleanup patterns
echo "\nChecking other cleanup patterns:"
rg -B 5 -A 15 "useEffect.*{.*return" app/components/realtime-chat/realtime-chat.tsx

Length of output: 1029

app/lib/audio.ts Show resolved Hide resolved
app/lib/audio.ts Show resolved Hide resolved
app/lib/audio.ts Show resolved Hide resolved
app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
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: 4

🧹 Outside diff range and nitpick comments (3)
app/components/realtime-chat/realtime-chat.tsx (3)

24-28: Add JSDoc documentation for the RealtimeChatProps interface.

Adding documentation will improve code maintainability and help other developers understand the purpose of each callback prop.

+/**
+ * Props for the RealtimeChat component
+ * @interface RealtimeChatProps
+ * @property {() => void} [onClose] - Callback fired when the chat is closed
+ * @property {() => void} [onStartVoice] - Callback fired when voice recording starts
+ * @property {() => void} [onPausedVoice] - Callback fired when voice recording is paused
+ */
 interface RealtimeChatProps {
   onClose?: () => void;
   onStartVoice?: () => void;
   onPausedVoice?: () => void;
 }

42-43: Add type safety for modality and VAD states.

The modality state should be constrained to specific values for type safety.

-  const [modality, setModality] = useState("audio");
+  type ModalityType = "audio" | "text";
+  const [modality, setModality] = useState<ModalityType>("audio");
   const [useVAD, setUseVAD] = useState(true);

228-255: Improve recording state management with a state machine approach.

The recording state transitions could be more robust using a state machine pattern.

+type RecordingState = 'idle' | 'recording' | 'processing';
+const [recordingState, setRecordingState] = useState<RecordingState>('idle');

 const toggleRecording = async () => {
-  if (!isRecording && clientRef.current) {
+  if (recordingState === 'idle' && clientRef.current) {
     try {
+      setRecordingState('recording');
       if (!audioHandlerRef.current) {
         audioHandlerRef.current = new AudioHandler();
         await audioHandlerRef.current.initialize();
       }
       await audioHandlerRef.current.startRecording(async (chunk) => {
         await clientRef.current?.sendAudio(chunk);
       });
-      setIsRecording(true);
+      onStartVoice?.();
     } catch (error) {
       console.error("Failed to start recording:", error);
+      setRecordingState('idle');
     }
-  } else if (audioHandlerRef.current) {
+  } else if (recordingState === 'recording' && audioHandlerRef.current) {
     try {
+      setRecordingState('processing');
       audioHandlerRef.current.stopRecording();
       if (!useVAD) {
         const inputAudio = await clientRef.current?.commitAudio();
         await handleInputAudio(inputAudio!);
         await clientRef.current?.generateResponse();
       }
-      setIsRecording(false);
+      setRecordingState('idle');
+      onPausedVoice?.();
     } catch (error) {
       console.error("Failed to stop recording:", error);
+      setRecordingState('idle');
     }
   }
 };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 50e6310 and a4c9eaf.

📒 Files selected for processing (1)
  • app/components/realtime-chat/realtime-chat.tsx (1 hunks)

app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
@lloydzhou lloydzhou requested review from ElricLiu and removed request for ElricLiu November 8, 2024 05:56
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: 4

🧹 Outside diff range and nitpick comments (5)
app/components/realtime-chat/realtime-chat.tsx (4)

43-44: Consider making modality and VAD configurable.

The modality and useVAD states are hardcoded without user configuration options. Consider:

  1. Adding these options to the realtime configuration store
  2. Exposing controls in the UI to toggle these settings
- const [modality, setModality] = useState("audio");
- const [useVAD, setUseVAD] = useState(true);
+ const [modality, setModality] = useState(config.realtimeConfig.modality ?? "audio");
+ const [useVAD, setUseVAD] = useState(config.realtimeConfig.useVAD ?? true);

105-105: Remove or implement commented code.

The commented out generateResponse() call suggests incomplete implementation. Either remove it or implement the necessary functionality.


260-260: Use English comments for consistency.

Replace the Chinese comment with its English translation for better maintainability.

- // 防止重复初始化
+ // Prevent duplicate initialization

286-308: Optimize effect dependencies.

The effect managing animation frames could be split into smaller effects to prevent unnecessary re-renders.

Consider separating the frequency data polling into its own custom hook:

function useFrequencyData(audioHandler: AudioHandler | null, isActive: boolean) {
  const [frequencies, setFrequencies] = useState<Uint8Array>();

  useEffect(() => {
    if (!isActive || !audioHandler) {
      setFrequencies(undefined);
      return;
    }

    const animationFrame = () => {
      const freqData = audioHandler.getByteFrequencyData();
      setFrequencies(freqData);
      return requestAnimationFrame(animationFrame);
    };

    const animationFrameId = requestAnimationFrame(animationFrame);
    return () => cancelAnimationFrame(animationFrameId);
  }, [audioHandler, isActive]);

  return frequencies;
}
app/components/voice-print/voice-print.tsx (1)

12-12: Standardize code comments to English for consistency

The comment // 保存10帧历史数据 is in Chinese. For consistency and maintainability, consider translating all comments into English.

Apply this change:

 const historyLengthRef = useRef(10); 
-// 保存10帧历史数据
+// Save 10 frames of historical data
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a4c9eaf and 8b4ca13.

📒 Files selected for processing (5)
  • app/components/realtime-chat/realtime-chat.module.scss (1 hunks)
  • app/components/realtime-chat/realtime-chat.tsx (1 hunks)
  • app/components/voice-print/index.ts (1 hunks)
  • app/components/voice-print/voice-print.module.scss (1 hunks)
  • app/components/voice-print/voice-print.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • app/components/voice-print/index.ts
  • app/components/voice-print/voice-print.module.scss
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/realtime-chat/realtime-chat.module.scss

app/components/realtime-chat/realtime-chat.tsx Outdated Show resolved Hide resolved
app/components/voice-print/voice-print.tsx Outdated Show resolved Hide resolved
app/components/voice-print/voice-print.tsx Outdated Show resolved Hide resolved
app/components/voice-print/voice-print.tsx Outdated Show resolved Hide resolved
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: 1

🧹 Outside diff range and nitpick comments (3)
app/components/realtime-chat/realtime-chat.tsx (3)

25-29: Add JSDoc documentation for the component props.

Adding detailed documentation for the props would improve maintainability and developer experience.

+/**
+ * Props for the RealtimeChat component
+ * @interface RealtimeChatProps
+ * @property {() => void} [onClose] - Callback fired when the chat is closed
+ * @property {() => void} [onStartVoice] - Callback fired when voice recording starts
+ * @property {() => void} [onPausedVoice] - Callback fired when voice recording is paused
+ */
 interface RealtimeChatProps {
   onClose?: () => void;
   onStartVoice?: () => void;
   onPausedVoice?: () => void;
 }

88-109: Address the TODO comment for message history.

The commented-out code for handling message history needs to be completed or removed. If this functionality is required, it should be properly implemented with appropriate error handling.

Would you like me to help implement the message history functionality or create a GitHub issue to track this task?


337-356: Add loading states for better user feedback.

The UI should indicate when operations are in progress, such as connecting or processing audio.

 <div className={styles["bottom-icons"]}>
   <div>
     <IconButton
       icon={isRecording ? <VoiceIcon /> : <VoiceOffIcon />}
       onClick={toggleRecording}
-      disabled={!isConnected}
+      disabled={!isConnected || isConnecting}
+      loading={isConnecting}
       shadow
       bordered
     />
   </div>
-  <div className={styles["icon-center"]}>{status}</div>
+  <div className={styles["icon-center"]}>
+    {isConnecting ? (
+      <span className={styles["loading-text"]}>Connecting...</span>
+    ) : (
+      status
+    )}
+  </div>
   <div>
     <IconButton
       icon={<PowerIcon />}
       onClick={handleClose}
+      disabled={isConnecting}
       shadow
       bordered
     />
   </div>
 </div>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d33e772 and 4629b39.

📒 Files selected for processing (1)
  • app/components/realtime-chat/realtime-chat.tsx (1 hunks)

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: 0

🧹 Outside diff range and nitpick comments (1)
README.md (1)

138-139: Fix list item indentation.

The list items have inconsistent indentation. Please adjust the indentation to match the surrounding list items.

-- [x] 支持 Realtime Chat [#5672](https://github.com/ChatGPTNextWeb/ChatGPT-Next-Web/issues/5672)
-- [ ] 本地知识库
+- [x] 支持 Realtime Chat [#5672](https://github.com/ChatGPTNextWeb/ChatGPT-Next-Web/issues/5672)
+- [ ] 本地知识库
🧰 Tools
🪛 Markdownlint

138-138: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level

(MD005, list-indent)


139-139: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level

(MD005, list-indent)


138-138: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


139-139: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4629b39 and 14f444e.

📒 Files selected for processing (1)
  • README.md (2 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md

138-138: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level

(MD005, list-indent)


139-139: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level

(MD005, list-indent)


138-138: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


139-139: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)

🔇 Additional comments (1)
README.md (1)

99-99: LGTM! Documentation updates for real-time chat feature.

The changes consistently document the new real-time chat feature in both English and Chinese sections, with proper issue references (#5672) for traceability.

Also applies to: 103-103, 138-138, 142-142

app/icons/close-24.svg Outdated Show resolved Hide resolved
@lloydzhou lloydzhou merged commit 289aeec into ChatGPTNextWeb:main Nov 11, 2024
1 check failed
frostime added a commit to frostime/ChatGPT-Next-Web that referenced this pull request Nov 12, 2024
commit 38fa305
Author: lloydzhou <[email protected]>
Date:   Mon Nov 11 13:26:08 2024 +0800

    update version

commit 289aeec
Merge: f8f6954 7d71da9
Author: Lloyd Zhou <[email protected]>
Date:   Mon Nov 11 13:19:26 2024 +0800

    Merge pull request ChatGPTNextWeb#5786 from ConnectAI-E/feature/realtime-chat

    Feature/realtime chat

commit 7d71da9
Author: lloydzhou <[email protected]>
Date:   Mon Nov 11 13:15:09 2024 +0800

    remove close-24 svg

commit f8f6954
Merge: 6e03f32 64aa760
Author: Lloyd Zhou <[email protected]>
Date:   Mon Nov 11 13:13:09 2024 +0800

    Merge pull request ChatGPTNextWeb#5779 from ConnectAI-E/feature/model/claude35haiku

    add claude35haiku & not support vision

commit 6e03f32
Merge: 108069a 18a6571
Author: Lloyd Zhou <[email protected]>
Date:   Mon Nov 11 13:10:00 2024 +0800

    Merge pull request ChatGPTNextWeb#5795 from JingSyue/main

    fix: built-in plugin dalle3 error ChatGPTNextWeb#5787

commit 18a6571
Author: JingSyue <[email protected]>
Date:   Mon Nov 11 12:59:29 2024 +0800

    Update proxy.ts

    Update proxy.ts

commit 14f444e
Author: Dogtiti <[email protected]>
Date:   Mon Nov 11 11:47:41 2024 +0800

    doc: realtime chat

commit 2b0f2e5
Author: JingSyue <[email protected]>
Date:   Sun Nov 10 10:28:25 2024 +0800

    fix: built-in plugin dalle3 error ChatGPTNextWeb#5787

commit 4629b39
Author: Dogtiti <[email protected]>
Date:   Sat Nov 9 16:22:01 2024 +0800

    chore: comment context history

commit d33e772
Author: Dogtiti <[email protected]>
Date:   Fri Nov 8 22:39:17 2024 +0800

    feat: voice print

commit 89136fb
Author: Dogtiti <[email protected]>
Date:   Fri Nov 8 22:18:39 2024 +0800

    feat: voice print

commit 8b4ca13
Author: Dogtiti <[email protected]>
Date:   Fri Nov 8 22:02:31 2024 +0800

    feat: voice print

commit a4c9eaf
Author: lloydzhou <[email protected]>
Date:   Fri Nov 8 13:43:13 2024 +0800

    do not save empty audio file

commit 50e6310
Author: lloydzhou <[email protected]>
Date:   Fri Nov 8 13:21:40 2024 +0800

    merge code and get analyser data

commit 48a1e8a
Author: Dogtiti <[email protected]>
Date:   Thu Nov 7 21:32:47 2024 +0800

    chore: i18n

commit e44ebe3
Author: Dogtiti <[email protected]>
Date:   Thu Nov 7 21:28:23 2024 +0800

    feat: realtime config

commit 108069a
Merge: fbb9385 d5bda29
Author: Lloyd Zhou <[email protected]>
Date:   Thu Nov 7 20:06:30 2024 +0800

    Merge pull request ChatGPTNextWeb#5788 from ConnectAI-E/fix-o1-maxtokens

    chore: o1模型使用max_completion_tokens

commit d5bda29
Author: DDMeaqua <[email protected]>
Date:   Thu Nov 7 19:45:27 2024 +0800

    chore: o1模型使用max_completion_tokens

commit 283caba
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 18:57:57 2024 +0800

    stop streaming play after get input audio.

commit b78e5db
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 17:55:51 2024 +0800

    add temperature config

commit 46c469b
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 17:47:55 2024 +0800

    add voice config

commit c00ebbe
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 17:40:03 2024 +0800

    update

commit c526ff8
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 17:23:20 2024 +0800

    update

commit 0037b0c
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 17:03:04 2024 +0800

    ts error

commit 6f81bb3
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 16:56:15 2024 +0800

    add context after connected

commit 7bdc45e
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 16:41:24 2024 +0800

    connect realtime model when open panel

commit 88cd3ac
Author: Dogtiti <[email protected]>
Date:   Thu Nov 7 12:16:11 2024 +0800

    fix: ts error

commit 4988d2e
Author: Dogtiti <[email protected]>
Date:   Thu Nov 7 11:56:58 2024 +0800

    fix: ts error

commit 8deb7a9
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 11:53:01 2024 +0800

    hotfix for update target session

commit db060d7
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 11:45:38 2024 +0800

    upload save record wav file

commit 5226278
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 09:36:22 2024 +0800

    upload save wav file logic

commit cf46d5a
Author: lloydzhou <[email protected]>
Date:   Thu Nov 7 01:12:08 2024 +0800

    upload response audio, and update audio_url to session message

commit a494152
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 22:30:02 2024 +0800

    feat: audio to message

commit f6e1f83
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 22:07:33 2024 +0800

    wip

commit d544eea
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 21:14:45 2024 +0800

    feat: realtime chat ui

commit fbb9385
Merge: 6ded4e9 18144c3
Author: Lloyd Zhou <[email protected]>
Date:   Wed Nov 6 20:33:51 2024 +0800

    Merge pull request ChatGPTNextWeb#5782 from ConnectAI-E/style/classname

    style: improve classname by clsx

commit 18144c3
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 20:16:38 2024 +0800

    chore: clsx

commit 64aa760
Author: opchips <[email protected]>
Date:   Wed Nov 6 19:18:05 2024 +0800

    update claude rank

commit e0bbb8b
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 16:58:26 2024 +0800

    style: improve classname by clsx

commit 6667ee1
Merge: 3086a2f 6ded4e9
Author: opchips <[email protected]>
Date:   Wed Nov 6 15:08:18 2024 +0800

    merge main

commit 6ded4e9
Merge: f4c9410 85cdcab
Author: Lloyd Zhou <[email protected]>
Date:   Wed Nov 6 15:04:46 2024 +0800

    Merge pull request ChatGPTNextWeb#5778 from ConnectAI-E/fix/5436

    fix: botMessage reply date

commit 85cdcab
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 14:53:08 2024 +0800

    fix: botMessage reply date

commit f4c9410
Merge: f526d6f adf7d82
Author: Lloyd Zhou <[email protected]>
Date:   Wed Nov 6 14:02:20 2024 +0800

    Merge pull request ChatGPTNextWeb#5776 from ConnectAI-E/feat-glm

    fix: glm chatpath

commit adf7d82
Author: DDMeaqua <[email protected]>
Date:   Wed Nov 6 13:55:57 2024 +0800

    fix: glm chatpath

commit 3086a2f
Author: opchips <[email protected]>
Date:   Wed Nov 6 12:56:24 2024 +0800

    add claude35haiku not vision

commit f526d6f
Merge: f3603e5 106461a
Author: Lloyd Zhou <[email protected]>
Date:   Wed Nov 6 11:16:33 2024 +0800

    Merge pull request ChatGPTNextWeb#5774 from ConnectAI-E/feature/update-target-session

    fix: updateCurrentSession => updateTargetSession

commit 106461a
Merge: c4e19db f3603e5
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 11:08:41 2024 +0800

    Merge branch 'main' of https://github.com/ConnectAI-E/ChatGPT-Next-Web into feature/update-target-session

commit c4e19db
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 11:06:18 2024 +0800

    fix: updateCurrentSession => updateTargetSession

commit f3603e5
Merge: 00d6cb2 8e2484f
Author: Dogtiti <[email protected]>
Date:   Wed Nov 6 10:49:28 2024 +0800

    Merge pull request ChatGPTNextWeb#5769 from ryanhex53/fix-model-multi@

    Custom model names can include the `@` symbol by itself.

commit 8e2484f
Author: ryanhex53 <[email protected]>
Date:   Tue Nov 5 13:52:54 2024 +0000

    Refactor: Replace all provider split occurrences with getModelProvider utility method

commit 00d6cb2
Author: lloydzhou <[email protected]>
Date:   Tue Nov 5 17:42:55 2024 +0800

    update version

commit b844045
Author: ryanhex53 <[email protected]>
Date:   Tue Nov 5 07:44:12 2024 +0000

    Custom model names can include the `@` symbol by itself.

    To specify the model's provider, append it after the model name using `@` as before.

    This format supports cases like `google vertex ai` with a model name like `claude-3-5-sonnet@20240620`.

    For instance, `claude-3-5-sonnet@20240620@vertex-ai` will be split by `split(/@(?!.*@)/)` into:

    `[ 'claude-3-5-sonnet@20240620', 'vertex-ai' ]`, where the former is the model name and the latter is the custom provider.

commit e49fe97
Merge: 14f7519 e49466f
Author: Lloyd Zhou <[email protected]>
Date:   Tue Nov 5 15:07:52 2024 +0800

    Merge pull request ChatGPTNextWeb#5765 from ConnectAI-E/feature/onfinish

    feat: update real 'currentSession'

commit 14f7519
Merge: 820ab54 0ec4233
Author: Dogtiti <[email protected]>
Date:   Tue Nov 5 11:07:52 2024 +0800

    Merge pull request ChatGPTNextWeb#5767 from ConnectAI-E/feat-glm

    chore: update readme

commit 0ec4233
Author: DDMeaqua <[email protected]>
Date:   Tue Nov 5 11:06:20 2024 +0800

    chore: update readme

commit 820ab54
Merge: 0dc4071 a6c1eb2
Author: Dogtiti <[email protected]>
Date:   Tue Nov 5 10:54:52 2024 +0800

    Merge pull request ChatGPTNextWeb#5766 from ConnectAI-E/feature/add-claude-haiku3.5

    Feature/add claude haiku3.5

commit a6c1eb2
Merge: 801dc41 0dc4071
Author: lloydzhou <[email protected]>
Date:   Tue Nov 5 10:23:15 2024 +0800

    add claude 3.5 haiku

commit 0dc4071
Merge: aef535f 4d39497
Author: Lloyd Zhou <[email protected]>
Date:   Tue Nov 5 01:10:06 2024 +0800

    Merge pull request ChatGPTNextWeb#5464 from endless-learner/main

    Added 1-click deployment link for Alibaba Cloud.

commit 4d39497
Author: Lloyd Zhou <[email protected]>
Date:   Tue Nov 5 01:09:27 2024 +0800

    merge main

commit aef535f
Merge: 686a80e fbb7a1e
Author: Dogtiti <[email protected]>
Date:   Mon Nov 4 21:41:11 2024 +0800

    Merge pull request ChatGPTNextWeb#5753 from ChatGPTNextWeb/feat-bt-doc

    Feat bt doc

commit 686a80e
Merge: 5733e3c 4b93370
Author: Dogtiti <[email protected]>
Date:   Mon Nov 4 21:37:34 2024 +0800

    Merge pull request ChatGPTNextWeb#5764 from ChatGPTNextWeb/dependabot/npm_and_yarn/testing-library/react-16.0.1

    chore(deps-dev): bump @testing-library/react from 16.0.0 to 16.0.1

commit e49466f
Author: Dogtiti <[email protected]>
Date:   Mon Nov 4 21:25:56 2024 +0800

    feat: update real 'currentSession'

commit 4b93370
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Nov 4 10:24:30 2024 +0000

    chore(deps-dev): bump @testing-library/react from 16.0.0 to 16.0.1

    Bumps [@testing-library/react](https://github.com/testing-library/react-testing-library) from 16.0.0 to 16.0.1.
    - [Release notes](https://github.com/testing-library/react-testing-library/releases)
    - [Changelog](https://github.com/testing-library/react-testing-library/blob/main/CHANGELOG.md)
    - [Commits](testing-library/react-testing-library@v16.0.0...v16.0.1)

    ---
    updated-dependencies:
    - dependency-name: "@testing-library/react"
      dependency-type: direct:development
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <[email protected]>

commit 5733e3c
Merge: d66bfc6 44fc5b5
Author: Dogtiti <[email protected]>
Date:   Mon Nov 4 17:16:44 2024 +0800

    Merge pull request ChatGPTNextWeb#5759 from ConnectAI-E/feature/onfinish

    Feature/onfinish

commit 44fc5b5
Author: Dogtiti <[email protected]>
Date:   Mon Nov 4 17:00:45 2024 +0800

    fix: onfinish responseRes

commit 2d3f7c9
Author: Dogtiti <[email protected]>
Date:   Wed Oct 16 15:17:08 2024 +0800

    fix: vision model dalle3

commit fe8cca3
Merge: adf97c6 d66bfc6
Author: GH Action - Upstream Sync <[email protected]>
Date:   Sat Nov 2 01:12:09 2024 +0000

    Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next-Web

commit fbb7a1e
Author: weige <[email protected]>
Date:   Fri Nov 1 18:20:16 2024 +0800

    fix

commit fb2c155
Author: weige <[email protected]>
Date:   Fri Nov 1 17:45:50 2024 +0800

    fix

commit c2c52a1
Author: weige <[email protected]>
Date:   Fri Nov 1 17:35:34 2024 +0800

    fix

commit 106ddc1
Author: weige <[email protected]>
Date:   Fri Nov 1 17:35:09 2024 +0800

    fix

commit 17d5209
Author: weige <[email protected]>
Date:   Fri Nov 1 17:28:20 2024 +0800

    add bt install doc

commit adf97c6
Merge: 7c466c9 0581e37
Author: GH Action - Upstream Sync <[email protected]>
Date:   Fri Nov 1 01:18:59 2024 +0000

    Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next-Web

commit 7c466c9
Merge: b0d28eb a0fa4d7
Author: GH Action - Upstream Sync <[email protected]>
Date:   Thu Oct 31 01:14:28 2024 +0000

    Merge branch 'main' of https://github.com/ChatGPTNextWeb/ChatGPT-Next-Web

commit b0d28eb
Merge: 064e964 613d67e
Author: endless-learner <[email protected]>
Date:   Tue Oct 29 14:38:49 2024 -0700

    Merge branch 'main' into main

commit 801dc41
Author: lloydzhou <[email protected]>
Date:   Thu Oct 24 15:28:05 2024 +0800

    add claude-3.5-haiku

commit 064e964
Author: endless-learner <[email protected]>
Date:   Tue Sep 24 23:05:32 2024 -0700

    Updated link to deploy on Alibaba Cloud, readable when not logged in, also, able to choose region.

commit 47fb40d
Merge: 9e18cc2 4c84182
Author: endless-learner <[email protected]>
Date:   Tue Sep 24 23:03:03 2024 -0700

    Merge branch 'ChatGPTNextWeb:main' into main

commit 9e18cc2
Author: endless-learner <[email protected]>
Date:   Tue Sep 24 13:55:00 2024 -0700

    Update README.md

    Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

commit 03268ce
Author: endless-learner <[email protected]>
Date:   Wed Sep 18 20:38:20 2024 -0700

    Added 1-click deployment link for Alibaba Cloud.
@coderabbitai coderabbitai bot mentioned this pull request Nov 20, 2024
10 tasks
This was referenced Dec 12, 2024
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