feat: add auto-save speech composition drafts and crash recovery - #1193
Conversation
|
@hrshjswniii is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
✍️ DCO Sign-off NeededHey @hrshjswniii! 👋 One or more commits in this PR are missing a Warning
How to fix: For the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits, replace git rebase --signoff HEAD~N
git push --force-with-leaseThis comment will update automatically after you push. 🤖 VoiceForge Automation · Updates automatically on edits |
github-actions
Bot
added
accessibility
assignee-match
client
enhancement
📝 WalkthroughWalkthroughTextToSpeech and VoiceForge now persist speech drafts in ChangesDraft persistence and composer behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TextToSpeech
participant VoiceForge
participant localStorage
User->>TextToSpeech: Enter speech text
TextToSpeech->>localStorage: Save voiceforge:draft_speech
User->>VoiceForge: Open composer
VoiceForge->>localStorage: Read voiceforge:draft_speech
localStorage-->>VoiceForge: Return saved draft
User->>TextToSpeech: Submit speech
TextToSpeech->>localStorage: Remove voiceforge:draft_speech
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
🎉 PR Ready for Mentor ReviewHey @hrshjswniii! 👋 Your PR passed all checks and is now in the GSSoC review queue. Note 🔗 Closing: #1187 · 📐 83 lines across 3 file(s) · 📬 Review requested @sabeenaviklar @Anushreebasics @itsdakshjain @snehkris @Mrigakshi-Rathore @Itzzavdheshh @Nitya-003 @4f4d @lovestaco, 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
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.test.jsx">
<violation number="1" location="client/src/components/TextToSpeech.test.jsx:9">
P3: `toBeDefined()` after `getByText()`/`getByPlaceholderText()` is redundant — these queries throw if the element is absent, so the assertion never fails. Use `toBeInTheDocument()` (from @testing-library/jest-dom) for a meaningful presence check, or drop the matcher and rely on the query throwing.</violation>
<violation number="2" location="client/src/components/TextToSpeech.test.jsx:15">
P1: The "handles draft localStorage persistence safely" test doesn't test the TextToSpeech component at all. It only exercises localStorage primitives directly, which is a browser/DOM implementation detail with no relation to the component's draft-read-on-mount or persist-on-typing behavior. Replace the test body with one that (1) sets localStorage before rendering, (2) renders TextToSpeech and asserts the textarea pre-fills from the draft, (3) simulates a typing event, and (4) asserts localStorage is updated. Drop the `expect(true).toBe(true)` tautology in the else branch — jsdom provides localStorage, and the fallback silently hides failures.</violation>
</file>
<file name="client/src/components/VoiceForge.jsx">
<violation number="1" location="client/src/components/VoiceForge.jsx:36">
P2: The draft key `voiceforge:draft_speech` persists in localStorage after "Speak & Save" because `handleSpeak` never clears `inputText`. The new `useEffect` persists `inputText` to localStorage on every change, and since `inputText` is unchanged after speaking, the old draft stays in localStorage. Next time the user visits VoiceForge, the text they already spoke will be restored. This differs from `TextToSpeech.jsx` where `submit()` explicitly calls `setText("")` which triggers the cleanup effect and removes the draft key.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| it("handles draft localStorage persistence safely", () => { | ||
| if (typeof localStorage !== "undefined") { | ||
| localStorage.setItem("voiceforge:draft_call_speech", "Draft speech text"); |
There was a problem hiding this comment.
P1: The "handles draft localStorage persistence safely" test doesn't test the TextToSpeech component at all. It only exercises localStorage primitives directly, which is a browser/DOM implementation detail with no relation to the component's draft-read-on-mount or persist-on-typing behavior. Replace the test body with one that (1) sets localStorage before rendering, (2) renders TextToSpeech and asserts the textarea pre-fills from the draft, (3) simulates a typing event, and (4) asserts localStorage is updated. Drop the expect(true).toBe(true) tautology in the else branch — jsdom provides localStorage, and the fallback silently hides failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/TextToSpeech.test.jsx, line 15:
<comment>The "handles draft localStorage persistence safely" test doesn't test the TextToSpeech component at all. It only exercises localStorage primitives directly, which is a browser/DOM implementation detail with no relation to the component's draft-read-on-mount or persist-on-typing behavior. Replace the test body with one that (1) sets localStorage before rendering, (2) renders TextToSpeech and asserts the textarea pre-fills from the draft, (3) simulates a typing event, and (4) asserts localStorage is updated. Drop the `expect(true).toBe(true)` tautology in the else branch — jsdom provides localStorage, and the fallback silently hides failures.</comment>
<file context>
@@ -0,0 +1,22 @@
+
+ it("handles draft localStorage persistence safely", () => {
+ if (typeof localStorage !== "undefined") {
+ localStorage.setItem("voiceforge:draft_call_speech", "Draft speech text");
+ expect(localStorage.getItem("voiceforge:draft_call_speech")).toBe("Draft speech text");
+ localStorage.removeItem("voiceforge:draft_call_speech");
</file context>
| try { | ||
| if (typeof localStorage !== "undefined") { | ||
| if (inputText) { | ||
| localStorage.setItem(COMPOSE_DRAFT_KEY, inputText); |
There was a problem hiding this comment.
P2: The draft key voiceforge:draft_speech persists in localStorage after "Speak & Save" because handleSpeak never clears inputText. The new useEffect persists inputText to localStorage on every change, and since inputText is unchanged after speaking, the old draft stays in localStorage. Next time the user visits VoiceForge, the text they already spoke will be restored. This differs from TextToSpeech.jsx where submit() explicitly calls setText("") which triggers the cleanup effect and removes the draft key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/VoiceForge.jsx, line 36:
<comment>The draft key `voiceforge:draft_speech` persists in localStorage after "Speak & Save" because `handleSpeak` never clears `inputText`. The new `useEffect` persists `inputText` to localStorage on every change, and since `inputText` is unchanged after speaking, the old draft stays in localStorage. Next time the user visits VoiceForge, the text they already spoke will be restored. This differs from `TextToSpeech.jsx` where `submit()` explicitly calls `setText("")` which triggers the cleanup effect and removes the draft key.</comment>
<file context>
@@ -15,9 +15,34 @@ import { LanguageSelector } from "./LanguageSelector.jsx";
+ try {
+ if (typeof localStorage !== "undefined") {
+ if (inputText) {
+ localStorage.setItem(COMPOSE_DRAFT_KEY, inputText);
+ } else {
+ localStorage.removeItem(COMPOSE_DRAFT_KEY);
</file context>
| describe("TextToSpeech component and draft persistence", () => { | ||
| it("renders TextToSpeech component and character limit indicator", () => { | ||
| render(<TextToSpeech onSpeak={() => {}} />); | ||
| expect(screen.getByText("Type to speak")).toBeDefined(); |
There was a problem hiding this comment.
P3: toBeDefined() after getByText()/getByPlaceholderText() is redundant — these queries throw if the element is absent, so the assertion never fails. Use toBeInTheDocument() (from @testing-library/jest-dom) for a meaningful presence check, or drop the matcher and rely on the query throwing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/TextToSpeech.test.jsx, line 9:
<comment>`toBeDefined()` after `getByText()`/`getByPlaceholderText()` is redundant — these queries throw if the element is absent, so the assertion never fails. Use `toBeInTheDocument()` (from @testing-library/jest-dom) for a meaningful presence check, or drop the matcher and rely on the query throwing.</comment>
<file context>
@@ -0,0 +1,22 @@
+describe("TextToSpeech component and draft persistence", () => {
+ it("renders TextToSpeech component and character limit indicator", () => {
+ render(<TextToSpeech onSpeak={() => {}} />);
+ expect(screen.getByText("Type to speak")).toBeDefined();
+ expect(screen.getByPlaceholderText("Type what you want to say...")).toBeDefined();
+ });
</file context>
Nitya-003
left a comment
Nitya-003
left a comment
There was a problem hiding this comment.
@hrshjswniii Resolve the conflicts and comments.
itsdakshjain
left a comment
itsdakshjain
left a comment
There was a problem hiding this comment.
This one also has same verdict
Conflicts presents with unresolved comments
github-actions
Bot
added
mentor:Nitya-003
🔄 Changes RequestedHey @hrshjswniii! 👋 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/src/components/TextToSpeech.test.jsx (1)
13-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest
TextToSpeechdraft behavior through the component.This test only verifies the browser
localStorageAPI. It does not test draft restoration, input persistence, explicit clearing, or cleanup after a successfulonSpeakcall.Preload the draft key before rendering. Assert the textarea value. Change and clear the textarea. Then assert the stored value is added and removed by
TextToSpeech.🤖 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.test.jsx` around lines 13 - 21, Replace the API-only test in the draft persistence case with a rendered TextToSpeech interaction test: preload the voiceforge:draft_call_speech key before rendering, assert the textarea restores that value, then change and clear the textarea while verifying the component adds and removes the stored draft. Also invoke the successful onSpeak flow and verify the draft is cleaned up afterward, using the component’s existing test selectors and handlers.
🤖 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 69-79: Restore the activeEmotion state declaration inside
TextToSpeech alongside the existing text state, exposing both activeEmotion and
setActiveEmotion for the reads and updates already used in the component.
Initialize it to the component’s expected default emotion value.
In `@client/src/components/VoiceForge.jsx`:
- Around line 35-47: Update handleSpeak to inspect the explicit success result
returned by speak and call setInputText("") only when speech generation
succeeds; preserve the current input and draft when speak fails. Ensure speak
returns a success indicator on successful completion and a failure result on
errors.
---
Nitpick comments:
In `@client/src/components/TextToSpeech.test.jsx`:
- Around line 13-21: Replace the API-only test in the draft persistence case
with a rendered TextToSpeech interaction test: preload the
voiceforge:draft_call_speech key before rendering, assert the textarea restores
that value, then change and clear the textarea while verifying the component
adds and removes the stored draft. Also invoke the successful onSpeak flow and
verify the draft is cleaned up afterward, using the component’s existing test
selectors and handlers.
🪄 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: c107032f-b48f-49db-9359-788b5bb78280
📒 Files selected for processing (3)
client/src/components/TextToSpeech.jsxclient/src/components/TextToSpeech.test.jsxclient/src/components/VoiceForge.jsx
| export default function TextToSpeech({ onSpeak, disabled = false, status = "idle" }) { | ||
| const [text, setText] = React.useState(""); | ||
| const [activeEmotion, setActiveEmotion] = React.useState("neutral"); | ||
| const [text, setText] = React.useState(() => { | ||
| try { | ||
| if (typeof localStorage !== "undefined") { | ||
| return localStorage.getItem(DRAFT_KEY) || ""; | ||
| } | ||
| } catch { | ||
| // Storage unavailable | ||
| } | ||
| return ""; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Restore the activeEmotion state declaration.
activeEmotion is read at Line 118. setActiveEmotion is called at Line 206. Neither identifier is declared after this change. Rendering TextToSpeech throws a ReferenceError.
Proposed fix
export default function TextToSpeech({ onSpeak, disabled = false, status = "idle" }) {
+ const [activeEmotion, setActiveEmotion] = React.useState("neutral");
const [text, setText] = React.useState(() => {📝 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 default function TextToSpeech({ onSpeak, disabled = false, status = "idle" }) { | |
| const [text, setText] = React.useState(""); | |
| const [activeEmotion, setActiveEmotion] = React.useState("neutral"); | |
| const [text, setText] = React.useState(() => { | |
| try { | |
| if (typeof localStorage !== "undefined") { | |
| return localStorage.getItem(DRAFT_KEY) || ""; | |
| } | |
| } catch { | |
| // Storage unavailable | |
| } | |
| return ""; | |
| }); | |
| export default function TextToSpeech({ onSpeak, disabled = false, status = "idle" }) { | |
| const [activeEmotion, setActiveEmotion] = React.useState("neutral"); | |
| const [text, setText] = React.useState(() => { | |
| try { | |
| if (typeof localStorage !== "undefined") { | |
| return localStorage.getItem(DRAFT_KEY) || ""; | |
| } | |
| } catch { | |
| // Storage unavailable | |
| } | |
| return ""; | |
| }); |
🤖 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 69 - 79, Restore the
activeEmotion state declaration inside TextToSpeech alongside the existing text
state, exposing both activeEmotion and setActiveEmotion for the reads and
updates already used in the component. Initialize it to the component’s expected
default emotion value.
| useEffect(() => { | ||
| try { | ||
| if (typeof localStorage !== "undefined") { | ||
| if (inputText) { | ||
| localStorage.setItem(COMPOSE_DRAFT_KEY, inputText); | ||
| } else { | ||
| localStorage.removeItem(COMPOSE_DRAFT_KEY); | ||
| } | ||
| } | ||
| } catch { | ||
| // Storage unavailable | ||
| } | ||
| }, [inputText]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear the compose draft after successful speech submission.
handleSpeak does not clear inputText after speak(text) succeeds. This effect therefore keeps voiceforge:draft_speech after submission. A later refresh restores an already sent message.
Return an explicit success result from speak. When that result is successful, call setInputText(""). Do not clear the draft when speech generation fails.
🤖 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/VoiceForge.jsx` around lines 35 - 47, Update
handleSpeak to inspect the explicit success result returned by speak and call
setInputText("") only when speech generation succeeds; preserve the current
input and draft when speak fails. Ensure speak returns a success indicator on
successful completion and a failure result on errors.
🎊 PR Merged SuccessfullyHey @hrshjswniii! 👋 Congratulations and thank you for your contribution to VoiceForge! Note 🔗 Linked issue(s): #1187 · ✅ Marked as merged and complete Maintainers may still handle final cleanup, release notes, or follow-up tracking after the merge. 🤖 VoiceForge Automation · Updates automatically on edits |
itzzavdhesh
added
SSoC26
4 participants
🚀 Program
GSSoC
📝 Description
This PR implements Auto-Save Speech Composition Drafts & Crash Recovery in
VoiceForge.jsxandTextToSpeech.jsx, preserving typed text compositions in real-time across accidental browser refreshes, tab closures, or call dropouts.Key additions:
TextToSpeech.jsx): Initialized in-calltextstate fromlocalStorage.getItem("voiceforge:draft_call_speech")and auto-saved input changes on type.VoiceForge.jsx): Initialized composerinputTextstate fromlocalStorage.getItem("voiceforge:draft_speech")and persisted updates automatically.TextToSpeech.test.jsx): Added Vitest unit test assertions verifying component rendering and draft persistence handling.🔗 Related Issue
Closes #1187
🔄 Type of Change
🧪 How to Test
http://localhost:5173in a browser.npm run test --workspace clientand verify all tests pass.✅ Checklist
feat: add auto-save speech composition drafts and crash recovery)Summary by cubic
Adds auto-save for speech drafts in the composer and in-call views to prevent text loss on refresh, tab close, or crashes. Drafts are restored on load and cleared after speaking or when the input is emptied.
voiceforge:draft_speechandvoiceforge:draft_call_speech, using safe reads/writes with graceful fallback.Written for commit 9ffd76a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests