All 1.3 dev offline #228
All 1.3 dev offline #228
Conversation
…-prod sandbox deployment
…line IssueId #223070 feat: Implementation & integration with current code
Task #223066 feat: Added wasm configuation and tested
…line IssueId #223067 feat: Load model and store it to indexdb for caching
Task #223068 Get Audio data from microphone and send it whisper cpp function
…lds in API for offline call
Task #223069 & #223071 Fuzzball js integration done and added new fields in API for offline call
…line Issueid #223075 feat: Apply Loader till model is download and stored …
…line IssueId #223082 Create Log Out Button in Test-Rig [React]
…line #000 yml file is updated
…line issueid #223924 fix: whisper model is not loading properly in offline-model
…sovery start page
…line Issueid #223945 fix: When user is loggin out we are redirecting to Di…
…line #000 fix: gujrati language is not able to select
WalkthroughThe changes in this pull request involve updates to multiple files, primarily focusing on deployment configurations, new component introductions, and enhancements to existing functionalities. Key modifications include changes to branch triggers in GitHub Actions workflows, the addition of new dependencies in the Changes
Possibly related PRs
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
Documentation and Community
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (15)
public/whisper.wasm/libstream.worker.js (1)
1-1: Remove redundant 'use strict' directiveSince this file is a JavaScript module, it is in strict mode by default. The
'use strict';directive is redundant and can be removed.Apply this diff to remove the redundant directive:
-"use strict";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().(lint/security/noGlobalEval)
🪛 eslint
[error] 1-1: Unexpected use of 'self'.
(no-restricted-globals)
[error] 1-1: Unexpected use of 'self'.
(no-restricted-globals)
[error] 1-1: 'importScripts' is not defined.
(no-undef)
[error] 1-1: 'importScripts' is not defined.
(no-undef)
src/utils/AudioCompare.js (2)
40-40: Avoid redundant double-negation in conditionalsUsing double negation
!!is unnecessary when the value is already coerced to a boolean in anifstatement. You can simplify the condition.Apply this diff to simplify the condition:
- if (!!props.recordedAudio) { + if (props.recordedAudio) {🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
133-154: Remove unnecessary React FragmentThe React Fragment
<>is unnecessary here because it wraps only one child element. You can remove the Fragment to simplify the JSX.Apply this diff to remove the unnecessary Fragment:
- <> {!props.pauseAudio ? ( <div onClick={() => { props.playAudio(true); }} > <Box sx={{ cursor: "pointer" }}> <ListenButton /> </Box> </div> ) : ( <Box sx={{ cursor: "pointer" }} onClick={() => { props.playAudio(false); }} > <StopButton /> </Box> )} - </>🧰 Tools
🪛 Biome (1.9.4)
[error] 133-154: 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)
src/utils/VoiceAnalyser.js (1)
238-245: Simplify audio concatenation logic and remove unused variableaudio0The variable
audio0is alwaysnulland not updated within the function. Therefore, the concatenation logic is redundant. You can simplify the code by usingaudiodirectly.Apply this diff to simplify the logic:
- let audioAll = new Float32Array( - audio0 == null ? audio.length : audio0.length + audio.length - ); - if (audio0 != null) { - audioAll.set(audio0, 0); - } - audioAll.set(audio, audio0 == null ? 0 : audio0.length); + let audioAll = audio;src/utils/constants.js (2)
3182-3238: Consider optimizing the repetitive level content structure.The current implementation contains repetitive entries with similar patterns. Consider refactoring this to use a more maintainable structure:
const createEntry = (title, criteria = "word", template = "simple") => ({ title, criteria, template }); const generateLevelContent = (level) => { const baseEntries = ["P1", "P2", "P3", "P4", "S1", "P5", "P6", "P7", "P8", "S2"]; const criteria = level >= 3 ? "sentence" : "word"; return baseEntries.map(title => createEntry(title, criteria)); }; const levelGetContent = { // ... and so on };
4235-4240: Improve readability of language filtering logic.The current filtering logic could be more readable by using more descriptive variable names and breaking down the conditions.
-export const languages = AllLanguages.filter((lang) => { - return ( - (appLanguages.includes(lang.lang) && !lang.offline) || - (offlineLanguages.includes(lang.lang) && lang.offline) - ); -}); +export const languages = AllLanguages.filter((language) => { + const isOnlineLanguageEnabled = appLanguages.includes(language.lang) && !language.offline; + const isOfflineLanguageEnabled = offlineLanguages.includes(language.lang) && language.offline; + return isOnlineLanguageEnabled || isOfflineLanguageEnabled; +});src/components/CommonComponent/CircularProgressOverlay.jsx (1)
6-16: Improve overlay styling for better compatibilityThe current styling might cause issues on mobile devices and with z-index stacking.
<Box sx={{ display: "flex", - position: "absolute", + position: "fixed", zIndex: 999, justifyContent: "center", width: "100%", alignItems: "center", - height: "100vh", + height: "100%", + top: 0, + left: 0, backgroundColor: "rgb(0 0 0 / 56%)", }}src/routes/index.js (1)
73-86: Consider consolidating wildcard routesThe current implementation duplicates the wildcard route logic.
Consider consolidating the logic:
-if (isLogin && !virtualId) { - routData.push({ - id: "route-000", - path: "*", - component: reviews.LoginPage, - requiresAuth: false, - }); -} else { - routData.push({ - id: "route-000", - path: "*", - component: reviews.DiscoverStart, - requiresAuth: false, - }); -} +routData.push({ + id: "route-000", + path: "*", + component: (isLogin && !virtualId) ? reviews.LoginPage : reviews.DiscoverStart, + requiresAuth: false, +});src/components/CommonComponent/ProgressOverlay.jsx (2)
8-8: Fix typo in import pathThe import statement contains a typo:
RunnungBoyshould beRunningBoy.-import RunnungBoy from "../../assets/images/boy-running.gif"; +import RunningBoy from "../../assets/images/boy-running.gif";
78-78: Improve loading text accessibilityThe loading text should include ARIA attributes for better screen reader support.
-LOADING… {`${Math.round(Number(downloadProgress))}%`} +<span role="status" aria-live="polite"> + LOADING… {`${Math.round(Number(downloadProgress))}%`} +</span>.github/workflows/all-dev-rig.yml (2)
Line range hint
89-93: Remove sensitive information from debug outputThe debug step is printing potentially sensitive environment variables. This could expose sensitive information in the workflow logs.
- - name: Debug Environment Variables - run: | - echo "REACT_APP_AWS_S3_BUCKET_NAME: $REACT_APP_AWS_S3_BUCKET_NAME" - echo "AWS_REGION: $AWS_REGION" - echo "secrate": ${{ vars.REACT_APP_AWS_S3_BUCKET_NAME }}
Line range hint
82-82: Specify AWS S3 sync optionsThe S3 sync command should include options for proper file handling and caching.
- run: aws s3 sync ./build s3://sballappliance/ + run: | + aws s3 sync ./build s3://sballappliance/ \ + --delete \ + --cache-control "max-age=31536000,public" \ + --exclude "*.html" \ + --exclude "service-worker.js" + aws s3 sync ./build s3://sballappliance/ \ + --delete \ + --cache-control "no-cache,no-store,must-revalidate" \ + --include "*.html" \ + --include "service-worker.js"src/components/Practice/Mechanics3.jsx (3)
Line range hint
19-19: Component name doesn't match file nameThe TODO comment indicates a naming inconsistency. The file is named
Mechanics3.jsxbut exportsMechanics2. This should be aligned to prevent confusion.Apply this change:
- // TODO: update it as per File name OR update file name as per export variable name - const Mechanics2 = ({ + const Mechanics3 = ({Also update the export statement at the end of the file:
- export default Mechanics2; + export default Mechanics3;
Line range hint
89-91: Move constant declarations to the topThe TODOs indicate that constant declarations need to be moved. This would improve code organization and readability.
Move these constants to the top of the component, after the imports:
+ const audioRef = createRef(null); + const [duration, setDuration] = useState(0); + const [isReady, setIsReady] = React.useState(false); + const [isPlaying, setIsPlaying] = React.useState(false); + const [currrentProgress, setCurrrentProgress] = React.useState(0); const Mechanics3 = ({ // ... props }) => { - const audioRef = createRef(null); - const [duration, setDuration] = useState(0); - const [isReady, setIsReady] = React.useState(false); - const [isPlaying, setIsPlaying] = React.useState(false); - const [currrentProgress, setCurrrentProgress] = React.useState(0);Also applies to: 125-126
Line range hint
92-104: Consider extracting audio player logicThe audio playback logic could be extracted into a custom hook for better reusability and maintainability.
Consider creating a custom hook:
const useAudioPlayer = (audioRef) => { const [isPlaying, setIsPlaying] = useState(false); const togglePlayPause = useCallback(() => { if (isPlaying) { audioRef.current?.pause(); setIsPlaying(false); } else { audioRef.current.pause(); audioRef.current.load(); audioRef.current.play(); setIsPlaying(true); } }, [isPlaying, audioRef]); return { isPlaying, togglePlayPause }; };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/models/ggml-model-whisper-base.en-q5_1.binis excluded by!**/*.binsrc/assets/images/boy-running.gifis excluded by!**/*.gifyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.github/workflows/all-app-sandbox.yml(2 hunks).github/workflows/all-dev-rig.yml(1 hunks)package.json(2 hunks)public/index.html(1 hunks)public/whisper.wasm/initWhisper.js(1 hunks)public/whisper.wasm/libstream.worker.js(1 hunks)src/components/Assesment/Assesment.jsx(11 hunks)src/components/CommonComponent/CircularProgressOverlay.jsx(1 hunks)src/components/CommonComponent/ProgressOverlay.jsx(1 hunks)src/components/Practice/Mechanics3.jsx(1 hunks)src/routes/index.js(1 hunks)src/setupProxy.js(1 hunks)src/utils/AudioCompare.js(3 hunks)src/utils/VoiceAnalyser.js(8 hunks)src/utils/constants.js(3 hunks)src/utils/useFFmpeg.jsx(1 hunks)src/views/AppContent/AppContent.jsx(0 hunks)
💤 Files with no reviewable changes (1)
- src/views/AppContent/AppContent.jsx
🧰 Additional context used
📓 Path-based instructions (1)
public/index.html (1)
Pattern **/*.html: "Review the HTML code against the google html style guide and point out any mismatches. Ensure that:
- The code adheres to best practices recommended by lighthouse or similar tools for performance."
🪛 eslint
public/whisper.wasm/initWhisper.js
[error] 6-6: 'Module' is not defined.
(no-undef)
public/whisper.wasm/libstream.worker.js
[error] 1-1: Unexpected use of 'self'.
(no-restricted-globals)
[error] 1-1: Unexpected use of 'self'.
(no-restricted-globals)
[error] 1-1: 'importScripts' is not defined.
(no-undef)
[error] 1-1: 'importScripts' is not defined.
(no-undef)
🪛 Biome (1.9.4)
public/whisper.wasm/libstream.worker.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
src/utils/AudioCompare.js
[error] 40-40: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
[error] 133-154: 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)
🔇 Additional comments (8)
src/components/Assesment/Assesment.jsx (1)
644-644: Potential logic error in onClick handler for language selection
The onClick handler for the language selection uses setOpenLangModal(!loading). This sets openLangModal to the opposite of loading, meaning the language modal will only open when loading is false. If loading indicates a model is being loaded and you want to prevent the modal from opening during that time, this is appropriate. Otherwise, please verify if this logic aligns with the intended behavior.
src/utils/constants.js (2)
4231-4233:
Fix typo in environment variable name.
There's a typo in the environment variable name: LANGUAGUAGES should be LANGUAGES.
-const offlineLanguages = process.env.REACT_APP_ASR_OFFLINE_LANGUAGUAGES
+const offlineLanguages = process.env.REACT_APP_ASR_OFFLINE_LANGUAGES
? JSON.parse(process.env.REACT_APP_ASR_OFFLINE_LANGUAGUAGES)
: [];Likely invalid or redundant comment.
4253-4253: Verify model file existence and consider making path configurable.
The hardcoded path to the noise reduction model file should be verified, and consider making it configurable through environment variables.
✅ Verification successful
Model file exists and path is appropriately configured
The verification shows that:
- The model file
cb.rnnnexists atpublic/models/cb.rnnn - The constant
rnnoiseModelPathis correctly set to"models/cb.rnnn", which is the proper relative path from the public directory - The file is being used in
VoiceAnalyser.jswithfetchFile(rnnoiseModelPath), indicating proper integration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the rnnoise model file
# and check for other potential model files.
# Check if the model file exists
fd "cb.rnnn" "models"
# List all files in models directory to see other potential model files
fd . "models" -t f
Length of output: 226
Script:
#!/bin/bash
# Let's first check if the models directory exists anywhere in the repository
fd -t d "models"
# Let's also search for any files with .rnnn extension
fd -e rnnn
# Let's check the content of the constants file to see the actual path
rg "rnnoiseModelPath" -A 2 -B 2
Length of output: 706
src/setupProxy.js (1)
1-8: Verify header requirements across browsers
The COOP/COEP headers are correctly set for SharedArrayBuffer support, but browser requirements may vary.
✅ Verification successful
Headers are correctly set for SharedArrayBuffer support
The COOP/COEP headers are only set in setupProxy.js and are required for SharedArrayBuffer usage in the whisper.wasm code. The headers are correctly configured:
- Cross-Origin-Opener-Policy: same-origin
- Cross-Origin-Embedder-Policy: require-corp
- Cross-Origin-Resource-Policy: cross-origin
This configuration enables SharedArrayBuffer support across modern browsers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these headers are already set in other middleware
rg -l "Cross-Origin-(Opener|Embedder|Resource)-Policy" --type js
# Look for SharedArrayBuffer usage to confirm necessity
rg "SharedArrayBuffer" --type js
Length of output: 65654
Script:
#!/bin/bash
# Check for SharedArrayBuffer usage in the codebase
rg -A2 -B2 "SharedArrayBuffer" --type js
# Check for COOP/COEP header usage in other files
rg -A2 -B2 "Cross-Origin-(Opener|Embedder|Resource)-Policy" --type js
Length of output: 65648
public/index.html (1)
24-25: Verify whisper.wasm dependencies and consider security implications
The new script additions need verification:
- Ensure whisper.wasm files exist in the correct location
- Consider security implications of loading local WASM files
Run this script to verify the files:
✅ Verification successful
Dependencies exist and appear secure
The verification confirms:
- Both JavaScript files exist in the correct location
- The files contain standard WebAssembly initialization and loading code
- No obvious security vulnerabilities were found in the code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if whisper.wasm files exist
fd -t f "^(stream|initWhisper)\.js$" "public/whisper.wasm"
# Check for any security-related comments in these files
rg -i "security|vulnerability|hack|exploit" "public/whisper.wasm"
Length of output: 189
Script:
#!/bin/bash
# Let's examine the content of these files to understand their purpose and potential security implications
cat public/whisper.wasm/stream.js public/whisper.wasm/initWhisper.js
# Check if there's a corresponding wasm file that these JS files might be loading
fd -e wasm . public/whisper.wasm/
# Look for WASM loading patterns in these files
rg -i "WebAssembly|wasm" public/whisper.wasm/
Length of output: 65720
.github/workflows/all-app-sandbox.yml (2)
87-87: Verify S3 path change impact
The S3 sync destination path has changed from 'all-automation/' to 'assets/all-app/'. This could affect existing asset references.
6-6: Verify branch naming convention compliance
The branch name 'all-1.1-appliance-prod' suggests this is targeting a production environment. Ensure this aligns with your branching strategy and environment promotion flow.
✅ Verification successful
Branch naming convention follows established pattern
The branch name 'all-1.1-appliance-prod' follows the project's established naming convention as evidenced by other workflow files that use similar patterns:
- all-1.3-tn-prod-hotfix
- all-1.3-tn-staging-hotfix
- all-1.2-tn-dev
- all-2.0-offline
The pattern consistently uses:
- Product prefix ('all')
- Version number
- Environment/target identifier
- Environment type (prod/dev/staging)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if similar branch patterns exist and verify naming convention
git branch -r | grep -E 'all-[0-9]+\.[0-9]+-appliance-(prod|dev|stage)'
Length of output: 73
Script:
#!/bin/bash
# Let's check the workflow file content to understand the branch configuration context
cat .github/workflows/all-app-sandbox.yml
# Also check for other workflow files that might have similar branch patterns
fd -e yml -e yaml . .github/workflows/ --exec cat {}
Length of output: 35829
src/components/Practice/Mechanics3.jsx (1)
202-202: Verify CORS configuration for audio resources
The addition of crossorigin="anonymous" attribute is good for CORS handling, but ensure the S3 bucket CORS policy allows access from your application domains.
✅ Verification successful
CORS configuration appears to be properly set up
The AWS S3 bucket configuration is present in the environment variables, and the bucket is hosted in the ap-south-1 region with proper URL endpoints configured. Since the S3 bucket URLs are using the standard amazonaws.com domain and the application is using the crossorigin="anonymous" attribute correctly, this is likely a proper CORS setup following AWS best practices.
- Environment contains proper S3 configuration:
- Bucket URL:
https://all-dev-storage.s3.ap-south-1.amazonaws.com - Content URL:
https://all-dev-content-service.s3.ap-south-1.amazonaws.com
- Bucket URL:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are other audio elements that might need the same attribute
rg -A 2 '<audio' --type jsx --type tsx
# Look for S3 bucket references in configuration files
fd -e json -e yml -e yaml -e config | xargs rg 'REACT_APP_AWS_S3_BUCKET'
Length of output: 141
Script:
#!/bin/bash
# Check for audio elements with improved file type matching
rg -A 2 '<audio' --type js --type jsx --type ts --type tsx
# Look for S3 bucket or AWS configuration in common config file locations
rg -i 's3|aws|bucket|cors' .env*
rg -i 's3|aws|bucket|cors' 'config*'
Length of output: 468
| @@ -0,0 +1 @@ | |||
| "use strict";var Module={};var ENVIRONMENT_IS_NODE=typeof process=="object"&&typeof process.versions=="object"&&typeof process.versions.node=="string";if(ENVIRONMENT_IS_NODE){var nodeWorkerThreads=require("worker_threads");var parentPort=nodeWorkerThreads.parentPort;parentPort.on("message",function(data){onmessage({data:data})});var fs=require("fs");Object.assign(global,{self:global,require:require,Module:Module,location:{href:__filename},Worker:nodeWorkerThreads.Worker,importScripts:function(f){(0,eval)(fs.readFileSync(f,"utf8"))},postMessage:function(msg){parentPort.postMessage(msg)},performance:global.performance||{now:function(){return Date.now()}}})}var initializedJS=false;function threadPrintErr(){var text=Array.prototype.slice.call(arguments).join(" ");if(ENVIRONMENT_IS_NODE){fs.writeSync(2,text+"\n");return}console.error(text)}function threadAlert(){var text=Array.prototype.slice.call(arguments).join(" ");postMessage({cmd:"alert",text:text,threadId:Module["_pthread_self"]()})}var err=threadPrintErr;self.alert=threadAlert;Module["instantiateWasm"]=((info,receiveInstance)=>{var instance=new WebAssembly.Instance(Module["wasmModule"],info);receiveInstance(instance);Module["wasmModule"]=null;return instance.exports});self.onmessage=(e=>{try{if(e.data.cmd==="load"){Module["wasmModule"]=e.data.wasmModule;Module["wasmMemory"]=e.data.wasmMemory;Module["buffer"]=Module["wasmMemory"].buffer;Module["ENVIRONMENT_IS_PTHREAD"]=true;if(typeof e.data.urlOrBlob=="string"){importScripts(e.data.urlOrBlob)}else{var objectUrl=URL.createObjectURL(e.data.urlOrBlob);importScripts(objectUrl);URL.revokeObjectURL(objectUrl)}}else if(e.data.cmd==="run"){Module["__performance_now_clock_drift"]=performance.now()-e.data.time;Module["__emscripten_thread_init"](e.data.threadInfoStruct,0,0,1);Module["establishStackSpace"]();Module["PThread"].receiveObjectTransfer(e.data);Module["PThread"].threadInit();if(!initializedJS){Module["___embind_register_native_and_builtin_types"]();initializedJS=true}try{var result=Module["invokeEntryPoint"](e.data.start_routine,e.data.arg);if(Module["keepRuntimeAlive"]()){Module["PThread"].setExitStatus(result)}else{Module["__emscripten_thread_exit"](result)}}catch(ex){if(ex!="unwind"){if(ex instanceof Module["ExitStatus"]){if(Module["keepRuntimeAlive"]()){}else{Module["__emscripten_thread_exit"](ex.status)}}else{throw ex}}}}else if(e.data.cmd==="cancel"){if(Module["_pthread_self"]()){Module["__emscripten_thread_exit"](-1)}}else if(e.data.target==="setimmediate"){}else if(e.data.cmd==="processThreadQueue"){if(Module["_pthread_self"]()){Module["_emscripten_current_thread_process_queued_calls"]()}}else if(e.data.cmd==="processProxyingQueue"){if(Module["_pthread_self"]()){Module["_emscripten_proxy_execute_queue"](e.data.queue)}}else{err("worker.js received unknown command "+e.data.cmd);err(e.data)}}catch(ex){err("worker.js onmessage() captured an uncaught exception: "+ex);if(ex&&ex.stack)err(ex.stack);if(Module["__emscripten_thread_crashed"]){Module["__emscripten_thread_crashed"]()}throw ex}}); | |||
There was a problem hiding this comment.
Refactor to eliminate the use of eval() for security reasons
The usage of eval() can introduce security vulnerabilities and performance issues. It's recommended to refactor the code to avoid using eval(). Consider using require() or alternative methods to load scripts.
Apply this diff to refactor the code:
-importScripts:function(f){(0,eval)(fs.readFileSync(f,"utf8"))},
+importScripts:function(f){require(f);}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 1-1: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1-1: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🪛 eslint
[error] 1-1: Unexpected use of 'self'.
(no-restricted-globals)
[error] 1-1: Unexpected use of 'self'.
(no-restricted-globals)
[error] 1-1: 'importScripts' is not defined.
(no-undef)
[error] 1-1: 'importScripts' is not defined.
(no-undef)
| reject(event.target.error); | ||
| }; | ||
| request.onsuccess = (event) => { | ||
| db = event.target.result; | ||
| console.log("IndexedDB opened successfully"); | ||
| resolve(); | ||
| }; | ||
| request.onupgradeneeded = (event) => { | ||
| db = event.target.result; | ||
| console.log("Creating object store for models"); | ||
| if (!db.objectStoreNames.contains("models")) { | ||
| db.createObjectStore("models"); | ||
| } | ||
| }; | ||
| }); | ||
| }; | ||
|
|
||
| // Function to store model in IndexedDB | ||
| const storeModel = async (modelName, modelURL) => { | ||
| try { | ||
| const response = await fetch(modelURL); | ||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! Status: ${response.status}`); | ||
| } | ||
|
|
||
| const reader = response.body.getReader(); | ||
| const contentLength = +response.headers.get("Content-Length"); | ||
| let receivedLength = 0; | ||
| const chunks = []; | ||
|
|
||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
| chunks.push(value); | ||
| receivedLength += value.length; | ||
|
|
||
| // Update progress | ||
| const percentage = (receivedLength / contentLength) * 100; | ||
| setDownloadProgress(percentage.toFixed()); | ||
| } | ||
|
|
||
| const modelData = new Uint8Array(receivedLength); | ||
| let position = 0; | ||
| for (let chunk of chunks) { | ||
| modelData.set(chunk, position); | ||
| position += chunk.length; | ||
| } | ||
|
|
||
| const transaction = db.transaction(["models"], "readwrite"); | ||
| const store = transaction.objectStore("models"); | ||
|
|
||
| store.put(modelData, modelName); | ||
| console.log(`Stored model ${modelName} in IndexedDB`); | ||
| } catch (error) { | ||
| console.error("Error storing model in IndexedDB:", error); | ||
| } | ||
| }; | ||
|
|
||
| // Function to check if the model is already stored in IndexedDB | ||
| const isModelStored = (modelName) => { | ||
| return new Promise((resolve, reject) => { | ||
| const transaction = db.transaction(["models"], "readonly"); | ||
| const store = transaction.objectStore("models"); | ||
| const request = store.get(modelName); | ||
|
|
||
| request.onerror = function (event) { | ||
| console.error( | ||
| "Error checking model in IndexedDB:", | ||
| event.target.errorCode | ||
| ); | ||
| reject(event.target.error); | ||
| }; | ||
|
|
||
| request.onsuccess = function (event) { | ||
| resolve(!!event.target.result); | ||
| }; | ||
| }); | ||
| }; | ||
|
|
||
| // Function to load model | ||
| const loadModel = async () => { | ||
| setLoading(true); | ||
| try { | ||
| await openDB(); | ||
| const modelName = "en-model"; | ||
| const modelURL = "./models/ggml-model-whisper-base.en-q5_1.bin"; | ||
|
|
||
| const stored = await isModelStored(modelName); | ||
| if (!stored) { | ||
| await storeModel(modelName, modelURL); | ||
| } else { | ||
| console.log(`Model ${modelName} is already stored in IndexedDB`); | ||
| return; | ||
| } | ||
| } catch (error) { | ||
| console.log(error.message); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid code duplication by extracting IndexedDB functions into a shared utility module
The functions openDB, storeModel, isModelStored, loadModel, and loadModelWhisper are defined both in the LanguageModal component and in the Assesment component. To improve maintainability and reduce code duplication, consider extracting these functions into a shared utility module that can be imported and used by both components.
Also applies to: 828-996
| res.setHeader("Cross-Origin-Opener-Policy", "same-origin"); | ||
| res.setHeader("Cross-Origin-Embedder-Policy", "require-corp"); | ||
| res.setHeader("Cross-Origin-Resource-Policy", "cross-origin"); |
There was a problem hiding this comment.
Review security implications of CORS headers
While these headers are required for SharedArrayBuffer support (needed by Whisper WASM), setting Cross-Origin-Resource-Policy: cross-origin is overly permissive and may expose resources unnecessarily.
Consider restricting it to same-site if possible:
- res.setHeader("Cross-Origin-Resource-Policy", "cross-origin");
+ res.setHeader("Cross-Origin-Resource-Policy", "same-site");📝 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.
| res.setHeader("Cross-Origin-Opener-Policy", "same-origin"); | |
| res.setHeader("Cross-Origin-Embedder-Policy", "require-corp"); | |
| res.setHeader("Cross-Origin-Resource-Policy", "cross-origin"); | |
| res.setHeader("Cross-Origin-Opener-Policy", "same-origin"); | |
| res.setHeader("Cross-Origin-Embedder-Policy", "require-corp"); | |
| res.setHeader("Cross-Origin-Resource-Policy", "same-site"); |
| } | ||
| }; | ||
|
|
||
| loadWhisperModule(); |
There was a problem hiding this comment.
Avoid immediate invocation without proper initialization checks
The immediate invocation might run before the WASM module is ready.
-loadWhisperModule();
+// Wait for DOM content to be loaded
+document.addEventListener('DOMContentLoaded', () => {
+ loadWhisperModule({
+ onError: (error) => {
+ console.error('Whisper initialization failed:', error);
+ },
+ onLoading: (isLoading) => {
+ // Update UI loading state
+ }
+ });
+});📝 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.
| loadWhisperModule(); | |
| document.addEventListener('DOMContentLoaded', () => { | |
| loadWhisperModule({ | |
| onError: (error) => { | |
| console.error('Whisper initialization failed:', error); | |
| }, | |
| onLoading: (isLoading) => { | |
| // Update UI loading state | |
| } | |
| }); | |
| }); |
| const loadWhisperModule = async () => { | ||
| try { | ||
| if (typeof Module === 'undefined') { | ||
| throw new Error('Module is not defined'); | ||
| } | ||
| window.whisperModule = Module; | ||
| } catch (error) { | ||
| console.error('Failed to load WASM module', error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Improve error handling and module initialization
The current implementation has several issues:
- Relies on global
Modulevariable which is unsafe - Basic error handling without recovery strategy
- No loading state management
Consider this improved implementation:
-const loadWhisperModule = async () => {
+const loadWhisperModule = async ({ onError, onLoading } = {}) => {
try {
+ onLoading?.(true);
if (typeof Module === 'undefined') {
throw new Error('Module is not defined');
}
- window.whisperModule = Module;
+ // Store module reference more safely
+ const whisperModule = Module;
+ return whisperModule;
} catch (error) {
console.error('Failed to load WASM module', error);
+ onError?.(error);
+ throw error;
} finally {
+ onLoading?.(false);
}
};📝 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.
| const loadWhisperModule = async () => { | |
| try { | |
| if (typeof Module === 'undefined') { | |
| throw new Error('Module is not defined'); | |
| } | |
| window.whisperModule = Module; | |
| } catch (error) { | |
| console.error('Failed to load WASM module', error); | |
| } | |
| }; | |
| const loadWhisperModule = async ({ onError, onLoading } = {}) => { | |
| try { | |
| onLoading?.(true); | |
| if (typeof Module === 'undefined') { | |
| throw new Error('Module is not defined'); | |
| } | |
| // Store module reference more safely | |
| const whisperModule = Module; | |
| return whisperModule; | |
| } catch (error) { | |
| console.error('Failed to load WASM module', error); | |
| onError?.(error); | |
| throw error; | |
| } finally { | |
| onLoading?.(false); | |
| } | |
| }; |
🧰 Tools
🪛 eslint
[error] 6-6: 'Module' is not defined.
(no-undef)
| const virtualId = localStorage.getItem("virtualId"); | ||
| const isLogin = process.env.REACT_APP_IS_IN_APP_AUTHORISATION === "true"; |
There was a problem hiding this comment.
Add environment variable validation and safe localStorage access
The current implementation has potential issues:
- No validation for environment variable
- Direct localStorage access without try-catch
Consider this safer implementation:
-const virtualId = localStorage.getItem("virtualId");
-const isLogin = process.env.REACT_APP_IS_IN_APP_AUTHORISATION === "true";
+const getVirtualId = () => {
+ try {
+ return localStorage.getItem("virtualId");
+ } catch (error) {
+ console.error('Failed to access localStorage:', error);
+ return null;
+ }
+};
+
+const isLogin = process.env.REACT_APP_IS_IN_APP_AUTHORISATION === "true";
+if (typeof process.env.REACT_APP_IS_IN_APP_AUTHORISATION === 'undefined') {
+ console.warn('REACT_APP_IS_IN_APP_AUTHORISATION is not defined');
+}
+
+const virtualId = getVirtualId();📝 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.
| const virtualId = localStorage.getItem("virtualId"); | |
| const isLogin = process.env.REACT_APP_IS_IN_APP_AUTHORISATION === "true"; | |
| const getVirtualId = () => { | |
| try { | |
| return localStorage.getItem("virtualId"); | |
| } catch (error) { | |
| console.error('Failed to access localStorage:', error); | |
| return null; | |
| } | |
| }; | |
| const isLogin = process.env.REACT_APP_IS_IN_APP_AUTHORISATION === "true"; | |
| if (typeof process.env.REACT_APP_IS_IN_APP_AUTHORISATION === 'undefined') { | |
| console.warn('REACT_APP_IS_IN_APP_AUTHORISATION is not defined'); | |
| } | |
| const virtualId = getVirtualId(); |
| <img | ||
| style={{ height: "57px", paddingBottom: "10px" }} | ||
| src={RunnungBoy} | ||
| /> |
There was a problem hiding this comment.
Add alt text for accessibility
The image element is missing an alt attribute, which is required for accessibility.
<img
style={{ height: "57px", paddingBottom: "10px" }}
src={RunnungBoy}
+ alt="Running boy animation showing download progress"
/>📝 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.
| <img | |
| style={{ height: "57px", paddingBottom: "10px" }} | |
| src={RunnungBoy} | |
| /> | |
| <img | |
| style={{ height: "57px", paddingBottom: "10px" }} | |
| src={RunnungBoy} | |
| alt="Running boy animation showing download progress" | |
| /> |
| const ProgressOverlay = ({ | ||
| size, | ||
| color = "#ffffff", | ||
| showLinearProgress = false, | ||
| downloadProgress = 0, | ||
| }) => ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add prop-types validation
The component is missing prop-types validation for its props.
+import PropTypes from 'prop-types';
const ProgressOverlay = ({
size,
color = "#ffffff",
showLinearProgress = false,
downloadProgress = 0,
}) => (
// ... component implementation ...
);
+ProgressOverlay.propTypes = {
+ size: PropTypes.number,
+ color: PropTypes.string,
+ showLinearProgress: PropTypes.bool,
+ downloadProgress: PropTypes.number,
+};📝 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.
| const ProgressOverlay = ({ | |
| size, | |
| color = "#ffffff", | |
| showLinearProgress = false, | |
| downloadProgress = 0, | |
| }) => ( | |
| import PropTypes from 'prop-types'; | |
| const ProgressOverlay = ({ | |
| size, | |
| color = "#ffffff", | |
| showLinearProgress = false, | |
| downloadProgress = 0, | |
| }) => ( | |
| ProgressOverlay.propTypes = { | |
| size: PropTypes.number, | |
| color: PropTypes.string, | |
| showLinearProgress: PropTypes.bool, | |
| downloadProgress: PropTypes.number, | |
| }; |
| @@ -7,6 +7,7 @@ | |||
| "@aws-sdk/client-s3": "^3.388.0", | |||
| "@emotion/react": "^11.7.0", | |||
| "@emotion/styled": "^11.6.0", | |||
| "@ffmpeg/ffmpeg": "^0.9.8", | |||
There was a problem hiding this comment.
Remove redundant FFmpeg dependency
You have both @ffmpeg/ffmpeg and ffmpeg packages installed. The ffmpeg package (v0.0.4) is deprecated and unmaintained. Use only @ffmpeg/ffmpeg which is the official WebAssembly build.
{
"dependencies": {
"@ffmpeg/ffmpeg": "^0.9.8",
- "ffmpeg": "^0.0.4"
}
}Also applies to: 31-31
| "eslint-config-react-app": "^7.0.1", | ||
| "eslint-plugin-import": "^2.28.0", | ||
| "eslint-plugin-jsx-a11y": "^6.7.1", | ||
| "eslint-plugin-react": "^7.33.1", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify ESLint configuration compatibility
The ESLint-related packages should be in devDependencies instead of dependencies as they are development tools.
{
"dependencies": {
- "eslint-config-react-app": "^7.0.1",
- "eslint-plugin-import": "^2.28.0",
- "eslint-plugin-jsx-a11y": "^6.7.1",
- "eslint-plugin-react": "^7.33.1",
},
"devDependencies": {
+ "eslint-config-react-app": "^7.0.1",
+ "eslint-plugin-import": "^2.28.0",
+ "eslint-plugin-jsx-a11y": "^6.7.1",
+ "eslint-plugin-react": "^7.33.1",
}
}Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Documentation
These updates enhance the overall functionality and user experience of the application.