Skip to content

ECHO-549 simplify the projectconversation route#350

Merged
ussaama merged 2 commits intomainfrom
simplify-project-conversation-route
Oct 29, 2025
Merged

ECHO-549 simplify the projectconversation route#350
ussaama merged 2 commits intomainfrom
simplify-project-conversation-route

Conversation

@ussaama
Copy link
Copy Markdown
Contributor

@ussaama ussaama commented Oct 28, 2025

Summary by CodeRabbit

  • New Features

    • Added audio conversation mode with voice recording, streaming replies, microphone device controls, permissions troubleshooting, cooldown countdown, and a dynamic recording timer
    • Added text conversation mode with message input, auto-scroll, submit/finish workflow, and finish confirmation modal
    • Shows “Thinking…” placeholder while replies are being prepared
  • Refactor

    • Split conversation UI into dedicated audio and text components for simpler routing and composition

@linear
Copy link
Copy Markdown

linear bot commented Oct 28, 2025

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 28, 2025

Walkthrough

Splits the previous monolithic participant conversation page into two exported components: ParticipantConversationAudio (audio streaming/recording flow) and ParticipantConversationText (text input flow). Routes now delegate directly to these components; route-level inline logic was removed.

Changes

Cohort / File(s) Summary
Audio conversation component
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
New exported React component implementing audio recording, chunked uploads, wake lock, mic device handling, conversation deletion detection, ECHO cooldown timer, streaming replies integration, permission/troubleshooting and finish-confirmation modals, UI controls and timers.
Text conversation component
echo/frontend/src/components/participant/ParticipantConversationText.tsx
New exported React component handling text input flow: data loading, text-chunk uploads/mutations, sticky textarea with Submit/Finish, finish-confirmation modal, auto-scroll to new chunks, and error/loading states.
Route refactor
echo/frontend/src/routes/participant/ParticipantConversation.tsx
Replaced large route implementations with minimal delegating routes that render <ParticipantConversationAudio /> and <ParticipantConversationText />; removed prior inline state, hooks, and UI composition from the routes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Attention points:
    • Audio recording lifecycle, wake lock and mic device switching handling
    • Streaming reply integration and cooldown/timer correctness
    • Conversation deletion/race conditions during recording and finish flow
    • Cleanup of side effects (wake locks, media streams) on unmount

Possibly related PRs

LGTM.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues Check ❓ Inconclusive The linked issue ECHO-549 provides only a title "Simplify project conversation route" without specific coding requirements or acceptance criteria. While the changeset demonstrably simplifies the route by extracting logic into subcomponents and removing data-loading/interaction complexity from the route layer, there are insufficient details in the provided linked issue to definitively validate that all specified requirements have been met or to assess completeness against measurable objectives. Verify the full requirements in ECHO-549 (details not provided in the linked_issues section) to confirm that the refactoring achieves all stated objectives. Consider adding more context about what "simplification" entails: performance improvements, testability gains, code maintainability metrics, or other success criteria.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "ECHO-549 simplify the projectconversation route" is clear and directly summarizes the main change: the ParticipantConversation route has been refactored to extract business logic into dedicated subcomponents (ParticipantConversationAudio and ParticipantConversationText), replacing the previous concatenated route logic with minimal route wrappers. The title is specific, concise, and accurately reflects the primary objective of simplifying the route structure without being vague or misleading.
Out of Scope Changes Check ✅ Passed All changes in this PR directly support the route simplification objective. The two new files (ParticipantConversationAudio.tsx and ParticipantConversationText.tsx) contain logic extracted from the original route, while ParticipantConversation.tsx is simplified to minimal route wrappers that delegate to these subcomponents. No unrelated modifications such as formatting-only changes, dependency updates, or tangential refactoring are present in the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch simplify-project-conversation-route

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added the Feature label Oct 28, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f160eb7 and 9474dd7.

📒 Files selected for processing (3)
  • echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (1 hunks)
  • echo/frontend/src/components/participant/ParticipantConversationText.tsx (1 hunks)
  • echo/frontend/src/routes/participant/ParticipantConversation.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (13)
echo/frontend/src/components/participant/hooks/index.ts (5)
  • useParticipantProjectById (279-284)
  • useConversationQuery (326-346)
  • useConversationChunksQuery (348-358)
  • useConversationRepliesQuery (308-324)
  • useUploadConversationChunk (51-145)
echo/frontend/src/hooks/useElementOnScreen.ts (1)
  • useElementOnScreen (3-25)
echo/frontend/src/hooks/useI18nNavigate.ts (1)
  • useI18nNavigate (10-38)
echo/frontend/src/hooks/useLanguage.ts (1)
  • useLanguage (24-45)
echo/frontend/src/components/project/ProjectQRCode.tsx (1)
  • useProjectSharingLink (23-59)
echo/frontend/src/hooks/useWakeLock.ts (1)
  • useWakeLock (3-57)
echo/frontend/src/config.ts (1)
  • API_BASE_URL (7-7)
echo/frontend/src/lib/utils.ts (1)
  • checkPermissionError (18-34)
echo/frontend/src/lib/api.ts (1)
  • finishConversation (1053-1057)
echo/frontend/src/components/participant/ParticipantBody.tsx (1)
  • ParticipantBody (27-263)
echo/frontend/src/components/participant/EchoErrorAlert.tsx (1)
  • EchoErrorAlert (5-30)
echo/frontend/src/components/common/ScrollToBottom.tsx (1)
  • ScrollToBottomButton (10-34)
echo/frontend/src/components/common/i18nLink.tsx (1)
  • I18nLink (6-29)
echo/frontend/src/components/participant/ParticipantConversationText.tsx (6)
echo/frontend/src/components/participant/hooks/index.ts (4)
  • useParticipantProjectById (279-284)
  • useConversationQuery (326-346)
  • useConversationChunksQuery (348-358)
  • useUploadConversationTextChunk (147-241)
echo/frontend/src/components/project/ProjectQRCode.tsx (1)
  • useProjectSharingLink (23-59)
echo/frontend/src/hooks/useElementOnScreen.ts (1)
  • useElementOnScreen (3-25)
echo/frontend/src/hooks/useI18nNavigate.ts (1)
  • useI18nNavigate (10-38)
echo/frontend/src/components/participant/ParticipantBody.tsx (1)
  • ParticipantBody (27-263)
echo/frontend/src/components/common/i18nLink.tsx (1)
  • I18nLink (6-29)
