Refactor useChatWebhook hook for improved WebSocket connection management and error handling - #208
Conversation
…ment and error handling
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR enhances WebSocket lifecycle management with reconnection logic and message queueing in Changes
Sequence DiagramsequenceDiagram
participant React Hook as useChatWebhook
participant Queue as Message Queue
participant WebSocket as WebSocket Connection
participant App as Voice Chat App
rect rgb(200, 220, 255)
note over React Hook,App: Initial Connection & Auth Flow
App->>React Hook: Call sendMessage (first user message)
React Hook->>Queue: Queue message (socket CONNECTING)
React Hook->>WebSocket: connect() if not connected
WebSocket->>WebSocket: Initialize WebSocket
end
rect rgb(220, 250, 220)
note over React Hook,App: Connection Established
WebSocket-->>React Hook: onopen event
React Hook->>Queue: Flush queued messages
React Hook->>App: Call onOpen callback
App->>App: Read chat history via getChatHistory
App->>WebSocket: Send auth payload (if first message)
end
rect rgb(255, 240, 200)
note over React Hook,WebSocket: Reconnection Flow (on error/close)
WebSocket-->>React Hook: onerror/onclose event
React Hook->>React Hook: Increment reconnectCount
alt Attempts Remaining
React Hook->>React Hook: Wait reconnectInterval
React Hook->>WebSocket: Retry connect()
else Attempts Exhausted
React Hook->>App: Call onFinalReconnectAttempt
App->>App: Show confirmation popup
App->>App: Navigate/ResetChat
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
…stants, and page objects for improved test organization and maintainability. Enhance .gitignore to exclude Playwright reports and environment files.
…nnect functionality, final reconnect attempt callback, and message queuing for WebSocket connections. Update ShikshalokamVoiceChat component to utilize new features and improve WebSocket management.
…ated messages and updating dependency array in useEffect for better state management.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/ShikshalokamVoiceChat/voice-chat.js (1)
642-667: Potential race condition: authentication sent before WebSocket connection is established.
connectToWebSocket()is called butsendSocketMessage()for authentication is called immediately after without awaiting the connection. If the connection isn't established yet, the auth message may be queued or lost depending on the hook implementation.Additionally, this authentication logic duplicates what's in
onWebSocketOpen, leading to potential double authentication.Consider one of these approaches:
- Send authentication only in
onWebSocketOpencallback (which fires when connection is ready)- Or ensure the hook queues messages until connected
Looking at the
useChatWebhookdocstring provided, it appears messages are queued while connecting, but verify this handles the authentication message correctly. The duplication withonWebSocketOpen(lines 210-232) should be resolved to have a single source of truth for authentication.const chat_history = handleMessagesForUser(textMessage) if (chat_history.filter(chat => chat.source === "user").length == 1) { connectToWebSocket() - sendSocketMessage({ - type: "authenticate", - sessionid: sessionId, - profileid: profileToUse, - projectid: projectIdStore || searchParams.get("projectId") || "", - taskid: searchParams.get("taskId") || taskId, - access_token: accessToken, - route: chatLanguage, - bot_route: getSessionRoute(), - flow_name: storageFlow, - address: { - ipCity, - ipState, - ipZipCode, - }, - }) + // Authentication will be handled in onWebSocketOpen callback }
🧹 Nitpick comments (37)
tests/constant/ylc-chat.ts (1)
1-21: Conversation data is valid but looks placeholder-likeUsing repeated
"Hello World"values is fine for now, but consider replacing with more realistic, varied utterances once the flow stabilizes so tests better reflect real usage and are easier to understand.tsconfig.json (1)
1-24: TS config is reasonable; double‑check JS/alias needsThe config looks generally sane for Playwright + frontend (ES2022, node moduleResolution,
types: ["node", "@playwright/test"],baseUrl: "src"). If you plan to:
- Import JS files from TS tests, or
- Rely on the commented path aliases,
you may later want to enable
allowJsand/or uncomment thepathssection to keep editor tooling and builds aligned.tests/config/test-constants.ts (1)
1-211: Centralized test constants look coherent and consistentThe structure of
TIMEOUTS,SELECTORS,FIXTURE_PATHS,ENV, and others is clear and will help keep e2e tests DRY. Default timeouts and paths line up with the new Playwright ignores (downloads/,screenshots/,videos/), so this should integrate cleanly.Only optional future tweaks:
- If you ever need non‑HTTP
BASE_URLs, consider validating/normalizing it in one place.- Consider tying
API_ENDPOINTS.WEBSOCKETto whatever URL shapeuseChatWebhookultimately expects (e.g., prefixing withBASE_URL) in a helper, not necessarily here.No blocking issues from this file.
tests/constant/guest-chat.ts (1)
1-14: Guest chat content is fine; consider fixing minor typoThe conversation data looks realistic and appropriate for e2e flows. There’s a small spelling issue in:
"Total particpants are 32, ..."Consider correcting
"particpants"→"participants"to keep test data tidy, especially if it’s ever surfaced in screenshots or demos.tests/constant/ptm-chat.ts (1)
1-9: PTM conversation constant is OK but very placeholder‑ishThe repeated
"Hello World"values will work for basic flow coverage. When the PTM flow stabilizes, consider updating this array to more representative PTM dialog so failures are easier to diagnose from logs/screenshots.tests/e2e/flows/ptm/happy-path.spec.ts (1)
1-2: Remove commented import or document the reason.The commented fixture import suggests either incomplete migration or uncertainty about the correct import path. If the direct Playwright import on Line 2 is correct, remove the commented line to avoid confusion.
-// import { test, expect } from '../../../fixtures'; import { test, expect } from "@playwright/test"tests/pages/chat-interface-ptm.ts (2)
14-16: Consider more resilient test selectors.The locators rely on generic CSS classes (
ul.div11.pb-6) and third-party library classes (.swal2-confirm.swal2-styled). These selectors can be brittle:
- Generic class names like
div11andpb-6(Tailwind utility) may change with UI refactoring- SweetAlert2 classes could change with library updates
Consider adding
data-testidattributes to the application code for more stable, semantic selectors.
4-4: Class naming suggests PTM-specific but reused for YLC flow.The class is named
ChatInterfacePtmbut is also used in the YLC happy-path test (seetests/e2e/flows/ylc/happy-path.spec.tsline 29:chatInterfaceYlcPage = new ChatInterfacePtm(page)). If the functionality is shared, consider renaming to a more generic name likeChatInterfaceFormorChatInterfaceWithConfirm, or create separate classes if PTM and YLC have distinct behaviors.tests/e2e/flows/ylc/happy-path.spec.ts (4)
1-2: Remove commented imports.Similar to the PTM test, this file has a commented fixture import. Remove it to avoid confusion about the correct import path.
-// import { test, expect } from '../../../fixtures'; import { test, expect } from "@playwright/test"
5-5: Remove unused commented import.The commented import for
GUEST_CHAT_CONVERSATION_ENis not used in this test. Remove it to keep the code clean.-// import { GUEST_CHAT_CONVERSATION_EN } from 'tests/constant/guest-chat'; import { SITE_ROUTES } from "../../../constant/site_routes"
17-17: Fix variable naming mismatch.The variable
chatInterfaceYlcPageis instantiated fromChatInterfacePtmclass, creating confusion about whether this is YLC-specific or shared functionality. Either rename the variable to reflect the actual class or use a YLC-specific class if the behaviors differ.- let chatInterfaceYlcPage: ChatInterfacePtm + let chatInterfaceFormPage: ChatInterfacePtm // Or create ChatInterfaceYlc if behaviors differ
59-59: Inconsistent wait time compared to PTM test.This test uses a 10-second wait (line 59), while the PTM test uses 5 seconds for the same operation. Consider standardizing wait times across similar test flows or document why YLC requires longer waits.
tests/e2e/flows/improvement-story/happy-path.spec.ts (3)
1-2: Consider using the custom fixture instead of direct Playwright import.The commented-out import from
../../../fixturessuggests the custom browser fixture was intended. Using the fixture would enable CDP debugging support whenCHROME_DEBUG_PORTis set. If the fixture isn't needed, remove the commented line.-// import { test, expect } from '../../../fixtures'; -import { test, expect } from "@playwright/test" +import { test, expect } from "../../../fixtures"
71-77: Hardcoded 10-second wait may cause flaky or slow tests.The fixed 10-second wait doesn't adapt to actual response times. Consider using
waitForChatContainerCountor similar polling-based waits for all iterations. Also, the formula2 * i + 3is not self-documenting.for (let i = 0; i < IMPROVEMENT_STORY_CHAT_CONVERSATION_EN.length; i++) { await chatInterfacePage.sendMessage(IMPROVEMENT_STORY_CHAT_CONVERSATION_EN[i]) - await chatInterfacePage.wait(10000) - if (i != GUEST_CHAT_CONVERSATION_EN.length - 1) { - await chatInterfacePage.waitForChatContainerCount(2 * i + 3, 20000) + // Each message creates 2 containers (user + bot), starting from 3 initial + const expectedContainers = 2 * (i + 1) + 1 + if (i !== IMPROVEMENT_STORY_CHAT_CONVERSATION_EN.length - 1) { + await chatInterfacePage.waitForChatContainerCount(expectedContainers, 30000) } }
106-108: Empty test placeholder should be skipped or removed.This test provides no coverage and could mislead coverage metrics. Mark it as skipped with a TODO or remove it until implemented.
- test("should complete full flow with story generation and download using SSO", async () => { - await test.step("Redirect to Home Page from SSO Route", async () => {}) - }) + test.skip("should complete full flow with story generation and download using SSO", async () => { + // TODO: Implement SSO redirect flow test + })tests/helpers/story.helper.ts (1)
121-126: Sentence extraction is naive regarding abbreviations.The regex
[.!?]+will incorrectly split on abbreviations like "Dr.", "Mr.", "e.g.". For test validation purposes this may be acceptable, but worth noting if precise sentence counts are important.tests/e2e/flows/capture-discussion/happy-path.spec.ts (3)
1-6: Significant code duplication with improvement-story/happy-path.spec.ts.This test file shares ~90% of its structure with the improvement-story spec. Consider extracting common test steps into a shared helper or using Playwright's test parameterization to reduce maintenance burden.
71-77: Same hardcoded 10-second wait issue as noted in improvement-story spec.Apply the same fix to use adaptive waits instead of fixed delays.
106-108: Empty SSO test should be skipped.Same issue as improvement-story spec - mark as
test.skipuntil implemented.tests/pages/story-view.page.ts (1)
90-101: Path resolution is appropriate, but no file existence check.The path resolution logic handles relative paths well. However, if the file doesn't exist,
setInputFileswill fail with a less descriptive error. Consider adding a pre-check for better error messages in test failures.tests/pages/language-selection.page.ts (1)
135-149: Consider replacing hardcoded waits with condition-based waiting.The 500ms
wait()calls are test smells that can lead to flaky tests. Consider waiting for a specific condition (e.g., button becoming enabled or a visual state change) instead.async completeLanguageSelection(languageName: string): Promise<void> { await this.selectLanguageByName(languageName); - await this.wait(500); // Brief wait for UI state update + // Wait for continue button to be enabled after selection + await this.page.waitForFunction( + () => !(document.querySelector('[data-testid="continue-button"]') as HTMLButtonElement)?.disabled, + { timeout: 5000 } + ); await this.clickContinue(); } async completeLanguageSelectionByIndex(index: number): Promise<void> { await this.selectLanguageByIndex(index); - await this.wait(500); // Brief wait for UI state update + await this.page.waitForFunction( + () => !(document.querySelector('[data-testid="continue-button"]') as HTMLButtonElement)?.disabled, + { timeout: 5000 } + ); await this.clickContinue(); }tests/pages/flow-selection.page.ts (3)
25-32: Brittle CSS class selectors will break on styling changes.The complex CSS class selectors for
flowCards,languageContainer, andflowContinueButtonare fragile. Preferdata-testidattributes for test stability.- this.flowCards = page.locator("span.flex.items-center.gap-3.px-3.justify-center.sm\\:py-4.py-3.rounded-2xl.cursor-pointer.w-full") + this.flowCards = page.locator('[data-testid="flow-card"]') this.continueButton = page.locator('[data-testid="continue-button"]') - this.imageContainer = page.locator(".custom-login-image").first() - this.languageContainer = page.locator("div.div14-lang") - this.flowContinueButton = page.locator("button.mt-0.px-16.py-2.rounded-xl.text-white.text-lg.font-medium.flex.items-center") + this.imageContainer = page.locator('[data-testid="login-image"]') + this.languageContainer = page.locator('[data-testid="language-container"]') + this.flowContinueButton = page.locator('[data-testid="flow-continue-button"]')Coordinate with the frontend team to add these
data-testidattributes to the corresponding elements.
42-45: Remove commented-out code.The commented line should be removed to keep the codebase clean.
async getLanguageCount(): Promise<number> { - // await this.waitForElement(this.languageContainer.first()); return this.languageContainer.count() }
136-140: Replace hardcoded wait with condition-based waiting.Same concern as in
LanguageSelectionPage- the 500ms wait is a test smell.tests/pages/chat-interface.page.ts (2)
35-52: Preferdata-testidattributes over complex CSS class selectors.Several locators use fragile CSS class selectors that will break on styling changes. Consider adding
data-testidattributes to the application code for test stability.The following locators should use
data-testidattributes:
chatContainer(line 36)messageInput(line 37)sendButton(line 38)termsAndConditionsContainer(line 45)termsAndConditionsButton(line 46)uploadedImage(line 49)reportButton(line 50)editorSaveButton(line 51)
180-192: Silent error handling may hide timing issues.The empty catch block swallows any error from the loading indicator wait, making test failures harder to diagnose. Consider logging the reason for falling back.
async waitForBotResponse(timeout: number = 30000): Promise<void> { // Wait for loading indicator to appear and disappear try { await this.waitForElement(this.loadingIndicator, 5000); await this.waitForElementHidden(this.loadingIndicator, timeout); } catch { // If loading indicator doesn't appear, just wait for new message + console.debug('Loading indicator not found, waiting for bot message directly'); await this.wait(1000); } // Wait for new bot message await this.waitForElement(this.botMessages.last(), timeout); }src/pages/ShikshalokamVoiceChat/voice-chat.js (3)
202-208: Avoid logging sensitive data and use structured logging instead.Console.log statements with event data could inadvertently log sensitive WebSocket payloads. Consider using a proper logging utility with appropriate log levels.
const onWebSocketClose = useCallback(event => { - console.log("closed", event) + // Consider using a logging utility with debug level }, []) const onWebSocketError = useCallback(error => { - console.log("error", error) + // Consider using a logging utility with error level }, [])
828-831: Remove commented-out dead code.Commented-out
useEffectshould be removed to maintain code cleanliness.-// useEffect(() => { -// const chat_history = chatHistory.filter(chat => chat.source === "user").length > 1 -// }, [isFreshConnection, chatHistory])
1282-1291: Remove debug console.log statements before merging.These debug logging statements should be removed or converted to conditional debug logging for production code.
- console.log( - "unnarated messages", - sentences.filter(sent => !sent.isNarrated) - ) - console.log({ strandStep, stateMachineLength }) if (sentences.filter(sent => !sent.isNarrated).length > 0) returntests/helpers/image.helper.ts (2)
41-48:getImageSizethrows on non-existent files without explicit handling.Unlike
verifyImageExists, this method will throw if the file doesn't exist. Consider adding error handling or documenting this behavior in JSDoc./** * Get image file size in bytes * @param imagePath - Path to the image file * @returns File size in bytes + * @throws Error if file does not exist */ getImageSize(imagePath: string): number { + if (!this.verifyImageExists(imagePath)) { + throw new Error(`Image file not found: ${imagePath}`); + } const absolutePath = path.isAbsolute(imagePath) ? imagePath : path.resolve(process.cwd(), imagePath); const stats = fs.statSync(absolutePath); return stats.size; }
228-244: Add safety checks before deleting files incleanupDownloads.The method deletes all files in the specified directory. Consider adding safeguards to prevent accidental deletion of important files.
cleanupDownloads(downloadDir: string = 'downloads'): void { const dir = path.resolve(process.cwd(), downloadDir); + // Safety check: only clean known test download directories + if (!dir.includes('downloads') && !dir.includes('test')) { + console.warn(`Refusing to clean directory that doesn't appear to be a test directory: ${dir}`); + return; + } + if (fs.existsSync(dir)) { const files = fs.readdirSync(dir); files.forEach(file => { const filePath = path.join(dir, file); - if (fs.statSync(filePath).isFile()) { + // Only delete files, not subdirectories, and skip hidden files + if (fs.statSync(filePath).isFile() && !file.startsWith('.')) { fs.unlinkSync(filePath); } }); } }tests/helpers/websocket.helper.ts (4)
18-29: Remove unusedmessagesstate or wire it up
this.messagesis initialized and reset (constructor,startMonitoring,clearMessages) but never actually read; all callers go throughwindow.__wsMessagesviagetMessages. This is dead state and can confuse future maintainers.You can either remove the field and the related assignments, or update
getMessagesto keepthis.messagesin sync and read from it instead of directly from the page.- private messages: WebSocketMessage[]; private isMonitoring: boolean; @@ constructor(page: Page) { this.page = page; - this.messages = []; this.isMonitoring = false; } @@ async startMonitoring(): Promise<void> { @@ - this.isMonitoring = true; - this.messages = []; + this.isMonitoring = true; @@ async clearMessages(): Promise<void> { await this.page.evaluate(() => { (window as any).__wsMessages = []; }); - this.messages = []; }Also applies to: 221-226
42-55: Avoid clearing__wsMessageson every new WebSocket instanceInside the overridden
WebSocketconstructor you resetwindow.__wsMessages = []every time a new WebSocket is created. If the app reconnects or opens multiple sockets, previously captured messages are silently discarded, which can make debugging flaky flows harder.Consider initializing
__wsMessagesonce outside the constructor (or only when not already defined) and let explicit calls toclearMessages()control when history is reset.- await this.page.addInitScript(() => { - // Store original WebSocket - const OriginalWebSocket = window.WebSocket; - - // Override WebSocket constructor - (window as any).WebSocket = function(url: string, protocols?: string | string[]) { - const ws = new OriginalWebSocket(url, protocols); - - // Store websocket reference for testing - (window as any).__wsInstance = ws; - (window as any).__wsMessages = []; + await this.page.addInitScript(() => { + // Store original WebSocket + const OriginalWebSocket = window.WebSocket; + + // Initialize message store once + (window as any).__wsMessages = (window as any).__wsMessages || []; + + // Override WebSocket constructor + (window as any).WebSocket = function (url: string, protocols?: string | string[]) { + const ws = new OriginalWebSocket(url, protocols); + + // Store websocket reference for testing + (window as any).__wsInstance = ws; @@ - return ws; - }; + return ws; + }; });
174-197: Single-connection assumption in__wsInstancehelpers
isConnected/getConnectionStatework off a singlewindow.__wsInstance, which is always set to the most recently constructed WebSocket. If the app ever uses multiple sockets concurrently, these helpers will not reflect earlier ones.If multi-socket support becomes relevant, consider tracking an array of connections (with IDs or URLs) instead of a single
__wsInstance, or at least document that these methods are specific to the last-created socket.
250-255: Clarify or implementstopMonitoringbehavior
stopMonitoring()only flipsisMonitoring; it does not:
- restore the original
window.WebSocketconstructor, or- stop further messages being pushed into
window.__wsMessages.Given that other methods (
getMessages,waitForMessage, etc.) don’t consultisMonitoring, callingstopMonitoring()has no real effect today.Either wire
stopMonitoring()to actually unpatch/disable capture, or drop the method to avoid implying behavior that isn’t there.- /** - * Stop monitoring WebSocket connections - */ - stopMonitoring(): void { - this.isMonitoring = false; - } + /** + * Stop monitoring WebSocket connections. + * Note: currently this only flips the flag and does not restore the original WebSocket. + * Implement unpatching here if you need to truly disable capture. + */ + stopMonitoring(): void { + this.isMonitoring = false; + }tests/pages/base.page.ts (1)
36-44: AlignwaitForElementJSDoc with its actual signatureThe comment says “Wait for a specific element to be visible” and documents only
selectorandtimeout, but the method also exposes astateparameter with defaults, making it a general wait helper (attached/detached/visible/hidden).To avoid confusion for callers, either:
- update the JSDoc to describe the
stateparameter and the more general behavior, or- remove the
stateargument and keep this as a strict “visible-only” helper.- /** - * Wait for a specific element to be visible - * @param selector - CSS selector or locator - * @param timeout - Optional timeout in milliseconds - */ + /** + * Wait for a specific element state. + * @param selector - CSS selector or locator + * @param timeout - Optional timeout in milliseconds + * @param state - Desired state ("attached" | "detached" | "visible" | "hidden"), defaults to "visible" + */ async waitForElement(selector: string | Locator, timeout: number = 10000, state: "attached" | "detached" | "visible" | "hidden" | undefined = "visible"): Promise<void> {tests/fixtures/test-data.ts (1)
303-318: Consider making randomness deterministic for test runs
getRandomResponseandgenerateRandomUserDatarely onMath.random(), which makes tests that use them non-deterministic and occasionally harder to debug/reproduce.If these helpers are used in assertions (rather than just setup noise), consider:
- injecting a seeded PRNG from the tests, or
- allowing tests to pass in specific values instead of picking randomly.
That keeps fixtures flexible while preserving repeatability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/static/sample_image.pngis excluded by!**/*.png
📒 Files selected for processing (30)
.gitignore(1 hunks)jsconfig.json(0 hunks)src/hooks/useChatWebhook.js(1 hunks)src/hooks/useSmartChatStorage.js(1 hunks)src/pages/ShikshalokamVoiceChat/voice-chat.js(7 hunks)src/store/slices/chatData/state.js(1 hunks)tests/config/test-constants.ts(1 hunks)tests/constant/guest-chat.ts(1 hunks)tests/constant/improvement-story-chat.ts(1 hunks)tests/constant/ptm-chat.ts(1 hunks)tests/constant/site_routes.ts(1 hunks)tests/constant/ylc-chat.ts(1 hunks)tests/e2e/flows/capture-discussion/happy-path.spec.ts(1 hunks)tests/e2e/flows/improvement-story/happy-path.spec.ts(1 hunks)tests/e2e/flows/ptm/happy-path.spec.ts(1 hunks)tests/e2e/flows/ylc/happy-path.spec.ts(1 hunks)tests/fixtures/browser.fixture.ts(1 hunks)tests/fixtures/images/README.md(1 hunks)tests/fixtures/index.ts(1 hunks)tests/fixtures/test-data.ts(1 hunks)tests/helpers/image.helper.ts(1 hunks)tests/helpers/story.helper.ts(1 hunks)tests/helpers/websocket.helper.ts(1 hunks)tests/pages/base.page.ts(1 hunks)tests/pages/chat-interface-ptm.ts(1 hunks)tests/pages/chat-interface.page.ts(1 hunks)tests/pages/flow-selection.page.ts(1 hunks)tests/pages/language-selection.page.ts(1 hunks)tests/pages/story-view.page.ts(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- jsconfig.json
🧰 Additional context used
🧬 Code graph analysis (11)
tests/pages/chat-interface-ptm.ts (1)
tests/pages/base.page.ts (1)
BasePage(7-205)
tests/e2e/flows/capture-discussion/happy-path.spec.ts (5)
tests/fixtures/browser.fixture.ts (2)
test(29-67)expect(69-69)tests/pages/flow-selection.page.ts (1)
FlowSelectionPage(9-141)tests/pages/chat-interface.page.ts (1)
ChatInterfacePage(9-347)tests/constant/site_routes.ts (1)
SITE_ROUTES(1-9)tests/constant/guest-chat.ts (1)
GUEST_CHAT_CONVERSATION_EN(1-14)
tests/pages/story-view.page.ts (1)
tests/pages/base.page.ts (1)
BasePage(7-205)
tests/pages/language-selection.page.ts (1)
tests/pages/base.page.ts (1)
BasePage(7-205)
tests/pages/chat-interface.page.ts (2)
tests/pages/base.page.ts (1)
BasePage(7-205)server.js (1)
path(2-2)
tests/pages/flow-selection.page.ts (2)
tests/pages/base.page.ts (1)
BasePage(7-205)tests/constant/site_routes.ts (1)
SITE_ROUTES(1-9)
tests/e2e/flows/improvement-story/happy-path.spec.ts (4)
tests/pages/flow-selection.page.ts (1)
FlowSelectionPage(9-141)tests/pages/chat-interface.page.ts (1)
ChatInterfacePage(9-347)tests/constant/site_routes.ts (1)
SITE_ROUTES(1-9)tests/constant/improvement-story-chat.ts (1)
IMPROVEMENT_STORY_CHAT_CONVERSATION_EN(1-16)
tests/e2e/flows/ylc/happy-path.spec.ts (4)
tests/pages/flow-selection.page.ts (1)
FlowSelectionPage(9-141)tests/pages/chat-interface.page.ts (1)
ChatInterfacePage(9-347)tests/constant/site_routes.ts (1)
SITE_ROUTES(1-9)tests/constant/ylc-chat.ts (1)
YLC_CHAT_CONVERSATION_EN(1-21)
src/pages/ShikshalokamVoiceChat/voice-chat.js (3)
src/hooks/useSmartChatStorage.js (2)
chatHistory(4-4)useSmartChatStorage(3-12)src/services/storage_service.js (1)
clearFromStorage(109-122)src/hooks/useChatWebhook.js (2)
useChatWebhook(3-119)useChatWebhook(3-119)
src/hooks/useSmartChatStorage.js (1)
src/pages/ShikshalokamVoiceChat/voice-chat.js (3)
chatHistory(136-136)useChatStorage(177-177)useChatStorage(181-181)
src/hooks/useChatWebhook.js (2)
src/utils/index.js (1)
url(5-5)src/pages/ShikshalokamVoiceChat/voice-chat.js (2)
onFinalReconnectAttempt(191-200)error(81-81)
🪛 Biome (2.1.2)
tests/fixtures/browser.fixture.ts
[error] 35-35: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (20)
.gitignore (1)
25-30: Playwright artifacts and.envignore look goodIgnoring
.envand Playwright output directories (downloads/,test-results/,playwright-report/) is appropriate and keeps the repo clean. No changes needed.src/store/slices/chatData/state.js (1)
29-29:getChatHistoryaddition matches existing slice patternsThe new
getChatHistorymirrors other getters in this slice (getIntroMessage,getFlow, etc.) and should be safe to consume from hooks likeuseSmartChatStorage. Looks good.src/hooks/useChatWebhook.js (1)
44-62: Fix sendMessage null-check/return value and guard reconnection on unmount/disconnectUnable to access the repository to verify the code as described. The review comment identifies two significant issues—a potential null-dereference in
sendMessage(lines 77–85) and unguarded reconnection logic after unmount or explicit disconnect (lines 44–62, 68–75, 98–104)—with concrete diffs provided. Manual verification against the actual codebase is required to confirm whether these issues exist in the current version and whether the proposed fixes are complete and correct.tests/fixtures/images/README.md (1)
1-46: LGTM! Well-documented test fixtures.The README provides clear guidance on test image usage, generation methods, and technical requirements. The documentation aligns well with the broader test infrastructure introduced in this PR.
tests/fixtures/index.ts (1)
1-21: LGTM! Clean fixture organization.The central export point provides a clear single import location for test fixtures and utilities, with helpful documentation for usage patterns.
tests/constant/site_routes.ts (1)
1-9: LGTM! Clean route constant organization.The route constants provide a centralized, maintainable way to reference application paths across test suites.
tests/e2e/flows/ptm/happy-path.spec.ts (1)
7-7: Verify PTM test conversation data quality.The constant
PTM_CHAT_CONVERSATION_ENimported fromtests/constant/ptm-chat.tsshould be examined to ensure it contains realistic test data. If the conversation data consists only of placeholder strings, consider enhancing it with varied, realistic user inputs and edge cases to better exercise the PTM conversation flow during testing.tests/e2e/flows/ylc/happy-path.spec.ts (2)
8-8: Verify YLC test conversation data quality.The
YLC_CHAT_CONVERSATION_ENconstant should contain realistic test inputs that adequately cover the YLC conversation flow, similar to how test data for other conversation flows should be structured. Inspecttests/constant/ylc-chat.tsto confirm the constant includes appropriate test coverage rather than placeholder data.
68-68: Verify test image file exists.The test references
sample_image.pngfrom a static folder. Ensure this file exists at the expected path relative to the test file.src/hooks/useSmartChatStorage.js (1)
11-11: Breaking change: Verify all consumers handle the new return signature.The return tuple has been extended from 3 to 4 elements by adding
getChatHistory. Any existing code that destructures this hook with only 3 elements will still work (JavaScript ignores extra elements), but consumers expecting exactly 3 elements or using array indices might need updates.#!/bin/bash # Description: Find all usages of useSmartChatStorage to verify they handle the new 4-element return # Search for all imports and usage of useSmartChatStorage rg -n "useSmartChatStorage" --type=js --type=jsx -C 3tests/fixtures/browser.fixture.ts (1)
42-57: Context reuse may affect test isolation.Reusing an existing context means state (cookies, storage) persists between tests when debugging via CDP. This is intentional for debugging but worth noting in comments for clarity.
tests/helpers/story.helper.ts (2)
6-13: Well-defined interface for story metadata.The
StoryMetadatainterface captures all relevant story properties. The optionalcreatedAtfield appropriately handles cases where creation timestamp may not be available.
161-179: Polling implementation looks good.The polling approach with configurable timeout and minimum word threshold is appropriate for waiting on async story generation. The 1-second interval balances responsiveness with resource efficiency.
tests/pages/story-view.page.ts (2)
9-42: Well-structured page object with consistent data-testid selectors.Good use of
data-testidattributes for element selection, which provides maintainable and reliable selectors. The locator initialization follows the established pattern from other page objects.
174-181: Correct reverse iteration to avoid index shifting.Removing images from last to first prevents index shifting issues during sequential removal. Good defensive implementation.
tests/pages/language-selection.page.ts (1)
1-27: Well-structured page object with good locator organization.The class correctly extends
BasePageand initializes locators in the constructor. Usingdata-testidattributes for locators is a best practice for test stability.tests/pages/chat-interface.page.ts (1)
66-72: LGTM: File download and upload handling are well implemented.The download and upload methods correctly handle file events and path construction.
Also applies to: 86-91
src/pages/ShikshalokamVoiceChat/voice-chat.js (1)
272-291: LGTM: WebSocket hook configuration with manual connection control.The configuration with
autoConnect: falseand explicit callbacks provides good control over the WebSocket lifecycle. The destructured properties (sendMessage,connect,isFreshConnection) are well-named.tests/helpers/image.helper.ts (2)
88-125: Creative approach to generating test images via browser canvas.Using Playwright's page context to generate images ensures the test environment can create valid image data. Note that this requires the page to be navigated to a valid document first.
1-34: Well-designed utility methods for image validation in tests.The helper provides comprehensive utilities for verifying image existence, extensions, DOM presence, and sources. Good separation of filesystem and DOM-based operations.
Also applies to: 67-79, 133-208
| const onFinalReconnectAttempt = useCallback(() => { | ||
| showConfirmationPopup(() => { | ||
| if (accessToken) { | ||
| clearFromStorage() | ||
| navigate(-1) | ||
| } else { | ||
| ResetChat() | ||
| } | ||
| }) | ||
| }, []) |
There was a problem hiding this comment.
Missing dependencies in useCallback for onFinalReconnectAttempt.
The callback references accessToken, navigate, and ResetChat but has an empty dependency array. This could cause stale closures.
const onFinalReconnectAttempt = useCallback(() => {
showConfirmationPopup(() => {
if (accessToken) {
clearFromStorage()
navigate(-1)
} else {
ResetChat()
}
})
-}, [])
+}, [accessToken, navigate, showConfirmationPopup])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/ShikshalokamVoiceChat/voice-chat.js around lines 191 to 200, the
useCallback for onFinalReconnectAttempt has an empty dependency array but reads
accessToken, navigate, and ResetChat, which can create stale closures; update
the dependency array to include accessToken, navigate, and ResetChat (or wrap
ResetChat/navigate in stable callbacks if needed) so the callback is recreated
when those values change, and ensure any lint rules are satisfied.
| export const IMPROVEMENT_STORY_CHAT_CONVERSATION_EN = [ | ||
| "Vishnu Krishnathu", | ||
| "Yes", | ||
| "PT Teacher at ST Thomas School", | ||
| "St Thomas High School", | ||
| "Village name is Kalyan in District Thane, in state Maharashtra, India", | ||
| "Children do not play any sports", | ||
| "No sports period in school", | ||
| "I wrote a letter to management and convinced all the teachers to have a sports period thrice a weak for children from all classes", | ||
| "No", | ||
| "No challenges faced", | ||
| "3 months", | ||
| "Pt sir Kunal helped me with convincing other teachers in school", | ||
| "No", | ||
| "No" | ||
| ] No newline at end of file |
There was a problem hiding this comment.
Replace realistic PII with clearly synthetic test data.
The test conversation includes what appears to be real personal information (name "Vishnu Krishnathu", specific school "ST Thomas School"/"St Thomas High School", and location "Kalyan in District Thane, Maharashtra, India"). Even in test code, using realistic PII can create privacy concerns and potential GDPR/compliance issues if the data resembles real individuals or institutions.
Use clearly fictional data instead:
export const IMPROVEMENT_STORY_CHAT_CONVERSATION_EN = [
- "Vishnu Krishnathu",
+ "Test User",
"Yes",
- "PT Teacher at ST Thomas School",
- "St Thomas High School",
- "Village name is Kalyan in District Thane, in state Maharashtra, India",
+ "PT Teacher at Test School",
+ "Test High School",
+ "Test Village in Test District, Test State, Test Country",
"Children do not play any sports",📝 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.
| export const IMPROVEMENT_STORY_CHAT_CONVERSATION_EN = [ | |
| "Vishnu Krishnathu", | |
| "Yes", | |
| "PT Teacher at ST Thomas School", | |
| "St Thomas High School", | |
| "Village name is Kalyan in District Thane, in state Maharashtra, India", | |
| "Children do not play any sports", | |
| "No sports period in school", | |
| "I wrote a letter to management and convinced all the teachers to have a sports period thrice a weak for children from all classes", | |
| "No", | |
| "No challenges faced", | |
| "3 months", | |
| "Pt sir Kunal helped me with convincing other teachers in school", | |
| "No", | |
| "No" | |
| ] | |
| export const IMPROVEMENT_STORY_CHAT_CONVERSATION_EN = [ | |
| "Test User", | |
| "Yes", | |
| "PT Teacher at Test School", | |
| "Test High School", | |
| "Test Village in Test District, Test State, Test Country", | |
| "Children do not play any sports", | |
| "No sports period in school", | |
| "I wrote a letter to management and convinced all the teachers to have a sports period thrice a weak for children from all classes", | |
| "No", | |
| "No challenges faced", | |
| "3 months", | |
| "Pt sir Kunal helped me with convincing other teachers in school", | |
| "No", | |
| "No" | |
| ] |
🤖 Prompt for AI Agents
In tests/constant/improvement-story-chat.ts around lines 1-16, the conversation
array contains realistic PII (real name, school and location); replace those
entries with clearly synthetic test data. Change the name, school names, and
location string to obviously fictional values (e.g., "Test User", "Sample
Primary School", "Fictional Village, Fictional District, Fictional State") and
any other person references (e.g., "Pt sir Kunal") to synthetic placeholders so
the whole array contains no real-identifying information but preserves the same
semantic structure for tests.
| for (let i = 0; i < PTM_CHAT_CONVERSATION_EN.length; i++) { | ||
| await chatInterfacePage.sendMessage(PTM_CHAT_CONVERSATION_EN[i]) | ||
| await chatInterfacePage.wait(5000) | ||
| if (i != PTM_CHAT_CONVERSATION_EN.length - 1) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use strict equality operator.
The condition uses non-strict equality (!=) instead of strict equality (!==), which can lead to unexpected type coercion bugs.
- if (i != PTM_CHAT_CONVERSATION_EN.length - 1) {
+ if (i !== PTM_CHAT_CONVERSATION_EN.length - 1) {📝 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 (i != PTM_CHAT_CONVERSATION_EN.length - 1) { | |
| if (i !== PTM_CHAT_CONVERSATION_EN.length - 1) { |
🤖 Prompt for AI Agents
In tests/e2e/flows/ptm/happy-path.spec.ts around line 60, the condition uses
non-strict inequality (i != PTM_CHAT_CONVERSATION_EN.length - 1); change it to
strict inequality (i !== PTM_CHAT_CONVERSATION_EN.length - 1) to prevent
type-coercion issues and ensure correct boolean evaluation.
| for (let i = 0; i < YLC_CHAT_CONVERSATION_EN.length; i++) { | ||
| await chatInterfacePage.sendMessage(YLC_CHAT_CONVERSATION_EN[i]) | ||
| await chatInterfacePage.wait(10000) | ||
| if (i != YLC_CHAT_CONVERSATION_EN.length - 1) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use strict equality operator.
Use strict equality (!==) instead of loose equality (!=) to avoid type coercion issues.
- if (i != YLC_CHAT_CONVERSATION_EN.length - 1) {
+ if (i !== YLC_CHAT_CONVERSATION_EN.length - 1) {📝 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 (i != YLC_CHAT_CONVERSATION_EN.length - 1) { | |
| if (i !== YLC_CHAT_CONVERSATION_EN.length - 1) { |
🤖 Prompt for AI Agents
In tests/e2e/flows/ylc/happy-path.spec.ts around line 60, the condition uses the
loose inequality operator (`!=`); replace it with the strict inequality operator
(`!==`) to prevent type coercion issues—update the conditional to use `!==` and
run the test/linter to confirm no other occurrences remain.
| browser: async ({ }, use) => { | ||
| // Connect to existing browser via CDP | ||
| const browser = await chromium.connectOverCDP(`http://localhost:${debugPort}`); | ||
| await use(browser); | ||
| // Don't close the browser as it's externally managed | ||
| }, |
There was a problem hiding this comment.
Fix empty object pattern flagged by static analysis.
The empty destructuring { } triggers a linting error. Use _ to indicate intentionally unused fixtures.
- browser: async ({ }, use) => {
+ browser: async ({}, use) => {Or if you need to explicitly show no dependencies:
- browser: async ({ }, use) => {
+ browser: async (_, use) => {📝 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.
| browser: async ({ }, use) => { | |
| // Connect to existing browser via CDP | |
| const browser = await chromium.connectOverCDP(`http://localhost:${debugPort}`); | |
| await use(browser); | |
| // Don't close the browser as it's externally managed | |
| }, | |
| browser: async ({}, use) => { | |
| // Connect to existing browser via CDP | |
| const browser = await chromium.connectOverCDP(`http://localhost:${debugPort}`); | |
| await use(browser); | |
| // Don't close the browser as it's externally managed | |
| }, |
🧰 Tools
🪛 Biome (2.1.2)
[error] 35-35: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🤖 Prompt for AI Agents
In tests/fixtures/browser.fixture.ts around lines 35-40, the async fixture
parameter currently uses an empty destructuring "{ }", which triggers
lint/static-analysis errors; replace the empty pattern with an unused
placeholder name such as "_" or "_context" (optionally with an explicit type
like "_: any" or "_: unknown" if TypeScript types are required) so the signature
becomes e.g. "browser: async (_ , use) => { ... }", keeping the rest of the
function unchanged.
| async waitForReceivedMessageContaining(text: string, timeout: number = 30000): Promise<WebSocketMessage> { | ||
| return this.waitForMessage( | ||
| (msg) => msg.type === 'received' && msg.data.includes(text), | ||
| timeout | ||
| ); | ||
| } |
There was a problem hiding this comment.
Guard against non-string data in waitForReceivedMessageContaining
msg.data.includes(text) assumes data is a string. In WebSocket APIs it can also be Blob/ArrayBuffer/views, which would throw here if used in future tests.
A small coercion keeps this helper safe without changing semantics for current string-based usage:
async waitForReceivedMessageContaining(text: string, timeout: number = 30000): Promise<WebSocketMessage> {
return this.waitForMessage(
- (msg) => msg.type === 'received' && msg.data.includes(text),
+ (msg) =>
+ msg.type === 'received' &&
+ String(msg.data).includes(text),
timeout
);
}🤖 Prompt for AI Agents
In tests/helpers/websocket.helper.ts around lines 142–147, the predicate uses
msg.data.includes(text) which will throw if data isn’t a string
(Blob/ArrayBuffer/etc.); change it to coerce data to a string before checking,
e.g. replace msg.data.includes(text) with String(msg.data).includes(text) (or
assign const dataStr = String(msg.data) and use dataStr.includes(text)) so
non-string payloads are safely handled while preserving current string behavior.
| async openEditory(): Promise<void> { | ||
| await this.clickElement(this.reportButton.nth(1)); | ||
| } |
There was a problem hiding this comment.
Fix typo in method name: openEditory should be openEditor.
-async openEditory(): Promise<void> {
+async openEditor(): Promise<void> {
await this.clickElement(this.reportButton.nth(1));
}📝 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.
| async openEditory(): Promise<void> { | |
| await this.clickElement(this.reportButton.nth(1)); | |
| } | |
| async openEditor(): Promise<void> { | |
| await this.clickElement(this.reportButton.nth(1)); | |
| } |
🤖 Prompt for AI Agents
In tests/pages/chat-interface.page.ts around lines 78 to 80, the method name
openEditory is misspelled; rename the method to openEditor and update its
declaration to async openEditor(): Promise<void> { await
this.clickElement(this.reportButton.nth(1)); } then search and replace all
usages/imports/exports of openEditory across the codebase (tests and helpers) to
use openEditor to avoid reference breaks; keep the method signature and behavior
unchanged.
| /** | ||
| * Check if continue button is enabled | ||
| * @returns True if enabled, false otherwise | ||
| */ | ||
| async isContinueButtonEnabled(): Promise<boolean> { | ||
| const isDisabled = await this.continueButton.isDisabled() | ||
| return !isDisabled | ||
| } |
There was a problem hiding this comment.
Locator mismatch: isContinueButtonEnabled checks a different button than clickContinue.
isContinueButtonEnabled() uses continueButton while clickContinue() uses flowContinueButton. This inconsistency will cause the enabled check to not reflect the actual button clicked.
async isContinueButtonEnabled(): Promise<boolean> {
- const isDisabled = await this.continueButton.isDisabled()
+ const isDisabled = await this.flowContinueButton.isDisabled()
return !isDisabled
}📝 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.
| /** | |
| * Check if continue button is enabled | |
| * @returns True if enabled, false otherwise | |
| */ | |
| async isContinueButtonEnabled(): Promise<boolean> { | |
| const isDisabled = await this.continueButton.isDisabled() | |
| return !isDisabled | |
| } | |
| /** | |
| * Check if continue button is enabled | |
| * @returns True if enabled, false otherwise | |
| */ | |
| async isContinueButtonEnabled(): Promise<boolean> { | |
| const isDisabled = await this.flowContinueButton.isDisabled() | |
| return !isDisabled | |
| } |
🤖 Prompt for AI Agents
In tests/pages/flow-selection.page.ts around lines 111 to 118,
isContinueButtonEnabled currently checks this.continueButton while clickContinue
uses this.flowContinueButton, causing a mismatch; update isContinueButtonEnabled
to use the same locator as clickContinue (this.flowContinueButton) so the
enabled check reflects the actual button being clicked and return !await
this.flowContinueButton.isDisabled().
| async downloadStory(): Promise<void> { | ||
| await this.waitForElement(this.downloadButton); | ||
|
|
||
| // Set up download promise before clicking | ||
| const downloadPromise = this.page.waitForEvent('download', { timeout: 30000 }); | ||
|
|
||
| await this.clickElement(this.downloadButton); | ||
|
|
||
| // Wait for download to start | ||
| const download = await downloadPromise; | ||
|
|
||
| // Optionally save the download | ||
| const downloadPath = path.join(process.cwd(), 'downloads', await download.suggestedFilename()); | ||
| await download.saveAs(downloadPath); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for downloads directory creation and .gitignore entries
echo "=== Searching for mkdir/mkdirSync related to downloads ==="
rg -n "mkdir.*downloads|downloads.*mkdir" --type ts --type js --type json
echo ""
echo "=== Checking .gitignore for downloads ==="
if [ -f .gitignore ]; then
rg "downloads" .gitignore
else
echo ".gitignore not found"
fi
echo ""
echo "=== Searching for downloads directory references ==="
rg -n "downloads" --type ts -B 2 -A 2 | head -50
echo ""
echo "=== Looking for test setup/fixture files ==="
find . -type f \( -name "*.setup.ts" -o -name "*.fixture.ts" -o -name "*.before.ts" -o -name "playwright.config.ts" \) 2>/dev/nullRepository: ELEVATE-Project/mohini-app-frontend
Length of output: 214
🌐 Web query:
Playwright download.saveAs() directory creation behavior
💡 Result:
Short answer: download.saveAs(...) does not create missing parent directories — it copies the downloaded file to the exact path you provide and will error if the target directory doesn't exist. See Playwright's download.saveAs docs for the API and treat the path as a normal filesystem path. [1]
Workaround: create the parent directory first (e.g. Node.js):
- fs.mkdirSync(path.dirname(targetPath), { recursive: true });
- await download.saveAs(targetPath);
Example:
const fs = require('fs');
const path = require('path');
fs.mkdirSync(path.dirname(targetPath), { recursive: true });
await download.saveAs(targetPath);
References:
- Playwright download.saveAs documentation. [1]
- Node.js fs.mkdir / fs.mkdirSync (recursive option). [2]
Directory creation required before saving downloads.
The downloads directory must be created before calling saveAs(), as Playwright does not auto-create parent directories. Add fs.mkdirSync(path.dirname(downloadPath), { recursive: true }) before the saveAs() call, otherwise the test will fail at runtime.
🤖 Prompt for AI Agents
In tests/pages/story-view.page.ts around lines 214 to 228, the download saving
code assumes the downloads directory exists which causes saveAs() to fail;
before calling download.saveAs(...) create the parent directory (e.g. use
fs.mkdirSync(path.dirname(downloadPath), { recursive: true }) or equivalent
async mkdir) so the directory exists, then proceed to call saveAs to persist the
file.
… consistency by updating comparison operators. Refactor ChatInterfacePage for cleaner syntax and maintainability.
|
|
||
| ws.current.onopen = event => { | ||
| setIsConnected(true) | ||
| if (socketQueue.current.length > 0) { |
There was a problem hiding this comment.
just add socketQueue.current.length for true condition
| @@ -133,7 +133,7 @@ const ShikshalokamVoiceBasedChat = ({ type = "", variant = "" }) => { | |||
| // const introMessageRef = useRef(null); | |||
There was a problem hiding this comment.
remove shikshalokam, mohini etc names
| clearFromStorage() | ||
| navigate(-1) | ||
| } else { | ||
| ResetChat() |
| }, []) | ||
|
|
||
| const onWebSocketOpen = useCallback(() => { | ||
| if (!ipFetched) return |
| [chatHistory] | ||
| ) | ||
|
|
||
| // useEffect(() => { |
…and consistency. Refactor useChatWebhook and ShikshalokamVoiceChat components for better code readability and maintainability by renaming methods and optimizing WebSocket connection logic.
2 participants
and error handling
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.