feat: Add loading indicator during TTS audio generation#369
Conversation
|
@pixeltannu is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
client
needs-template
✅ DCO VerifiedHey @pixeltannu! 👋 All commits include a valid Note Your PR can continue through the normal review process. 🤖 VoiceForge Automation · Updates automatically on edits |
🎉 PR Ready for Mentor ReviewHey @pixeltannu! 👋 Your PR passed all checks and is now in the GSSoC review queue. Note 🔗 Closing: #363 · 📐 94 lines across 2 file(s) · 📬 Already requested or no eligible reviewer found @sabeenaviklar @Anushreebasics @itsdakshjain @snehkris @1754riya @Mrigakshi-Rathore @Itzzavdheshh @vedhapprakashni, this PR is ready for your review — please confirm scope, check behavior and tests, then approve or request changes. Important This is not an approval. Please wait for mentor feedback before expecting a merge. If changes are requested, push them to this same branch and keep the PR focused on the linked issue. 🤖 VoiceForge Automation · Updates automatically on edits |
📝 WalkthroughWalkthroughThe PR adds an ChangesTTS Loading Indicator and History Deduplication
Sequence DiagramsequenceDiagram
participant User
participant TextToSpeech
participant useSpeechHistory
participant History List
User->>TextToSpeech: Click Speak button
TextToSpeech->>TextToSpeech: isLoading check passes (not already speaking)
TextToSpeech->>TextToSpeech: Show Loader2 + "Generating..."<br/>Disable textarea
TextToSpeech->>useSpeechHistory: onSpeak(text)
useSpeechHistory->>History List: Check if text exists
alt Duplicate found
useSpeechHistory->>History List: Update timestamp to Date.now()
useSpeechHistory->>History List: Move to top
else New text
useSpeechHistory->>History List: Create entry with UUID
end
History List->>useSpeechHistory: Return updated history (capped to MAX)
Note over TextToSpeech: Audio streams from API
TextToSpeech->>TextToSpeech: status changes, isLoading clears
TextToSpeech->>TextToSpeech: Show SendHorizontal + "Speak"<br/>Enable textarea
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@client/src/components/TextToSpeech.jsx`:
- Around line 29-35: The submit() function has a race condition where rapid
clicks can both pass the guard clause before parent state updates, causing
duplicate onSpeak calls. Implement a local in-flight latch by creating a local
state variable (using useState) to track whether an API request is currently in
progress. In the submit() function, check this local state in the guard clause
and set it to true immediately before calling onSpeak, then set it back to false
after the call completes. Apply the same pattern to the other affected locations
mentioned at lines 68-69 and 86-87.
🪄 Autofix (Beta)
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: 572501f6-00cf-494e-b6ec-f098b3620615
📒 Files selected for processing (1)
client/src/components/TextToSpeech.jsx
| const isLoading = status === "speaking"; | ||
|
|
||
| async function submit() { | ||
| if (!trimmedText || disabled) return; | ||
| await onSpeak(trimmedText); | ||
| setText(""); | ||
| } | ||
| if (!trimmedText || disabled || isLoading) return; | ||
| await onSpeak(trimmedText); | ||
| setText(""); | ||
| } |
There was a problem hiding this comment.
Close the pre-rerender race that still allows duplicate onSpeak calls.
At Line 31/Line 32, duplicate prevention relies on parent-driven status (isLoading). Because that state arrives on a later render, two rapid clicks can still enter submit() before isLoading flips, triggering duplicate API requests.
Suggested patch (local in-flight latch)
export default function TextToSpeech({ onSpeak, disabled = false, status = "idle" }) {
const [text, setText] = React.useState("");
+ const [isSubmitting, setIsSubmitting] = React.useState(false);
const trimmedText = text.trim();
@@
const isLoading = status === "speaking";
+ const isBusy = isLoading || isSubmitting;
@@
async function submit() {
- if (!trimmedText || disabled || isLoading) return;
- await onSpeak(trimmedText);
- setText("");
+ if (!trimmedText || disabled || isBusy) return;
+ setIsSubmitting(true);
+ try {
+ await onSpeak(trimmedText);
+ setText("");
+ } finally {
+ setIsSubmitting(false);
+ }
}
@@
- disabled={disabled || isLoading}
+ disabled={disabled || isBusy}
@@
- {isLoading && (
+ {isBusy && (
@@
- disabled={disabled || !trimmedText || isLoading}
- aria-busy={isLoading}
+ disabled={disabled || !trimmedText || isBusy}
+ aria-busy={isBusy}
@@
- {isLoading ? (
+ {isBusy ? (
@@
- {isLoading ? "Generating..." : "Speak"}
+ {isBusy ? "Generating..." : "Speak"}
</button>Also applies to: 68-69, 86-87
🧰 Tools
🪛 ast-grep (0.43.0)
[error] 33-33: React's useState should not be directly called
Context: setText("")
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 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 `@client/src/components/TextToSpeech.jsx` around lines 29 - 35, The submit()
function has a race condition where rapid clicks can both pass the guard clause
before parent state updates, causing duplicate onSpeak calls. Implement a local
in-flight latch by creating a local state variable (using useState) to track
whether an API request is currently in progress. In the submit() function, check
this local state in the guard clause and set it to true immediately before
calling onSpeak, then set it back to false after the call completes. Apply the
same pattern to the other affected locations mentioned at lines 68-69 and 86-87.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/components/TextToSpeech.jsx">
<violation number="1" location="client/src/components/TextToSpeech.jsx:32">
P2: Race condition: the `isLoading` guard is derived from the parent's `status` prop, which only updates on a subsequent render. Two rapid clicks can both enter `submit()` before `isLoading` flips to `true`, resulting in duplicate TTS API requests. Add a local `isSubmitting` state that is set synchronously before `await onSpeak(trimmedText)` and cleared in a `finally` block to close this window.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| await onSpeak(trimmedText); | ||
| setText(""); | ||
| } | ||
| if (!trimmedText || disabled || isLoading) return; |
There was a problem hiding this comment.
P2: Race condition: the isLoading guard is derived from the parent's status prop, which only updates on a subsequent render. Two rapid clicks can both enter submit() before isLoading flips to true, resulting in duplicate TTS API requests. Add a local isSubmitting state that is set synchronously before await onSpeak(trimmedText) and cleared in a finally block to close this window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/TextToSpeech.jsx, line 32:
<comment>Race condition: the `isLoading` guard is derived from the parent's `status` prop, which only updates on a subsequent render. Two rapid clicks can both enter `submit()` before `isLoading` flips to `true`, resulting in duplicate TTS API requests. Add a local `isSubmitting` state that is set synchronously before `await onSpeak(trimmedText)` and cleared in a `finally` block to close this window.</comment>
<file context>
@@ -1,37 +1,38 @@
- await onSpeak(trimmedText);
- setText("");
-}
+ if (!trimmedText || disabled || isLoading) return;
+ await onSpeak(trimmedText);
+ setText("");
</file context>
|
PR is ready for review whenever a mentor has time. Thanks! |
…vdhesh#363) Signed-off-by: Tannu Kumari <tannu.kumarieng1@gmail.com>
Signed-off-by: Tannu Kumari <tannu.kumarieng1@gmail.com>
pixeltannu
force-pushed
the
feat/loading-indicator-tts
branch
from
e18bcf9 to
3878999
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
client/src/components/TextToSpeech.jsx (1)
31-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose the remaining pre-rerender duplicate-submit race.
isLoadingdepends on parentstatus, so two rapid submits can still pass the guard before the next render and trigger duplicateonSpeakcalls. Add a local in-flight latch and use it for guard + disabled states.Suggested patch
export default function TextToSpeech({ onSpeak, disabled = false, status = "idle" }) { const [text, setText] = React.useState(""); + const [isSubmitting, setIsSubmitting] = React.useState(false); const trimmedText = text.trim(); @@ const isLoading = status === "speaking"; + const isBusy = isLoading || isSubmitting; async function submit() { - if (!trimmedText || disabled || isLoading) return; - await onSpeak(trimmedText); - setText(""); + if (!trimmedText || disabled || isBusy) return; + setIsSubmitting(true); + try { + await onSpeak(trimmedText); + setText(""); + } finally { + setIsSubmitting(false); + } } @@ - disabled={disabled || isLoading} + disabled={disabled || isBusy} @@ - {isLoading && ( + {isBusy && ( @@ - disabled={disabled || !trimmedText || isLoading} - aria-busy={isLoading} + disabled={disabled || !trimmedText || isBusy} + aria-busy={isBusy} @@ - {isLoading ? ( + {isBusy ? ( @@ - {isLoading ? "Generating..." : "Speak"} + {isBusy ? "Generating..." : "Speak"}Also applies to: 68-69, 86-87
🤖 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 `@client/src/components/TextToSpeech.jsx` around lines 31 - 35, The submit function in TextToSpeech has a race condition where two rapid submits can bypass the isLoading guard before the parent re-renders, causing duplicate onSpeak calls. Add a local state variable (an in-flight latch) to track whether a request is currently in progress. Update the submit function to check this local state in the guard condition before calling onSpeak, set the latch to true when the async onSpeak call starts, and reset it to false when complete. Also update the disabled state logic to use this local latch alongside the existing checks. Apply the same pattern to the other submit locations referenced at lines 68-69 and 86-87.
🤖 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.
Duplicate comments:
In `@client/src/components/TextToSpeech.jsx`:
- Around line 31-35: The submit function in TextToSpeech has a race condition
where two rapid submits can bypass the isLoading guard before the parent
re-renders, causing duplicate onSpeak calls. Add a local state variable (an
in-flight latch) to track whether a request is currently in progress. Update the
submit function to check this local state in the guard condition before calling
onSpeak, set the latch to true when the async onSpeak call starts, and reset it
to false when complete. Also update the disabled state logic to use this local
latch alongside the existing checks. Apply the same pattern to the other submit
locations referenced at lines 68-69 and 86-87.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f1abdb60-3a56-46a9-a7bb-804717ecf68d
📒 Files selected for processing (2)
client/src/components/TextToSpeech.jsxclient/src/hooks/useSpeechHistory.js
|
Please review this PR when you get a chance. This PR closes issue #363. CodeRabbit also flagged a minor race condition issue in TextToSpeech.jsx — should I fix it before merging, or is it okay to proceed? |
github-actions
Bot
added
mentor:Anushreebasics
🔄 Changes RequestedHey @pixeltannu! 👋 A mentor has reviewed your PR and requested some changes. Warning Please review the feedback above, update this same branch, and keep the PR focused on the linked issue. Once you push your updates, the review flow will continue automatically on this same PR. 🤖 VoiceForge Automation · Updates automatically on edits |
itsdakshjain
left a comment
itsdakshjain
left a comment
There was a problem hiding this comment.
Resolve bot comments and empty line as stated by Anushree
3 participants
🚀 Program
GSSoC
📝 Description
Adds a loading indicator while text-to-speech audio is being generated, so users get clear visual feedback instead of an unresponsive-looking Speak button.
🔗 Related Issue
Closes #363
🔄 Type of Change
🧪 How to Test
http://localhost:5173in a browser📸 Screenshots (if applicable)
N/A
✅ Checklist
feat: add voice preview)Summary by CodeRabbit
New Features
Bug Fixes