echo/frontend/src/routes/participant/ParticipantConversation.tsx (2)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (1)
  • ParticipantConversationAudio (57-718)
echo/frontend/src/components/participant/ParticipantConversationText.tsx (1)
  • ParticipantConversationText (36-244)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: ci-check-server

Comment on lines +96 to +103
const [isFinishing, _setIsFinishing] = useState(false);
const [isStopping, setIsStopping] = useState(false);
const [opened, { open, close }] = useDisclosure(false);
// Navigation and language
const navigate = useI18nNavigate();
const { iso639_1 } = useLanguage();
const newConversationLink = useProjectSharingLink(projectQuery.data);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove dead state and align loading flags.

isFinishing is never updated; Finish button’s loading/disabled props won’t reflect actual state. Use isStopping or drop the flags.

-const [isFinishing, _setIsFinishing] = useState(false);
+// removed dead state

...
-  <Button
+  <Button
     size="lg"
     radius="md"
     onClick={open}
     variant="light"
     rightSection={<IconCheck />}
     className="w-full md:w-auto"
-    loading={isFinishing}
-    disabled={isFinishing}
+    // loading/disabled governed by modal + isStopping during confirm
   >

Also applies to: 608-623

🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
around lines 96-103 (and also apply the same change to lines 608-623), remove
the dead state declaration "const [isFinishing, _setIsFinishing] =
useState(false);" and replace any uses of isFinishing with the active isStopping
flag (or the appropriate loading/disabled prop) so the Finish button reflects
actual state; also delete the unused _setIsFinishing identifier and ensure all
button loading/disabled props and any logic reference isStopping instead.

Comment on lines +205 to +209
experimental_prepareRequestBody() {
return {
language: iso639_1,
};
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Drop redundant experimental_prepareRequestBody.

You already pass body. Keeping both is confusing and risks drift.

- experimental_prepareRequestBody() {
-   return {
-     language: iso639_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.

Suggested change
experimental_prepareRequestBody() {
return {
language: iso639_1,
};
},
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
around lines 205 to 209, remove the redundant experimental_prepareRequestBody()
method (which returns { language: iso639_1 }) because the request body is
already passed elsewhere; delete this method and any direct calls to it, ensure
the existing body value is used where the request is constructed, and run a
quick build/test to verify there are no remaining references or type errors.

Comment on lines +251 to +259
while (uploadChunkMutation.isPending) {
await new Promise((resolve) => setTimeout(resolve, 1000));
}

// scroll to bottom of the page
setTimeout(() => {
if (scrollTargetRef.current) {
scrollTargetRef.current.scrollIntoView({ behavior: "smooth" });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Avoid busy-wait while uploads are pending.

Looping every 1s can stall UX indefinitely if a retry storms. Prefer early-return with user feedback and disable the ECHO button while pending.

-// Wait for pending uploads to complete
-while (uploadChunkMutation.isPending) {
-  await new Promise((resolve) => setTimeout(resolve, 1000));
-}
+if (uploadChunkMutation.isPending) {
+  toast.info(t`Please wait for the current upload to finish.`);
+  return;
+}

Also set the button disabled state:

-<Button
+<Button
   ...
-  loading={isLoading}
+  loading={isLoading || uploadChunkMutation.isPending}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +67 to +75
uploadChunkMutation.mutate({
content: text.trim(),
conversationId: conversationId ?? "",
source: "PORTAL_TEXT",
timestamp: new Date(),
});

setText("");
};
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Guard against empty conversationId on submit.

onChunk uses conversationId ?? "" which may enqueue a bad mutation if params ever fail. Cheap guard improves resilience.

- uploadChunkMutation.mutate({
+ if (!conversationId) return;
+ uploadChunkMutation.mutate({
📝 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.

Suggested change
uploadChunkMutation.mutate({
content: text.trim(),
conversationId: conversationId ?? "",
source: "PORTAL_TEXT",
timestamp: new Date(),
});
setText("");
};
if (!conversationId) return;
uploadChunkMutation.mutate({
content: text.trim(),
conversationId: conversationId ?? "",
source: "PORTAL_TEXT",
timestamp: new Date(),
});
setText("");
};
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationText.tsx
around lines 67 to 75, the submit handler calls uploadChunkMutation.mutate with
conversationId ?? "" which can send an invalid empty ID; add a guard that
verifies conversationId is non-empty (e.g., if (!conversationId) return or show
a user/error state) before calling mutate, and only clear the input after a
successful guard; optionally disable the submit action/UI when conversationId is
missing to prevent future attempts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conversation id will always be there, otherwise the conversation doesnt start. the id comes from params here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ussaama LGTM, makes sense! If the route structure guarantees the param's there, no need for the extra guard. Ship it! 🚀

Comment on lines +82 to +85
const handleConfirmFinishButton = () => {
navigate(finishUrl);
};

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Unify finish flow with audio: call API before navigating.

Audio path hits finishConversation then navigates; text path only navigates. Ensure server state is finalized.

If /finish doesn’t finalize server-side, adopt this:

+import { finishConversation } from "@/lib/api";
...
-const handleConfirmFinishButton = () => {
-  navigate(finishUrl);
-};
+const handleConfirmFinishButton = async () => {
+  try {
+    await finishConversation(conversationId ?? "");
+    navigate(finishUrl);
+  } catch (e) {
+    // optionally show a toast/error UI
+    console.error("Failed to finish conversation", e);
+  }
+};

Would you like me to scan the codebase to confirm whether the /finish route performs the finalization?


🏁 Script executed:

#!/bin/bash
rg -nP --type=ts --type=tsx -C3 'finishConversation\s*\('

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Search for finishConversation usage across TypeScript files
rg -nP -C3 'finishConversation' --type=ts

Length of output: 2561


🏁 Script executed:

#!/bin/bash
# Search for audio finish handler to compare with text handler
rg -nP -C3 'handleConfirmFinishButton|finishUrl' --type=ts

Length of output: 3396


🏁 Script executed:

#!/bin/bash
# Look for finish-related API endpoints or route handlers
rg -nP -C3 '\/finish|finishConversation' | head -100

Length of output: 5043


🏁 Script executed:

#!/bin/bash
# Find ParticipantPostConversation component
rg -nP -C5 'ParticipantPostConversation' --type=ts

Length of output: 2877


🏁 Script executed:

#!/bin/bash
# Find backend endpoint implementation to see what finalization occurs
rg -nP -A20 'async def run_when_conversation_is_finished' --type=py

Length of output: 1404


Call finishConversation API before navigating in text mode to match audio flow.

The audio path explicitly calls finishConversation() before navigating to /finish (ParticipantConversationAudio.tsx:281), which triggers the backend finish hook. The text path only navigates without calling the API, skipping server-side finalization. The ParticipantPostConversation component rendered at the /finish route handles post-conversation UI but doesn't trigger the backend endpoint.

Apply the suggested fix to ParticipantConversationText.tsx (lines 82-84):

+import { finishConversation } from "@/lib/api";
...
-const handleConfirmFinishButton = () => {
-  navigate(finishUrl);
-};
+const handleConfirmFinishButton = async () => {
+  try {
+    await finishConversation(conversationId ?? "");
+    navigate(finishUrl);
+  } catch (e) {
+    console.error("Failed to finish conversation", e);
+  }
+};
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationText.tsx
around lines 82-85, the handler currently navigates directly to finishUrl
without calling the finishConversation API like the audio flow; update
handleConfirmFinishButton to call the same finishConversation function used by
ParticipantConversationAudio, await its completion (or chain the promise), and
then navigate to finishUrl, wrapping the call in try/catch (or finally) to log
any error but still proceed to navigate so the backend finish hook runs before
arriving at /finish.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont have any audio chunks to finish here just text updates in directus. Also finish doesnt appear unless we save atleast one text message in directus.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check what ParticipantPostConversation component does and if it handles finalization
rg -nP -A30 'export.*ParticipantPostConversation' --type=ts

# Also check if there's any logic in the finish route that calls finishConversation
rg -nP -C5 'conversation.*finish|finishConversation' --type=ts | grep -A10 -B10 'ParticipantPostConversation\|/finish'

Length of output: 267135


🏁 Script executed:

#!/bin/bash
# Check the backend to see if finishConversation endpoint is required for all conversation types
rg -nP -A20 'def.*finish.*conversation|finish_conversation' --type=py

# Check if there's any difference in handling audio vs text conversations on finish
rg -nP -C5 'PORTAL_TEXT|audio.*text|conversation.*type' --type=py | head -80

Length of output: 34000


🏁 Script executed:

#!/bin/bash
# Look at the finishConversation API implementation in the frontend
cat frontend/src/lib/api.ts | grep -A20 -B5 'finishConversation'

Length of output: 55


@ussaama totally hear you — text mode has no audio chunks. The finish step isn’t about audio, it flips server state and triggers follow-ups.

Why this still matters in text mode:

  • Marks conversation as is_finished so further chunks are rejected (per server tests).
  • Triggers task_finish_conversation_hook (and RAG/text-only ETL where applicable).
  • Reporting/filters reference “finished” conversations.

Right now, text flow just navigates (ParticipantConversationText.tsx Line 82) and skips that server-side finalization. Let’s align it with audio by calling the existing API before navigating:

// echo/frontend/src/components/participant/ParticipantConversationText.tsx
import { finishConversation } from "@/lib/api"; // add this

// ...

const handleConfirmFinishButton = async () => {
  try {
    await finishConversation(conversationId ?? "");
  } catch (e) {
    // keep UX snappy; log and still navigate
    console.error("Failed to finish conversation", e);
  } finally {
    navigate(finishUrl);
  }
};

Impact:

  • Consistent semantics across audio/text.
  • Finished flag set immediately; reports and background jobs see the right state.
  • Zero behavioral change for the “button visibility” guard — finish still won’t show until there’s at least one text chunk saved.

Thumbs-up to ship this tweak? I can prep the patch. LGTM.


✏️ Learnings added
Learnt from: ussaama
PR: Dembrane/echo#350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.888Z
Learning: In text mode (echo/frontend/src/components/participant/ParticipantConversationText.tsx), participants only submit PORTAL_TEXT chunks (no audio). The “Finish” button is shown only after at least one text message is saved to Directus.

Learnt from: ussaama
PR: Dembrane/echo#350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.888Z
Learning: The participant “finish” flow should call finishConversation(conversationId) (echo/frontend/src/lib/api.ts) before navigating to /:projectId/conversation/:conversationId/finish so the backend sets is_finished and triggers task_finish_conversation_hook.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +223 to +239
<I18nLink to={audioModeUrl}>
<ActionIcon component="a" variant="default" size="50" radius="md">
<IconMicrophone />
</ActionIcon>
</I18nLink>
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
component="a"
variant="light"
rightSection={<IconCheck />}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
)}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Stop nesting anchors; fix button semantics.

I18nLink renders an anchor. Wrapping an ActionIcon rendered as nests anchors; the Finish button also forces without href. This breaks a11y and click targets. Switch to non-anchor components.

Apply:

-  <I18nLink to={audioModeUrl}>
-    <ActionIcon component="a" variant="default" size="50" radius="md">
+  <I18nLink to={audioModeUrl}>
+    <ActionIcon variant="default" size="50" radius="md">
       <IconMicrophone />
     </ActionIcon>
   </I18nLink>
...
-  <Button
+  <Button
     size="lg"
     radius="md"
     onClick={openFinishModal}
-    component="a"
     variant="light"
     rightSection={<IconCheck />}
   >
📝 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.

Suggested change
<I18nLink to={audioModeUrl}>
<ActionIcon component="a" variant="default" size="50" radius="md">
<IconMicrophone />
</ActionIcon>
</I18nLink>
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
component="a"
variant="light"
rightSection={<IconCheck />}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
)}
<I18nLink to={audioModeUrl}>
<ActionIcon variant="default" size="50" radius="md">
<IconMicrophone />
</ActionIcon>
</I18nLink>
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
variant="light"
rightSection={<IconCheck />}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
)}
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationText.tsx
around lines 223-239, you are nesting anchors because I18nLink renders an <a>
and ActionIcon/Finish Button are forced to render as <a>; change ActionIcon to
render a non-anchor element (e.g., remove component="a" or set component="div"
or "button") and remove component="a" from the Button so it renders as a native
<button> (or another non-anchor) while keeping I18nLink as the outer link for
audioModeUrl; ensure the Button uses onClick for navigation/modal only and that
any interactive element has proper role/aria attributes if you switch to a div.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ussaama LGTM! Ship it 🚀


🧠 Learnings used
Learnt from: ussaama
PR: Dembrane/echo#259
File: echo/frontend/src/components/layout/ParticipantLayout.tsx:33-33
Timestamp: 2025-08-08T10:39:31.114Z
Learning: In echo/frontend/src/components/layout/ParticipantLayout.tsx, prefer using simple pathname.includes("start") and pathname.includes("finish") to control the settings button visibility. No need to switch to segment-based matching or add a useEffect to auto-close the modal for these routes, per ussaama’s preference in PR #259.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Comment on lines 4 to 10
export const ParticipantConversationAudioRoute = () => {
const { projectId, conversationId } = useParams();

// Get device ID from cookies for audio recording
const savedDeviceId = Cookies.get("micDeviceId");
const deviceId = savedDeviceId || "";

const projectQuery = useParticipantProjectById(projectId ?? "");
const conversationQuery = useConversationQuery(projectId, conversationId);
const chunks = useConversationChunksQuery(projectId, conversationId);
const repliesQuery = useConversationRepliesQuery(conversationId);
const uploadChunkMutation = useUploadConversationChunk();

const onChunk = (chunk: Blob) => {
uploadChunkMutation.mutate({
chunk,
conversationId: conversationId ?? "",
runFinishHook: false,
source: "PORTAL_AUDIO",
timestamp: new Date(),
});
};

const [scrollTargetRef, isVisible] = useElementOnScreen({
root: null,
rootMargin: "-83px",
threshold: 0.1,
});

const [troubleShootingGuideOpened, setTroubleShootingGuideOpened] =
useState(false);
const [lastReplyTime, setLastReplyTime] = useState<Date | null>(null);
const [remainingCooldown, setRemainingCooldown] = useState(0);
const [showCooldownMessage, setShowCooldownMessage] = useState(false);
const [
conversationDeletedDuringRecording,
setConversationDeletedDuringRecording,
] = useState(false);

const [isFinishing, _setIsFinishing] = useState(false);
const [isStopping, setIsStopping] = useState(false);
const [opened, { open, close }] = useDisclosure(false);
// Navigation and language
const navigate = useI18nNavigate();
const { iso639_1 } = useLanguage();
const newConversationLink = useProjectSharingLink(projectQuery.data);

// Calculate remaining cooldown time
const getRemainingCooldown = useCallback(() => {
if (!lastReplyTime) return 0;
const cooldownSeconds = DEFAULT_REPLY_COOLDOWN;
const elapsedSeconds = Math.floor(
(new Date().getTime() - lastReplyTime.getTime()) / 1000,
);
return Math.max(0, cooldownSeconds - elapsedSeconds);
}, [lastReplyTime]);

// Update cooldown timer
useEffect(() => {
if (!lastReplyTime) return;

const interval = setInterval(() => {
const remaining = getRemainingCooldown();
setRemainingCooldown(remaining);

if (remaining <= 0) {
clearInterval(interval);
}
}, 1000);

return () => clearInterval(interval);
}, [lastReplyTime, getRemainingCooldown]);

const audioRecorder = useChunkedAudioRecorder({ deviceId, onChunk });
useWakeLock({ obtainWakeLockOnMount: true });

const {
startRecording,
stopRecording,
isRecording,
isPaused,
pauseRecording,
resumeRecording,
recordingTime,
errored,
permissionError,
} = audioRecorder;

const handleMicrophoneDeviceChanged = async () => {
try {
stopRecording();
} catch (error) {
toast.error(
t`Failed to stop recording on device change. Please try again.`,
);
console.error("Failed to stop recording on device change:", error);
}
};

useWindowEvent("microphoneDeviceChanged", handleMicrophoneDeviceChanged);

// Monitor conversation status during recording - handle deletion mid-recording
useEffect(() => {
if (!isRecording) return;

if (
conversationQuery.isError &&
!conversationQuery.isFetching &&
!conversationQuery.isLoading
) {
const error = conversationQuery.error;
const httpStatus = error?.response?.status;

if (
httpStatus &&
CONVERSATION_DELETION_STATUS_CODES.includes(httpStatus)
) {
console.warn(
"Conversation was deleted or is no longer accessible during recording",
{ message: error?.message, status: httpStatus },
);
stopRecording();
setConversationDeletedDuringRecording(true);
} else {
console.warn(
"Error fetching conversation during recording - continuing",
{ message: error?.message, status: httpStatus },
);
}
}
}, [
isRecording,
conversationQuery.isError,
conversationQuery.isLoading,
conversationQuery.isFetching,
conversationQuery.error,
stopRecording,
]);

const {
messages: echoMessages,
isLoading,
status,
error,
handleSubmit,
} = useChat({
api: `${API_BASE_URL}/conversations/${conversationId}/get-reply`,
body: { language: iso639_1 },
experimental_prepareRequestBody() {
return {
language: iso639_1,
};
},
initialMessages:
repliesQuery.data?.map((msg) => ({
content: msg.content_text ?? "",
id: String(msg.id),
role: msg.type === "assistant_reply" ? "assistant" : "user",
})) ?? [],
onError: (error) => {
console.error("onError", error);
},
});

// Handlers
const handleCheckMicrophoneAccess = async () => {
const permissionError = await checkPermissionError();
if (["granted", "prompt"].includes(permissionError ?? "")) {
window.location.reload();
} else {
alert(
t`Microphone access is still denied. Please check your settings and try again.`,
);
}
};

const handleReply = async (e: React.MouseEvent<HTMLButtonElement>) => {
const remaining = getRemainingCooldown();
if (remaining > 0) {
setShowCooldownMessage(true);
const minutes = Math.floor(remaining / 60);
const seconds = remaining % 60;
const timeStr =
minutes > 0
? t`${minutes} minutes and ${seconds} seconds`
: t`${seconds} seconds`;

toast.info(t`Please wait ${timeStr} before requesting another ECHO.`);
return;
}

try {
setShowCooldownMessage(false);
// Wait for pending uploads to complete
while (uploadChunkMutation.isPending) {
await new Promise((resolve) => setTimeout(resolve, 1000));
}

// scroll to bottom of the page
setTimeout(() => {
if (scrollTargetRef.current) {
scrollTargetRef.current.scrollIntoView({ behavior: "smooth" });
}
}, 0);

handleSubmit(e, { allowEmptySubmit: true });
setLastReplyTime(new Date());
setRemainingCooldown(DEFAULT_REPLY_COOLDOWN);
} catch (error) {
console.error("Error during echo:", error);
}
};

const handleStopRecording = () => {
if (isRecording) {
pauseRecording();
open();
}
};

const handleConfirmFinish = async () => {
setIsStopping(true);
try {
stopRecording();
await finishConversation(conversationId ?? "");
close();
navigate(finishUrl);
} catch (error) {
console.error("Error finishing conversation:", error);
toast.error(t`Failed to finish conversation. Please try again.`);
setIsStopping(false);
}
};

if (conversationQuery.isLoading || projectQuery.isLoading) {
return <LoadingOverlay visible />;
}

// Check if conversation is not present or failed to load
if (
conversationQuery.isError ||
!conversationQuery.data ||
conversationDeletedDuringRecording
) {
return (
<div className="container mx-auto flex h-full max-w-2xl flex-col items-center justify-center">
<div className="p-8 text-center">
<Text size="xl" fw={500} c="red" mb="md">
{conversationDeletedDuringRecording ? (
<Trans id="participant.conversation.ended">
Conversation Ended
</Trans>
) : (
<Trans id="participant.conversation.error">
Something went wrong
</Trans>
)}
</Text>
<Text size="md" c="dimmed" mb="lg">
{conversationDeletedDuringRecording ? (
<Trans id="participant.conversation.error.deleted">
It looks like the conversation was deleted while you were
recording. We've stopped the recording to prevent any issues.
You can start a new one anytime.
</Trans>
) : (
<Trans id="participant.conversation.error.loading">
The conversation could not be loaded. Please try again or
contact support.
</Trans>
)}
</Text>
<Group justify="center" gap="md">
<Button
variant="light"
size="md"
onClick={() => window.location.reload()}
leftSection={<IconReload />}
>
<Trans id="participant.button.reload">Reload Page</Trans>
</Button>
{newConversationLink && (
<Button
leftSection={<IconPlus size={16} />}
variant="filled"
size="md"
component="a"
href={newConversationLink}
>
<Trans id="participant.button.start.new.conversation">
Start New Conversation
</Trans>
</Button>
)}
</Group>
</div>
</div>
);
}

const textModeUrl = `/${projectId}/conversation/${conversationId}/text`;
const finishUrl = `/${projectId}/conversation/${conversationId}/finish`;

return (
<div className="container mx-auto flex h-full max-w-2xl flex-col">
{/* modal for permissions error */}
<Modal
opened={!!permissionError}
onClose={() => true}
centered
fullScreen
radius={0}
transitionProps={{ duration: 200, transition: "fade" }}
withCloseButton={false}
>
<div className="h-full rounded-md bg-white py-4">
<Stack className="container mx-auto mt-4 max-w-2xl px-2" gap="lg">
<div className="max-w-prose text-lg">
<Trans id="participant.alert.microphone.access.failure">
Oops! It looks like microphone access was denied. No worries,
though! We've got a handy troubleshooting guide for you. Feel
free to check it out. Once you've resolved the issue, come back
and visit this page again to check if your microphone is ready.
</Trans>
</div>

<Button
component="a"
href="https://dembrane.notion.site/Troubleshooting-Microphone-Permissions-All-Languages-bd340257647742cd9cd960f94c4223bb?pvs=74"
target="_blank"
size={troubleShootingGuideOpened ? "lg" : "xl"}
leftSection={<IconQuestionMark />}
variant={!troubleShootingGuideOpened ? "filled" : "light"}
onClick={() => setTroubleShootingGuideOpened(true)}
>
<Trans id="participant.button.open.troubleshooting.guide">
Open troubleshooting guide
</Trans>
</Button>
<Divider />
<Button
size={!troubleShootingGuideOpened ? "lg" : "xl"}
leftSection={<IconReload />}
variant={troubleShootingGuideOpened ? "filled" : "light"}
onClick={handleCheckMicrophoneAccess}
>
<Trans id="participant.button.check.microphone.access">
Check microphone access
</Trans>
</Button>
</Stack>
</div>
</Modal>

{/* modal for stop recording confirmation */}
<Modal
opened={opened}
onClose={isStopping ? () => {} : close}
closeOnClickOutside={!isStopping}
closeOnEscape={!isStopping}
centered
title={
<Text fw={500}>
<Trans id="participant.modal.stop.title">Finish Conversation</Trans>
</Text>
}
size="sm"
radius="md"
padding="xl"
>
<Stack gap="lg">
<Text>
<Trans id="participant.modal.stop.message">
Are you sure you want to finish the conversation?
</Trans>
</Text>
<Group grow gap="md">
<Button
variant="outline"
color="gray"
onClick={close}
disabled={isStopping}
miw={100}
radius="md"
size="md"
>
<Trans id="participant.button.stop.no">No</Trans>
</Button>
<Button
onClick={handleConfirmFinish}
loading={isStopping}
miw={100}
radius="md"
size="md"
>
<Trans id="participant.button.stop.yes">Yes</Trans>
</Button>
</Group>
</Stack>
</Modal>

<Box className={clsx("relative flex-grow p-4 pb-12 transition-all")}>
{projectQuery.data && conversationQuery.data && (
<ParticipantBody
interleaveMessages={false}
projectId={projectId ?? ""}
conversationId={conversationId ?? ""}
recordingStarted={isRecording}
/>
)}

<Stack gap="sm">
{echoMessages && echoMessages.length > 0 && (
<>
{echoMessages.map((message, index) => (
<SpikeMessage
key={message.id}
message={{
content_text: message.content,
date_created: new Date().toISOString(),
// @ts-expect-error - id is a string
id: Number.parseInt(message.id) || 0,
type:
message.role === "assistant" ? "assistant_reply" : "user",
}}
loading={index === echoMessages.length - 1 && isLoading}
className={`min-h-[180px] md:min-h-[169px] ${index !== echoMessages.length - 1 ? "border-b" : ""}`}
/>
))}
{status !== "streaming" && status !== "ready" && !error && (
<SpikeMessage
key="thinking"
message={{
content_text: t`Thinking...`,
date_created: new Date().toISOString(),
// @ts-expect-error - id is a string
id: 0,
type: "assistant_reply",
}}
loading={true}
className="min-h-[180px] md:min-h-[169px]"
/>
)}
</>
)}

{error && <EchoErrorAlert error={error} />}
</Stack>
<div ref={scrollTargetRef} />
</Box>

{!errored && (
<Stack
gap="lg"
className="sticky bottom-0 z-10 w-full border-t border-slate-300 bg-white p-4"
>
<Group
justify="center"
className={
"absolute bottom-[125%] left-1/2 z-50 translate-x-[-50%]"
}
>
<ScrollToBottomButton
elementRef={scrollTargetRef}
isVisible={isVisible}
/>
</Group>

{/* Recording time indicator */}
{isRecording && (
<div className="w-full border-slate-300 bg-white pb-4 pt-2">
<Group justify="center" align="center">
{isPaused ? (
<IconPlayerPause />
) : (
<div className="h-4 w-4 animate-pulse rounded-full bg-red-500" />
)}
<Text className="text-3xl">
{Math.floor(recordingTime / 3600) > 0 && (
<>
{Math.floor(recordingTime / 3600)
.toString()
.padStart(2, "0")}
:
</>
)}
{Math.floor((recordingTime % 3600) / 60)
.toString()
.padStart(2, "0")}
:{(recordingTime % 60).toString().padStart(2, "0")}
</Text>
</Group>
</div>
)}

<Group justify="center">
{!isRecording && (
<Group className="w-full">
{chunks?.data &&
chunks.data.length > 0 &&
!!projectQuery.data?.is_get_reply_enabled && (
<Group>
<Button
fullWidth
variant="default"
size="lg"
radius="md"
onClick={(e) => {
handleReply(e);
}}
loading={isLoading}
loaderProps={{ type: "dots" }}
>
{showCooldownMessage && remainingCooldown > 0 ? (
<Text>
<Trans>
<span className="hidden md:inline">Wait </span>
{Math.floor(remainingCooldown / 60)}:
{(remainingCooldown % 60)
.toString()
.padStart(2, "0")}
</Trans>
</Text>
) : (
<Trans id="participant.button.echo">ECHO</Trans>
)}
</Button>
</Group>
)}

<Button
size="lg"
radius="md"
rightSection={<IconMicrophone />}
onClick={startRecording}
className="flex-grow"
>
<Trans id="participant.button.record">Record</Trans>
</Button>

<I18nLink to={textModeUrl}>
<ActionIcon
component="a"
size="50"
variant="default"
radius="md"
>
<IconTextCaption />
</ActionIcon>
</I18nLink>

{!isRecording &&
!isStopping &&
chunks?.data &&
chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={open}
variant="light"
rightSection={<IconCheck />}
className="w-full md:w-auto"
loading={isFinishing}
disabled={isFinishing}
>
<Trans id="participant.button.finish">Finish</Trans>
</Button>
)}
</Group>
)}

{isRecording && (
<>
{chunks?.data &&
chunks.data.length > 0 &&
!!projectQuery.data?.is_get_reply_enabled && (
<Group>
<Button
fullWidth
variant="default"
size="lg"
radius="md"
onClick={(e) => {
handleReply(e);
}}
loading={isLoading}
loaderProps={{ type: "dots" }}
>
{showCooldownMessage && remainingCooldown > 0 ? (
<Text>
<Trans>
<span className="hidden md:inline">Wait </span>
{Math.floor(remainingCooldown / 60)}:
{(remainingCooldown % 60)
.toString()
.padStart(2, "0")}
</Trans>
</Text>
) : (
<Trans id="participant.button.is.recording.echo">
ECHO
</Trans>
)}
</Button>
</Group>
)}

{isPaused ? (
<Button
className="flex-1"
size="lg"
radius="md"
onClick={resumeRecording}
>
<span className="hidden md:block">
<Trans id="participant.button.resume">Resume</Trans>
</span>
<IconPlayerPlay size={18} className="ml-0 md:ml-1" />
</Button>
) : (
<Button
className="flex-1"
size="lg"
radius="md"
onClick={pauseRecording}
>
<span className="hidden md:block">
<Trans id="participant.button.pause">Pause</Trans>
</span>
<IconPlayerPause size={18} className="ml-0 md:ml-1" />
</Button>
)}

<Button
variant="outline"
size="lg"
radius="md"
color="red"
onClick={handleStopRecording}
disabled={isStopping}
className={
!chunks?.data ||
chunks.data.length === 0 ||
!projectQuery.data?.is_get_reply_enabled
? "flex-1"
: ""
}
>
<Trans id="participant.button.stop">Stop</Trans>
<IconPlayerStopFilled
size={18}
className="ml-0 hidden md:ml-1 md:block"
/>
</Button>
</>
)}
</Group>
</Stack>
)}
</div>
);
return <ParticipantConversationAudio />;
};

