-
Notifications
You must be signed in to change notification settings - Fork 0
[feature]:AI Chatbot support #4
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
base: master
Are you sure you want to change the base?
Conversation
Screen.Recording.2024-09-20.at.1.51.33.AM.1.1.mov |
Ok, I'll test it later |
Sure, please let me know the feedback I'll accommodate the changes. |
Take a look at the code, ci fail |
WalkthroughThis update introduces a new chatbot feature to the web application. It adds two new Angular components: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatbotComponent
participant HttpClient
participant AI_Backend
User->>ChatbotComponent: Clicks chatbot button / types message
ChatbotComponent->>ChatbotComponent: Shows popup / updates userInput
User->>ChatbotComponent: Sends message (button or Enter)
ChatbotComponent->>ChatbotComponent: Adds user message to messages
ChatbotComponent->>HttpClient: POST /api/ai with userInput
HttpClient->>AI_Backend: Sends request
AI_Backend-->>HttpClient: Returns AI response
HttpClient-->>ChatbotComponent: Receives response
ChatbotComponent->>ChatbotComponent: Adds AI message to messages
User->>ChatbotComponent: Clicks copy on AI message
ChatbotComponent->>ChatbotComponent: Copies text to clipboard, shows "Copied!" hint
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this 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
🧹 Nitpick comments (8)
web-app/src/app/app.component.ts (1)
1-1
: Remove unused importThe
HttpClient
import is added but not used in this component. It should be removed to keep the code clean and avoid confusion.-import { HttpClient } from '@angular/common/http';
web-app/src/app/shared/components/chat-message/chat-message.component.html (1)
1-10
: Add visual distinction between user and AI messagesCurrently, the only visual difference between AI and user messages is the copy button. Consider adding different styling or positioning for AI vs. user messages to improve readability.
web-app/src/app/shared/components/chatbot/chatbot.component.html (1)
1-19
: Consider adding auto-scroll for chat messagesAs more messages are added to the chat, the user will need to manually scroll to see the latest messages. Consider adding auto-scroll functionality to always show the most recent messages.
web-app/src/app/shared/components/chat-message/chat-message.component.css (2)
30-46
: Improve tooltip positioning for better accessibilityThe hint's absolute positioning with negative top value might cause it to be cut off in certain viewport sizes. Consider using a more reliable positioning approach.
.hint { position: absolute; - top: -10px; + top: -25px; left: 50%; transform: translateX(-50%); background-color: #3f51b5; color: white; padding: 3px 8px; border-radius: 4px; font-size: 0.75rem; opacity: 0; transition: opacity 0.3s ease-in-out; + z-index: 1001; /* Ensure tooltip is above other elements */ }
1-10
: Add support for different message typesThe current styling doesn't visually differentiate between user and AI messages. Consider adding different background colors or alignment.
.message { margin-bottom: 5px; padding: 5px; border-radius: 4px; background-color: #f1f1f1; max-width: 100%; word-wrap: break-word; display: flex; align-items: center; } + +.message.user-message { + background-color: #e3f2fd; + align-self: flex-end; +} + +.message.ai-message { + background-color: #f1f1f1; + align-self: flex-start; +}web-app/src/app/shared/components/chatbot/chatbot.component.ts (1)
43-45
: Add confirmation for chat resetConsider adding a confirmation dialog before clearing all messages to prevent accidental data loss.
resetChat() { - this.messages = []; + if (confirm('Are you sure you want to reset the chat? All messages will be lost.')) { + this.messages = []; + } }web-app/src/app/shared/components/chatbot/chatbot.component.css (2)
55-62
: Add loading indicator for ongoing AI responsesSince the AI will likely take time to respond, add a styling for a loading indicator.
.chat-messages { flex-grow: 1; overflow-y: auto; /* Enables vertical scrolling */ max-height: 200px; /* Set a maximum height for the chat window */ margin-bottom: 10px; width: 100%; padding-right: 5px; /* Optional: Adds some padding for better readability */ } + +.loading-indicator { + display: inline-block; + width: 20px; + height: 20px; + border: 2px solid rgba(0, 0, 0, 0.1); + border-radius: 50%; + border-top-color: #3f51b5; + animation: spin 1s ease-in-out infinite; +} + +@keyframes spin { + to { transform: rotate(360deg); } +}
16-30
: Add responsive styling for mobile devicesThe current fixed width might not work well on small mobile screens. Consider adding media queries.
.popup { position: fixed; bottom: 5rem; right: 2rem; width: 310px; max-height: 400px; border: 1px solid #ccc; border-radius: 8px; box-shadow: 0 2px 10px rgba(0, 0, 0, 0.1); background-color: white; z-index: 1000; display: flex; flex-direction: column; justify-content: space-between; } + +/* Responsive styling for mobile devices */ +@media (max-width: 480px) { + .popup { + width: 90%; + right: 5%; + left: 5%; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
web-app/src/assets/img/chatbot-icon.svg
is excluded by!**/*.svg
📒 Files selected for processing (8)
web-app/src/app/app.component.ts
(2 hunks)web-app/src/app/shared/components/chat-message/chat-message.component.css
(1 hunks)web-app/src/app/shared/components/chat-message/chat-message.component.html
(1 hunks)web-app/src/app/shared/components/chat-message/chat-message.component.ts
(1 hunks)web-app/src/app/shared/components/chatbot/chatbot.component.css
(1 hunks)web-app/src/app/shared/components/chatbot/chatbot.component.html
(1 hunks)web-app/src/app/shared/components/chatbot/chatbot.component.ts
(1 hunks)web-app/src/app/shared/shared.module.ts
(3 hunks)
🔇 Additional comments (5)
web-app/src/app/app.component.ts (1)
14-17
: LGTM! Integration of chatbot componentThe chatbot component is properly integrated into the main application template, allowing it to be available across the entire application.
web-app/src/app/shared/shared.module.ts (2)
25-26
: LGTM! Components properly importedThe chatbot components are correctly imported.
35-37
: LGTM! Components properly registeredThe components are correctly added to the COMPONENTS array, ensuring they are declared and exported by the SharedModule.
web-app/src/app/shared/components/chat-message/chat-message.component.ts (1)
1-23
: Well-structured component for displaying chat messagesThe implementation looks good. The component takes a message input and properly implements the copy-to-clipboard functionality with appropriate error handling and user feedback via the timeout.
web-app/src/app/shared/components/chatbot/chatbot.component.ts (1)
14-14
: LGTM: Proper dependency injectionGood use of Angular's dependency injection for HttpClient.
<div class="message" *ngIf="message.sender === 'AI'"> | ||
<p><strong>{{ message.sender }}:</strong> {{ message.text }}</p> | ||
<button class="copy-btn" (click)="copyToClipboard(message.text)"> | ||
📋 | ||
<span class="hint" *ngIf="copied">Copied!</span> | ||
</button> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve accessibility for the copy button
The copy button lacks proper accessibility attributes, which makes it difficult for screen reader users to understand its purpose.
-<button class="copy-btn" (click)="copyToClipboard(message.text)">
+<button class="copy-btn" (click)="copyToClipboard(message.text)" aria-label="Copy message to clipboard" title="Copy to clipboard">
📋
<span class="hint" *ngIf="copied">Copied!</span>
</button>
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | ||
</div> | ||
<div class="input-container"> | ||
<input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation
Currently, the user can send empty messages as there's no validation on the input. This could result in unnecessary API calls and empty messages in the chat.
-<input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." />
+<input type="text" [(ngModel)]="userInput" (keydown.enter)="userInput.trim() && sendMessage()" placeholder="Type your message..." />
-<button (click)="sendMessage()">Send</button>
+<button (click)="sendMessage()" [disabled]="!userInput.trim()">Send</button>
<button class="float" (click)="togglePopup()"> | ||
<img src="../assets/img/chatbot-icon.svg" alt="Ai Chat" /> | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the asset path for the chatbot icon
The image path uses a relative path which might be incorrect depending on the project structure. Angular typically uses absolute paths from the assets folder.
🏁 Script executed:
#!/bin/bash
# Check if the chatbot icon exists at the specified path
# Find the assets directory and check for the chatbot-icon.svg
echo "Checking for chatbot icon file..."
fd -t f "chatbot-icon.svg" --full-path
Length of output: 155
Use an absolute asset path for the chatbot icon
The file chatbot-icon.svg
does exist at web-app/src/assets/img/chatbot-icon.svg
and will be served from /assets/img/chatbot-icon.svg
. Relying on a relative path (../assets/...
) can break when the app is on a nested route, since URLs resolve against the current browser path. Update the src
to an absolute path:
<button class="float" (click)="togglePopup()">
- <img src="../assets/img/chatbot-icon.svg" alt="Ai Chat" />
+ <img src="/assets/img/chatbot-icon.svg" alt="Ai Chat" />
</button>
— Ensure that all other asset references follow this pattern to avoid similar routing issues.
<button class="close-btn" (click)="togglePopup()">✖</button> | ||
<h3>Chatbot</h3> | ||
<div class="chat-window"> | ||
<div class="chat-messages"> | ||
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | ||
</div> | ||
<div class="input-container"> | ||
<input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." /> | ||
<button (click)="sendMessage()">Send</button> | ||
<button class="reset-btn" (click)="resetChat()">Reset</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve accessibility for buttons and inputs
The buttons and inputs in the chatbot lack proper accessibility attributes, making them difficult to use with screen readers.
-<button class="close-btn" (click)="togglePopup()">✖</button>
+<button class="close-btn" (click)="togglePopup()" aria-label="Close chatbot">✖</button>
<h3>Chatbot</h3>
<div class="chat-window">
<div class="chat-messages">
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message>
</div>
<div class="input-container">
- <input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." />
+ <input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." aria-label="Message input" />
- <button (click)="sendMessage()">Send</button>
+ <button (click)="sendMessage()" aria-label="Send message">Send</button>
- <button class="reset-btn" (click)="resetChat()">Reset</button>
+ <button class="reset-btn" (click)="resetChat()" aria-label="Reset chat">Reset</button>
</div>
</div>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<button class="close-btn" (click)="togglePopup()">✖</button> | |
<h3>Chatbot</h3> | |
<div class="chat-window"> | |
<div class="chat-messages"> | |
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | |
</div> | |
<div class="input-container"> | |
<input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." /> | |
<button (click)="sendMessage()">Send</button> | |
<button class="reset-btn" (click)="resetChat()">Reset</button> | |
<button class="close-btn" (click)="togglePopup()" aria-label="Close chatbot">✖</button> | |
<h3>Chatbot</h3> | |
<div class="chat-window"> | |
<div class="chat-messages"> | |
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | |
</div> | |
<div class="input-container"> | |
<input | |
type="text" | |
[(ngModel)]="userInput" | |
(keydown.enter)="sendMessage()" | |
placeholder="Type your message..." | |
aria-label="Message input" | |
/> | |
<button (click)="sendMessage()" aria-label="Send message">Send</button> | |
<button class="reset-btn" (click)="resetChat()" aria-label="Reset chat">Reset</button> | |
</div> | |
</div> |
<button class="float" (click)="togglePopup()"> | ||
<img src="../assets/img/chatbot-icon.svg" alt="Ai Chat" /> | ||
</button> | ||
<div class="popup" *ngIf="isPopupVisible"> | ||
<div class="popup-content"> | ||
<button class="close-btn" (click)="togglePopup()">✖</button> | ||
<h3>Chatbot</h3> | ||
<div class="chat-window"> | ||
<div class="chat-messages"> | ||
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | ||
</div> | ||
<div class="input-container"> | ||
<input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." /> | ||
<button (click)="sendMessage()">Send</button> | ||
<button class="reset-btn" (click)="resetChat()">Reset</button> | ||
</div> | ||
</div> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add loading state and error handling
The chatbot UI doesn't show any loading indicators or error messages when interacting with the AI backend. This could lead to a poor user experience if the backend is slow or returns an error.
Add loading and error handling states to the template:
<div class="popup" *ngIf="isPopupVisible">
<div class="popup-content">
<button class="close-btn" (click)="togglePopup()">✖</button>
<h3>Chatbot</h3>
<div class="chat-window">
<div class="chat-messages">
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message>
+ <div *ngIf="isLoading" class="loading-indicator">AI is thinking...</div>
+ <div *ngIf="errorMessage" class="error-message">{{errorMessage}}</div>
</div>
<div class="input-container">
- <input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." />
+ <input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." [disabled]="isLoading" />
- <button (click)="sendMessage()">Send</button>
+ <button (click)="sendMessage()" [disabled]="isLoading || !userInput.trim()">Send</button>
<button class="reset-btn" (click)="resetChat()">Reset</button>
</div>
</div>
</div>
</div>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<button class="float" (click)="togglePopup()"> | |
<img src="../assets/img/chatbot-icon.svg" alt="Ai Chat" /> | |
</button> | |
<div class="popup" *ngIf="isPopupVisible"> | |
<div class="popup-content"> | |
<button class="close-btn" (click)="togglePopup()">✖</button> | |
<h3>Chatbot</h3> | |
<div class="chat-window"> | |
<div class="chat-messages"> | |
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | |
</div> | |
<div class="input-container"> | |
<input type="text" [(ngModel)]="userInput" (keydown.enter)="sendMessage()" placeholder="Type your message..." /> | |
<button (click)="sendMessage()">Send</button> | |
<button class="reset-btn" (click)="resetChat()">Reset</button> | |
</div> | |
</div> | |
</div> | |
</div> | |
<button class="float" (click)="togglePopup()"> | |
<img src="../assets/img/chatbot-icon.svg" alt="Ai Chat" /> | |
</button> | |
<div class="popup" *ngIf="isPopupVisible"> | |
<div class="popup-content"> | |
<button class="close-btn" (click)="togglePopup()">✖</button> | |
<h3>Chatbot</h3> | |
<div class="chat-window"> | |
<div class="chat-messages"> | |
<app-chat-message *ngFor="let message of messages" [message]="message"></app-chat-message> | |
<div *ngIf="isLoading" class="loading-indicator">AI is thinking...</div> | |
<div *ngIf="errorMessage" class="error-message">{{errorMessage}}</div> | |
</div> | |
<div class="input-container"> | |
<input | |
type="text" | |
[(ngModel)]="userInput" | |
(keydown.enter)="sendMessage()" | |
placeholder="Type your message..." | |
[disabled]="isLoading" | |
/> | |
<button (click)="sendMessage()" [disabled]="isLoading || !userInput.trim()">Send</button> | |
<button class="reset-btn" (click)="resetChat()">Reset</button> | |
</div> | |
</div> | |
</div> | |
</div> |
this.http.post('/api/ai', { text: this.userInput }).subscribe( | ||
(response: any) => { | ||
// Add AI's response to the chat window | ||
this.messages.push({ sender: 'AI', text: "sample answer" }); | ||
}, | ||
(error) => { | ||
// Handle error response | ||
this.messages.push({ sender: 'AI', text: 'Error communicating with AI' }); | ||
console.error('Error communicating with AI:', error); | ||
} | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix hardcoded AI response and add SSE streaming support
The current implementation doesn't use the actual API response and lacks support for SSE streaming mentioned in the PR description. This is likely causing the frontend display issue mentioned in the PR comments.
// Send the message to the AI backend
- this.http.post('/api/ai', { text: this.userInput }).subscribe(
- (response: any) => {
- // Add AI's response to the chat window
- this.messages.push({ sender: 'AI', text: "sample answer" });
- },
- (error) => {
- // Handle error response
- this.messages.push({ sender: 'AI', text: 'Error communicating with AI' });
- console.error('Error communicating with AI:', error);
- }
- );
+ // Add a placeholder response initially
+ const responseIndex = this.messages.length;
+ this.messages.push({ sender: 'AI', text: 'Thinking...' });
+
+ // Setup EventSource for SSE streaming
+ const source = new EventSource(`/api/ai/stream?query=${encodeURIComponent(this.userInput)}`);
+
+ let responseText = '';
+
+ source.onmessage = (event) => {
+ try {
+ const data = JSON.parse(event.data);
+ if (data.text) {
+ responseText += data.text;
+ // Update the ongoing response
+ this.messages[responseIndex] = { sender: 'AI', text: responseText };
+ }
+ } catch (e) {
+ console.error('Error parsing SSE data:', e);
+ }
+ };
+
+ source.onerror = (error) => {
+ console.error('SSE Error:', error);
+ // If responseText is empty, show error message, otherwise keep what we received
+ if (!responseText) {
+ this.messages[responseIndex] = { sender: 'AI', text: 'Error communicating with AI' };
+ }
+ source.close();
+ };
+
+ source.addEventListener('end', () => {
+ source.close();
+ });
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
this.http.post('/api/ai', { text: this.userInput }).subscribe( | |
(response: any) => { | |
// Add AI's response to the chat window | |
this.messages.push({ sender: 'AI', text: "sample answer" }); | |
}, | |
(error) => { | |
// Handle error response | |
this.messages.push({ sender: 'AI', text: 'Error communicating with AI' }); | |
console.error('Error communicating with AI:', error); | |
} | |
); | |
// Send the message to the AI backend | |
// Add a placeholder response initially | |
const responseIndex = this.messages.length; | |
this.messages.push({ sender: 'AI', text: 'Thinking...' }); | |
// Setup EventSource for SSE streaming | |
const source = new EventSource( | |
`/api/ai/stream?query=${encodeURIComponent(this.userInput)}` | |
); | |
let responseText = ''; | |
source.onmessage = (event) => { | |
try { | |
const data = JSON.parse(event.data); | |
if (data.text) { | |
responseText += data.text; | |
// Update the ongoing response | |
this.messages[responseIndex] = { sender: 'AI', text: responseText }; | |
} | |
} catch (e) { | |
console.error('Error parsing SSE data:', e); | |
} | |
}; | |
source.onerror = (error) => { | |
console.error('SSE Error:', error); | |
// If nothing has come through yet, show an error message | |
if (!responseText) { | |
this.messages[responseIndex] = { | |
sender: 'AI', | |
text: 'Error communicating with AI' | |
}; | |
} | |
source.close(); | |
}; | |
// Close the connection when the server signals end-of-stream | |
source.addEventListener('end', () => { | |
source.close(); | |
}); |
.float { | ||
position: fixed; | ||
bottom: 2rem; | ||
right: 2rem; | ||
border: none; | ||
background: transparent; | ||
padding: 0; | ||
cursor: pointer; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add accessibility attributes to the chat button
Improve accessibility by adding appropriate ARIA attributes to the floating chat button.
This will help screen readers identify the purpose of the button. Update the HTML template to include these attributes:
<button class="float" aria-label="Open chat assistant" title="Chat with AI assistant">
<img src="path-to-chat-icon.png" alt="Chat icon">
</button>
What's changed?
I've added an ai chatbot support, I'm unable to config the ai endpoints as I wasn't able to translate the ai config docs
closes apache#2303
Checklist
Add or update API
Summary by CodeRabbit
New Features
Style