Feature/non llm http api - #336
Conversation
…ata extraction and content block generation, enhancing flexibility for different flow types.
When a flow has child flows, show a dropdown to select the active child. Selected child's flowInfo drives all chat config (websocket, bot_route, editor); URL param stays as parent flow route. Resets chat on switch with confirmation popup if conversation already started.
Add `postNonLlmChat` endpoint + `NON_LLM_CHAT` URL. In DynamicVoiceChat, detect non-LLM steps via `stepMetadata` and route messages through HTTP instead of WebSocket. Inject first bot question from state machine metadata when step 0 is non-LLM. Transitions back to WebSocket when next step is LLM. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hini-app-frontend into feature/non-llm-http-api
Bot flow data static after fetch; refetch caused redundant calls and re-render flicker.
…hini-app-frontend into feature/non-llm-http-api
Backend now streams audio_s3_url on stop; fetch and cache it as base64 instead of hitting AI4Bharat TTS, falling back to TTS on fetch failure.
|
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 ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughDynamic voice chat now supports active child flows, non-LLM steps, configurable story editing, and bot S3 audio playback. Related routes, API constants, development startup, and story utilities were also updated. ChangesVoice chat flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 59: Update the package.json "dev" script to invoke react-scripts start
through env-cmd with the dev environment, restoring the documented npm run dev
behavior and loading the dev configuration from .env-cmdrc.
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js`:
- Line 2377: Update the PrivacyPolicyPopup render condition in the chat view to
require !shouldFetchChatSession, preventing this branch from rendering when the
existing shouldFetchChatSession branch is active. Preserve the current
chatLanguage, acceptedTnc, loading, activeFlowRoute, and popup props.
- Line 627: Update the non-LLM check in the surrounding submission flow to use 0
as the fallback only when strandStep is nullish, preserving an explicit
strandStep value of 0. Replace the truthiness-based fallback in isNonLlmStep
while leaving later step handling unchanged.
- Around line 1991-2001: Update handleChildFlowChange to persist newRoute in the
flow URL parameter before calling resetChat, in both the chat-history
confirmation callback and the no-history branch. Ensure the URL reflects the
selected child route so useUrlFlow and upload handling restore and submit that
route after reload.
- Around line 66-75: Update fetchAudioUrlAsBase64 in
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js lines 66-75 to reject
responses that are not OK or whose content type is not audio, and return the
complete FileReader data URL instead of stripping its MIME prefix. Apply the
same validation and MIME-preserving conversion in
src/pages/ShikshalokamVoiceChat/voice-chat.js lines 67-76 so invalid S3
responses trigger the TTS fallback and valid audio formats remain playable.
- Around line 670-678: Update the non-LLM HTTP failure path in the catch block
so the failed user input remains available for retry instead of being cleared by
the subsequent setTextMessage call. Remove or mark the optimistic conversation
message as failed, and provide a retry action if it remains visible; preserve
the existing error reporting while keeping the composer usable after failure.
In `@src/utils/story.js`:
- Around line 70-71: Update the QA branch in getEditorContentBlocks to pass
editorConfig into getQuestionAnswersFromBlocks. Make that extraction use
question_field, answer_field, and header_prefix when recognizing headers and
constructing each saved QA object, preserving the existing defaults only when
configuration is absent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 24949109-06ec-41e7-997f-d585a5272cba
📒 Files selected for processing (7)
package.jsonsrc/api/endpoints/chat_flow.jssrc/constants/urls.tssrc/pages/ShikshalokamVoiceChat/dynamic-voice-chat.jssrc/pages/ShikshalokamVoiceChat/voice-chat.jssrc/url.jssrc/utils/story.js
| }, | ||
| "scripts": { | ||
| "dev": "env-cmd -e dev react-scripts start", | ||
| "dev": "react-scripts start", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect the dev command and environment documentation to remain consistent.
rg -n -C 3 '"dev"\s*:' package.json
rg -n -C 4 'env-cmd|npm run dev|\.env-cmdrc' README.md package.jsonRepository: ELEVATE-Project/mohini-app-frontend
Length of output: 3349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- relevant files ---\n'
git ls-files | rg '(^|/)(package\.json|README\.md|\.env-cmdrc|\.env|sample\.env-cmdrc|sample\.env)$' || true
printf '\n--- .env-cmdrc contents ---\n'
if [ -f .env-cmdrc ]; then cat -n .env-cmdrc; else echo 'missing'; fi
printf '\n--- build-dev script and React env loading references ---\n'
sed -n '88,67p' README.md | sed -n '88,105p'
sed -n '58,69p' package.jsonRepository: ELEVATE-Project/mohini-app-frontend
Length of output: 264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(package\.json|README\.md|\.env-cmdrc|\.env|sample\.env-cmdrc|sample\.env)$' || true
printf '%s\n' ''
printf '%s\n' '--- .env-cmdrc contents ---'
if [ -f .env-cmdrc ]; then cat -n .env-cmdrc; else echo 'missing'; fi
printf '%s\n' ''
printf '%s\n' '--- scripts and docs references ---'
sed -n '88,96p' README.md
sed -n '58,66p' package.jsonRepository: ELEVATE-Project/mohini-app-frontend
Length of output: 1163
Restore the documented dev environment command.
README.md documents npm run dev as starting the dev server with the dev environment, but package.json now runs react-scripts start directly. react-scripts loads the root .env file, not the dev section from .env-cmdrc; restore env-cmd -e dev react-scripts start or update the docs/scripts together.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 59, Update the package.json "dev" script to invoke
react-scripts start through env-cmd with the dev environment, restoring the
documented npm run dev behavior and loading the dev configuration from
.env-cmdrc.
| async function fetchAudioUrlAsBase64(url) { | ||
| const response = await fetch(url) | ||
| const blob = await response.blob() | ||
| return new Promise((resolve, reject) => { | ||
| const reader = new FileReader() | ||
| reader.onloadend = () => resolve(reader.result.split(",")[1]) | ||
| reader.onerror = reject | ||
| reader.readAsDataURL(blob) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make invalid S3 audio responses trigger the TTS fallback.
fetch() resolves for HTTP error responses. Both helpers convert an error body to base64 and later force it into a data:audio/wav URL. Playback then fails and marks the message as narrated, so the AI4Bharat fallback does not run. The forced WAV MIME type also rejects valid non-WAV S3 audio.
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js#L66-L75: reject non-OK or non-audio responses, and return the completeFileReaderdata URL so its MIME type is retained.src/pages/ShikshalokamVoiceChat/voice-chat.js#L67-L76: apply the same validation and MIME-preserving conversion.
📍 Affects 2 files
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js#L66-L75(this comment)src/pages/ShikshalokamVoiceChat/voice-chat.js#L67-L76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` around lines 66 - 75,
Update fetchAudioUrlAsBase64 in
src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js lines 66-75 to reject
responses that are not OK or whose content type is not audio, and return the
complete FileReader data URL instead of stripping its MIME prefix. Apply the
same validation and MIME-preserving conversion in
src/pages/ShikshalokamVoiceChat/voice-chat.js lines 67-76 so invalid S3
responses trigger the TTS fallback and valid audio formats remain playable.
| if (chat_history.filter(chat => chat.source === "user").length == 1 || !isSocketConnected) { | ||
|
|
||
| // NON_LLM HTTP path: skip WebSocket for data-collection steps | ||
| if (isNonLlmStep(strandStep || 1)) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use step 0 for the first non-LLM submission.
The code injects a question from step 0 at lines 963-973. strandStep || 1 converts both null and 0 to 1. The first answer therefore uses the WebSocket path when only step 0 is non_llm.
Use a nullish fallback such as strandStep ?? 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` at line 627, Update
the non-LLM check in the surrounding submission flow to use 0 as the fallback
only when strandStep is nullish, preserving an explicit strandStep value of 0.
Replace the truthiness-based fallback in isNonLlmStep while leaving later step
handling unchanged.
| } catch (err) { | ||
| console.error("NON_LLM HTTP send failed:", err) | ||
| setLlmError("Failed to send message. Please try again.") | ||
| } | ||
|
|
||
| setAsrAudio(null) | ||
| handleScrollToView() | ||
| setTextMessage("") | ||
| return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the conversation retryable after an HTTP failure.
The user message is added before the request. The catch block then sets llmError, while the code later clears textMessage. The composer is hidden when llmError is set, so the user cannot resend the failed non-LLM answer.
On failure, retain the input and remove or mark the optimistic message as failed. Add a retry action if the message must remain visible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` around lines 670 -
678, Update the non-LLM HTTP failure path in the catch block so the failed user
input remains available for retry instead of being cleared by the subsequent
setTextMessage call. Remove or mark the optimistic conversation message as
failed, and provide a retry action if it remains visible; preserve the existing
error reporting while keeping the composer usable after failure.
| const handleChildFlowChange = e => { | ||
| const newRoute = e.target.value | ||
| if (chatHistory.length > 0) { | ||
| showGuestPopup(() => { | ||
| setSelectedChildFlowRoute(newRoute) | ||
| resetChat() | ||
| }, stayOnPage) | ||
| } else { | ||
| setSelectedChildFlowRoute(newRoute) | ||
| resetChat() | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist the selected child flow before reloading.
selectedChildFlowRoute exists only in component state. resetChat() reloads the page, and useUrlFlow() then restores the parent route from the unchanged URL. The effect at lines 858-867 selects the default child again, so a non-default selection is lost. Upload handling also reads the flow from the URL and can submit the parent flow.
Write newRoute to the flow URL parameter before resetting the session, or navigate to the selected child route before reload.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 1994-1994: Avoid using the initial state variable in setState
Context: setSelectedChildFlowRoute(newRoute)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 1998-1998: Avoid using the initial state variable in setState
Context: setSelectedChildFlowRoute(newRoute)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` around lines 1991 -
2001, Update handleChildFlowChange to persist newRoute in the flow URL parameter
before calling resetChat, in both the chat-history confirmation callback and the
no-history branch. Ensure the URL reflects the selected child route so
useUrlFlow and upload handling restore and submit that route after reload.
| {acceptedTnc === "ONGOING" && !isLoading && shouldFetchChatSession && <PrivacyPolicyPopup tncText={t(tncText)} onAccept={handleAcceptTnC} />} | ||
|
|
||
| {chatLanguage && acceptedTnc === "ONGOING" && !isLoading && storageFlow && <PrivacyPolicyPopup tncText={t(tncText)} onAccept={handleAcceptTnC} useStaticText={false} />} | ||
| {chatLanguage && acceptedTnc === "ONGOING" && !isLoading && activeFlowRoute && <PrivacyPolicyPopup tncText={t(tncText)} onAccept={handleAcceptTnC} useStaticText={false} />} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent duplicate privacy-policy popups for Reflection.
When shouldFetchChatSession is true, line 2375 already renders PrivacyPolicyPopup. This added condition also matches when chatLanguage and activeFlowRoute are available. Reflection users receive two overlapping popups.
Exclude shouldFetchChatSession from this new condition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ShikshalokamVoiceChat/dynamic-voice-chat.js` at line 2377, Update
the PrivacyPolicyPopup render condition in the chat view to require
!shouldFetchChatSession, preventing this branch from rendering when the existing
shouldFetchChatSession branch is active. Preserve the current chatLanguage,
acceptedTnc, loading, activeFlowRoute, and popup props.
| if (editorConfig.type === "qa") { | ||
| return { [editorConfig.data_key]: getQuestionAnswersFromBlocks(blocks) } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Honor the configured QA schema when saving editor content.
getEditorContentBlocks reads editorConfig.question_field, editorConfig.answer_field, and editorConfig.header_prefix at lines 216-220. This extraction path always saves question and answer fields and only recognizes headers that start with Q. A flow with different configured field names or header prefix saves data that renders as blank when the editor reopens.
Pass the QA configuration into getQuestionAnswersFromBlocks, parse the configured header format, and emit the configured field names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/story.js` around lines 70 - 71, Update the QA branch in
getEditorContentBlocks to pass editorConfig into getQuestionAnswersFromBlocks.
Make that extraction use question_field, answer_field, and header_prefix when
recognizing headers and constructing each saved QA object, preserving the
existing defaults only when configuration is absent.
…yment-ansible fix: ansible logic updated
Backend BOT_VERNACULAR endpoint return single obj not array. Drop [0]/results indexing across intro-message consumers. Also carry audio_s3_url through bot msg flow, mark sent msgs received, drop unused getAI4BharatAudio helper and non_llm step-0 injection.
…hini-app-frontend into feature/non-llm-http-api
…mi-discussion-config Fix configurable parent-child flow switching and browser back navigation
Removed shouldFetchIntro/isNewChatOpen from query enabled condition. App needs bot state (llm vs non-llm) on mount; without it, app defaults to llm step and routes messages over websocket incorrectly. Also: - add refetchOnWindowFocus:false on companyBot and chat-history queries to stop refetch on tab switch - fix recording useEffect infinite loop: read chatHistory via getChatHistory() instead of depending on chatHistory directly
…e-selection-home-redirect Feat/language selection home redirect
…hini-app-frontend into feature/non-llm-http-api
- setAsrAudio(null) -> [] avoid downstream null deref - aria-label used undefined t() ref, hardcode string
finish_reason, source, operation_type literals swapped for STATE_MACHINE_OPERATION_TYPE, FINISH_REASON_SOCKET, CONVERSATION_USER_TYPES enums. Drop unused CONVERSATION_USER_TYPES import in DefineChallenge.jsx.
…llm-http-api Feature/non llm http api
Payload sent asr_audio never; recorded IDs dropped silently. Reset asrAudio after send so next turn starts clean.
Summary by CodeRabbit