export const ParticipantConversationTextRoute = () => {
const { projectId, conversationId } = useParams();
const projectQuery = useParticipantProjectById(projectId ?? "");
const conversationQuery = useConversationQuery(projectId, conversationId);
const chunks = useConversationChunksQuery(projectId, conversationId);
const uploadChunkMutation = useUploadConversationTextChunk();
const newConversationLink = useProjectSharingLink(projectQuery.data);

const [text, setText] = useState("");
const [
finishModalOpened,
{ open: openFinishModal, close: closeFinishModal },
] = useDisclosure(false);

const [scrollTargetRef] = useElementOnScreen({
root: null,
rootMargin: "-158px",
threshold: 0.1,
});

const onChunk = () => {
if (!text || text.trim() === "") {
return;
}

setTimeout(() => {
if (scrollTargetRef.current) {
scrollTargetRef.current.scrollIntoView({ behavior: "smooth" });
}
}, 0);

uploadChunkMutation.mutate({
content: text.trim(),
conversationId: conversationId ?? "",
source: "PORTAL_TEXT",
timestamp: new Date(),
});

setText("");
};

const navigate = useI18nNavigate();

const audioModeUrl = `/${projectId}/conversation/${conversationId}`;
const finishUrl = `/${projectId}/conversation/${conversationId}/finish`;

const handleConfirmFinishButton = () => {
navigate(finishUrl);
};

if (conversationQuery.isLoading || projectQuery.isLoading) {
return <LoadingOverlay visible />;
}

// Check if conversation is not present or failed to load
if (conversationQuery.isError || !conversationQuery.data) {
return (
<div className="container mx-auto flex h-full max-w-2xl flex-col items-center justify-center">
<div className="p-8 text-center">
<Text size="xl" fw={500} c="red" mb="md">
<Trans id="participant.conversation.error.text.mode">
Something went wrong
</Trans>
</Text>
<Text size="md" c="dimmed" mb="lg">
<Trans id="participant.conversation.error.loading.text.mode">
The conversation could not be loaded. Please try again or contact
support.
</Trans>
</Text>
<Group justify="center" gap="md">
<Button
variant="light"
size="md"
onClick={() => window.location.reload()}
leftSection={<IconReload />}
>
<Trans id="participant.button.reload.page.text.mode">
Reload Page
</Trans>
</Button>
{newConversationLink && (
<Button
leftSection={<IconPlus size={16} />}
variant="filled"
size="md"
component="a"
href={newConversationLink}
>
<Trans id="participant.button.start.new.conversation.text.mode">
Start New Conversation
</Trans>
</Button>
)}
</Group>
</div>
</div>
);
}

return (
<div className="container mx-auto flex h-full max-w-2xl flex-col">
{/* modal for finish conversation confirmation */}
<Modal
opened={finishModalOpened}
onClose={closeFinishModal}
centered
title={
<Text fw={500}>
<Trans id="participant.modal.finish.title.text.mode">
Finish Conversation
</Trans>
</Text>
}
size="sm"
radius="md"
padding="xl"
>
<Stack gap="lg">
<Text>
<Trans id="participant.modal.finish.message.text.mode">
Are you sure you want to finish the conversation?
</Trans>
</Text>
<Group grow gap="md">
<Button
variant="outline"
color="gray"
onClick={closeFinishModal}
miw={100}
radius="md"
size="md"
>
<Trans id="participant.button.finish.no.text.mode">No</Trans>
</Button>
<Button
onClick={handleConfirmFinishButton}
miw={100}
radius="md"
size="md"
>
<Trans id="participant.button.finish.yes.text.mode">Yes</Trans>
</Button>
</Group>
</Stack>
</Modal>

<Box className={clsx("relative flex-grow px-4 py-12 transition-all")}>
{projectQuery.data && conversationQuery.data && (
<ParticipantBody
viewResponses
projectId={projectId ?? ""}
conversationId={conversationId ?? ""}
/>
)}

<div ref={scrollTargetRef} className="h-0" />
</Box>

<Stack className="sticky bottom-0 z-10 w-full border-slate-300 bg-white p-4">
<Group
justify="center"
className={"absolute bottom-[110%] left-1/2 z-50 translate-x-[-50%]"}
>
{/* <ScrollToBottomButton
elementRef={scrollTargetRef}
isVisible={isVisible}
/> */}
</Group>
<textarea
className="h-32 w-full rounded-md border border-slate-300 p-4"
placeholder={t`Type your response here`}
value={text}
onChange={(e) => setText(e.target.value)}
/>
<Group className="w-full">
<Button
size="lg"
radius="md"
rightSection={<IconUpload />}
onClick={onChunk}
loading={uploadChunkMutation.isPending}
className="flex-grow"
>
<Trans id="participant.button.submit.text.mode">Submit</Trans>
</Button>

<I18nLink to={audioModeUrl}>
<ActionIcon component="a" variant="default" size="50" radius="md">
<IconMicrophone />
</ActionIcon>
</I18nLink>
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
component="a"
variant="light"
rightSection={<IconCheck />}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
)}
</Group>
</Stack>
</div>
);
return <ParticipantConversationText />;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

