-
Notifications
You must be signed in to change notification settings - Fork 17
ECHO-613 ECHO-614 ECHO-591 ECHO-571 translation fixes, "Go deeper" or "Make it concrete" showing instead of "Refine", Refine was clickable when "Go deeper" was generating response #390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…e" selected from dashboard and show "Refine" button - ECHO-614 disable "Refine" button while "Go deeper" response is generating
WalkthroughThis PR implements a localStorage-based mechanism to disable the refine button while "Go Deeper" response generates, simplifies refine navigation logic by removing cooldown dependency, and updates translation identifiers from "experimental" to "beta" across participant and portal editor components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this 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
📜 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.
📒 Files selected for processing (5)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx(4 hunks)echo/frontend/src/components/participant/ParticipantConversationAudioContent.tsx(3 hunks)echo/frontend/src/components/participant/refine/RefineSelection.tsx(3 hunks)echo/frontend/src/components/project/ProjectPortalEditor.tsx(2 hunks)echo/frontend/src/locales/en-US.po(40 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
echo/frontend/**/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
echo/frontend/**/src/{routes,components}/**/*.tsx: Compose Mantine primitives (Stack,Group,ActionIcon, etc.) while layering Tailwind utility classes viaclassName, alongside toast feedback via@/components/common/Toaster
For Tailwind classes that need dynamic values, replace with inlinestyleprops using CSS variables instead of hardcoded hex values
Files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsx
echo/frontend/**/src/**/*.{css,tsx,ts}
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
Use CSS variables
var(--app-background)andvar(--app-text)instead of hardcoded colors like#F6F4F1or#2D2D2Cto ensure theme changes propagate
Files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsx
echo/frontend/**/src/{theme.tsx,components,routes}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
When adding new Mantine components, reference the global Mantine theme in
src/theme.tsxwith customprimarycolor palette and component defaults, preferring CSS variables in styles over hardcoded hex values
Files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsx
echo/frontend/**/src/{components,routes}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
UI mutations should surface inline feedback: pair toasts with contextual Mantine
Alertcomponents inside modals/forms for errors or warnings
Files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsx
🧠 Learnings (17)
📓 Common learnings
Learnt from: ussaama
Repo: Dembrane/echo PR: 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.
Learnt from: ussaama
Repo: Dembrane/echo PR: 205
File: echo/frontend/src/lib/query.ts:1444-1506
Timestamp: 2025-07-10T12:48:20.683Z
Learning: ussaama prefers string concatenation over template literals for simple cases where readability is clearer, even when linting tools suggest template literals. Human readability takes precedence over strict linting rules in straightforward concatenation scenarios.
📚 Learning: 2025-11-21T12:44:30.306Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 366
File: echo/frontend/src/components/participant/refine/RefineSelection.tsx:38-38
Timestamp: 2025-11-21T12:44:30.306Z
Learning: In echo/frontend/src/components/participant/refine/RefineSelection.tsx, the refine selection panels (Make it concrete / Go deeper) should use h-[50%] when only one panel is visible to maintain half-height spacing for consistent visual weight. The flexClass logic should remain: showVerify && showEcho ? "flex-1" : "h-[50%]".
Applied to files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-08-06T13:38:30.769Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 256
File: echo/frontend/src/components/participant/MicrophoneTest.tsx:54-54
Timestamp: 2025-08-06T13:38:30.769Z
Learning: In echo/frontend/src/components/participant/MicrophoneTest.tsx, the useDisclosure hook is used where only the `close` function is needed locally (called in handleConfirmMicChange and handleCancelMicChange), while `opened` and `open` are unused because the modal state is managed by a parent component. The `showSecondModal` state variable is used separately to control content switching within the modal between the main UI and confirmation step.
Applied to files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-08-08T10:39:31.114Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 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/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/**/*.tsx : Use Lingui macros `t` from `lingui/core/macro` and `Trans` from `lingui/react/macro` for localizing UI strings in routed screens
Applied to files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/{routes,components}/**/*.tsx : Compose Mantine primitives (`Stack`, `Group`, `ActionIcon`, etc.) while layering Tailwind utility classes via `className`, alongside toast feedback via `@/components/common/Toaster`
Applied to files:
echo/frontend/src/components/participant/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/hooks/useAppPreferences.tsx : User preferences (font family, color scheme) persist in localStorage under key `dembrane-app-preferences`; font preference is linked to color scheme: DM Sans→Parchment+Graphite, Space Grotesk→White+Black
Applied to files:
echo/frontend/src/components/participant/refine/RefineSelection.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/auth/**/*.tsx : Gentle login/logout flows use `useTransitionCurtain().runTransition()` before navigation—animations expect Directus session mutations to await that promise
Applied to files:
echo/frontend/src/components/participant/refine/RefineSelection.tsx
📚 Learning: 2025-10-28T13:47:02.926Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.926Z
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/refine/RefineSelection.tsxecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-10-28T13:47:02.926Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.926Z
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/ParticipantConversationAudio.tsxecho/frontend/src/components/participant/ParticipantConversationAudioContent.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/{components,routes}/**/*.tsx : UI mutations should surface inline feedback: pair toasts with contextual Mantine `Alert` components inside modals/forms for errors or warnings
Applied to files:
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/{routes/auth,components/settings}/**/hooks/index.ts : Auth session state depends on the shared `['auth','session']` React Query key; invalidate it on login/logout before fetching `['users','me']`
Applied to files:
echo/frontend/src/components/participant/ParticipantConversationAudioContent.tsx
📚 Learning: 2025-08-19T10:22:55.323Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 266
File: echo/frontend/src/components/conversation/ConversationAccordion.tsx:675-678
Timestamp: 2025-08-19T10:22:55.323Z
Learning: In echo/frontend/src/components/conversation/hooks/index.ts, the useConversationsCountByProjectId hook uses useSuspenseQuery with Directus aggregate, which always returns string numbers like "0", "1", "2" and suspends during loading instead of returning undefined. Therefore, Number(conversationsCountQuery.data) ?? 0 is safe and the Number() conversion is necessary for type conversion from string to number.
Applied to files:
echo/frontend/src/components/participant/ParticipantConversationAudioContent.tsx
📚 Learning: 2025-08-19T10:22:55.323Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 266
File: echo/frontend/src/components/conversation/ConversationAccordion.tsx:675-678
Timestamp: 2025-08-19T10:22:55.323Z
Learning: In echo/frontend/src/components/conversation/hooks/index.ts, the useConversationsCountByProjectId hook uses regular useQuery (not useSuspenseQuery), which means conversationsCountQuery.data can be undefined during loading states. When using Number(conversationsCountQuery.data) ?? 0, this creates NaN because Number(undefined) = NaN and NaN is not nullish, so the fallback doesn't apply. The correct pattern is Number(conversationsCountQuery.data ?? 0) to ensure the fallback happens before type conversion.
Applied to files:
echo/frontend/src/components/participant/ParticipantConversationAudioContent.tsx
📚 Learning: 2025-05-30T15:38:44.413Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 169
File: echo/frontend/src/components/project/ProjectPortalEditor.tsx:409-464
Timestamp: 2025-05-30T15:38:44.413Z
Learning: Badge-based selectors in ProjectPortalEditor.tsx: Keyboard navigation enhancements for accessibility are considered optional improvements rather than critical issues. The user acknowledges these suggestions but doesn't prioritize them as blockers.
Applied to files:
echo/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-08-19T10:14:31.647Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 266
File: echo/frontend/src/components/chat/ChatAccordion.tsx:214-221
Timestamp: 2025-08-19T10:14:31.647Z
Learning: In the Echo frontend codebase using Lingui, i18n IDs in Trans components (e.g., `<Trans id="any.string.here">`) can be arbitrary strings and don't need to follow specific naming conventions. They are just references used in .po files, and the actual translations need to be manually defined in the locale files.
Applied to files:
echo/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-09-16T08:34:38.109Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 293
File: echo/frontend/src/components/chat/ChatTemplatesMenu.tsx:15-16
Timestamp: 2025-09-16T08:34:38.109Z
Learning: In ChatTemplatesMenu.tsx, titles are preferred over IDs for template selection logic since titles are unique one-liners and work effectively as identifiers. IDs were added primarily for better list rendering rather than selection purposes.
Applied to files:
echo/frontend/src/locales/en-US.po
⏰ 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
🔇 Additional comments (4)
echo/frontend/src/locales/en-US.po (1)
291-305: i18n wiring for Beta / Refine / “Go deeper” / “Make it concrete” is coherentAll the new/updated Lingui IDs (beta label, refine modal titles/reason, “Go deeper”/“Make it concrete” buttons & descriptions, concrete topics, portal/thank-you content, live preview/refresh) line up with the TSX usages in the other files. Wording is consistent with the product language and the Echo→Go deeper / Verify→Make it concrete rename, and the obsolete experimental keys are correctly tombstoned with
#~. No action needed here.Also applies to: 324-337, 687-690, 790-808, 936-959, 1388-1423, 1736-1739, 1799-1807, 1898-1901, 2097-2130, 2178-2186, 2337-2340, 2545-2556, 2712-2715, 3623-3626, 3758-3761, 3807-3810, 3824-3827, 3426-3438, 4260-4263
echo/frontend/src/components/project/ProjectPortalEditor.tsx (1)
552-562: Beta labelling now correctly driven by shared translation IDsBoth “Go deeper” and “Make it concrete” badges use
dashboard.dembrane.concrete.beta, and the title/description/topic-select IDs (dashboard.dembrane.concrete.title/description/topic.select) match the locale file. This centralizes the Beta copy and removes the old experimental key cleanly. LGTM.Also applies to: 737-751
echo/frontend/src/components/participant/ParticipantConversationAudioContent.tsx (1)
2-3: LocalStorage cleanup for refine-disable flag is correctly hooked into chat lifecycleTying
removeValue()into bothuseChatonErrorandonFinishagainst the sharedrefine_disabled_${conversationId}key is the right shape to ensure the refine-disable flag doesn’t get stuck after a Go deeper run. The hook is only used for side effects here, which is fine given the small overhead. LGTM.Also applies to: 30-35, 60-66
echo/frontend/src/components/participant/refine/RefineSelection.tsx (1)
3-4: Refine selection now correctly marks the refine flow as disabled on “Go deeper”Using
useLocalStoragewith the per-conversation key and flipping it totrueinhandleEchoClickright after starting the echo cooldown is the right trigger point from this component’s POV; the rest of the refine UI (flex layout, cooldown messaging) is preserved exactly as before. The only caveat is ensuring the reader of this flag actually reacts to it—which I’m calling out in the parentParticipantConversationAudioreview.Also applies to: 15-18, 25-30
| import { useDisclosure, useLocalStorage, useWindowEvent } from "@mantine/hooks"; | ||
| import { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Mantine useLocalStorage same tab synchronization multiple components
💡 Result:
Short answer: Mantine's useLocalStorage syncs across browser tabs/windows via the storage event, but it does NOT automatically propagate updates to other hook instances in the same tab. Use one of these patterns to keep multiple components in the same tab in sync:
- Lift the state into a shared parent (or React context) and have components consume that single source of truth (recommended). [1]
- Use a small global pub/sub (mitt or an EventTarget) so setValue writes localStorage and emits a same-tab event other hook instances listen to. [3]
- Replace/use a custom "synced" hook that uses an emitter + storage events to sync both same-tab and cross-tab (examples exist, e.g. useSyncedLocalStorage). [3]
Background / why: Mantine's hook subscribes to the window "storage" event which only fires in other documents (other tabs/windows), not in the same tab where localStorage was changed — so same-tab updates won't trigger other hook instances. [1][2]
Sources:
- Mantine useLocalStorage docs (browser tabs sync via storage event). [1]
- Explanation that storage events don't fire in the same tab. [2]
- Example approach / custom hook that adds same-tab sync via an emitter (useSyncedLocalStorage). [3]
Refine disable flag is not synchronized within the same tab—Refine button likely stays enabled during "Go deeper" response generation
The refine disable mechanism is split across three components using separate useLocalStorage instances:
RefineSelectionwritestrueto localStorage viauseLocalStoragewhen starting "Go deeper"ParticipantConversationAudioContentclears it viaremoveValue()ononError/onFinishParticipantConversationAudioreads via its ownuseLocalStorageinstance and disables the button viadisabled={isStopping || isRefineDisabled}
The problem: Mantine's useLocalStorage only synchronizes state across browser tabs/windows via the storage event. Within the same tab, updating localStorage in one component does not automatically update the state in other components using the same key—each hook instance maintains isolated React state. When RefineSelection calls its setter, ParticipantConversationAudio's isRefineDisabled state does not update in the same tab. Additionally, navigating between nested routes keeps ParticipantConversationAudio mounted, so its initializer only runs once and won't pick up later writes.
Result: The Refine button is probably never actually disabled while a Go deeper response is generating, and may remain disabled longer than intended on cleanup. This defeats ECHO-614.
Recommendation: Replace the cross-component localStorage coordination with a single source of truth owned by the parent component and passed to children:
-const [isRefineDisabled, _setIsRefineDisabled] = useLocalStorage({
- defaultValue: false,
- key: `refine_disabled_${conversationId}`,
-});
+const [isRefineDisabled, setIsRefineDisabled] = useState(false);
...
<Outlet
context={{
isRecording,
+ isRefineDisabled,
+ setIsRefineDisabled,
}}
/>Then have RefineSelection and ParticipantConversationAudioContent call setIsRefineDisabled via useOutletContext() instead of their own useLocalStorage instances. You can optionally mirror to localStorage afterward if persistence across full page reloads is needed.
Applies to lines: 13–14, 50–56, 218–225, 458–474
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
around lines 13–14, 50–56, 218–225 and 458–474 the disable flag is implemented
with multiple useLocalStorage hooks which do not synchronize React state within
the same tab; replace this cross-component localStorage coordination with a
single boolean state (isRefineDisabled) declared in the parent component, pass
that state and its setter down to children via outlet context (useOutletContext)
or props, remove the child components' useLocalStorage usages so RefineSelection
calls setIsRefineDisabled(true) when starting "Go deeper" and
ParticipantConversationAudioContent calls setIsRefineDisabled(false) on
finish/error, and optionally mirror the parent state to localStorage only for
full-page reload persistence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Summary by CodeRabbit
Improvements
Localization
✏️ Tip: You can customize this high-level summary in your review settings.