Sentiment analyser playtime 24 #4024
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options. (For a CapTP with native promises, see @endo/eventual-send and @endo/captp) WalkthroughThe changes introduce significant enhancements to the functionality of a browser plugin for meetings, focusing on transcription and sentiment analysis features. The Changes
Suggested reviewers
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? 🪧 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 (
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
docs/samples/browser-plugin-meetings/app.js (2)
1233-1238: Correct the spelling of 'commulative' to 'cumulative'The term 'commulative' is misspelled. It should be 'cumulative' to accurately reflect the function's purpose and maintain consistency.
Apply this diff to fix the spelling errors:
-function updateCommulativeScore(scores, value) { +function updateCumulativeScore(scores, value) { scores.push(value); const total = scores.reduce((acc, value) => acc + value, 0); - const commulativeScore = total / scores.length; - return commulativeScore; + const cumulativeScore = total / scores.length; + return cumulativeScore; } // Update the function call -updateLinearBar(updateCommulativeScore(scores, value)); +updateLinearBar(updateCumulativeScore(scores, value));
1243-1246: Standardize variable naming: 'yellowGreenbLENDwidth' should be 'yellowGreenBlendWidth'The variable name
yellowGreenbLENDwidthhas inconsistent casing. Rename it toyellowGreenBlendWidthfor consistency and readability.Apply this diff to fix the variable name:
-let yellowGreenbLENDwidth = 0; +let yellowGreenBlendWidth = 0; // Update all occurrences - yellowGreenbLENDwidth = 10; + yellowGreenBlendWidth = 10; // Update the DOM manipulation - document.getElementById('yellowGreenbLENDwidth').style.width = `${yellowGreenbLENDwidth}%`; + document.getElementById('yellowGreenBlendWidth').style.width = `${yellowGreenBlendWidth}%`;docs/samples/browser-plugin-meetings/style.css (1)
539-541: Standardize class name: '.yellowGreenbLENDwidth' should be '.yellowGreenBlendWidth'The class name
.yellowGreenbLENDwidthhas inconsistent casing. Rename it to.yellowGreenBlendWidthto maintain consistency with the JavaScript variable names and improve readability.Apply this diff to fix the class name:
-.yellowGreenbLENDwidth { +.yellowGreenBlendWidth { background: linear-gradient(to right, yellow, green); }Ensure to update all references to this class in the HTML and JavaScript code.
docs/samples/browser-plugin-meetings/index.html (1)
Line range hint
703-723: Consider removing commented-out code for 'Breakout Sessions' and 'Events Published'The sections for 'Breakout Sessions' and 'Events Published' are commented out. If these features are no longer needed, consider removing the commented-out code to keep the codebase clean and maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
docs/samples/browser-plugin-meetings/speedometer1.pngis excluded by!**/*.png
📒 Files selected for processing (4)
docs/samples/browser-plugin-meetings/app.js(2 hunks)docs/samples/browser-plugin-meetings/index.html(9 hunks)docs/samples/browser-plugin-meetings/style.css(1 hunks)packages/calling/src/CallingClient/calling/call.ts(1 hunks)
🔇 Additional comments (1)
packages/calling/src/CallingClient/calling/call.ts (1)
233-235: LGTM!
Initializing mobiusUrl, receivedRoapOKSeq, and mediaNegotiationCompleted in the constructor enhances the call management process and ensures proper media negotiation handling.
| let currentRead = 0; | ||
|
|
||
| for (i=currentRead; i< payload.captions.length ; i++) { | ||
| let isSpeakerHost = payload.captions[i].speaker.speakerId === meeting.hostId; | ||
|
|
||
| currentRead = currentRead +1; | ||
|
|
||
| if(payload.captions[i].isFinal === true && isSpeakerHost !== true ) { | ||
| getSentimentScore(payload.captions[i].text); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix the for loop by declaring 'i' and adding an increment expression
The variable i is not declared, which will result in a reference error or the creation of a global variable. Additionally, the for loop lacks an increment expression, leading to a potential infinite loop. Incrementing currentRead inside the loop does not affect the loop variable i.
Apply this diff to fix the issues:
-let currentRead = 0;
-for (i=currentRead; i< payload.captions.length ; i++) {
+for (let i = 0; i < payload.captions.length; i++) {
- currentRead = currentRead +1;
let isSpeakerHost = payload.captions[i].speaker.speakerId === meeting.hostId;📝 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.
| let currentRead = 0; | |
| for (i=currentRead; i< payload.captions.length ; i++) { | |
| let isSpeakerHost = payload.captions[i].speaker.speakerId === meeting.hostId; | |
| currentRead = currentRead +1; | |
| if(payload.captions[i].isFinal === true && isSpeakerHost !== true ) { | |
| getSentimentScore(payload.captions[i].text); | |
| } | |
| } | |
| for (let i = 0; i < payload.captions.length; i++) { | |
| let isSpeakerHost = payload.captions[i].speaker.speakerId === meeting.hostId; | |
| if(payload.captions[i].isFinal === true && isSpeakerHost !== true ) { | |
| getSentimentScore(payload.captions[i].text); | |
| } | |
| } |
| if (xhr.status >= 200 && xhr.status < 300) { | ||
| const response = JSON.parse(this.responseText); | ||
|
|
||
| console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`); | ||
|
|
||
| let value = getShowScore(response[0].label, response[0].score); | ||
|
|
||
| updateNeedle(value); | ||
|
|
||
| updateLinearBar(updateCommulativeScore(scores,value)); | ||
| } else { | ||
| console.error('Error in request:', xhr.status, xhr.statusText); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON parsing
Parsing this.responseText without error handling may throw an exception if the response is not valid JSON. Wrap the JSON.parse call in a try-catch block to handle parsing errors gracefully.
Apply this diff to handle JSON parsing errors:
xhr.onload = function() {
if (xhr.status >= 200 && xhr.status < 300) {
+ try {
const response = JSON.parse(this.responseText);
console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`);
let value = getShowScore(response[0].label, response[0].score);
updateNeedle(value);
updateLinearBar(updateCommulativeScore(scores, value));
+ } catch (e) {
+ console.error('Failed to parse JSON response:', e);
+ }
} else {
console.error('Error in request:', xhr.status, xhr.statusText);
}
};📝 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.
| if (xhr.status >= 200 && xhr.status < 300) { | |
| const response = JSON.parse(this.responseText); | |
| console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`); | |
| let value = getShowScore(response[0].label, response[0].score); | |
| updateNeedle(value); | |
| updateLinearBar(updateCommulativeScore(scores,value)); | |
| } else { | |
| console.error('Error in request:', xhr.status, xhr.statusText); | |
| } | |
| }; | |
| if (xhr.status >= 200 && xhr.status < 300) { | |
| try { | |
| const response = JSON.parse(this.responseText); | |
| console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`); | |
| let value = getShowScore(response[0].label, response[0].score); | |
| updateNeedle(value); | |
| updateLinearBar(updateCommulativeScore(scores,value)); | |
| } catch (e) { | |
| console.error('Failed to parse JSON response:', e); | |
| } | |
| } else { | |
| console.error('Error in request:', xhr.status, xhr.statusText); | |
| } | |
| }; |
| let xhr = new XMLHttpRequest(); | ||
| let scores = []; | ||
|
|
||
| xhr.open("POST", "http://localhost:8080/analyze-sentiment"); | ||
|
|
||
| xhr.setRequestHeader("Content-Type", "application/json"); | ||
|
|
||
| console.log('Playtime: Transcript Sent:', transcriptText); | ||
|
|
||
| xhr.send(JSON.stringify({ transcript: transcriptText })); | ||
|
|
||
| xhr.onload = function() { | ||
| if (xhr.status >= 200 && xhr.status < 300) { | ||
| const response = JSON.parse(this.responseText); | ||
|
|
||
| console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`); | ||
|
|
||
| let value = getShowScore(response[0].label, response[0].score); | ||
|
|
||
| updateNeedle(value); | ||
|
|
||
| updateLinearBar(updateCommulativeScore(scores,value)); | ||
| } else { | ||
| console.error('Error in request:', xhr.status, xhr.statusText); | ||
| } | ||
| }; | ||
|
|
||
| xhr.onerror = function() { | ||
| console.error('Request failed'); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Declare the 'scores' array outside the 'getSentimentScore' function to accumulate values
The scores array is reinitialized every time getSentimentScore is called, which prevents it from accumulating values across multiple calls. To correctly compute the cumulative score, declare scores outside the function.
Apply this diff to fix the issue:
-function getSentimentScore(transcriptText) {
- let xhr = new XMLHttpRequest();
- let scores = [];
+let scores = [];
+
+function getSentimentScore(transcriptText) {
+ let xhr = new XMLHttpRequest();
// Rest of the code...📝 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.
| let xhr = new XMLHttpRequest(); | |
| let scores = []; | |
| xhr.open("POST", "http://localhost:8080/analyze-sentiment"); | |
| xhr.setRequestHeader("Content-Type", "application/json"); | |
| console.log('Playtime: Transcript Sent:', transcriptText); | |
| xhr.send(JSON.stringify({ transcript: transcriptText })); | |
| xhr.onload = function() { | |
| if (xhr.status >= 200 && xhr.status < 300) { | |
| const response = JSON.parse(this.responseText); | |
| console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`); | |
| let value = getShowScore(response[0].label, response[0].score); | |
| updateNeedle(value); | |
| updateLinearBar(updateCommulativeScore(scores,value)); | |
| } else { | |
| console.error('Error in request:', xhr.status, xhr.statusText); | |
| } | |
| }; | |
| xhr.onerror = function() { | |
| console.error('Request failed'); | |
| }; | |
| } | |
| let scores = []; | |
| function getSentimentScore(transcriptText) { | |
| let xhr = new XMLHttpRequest(); | |
| xhr.open("POST", "http://localhost:8080/analyze-sentiment"); | |
| xhr.setRequestHeader("Content-Type", "application/json"); | |
| console.log('Playtime: Transcript Sent:', transcriptText); | |
| xhr.send(JSON.stringify({ transcript: transcriptText })); | |
| xhr.onload = function() { | |
| if (xhr.status >= 200 && xhr.status < 300) { | |
| const response = JSON.parse(this.responseText); | |
| console.log(`Playtime: Score: ${response[0].score} Label: ${response[0].label}`); | |
| let value = getShowScore(response[0].label, response[0].score); | |
| updateNeedle(value); | |
| updateLinearBar(updateCommulativeScore(scores,value)); | |
| } else { | |
| console.error('Error in request:', xhr.status, xhr.statusText); | |
| } | |
| }; | |
| xhr.onerror = function() { | |
| console.error('Request failed'); | |
| }; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
docs/samples/browser-plugin-meetings/style.css (2)
458-466: Remove commented code and improve margin handlingThe
.box-sentimentclass contains:
- Commented out background-color that should be removed
- Hard-coded margin values that could affect responsiveness
Suggested improvements:
.box-sentiment { display: flex; justify-content: center; align-items: center; flex-direction: column; - margin-bottom: 50px; - margin-left: 70px; + margin: clamp(1rem, 3vw, 3rem); - /* background-color: #982e2e; */ }
469-478: Maintain consistent styling with box-sentimentThe
.progress-containerhas similar issues to.box-sentiment:
- Contains unnecessary commented code
- Uses hard-coded margin values
Suggested improvements for consistency:
.progress-container { display: flex; justify-content: left; text-align: center; align-items: center; flex-direction: column; - margin-bottom: 77px; - margin-left: 40px; + margin: clamp(1rem, 3vw, 3rem); - /* background-color: #c0d646; */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/samples/browser-plugin-meetings/style.css(1 hunks)
🔇 Additional comments (1)
docs/samples/browser-plugin-meetings/style.css (1)
480-502:
Improve speedometer image handling and scaling
Several concerns with the speedometer implementation:
- The background image path is relative and might break when deployed
- Fixed dimensions could affect scalability
- Needle positioning uses fixed percentages
Suggested improvements:
#speedometer {
- width: 25rem;
- height: 19rem;
+ width: min(25rem, 90vw);
+ aspect-ratio: 25/19;
background-image: url("speedometer1.png");
background-size: 100%, 100%;
background-repeat: no-repeat;
position: relative;
}
#needle {
- width: 3.3px;
- height: 80px;
+ width: calc(0.33% + 2px);
+ height: 40%;
border: 1.5px solid white;
border-radius: 5px;
background-color: rgb(248, 243, 243);
position: absolute;
top: 28%;
left: 48%;
z-index: 111;
transform-origin: bottom;
transform: rotate(-135deg);
transition: 1s;
}Also, consider using an SVG instead of a background image for better scaling and performance.
Let's verify the image path:
| display: flex; | ||
| /* justify-content: space-between; */ | ||
| gap: 150px; | ||
| align-items: center; | ||
| height: 31rem; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving responsive design
The .sentiment class has potential responsiveness issues:
- Fixed height (
31rem) might not work well on different screen sizes - Hard-coded gap (
150px) could break layout on mobile devices
Consider this more responsive approach:
.sentiment {
display: flex;
- /* justify-content: space-between; */
- gap: 150px;
+ gap: clamp(1rem, 5vw, 150px);
align-items: center;
- height: 31rem;
+ min-height: 31rem;
+ flex-wrap: wrap;
}📝 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.
| .sentiment { | |
| display: flex; | |
| /* justify-content: space-between; */ | |
| gap: 150px; | |
| align-items: center; | |
| height: 31rem; | |
| } | |
| .sentiment { | |
| display: flex; | |
| gap: clamp(1rem, 5vw, 150px); | |
| align-items: center; | |
| min-height: 31rem; | |
| flex-wrap: wrap; | |
| } |
| .progress-bar { | ||
| width: 21rem; | ||
| height: 5rem; | ||
| border: 2px solid black; | ||
| border-radius: 10px; | ||
| display: flex; | ||
| overflow: hidden; | ||
| position: relative; | ||
| } | ||
|
|
||
| .red, .yellow, .green, .redYellowBlendWidth, .yellowGreenbLENDwidth { | ||
| height: 100%; | ||
| transition: width 0.5s ease; | ||
| position: left; | ||
| top: 0; | ||
| bottom: 0; | ||
| } | ||
|
|
||
| .red { | ||
| background-color: red; | ||
| } | ||
|
|
||
| .yellow { | ||
| background-color: yellow; | ||
| } | ||
|
|
||
| .green { | ||
| background-color: green; | ||
| } | ||
|
|
||
| .redYellowBlendWidth { | ||
| /* background-color: rgb(215, 146, 43); */ | ||
| background: linear-gradient(to right, red, yellow); | ||
| } | ||
|
|
||
| .yellowGreenbLENDwidth { | ||
| background: linear-gradient(to right, yellow, green); | ||
| } |
There was a problem hiding this comment.
Fix CSS issues and naming consistency
Several issues in the progress bar implementation:
- Fixed dimensions affect responsiveness
- Invalid CSS property
position: left - Inconsistent class naming (bLEND vs Blend)
Apply these fixes:
.progress-bar {
- width: 21rem;
+ width: min(21rem, 90vw);
height: 5rem;
border: 2px solid black;
border-radius: 10px;
display: flex;
overflow: hidden;
position: relative;
}
.red, .yellow, .green, .redYellowBlendWidth, .yellowGreenbLENDwidth {
height: 100%;
transition: width 0.5s ease;
- position: left;
+ position: relative;
top: 0;
bottom: 0;
}
-.yellowGreenbLENDwidth {
+.yellowGreenBlendWidth {
background: linear-gradient(to right, yellow, green);
}📝 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.
| .progress-bar { | |
| width: 21rem; | |
| height: 5rem; | |
| border: 2px solid black; | |
| border-radius: 10px; | |
| display: flex; | |
| overflow: hidden; | |
| position: relative; | |
| } | |
| .red, .yellow, .green, .redYellowBlendWidth, .yellowGreenbLENDwidth { | |
| height: 100%; | |
| transition: width 0.5s ease; | |
| position: left; | |
| top: 0; | |
| bottom: 0; | |
| } | |
| .red { | |
| background-color: red; | |
| } | |
| .yellow { | |
| background-color: yellow; | |
| } | |
| .green { | |
| background-color: green; | |
| } | |
| .redYellowBlendWidth { | |
| /* background-color: rgb(215, 146, 43); */ | |
| background: linear-gradient(to right, red, yellow); | |
| } | |
| .yellowGreenbLENDwidth { | |
| background: linear-gradient(to right, yellow, green); | |
| } | |
| .progress-bar { | |
| width: min(21rem, 90vw); | |
| height: 5rem; | |
| border: 2px solid black; | |
| border-radius: 10px; | |
| display: flex; | |
| overflow: hidden; | |
| position: relative; | |
| } | |
| .red, .yellow, .green, .redYellowBlendWidth, .yellowGreenbLENDwidth { | |
| height: 100%; | |
| transition: width 0.5s ease; | |
| position: relative; | |
| top: 0; | |
| bottom: 0; | |
| } | |
| .red { | |
| background-color: red; | |
| } | |
| .yellow { | |
| background-color: yellow; | |
| } | |
| .green { | |
| background-color: green; | |
| } | |
| .redYellowBlendWidth { | |
| /* background-color: rgb(215, 146, 43); */ | |
| background: linear-gradient(to right, red, yellow); | |
| } | |
| .yellowGreenBlendWidth { | |
| background: linear-gradient(to right, yellow, green); | |
| } |
COMPLETES #< INSERT LINK TO ISSUE >
This pull request addresses
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
I certified that
I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly
Make sure to have followed the contributing guidelines before submitting.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style