LGTM. Route is finally thin.

Nice delegation to specialized components.

Consider lazy-loading to trim initial bundle:

  • const ParticipantConversationAudio = lazy(() => import(...))
  • const ParticipantConversationText = lazy(() => import(...))
🤖 Prompt for AI Agents
In echo/frontend/src/routes/participant/ParticipantConversation.tsx around lines
4 to 10, the route components currently import and render
ParticipantConversationAudio/Text eagerly; change them to lazy-loaded components
to reduce initial bundle size by: import React.lazy and Suspense at the top,
replace direct component imports with const ParticipantConversationAudio =
lazy(() => import('./ParticipantConversationAudio')) and likewise for
ParticipantConversationText, and wrap the returned component in a Suspense with
a lightweight fallback (spinner or null) in each route component so the UI
handles the async load.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
echo/frontend/src/components/participant/ParticipantConversationText.tsx (2)

24-35: Finish flow must hit the API before navigating.

Call finishConversation so the backend sets is_finished and triggers task_finish_conversation_hook; add a tiny loading state on confirm. Based on learnings.

@@
 import { useI18nNavigate } from "@/hooks/useI18nNavigate";
+import { finishConversation } from "@/lib/api";
 
 export const ParticipantConversationText = () => {
@@
-const handleConfirmFinishButton = () => {
-  navigate(finishUrl);
-};
+const [isFinishing, setIsFinishing] = useState(false);
+const handleConfirmFinishButton = async () => {
+  try {
+    setIsFinishing(true);
+    await finishConversation(conversationId ?? "");
+    navigate(finishUrl);
+  } catch (e) {
+    console.error("Failed to finish conversation", e);
+  } finally {
+    setIsFinishing(false);
+  }
+};
@@
-  <Button
+  <Button
     onClick={handleConfirmFinishButton}
+    loading={isFinishing}
+    disabled={isFinishing}
     miw={100}
     radius="md"
     size="md"
   >

Also applies to: 82-85, 171-178


223-227: Kill nested anchors; let I18nLink render the .

Remove component="a" from ActionIcon. Also add an aria-label.

-<I18nLink to={audioModeUrl}>
-  <ActionIcon component="a" variant="default" size="50" radius="md">
+<I18nLink to={audioModeUrl}>
+  <ActionIcon variant="default" size={50} radius="md" aria-label={t`Switch to audio mode`}>
     <IconMicrophone />
   </ActionIcon>
 </I18nLink>
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (3)

98-100: Drop dead isFinishing; wire buttons to isStopping/modal.

Lean and predictable UI.

-const [isFinishing, _setIsFinishing] = useState(false);
@@
 { !isRecording &&
   !isStopping &&
   chunks?.data &&
   chunks.data.length > 0 && (
   <Button
     size="lg"
     radius="md"
     onClick={open}
     variant="light"
     rightSection={<IconCheck />}
     className="w-full md:w-auto"
-    loading={isFinishing}
-    disabled={isFinishing}
+    disabled={isStopping}
   >
     <Trans id="participant.button.finish">Finish</Trans>
   </Button>
 )}

Also applies to: 602-618


205-211: Remove redundant experimental_prepareRequestBody; body already includes language.

Single source of truth.

   api: `${API_BASE_URL}/conversations/${conversationId}/get-reply`,
-  body: { language: iso639_1 },
-  experimental_prepareRequestBody() {
-    return {
-      language: iso639_1,
-    };
-  },
+  body: { language: iso639_1 },

250-256: Kill the busy-wait; prefer UX-friendly early return and reflect pending state.

Avoid indefinite stalls under retries; surface state on the button.

   setShowCooldownMessage(false);
-  // Wait for pending uploads to complete
-  while (uploadChunkMutation.isPending) {
-    await new Promise((resolve) => setTimeout(resolve, 1000));
-  }
+  if (uploadChunkMutation.isPending) {
+    toast.info(t`Please wait for the current upload to finish.`);
+    return;
+  }
@@
-  loading={isLoading}
+  loading={isLoading || uploadChunkMutation.isPending}

Also applies to: 558-568

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9474dd7 and 9f65a21.

📒 Files selected for processing (2)
  • echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (1 hunks)
  • echo/frontend/src/components/participant/ParticipantConversationText.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-28T13:47:02.888Z
Learnt from: ussaama
PR: Dembrane/echo#350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.888Z
Learning: In text mode (echo/frontend/src/components/participant/ParticipantConversationText.tsx), participants only submit PORTAL_TEXT chunks (no audio). The “Finish” button is shown only after at least one text message is saved to Directus.

Applied to files:

  • echo/frontend/src/components/participant/ParticipantConversationText.tsx
📚 Learning: 2025-10-28T13:47:02.888Z
Learnt from: ussaama
PR: Dembrane/echo#350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.888Z
Learning: The participant “finish” flow should call finishConversation(conversationId) (echo/frontend/src/lib/api.ts) before navigating to /:projectId/conversation/:conversationId/finish so the backend sets is_finished and triggers task_finish_conversation_hook.

Applied to files:

  • echo/frontend/src/components/participant/ParticipantConversationText.tsx
📚 Learning: 2025-08-08T10:39:31.114Z
Learnt from: ussaama
PR: Dembrane/echo#259
File: echo/frontend/src/components/layout/ParticipantLayout.tsx:33-33
Timestamp: 2025-08-08T10:39:31.114Z
Learning: In echo/frontend/src/components/layout/ParticipantLayout.tsx, prefer using simple pathname.includes("start") and pathname.includes("finish") to control the settings button visibility. No need to switch to segment-based matching or add a useEffect to auto-close the modal for these routes, per ussaama’s preference in PR #259.

Applied to files:

  • echo/frontend/src/components/participant/ParticipantConversationText.tsx
🧬 Code graph analysis (2)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (11)
echo/frontend/src/components/participant/hooks/index.ts (5)
  • useParticipantProjectById (279-284)
  • useConversationQuery (326-346)
  • useConversationChunksQuery (348-358)
  • useConversationRepliesQuery (308-324)
  • useUploadConversationChunk (51-145)
echo/frontend/src/hooks/useElementOnScreen.ts (1)
  • useElementOnScreen (3-25)
echo/frontend/src/hooks/useI18nNavigate.ts (1)
  • useI18nNavigate (10-38)
echo/frontend/src/hooks/useLanguage.ts (1)
  • useLanguage (24-45)
echo/frontend/src/components/project/ProjectQRCode.tsx (1)
  • useProjectSharingLink (23-59)
echo/frontend/src/hooks/useWakeLock.ts (1)
  • useWakeLock (3-57)
echo/frontend/src/lib/utils.ts (1)
  • checkPermissionError (18-34)
echo/frontend/src/lib/api.ts (1)
  • finishConversation (1053-1057)
echo/frontend/src/components/participant/ParticipantBody.tsx (1)
  • ParticipantBody (27-263)
echo/frontend/src/components/participant/EchoErrorAlert.tsx (1)
  • EchoErrorAlert (5-30)
echo/frontend/src/components/common/ScrollToBottom.tsx (1)
  • ScrollToBottomButton (10-34)
echo/frontend/src/components/participant/ParticipantConversationText.tsx (6)
echo/frontend/src/components/participant/hooks/index.ts (4)
  • useParticipantProjectById (279-284)
  • useConversationQuery (326-346)
  • useConversationChunksQuery (348-358)
  • useUploadConversationTextChunk (147-241)
echo/frontend/src/components/project/ProjectQRCode.tsx (1)
  • useProjectSharingLink (23-59)
echo/frontend/src/hooks/useElementOnScreen.ts (1)
  • useElementOnScreen (3-25)
echo/frontend/src/hooks/useI18nNavigate.ts (1)
  • useI18nNavigate (10-38)
echo/frontend/src/components/participant/ParticipantBody.tsx (1)
  • ParticipantBody (27-263)
echo/frontend/src/components/common/i18nLink.tsx (1)
  • I18nLink (6-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: ci-check-server

Comment on lines +228 to +237
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
variant="light"
rightSection={<IconCheck />}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Block finishing while a submit is inflight.

Avoid racing finalization vs. last text upload; disable the Finish CTA during mutate.

 {text.trim() === "" && chunks.data && chunks.data.length > 0 && (
   <Button
     size="lg"
     radius="md"
     onClick={openFinishModal}
     variant="light"
     rightSection={<IconCheck />}
+    disabled={uploadChunkMutation.isPending}
   >
     <Trans id="participant.button.finish.text.mode">Finish</Trans>
   </Button>
 )}
📝 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.

Suggested change
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
variant="light"
rightSection={<IconCheck />}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
{text.trim() === "" && chunks.data && chunks.data.length > 0 && (
<Button
size="lg"
radius="md"
onClick={openFinishModal}
variant="light"
rightSection={<IconCheck />}
disabled={uploadChunkMutation.isPending}
>
<Trans id="participant.button.finish.text.mode">Finish</Trans>
</Button>
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationText.tsx
around lines 228 to 237, the Finish button can be clicked while the final text
upload mutate is in flight; disable the Finish CTA when the mutation is running
to avoid a race. Add a disabled prop to the Button that checks the upload
mutation state (e.g., chunks.mutate.isLoading or chunks.isMutating depending on
your hook) and optionally show a loading state (size-appropriate spinner or
Mantine's loading prop) so the button is inactive until the mutate completes.

@ussaama ussaama requested a review from spashii October 28, 2025 14:01
@ussaama ussaama added this pull request to the merge queue Oct 29, 2025
Merged via the queue into main with commit 4b45474 Oct 29, 2025
11 checks passed
@ussaama ussaama deleted the simplify-project-conversation-route branch October 29, 2025 08:18
spashii pushed a commit that referenced this pull request Nov 18, 2025
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added audio conversation mode with voice recording, streaming replies,
microphone device controls, permissions troubleshooting, cooldown
countdown, and a dynamic recording timer
* Added text conversation mode with message input, auto-scroll,
submit/finish workflow, and finish confirmation modal
  * Shows “Thinking…” placeholder while replies are being prepared

* **Refactor**
* Split conversation UI into dedicated audio and text components for
simpler routing and composition
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants