-
Notifications
You must be signed in to change notification settings - Fork 17
ECHO-578 Fixes #377
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
ECHO-578 Fixes #377
Conversation
- backend modification to include translations in the seed function to populate verification topics
WalkthroughFixes multiple issues from ECHO-578: reworks chat message conversation mapping to handle mixed input shapes, adds transcript detection hook for conversations, implements dynamic locale resolution for language preferences across components, disables chat auto-select globally, enhances summary generation UI with mutation tracking and conditional state displays, and adds multi-language translations to backend verification topics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 2
📜 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 (7)
echo/frontend/src/components/chat/ChatHistoryMessage.tsx(1 hunks)echo/frontend/src/components/conversation/hooks/index.ts(1 hunks)echo/frontend/src/components/participant/verify/VerifySelection.tsx(3 hunks)echo/frontend/src/components/project/ProjectPortalEditor.tsx(4 hunks)echo/frontend/src/config.ts(1 hunks)echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx(5 hunks)echo/server/dembrane/seed.py(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
echo/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
echo/frontend/**/*.{ts,tsx}: Use Lingui macros for UI localization: importtfrom@lingui/core/macroandTransfrom@lingui/react/macroto localize UI strings in routed screens
Custom Directus POSTs (like 2FA) should calldirectus.requestwith a function signature rather thanrestRequest; reusepostDirectusfromsrc/components/settings/hooks/index.tsto stay consistent
Files:
echo/frontend/src/config.tsecho/frontend/src/components/conversation/hooks/index.tsecho/frontend/src/components/participant/verify/VerifySelection.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/components/chat/ChatHistoryMessage.tsxecho/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx
echo/frontend/**/components/**/hooks/index.ts
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
React Query hook hubs: Each feature should own a
hooks/index.tsfile exposinguseQuery/useMutationwrappers with shareduseQueryClientinvalidation logic
Files:
echo/frontend/src/components/conversation/hooks/index.ts
echo/frontend/**/hooks/**/*.{ts,tsx}
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
Auth session state depends on the shared
['auth','session']React Query key; invalidate it on login/logout before fetching['users','me']
Files:
echo/frontend/src/components/conversation/hooks/index.ts
echo/server/dembrane/**/*.py
📄 CodeRabbit inference engine (echo/.cursor/rules/async-threadpool.mdc)
echo/server/dembrane/**/*.py: Always wrap blocking I/O calls usingrun_in_thread_poolfromdembrane.async_helpersin backend code. Wrap calls todirectus.*,conversation_service.*,project_service.*, S3 helpers, and CPU-heavy utilities like token counting or summary generation if they are sync. Do not wrap already-async functions or LightRAG calls (e.g.,rag.aquery,rag.ainsert).
Prefer converting endpoints toasync defandawaitresults rather than using synchronous functions
echo/server/dembrane/**/*.py: Store all configuration changes indembrane/settings.py: add new env vars as fields onAppSettings, expose grouped accessors (e.g.,feature_flags,directus) if multiple modules read them, and fetch config at runtime withsettings = get_settings()—never import env vars directly
PopulateEMBEDDING_*env vars (model, key/base URL/version) before callingdembrane.embedding.embed_textto ensure embeddings use the correct configuration
Files:
echo/server/dembrane/seed.py
🧠 Learnings (14)
📓 Common learnings
Learnt from: ussaama
Repo: Dembrane/echo PR: 336
File: echo/server/dembrane/api/chat.py:593-630
Timestamp: 2025-10-15T11:06:42.397Z
Learning: In `echo/server/dembrane/api/chat.py`, the auto-select conversation flow (lines 589-631) deliberately uses incremental system message generation with `create_system_messages_for_chat` and `token_counter` for each candidate conversation to ensure accurate token count estimation before adding conversations. The user (ussaama) prioritizes accuracy over the O(n²) performance cost to stay within the 80% context threshold precisely.
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-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/conversation/hooks/index.tsecho/frontend/src/components/chat/ChatHistoryMessage.tsxecho/frontend/src/routes/project/conversation/ProjectConversationOverview.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/conversation/hooks/index.tsecho/frontend/src/components/chat/ChatHistoryMessage.tsxecho/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx
📚 Learning: 2025-11-27T11:27:01.580Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-11-27T11:27:01.580Z
Learning: Applies to echo/frontend/**/components/**/hooks/index.ts : React Query hook hubs: Each feature should own a `hooks/index.ts` file exposing `useQuery`/`useMutation` wrappers with shared `useQueryClient` invalidation logic
Applied to files:
echo/frontend/src/components/conversation/hooks/index.tsecho/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx
📚 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/conversation/hooks/index.ts
📚 Learning: 2025-11-27T11:27:01.580Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-11-27T11:27:01.580Z
Learning: Applies to echo/frontend/**/*.{ts,tsx} : Use Lingui macros for UI localization: import `t` from `lingui/core/macro` and `Trans` from `lingui/react/macro` to localize UI strings in routed screens
Applied to files:
echo/frontend/src/components/participant/verify/VerifySelection.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsx
📚 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/verify/VerifySelection.tsx
📚 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/verify/VerifySelection.tsx
📚 Learning: 2025-11-27T11:27:01.580Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-11-27T11:27:01.580Z
Learning: Applies to echo/frontend/src/lib/directus.ts : Standardize Directus error handling and add localization polish in `src/lib/directus.ts`
Applied to files:
echo/frontend/src/components/project/ProjectPortalEditor.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.tsx
📚 Learning: 2025-06-06T05:43:44.935Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 180
File: echo/frontend/src/locales/es-ES.ts:1-1
Timestamp: 2025-06-06T05:43:44.935Z
Learning: When users indicate they have already addressed translation issues in locale files, verify the current state before assuming the issue persists. Translation files are often updated in bulk and previously flagged English text may have been properly localized.
Applied to files:
echo/frontend/src/components/project/ProjectPortalEditor.tsx
📚 Learning: 2025-11-27T11:27:01.580Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-11-27T11:27:01.580Z
Learning: Applies to echo/frontend/**/hooks/**/*.{ts,tsx} : 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/routes/project/conversation/ProjectConversationOverview.tsx
📚 Learning: 2025-11-27T11:27:01.580Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-11-27T11:27:01.580Z
Learning: Applies to echo/frontend/**/*.{tsx} : Blend Mantine and Tailwind: 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/routes/project/conversation/ProjectConversationOverview.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: 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/routes/project/conversation/ProjectConversationOverview.tsx
🧬 Code graph analysis (4)
echo/frontend/src/components/conversation/hooks/index.ts (1)
echo/frontend/src/lib/directus.ts (1)
directus(16-16)
echo/frontend/src/components/participant/verify/VerifySelection.tsx (1)
echo/frontend/src/hooks/useLanguage.ts (1)
useLanguage(24-45)
echo/frontend/src/components/project/ProjectPortalEditor.tsx (1)
echo/frontend/src/hooks/useLanguage.ts (1)
useLanguage(24-45)
echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx (3)
echo/frontend/src/components/conversation/hooks/index.ts (1)
useConversationHasTranscript(1024-1066)echo/frontend/src/lib/api.ts (1)
generateConversationSummary(1004-1009)echo/frontend/src/components/common/Toaster.tsx (1)
toast(34-34)
⏰ 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 (12)
echo/frontend/src/config.ts (1)
45-45: Ship it — auto-select feature flag is properly wired and actively used throughout the application.Verified that
ENABLE_CHAT_AUTO_SELECTis imported and used across the codebase:
- ProjectChatRoute.tsx: Controls auto-select message handling during chat flow (lines 123, 153, 197, 213), progress display (line 418), and conversation selection logic (lines 503, 529)
- ConversationAccordion.tsx: Gates auto-select UI panel (line 884) and visual indicators (lines 162, 508)
- ChatHistoryMessage.tsx: Controls metadata rendering (lines 47, 79, 126)
- ChatContextProgress.tsx: Affects color logic based on auto-select state (line 38)
- Debug.tsx: Displays flag value for debugging (line 245)
Setting this to
falsewill comprehensively disable the auto-select feature globally as intended.echo/server/dembrane/seed.py (2)
71-77: LGTM, translations are shipped. 🚀Clean i18n structure for verification topics. All 5 locales covered consistently across all topics.
Also applies to: 92-98, 113-119, 133-139, 153-159, 173-179
210-221: Solid fallback logic.The
or []fallback after the list comprehension handles empty/missing translations gracefully, defaulting to the single-language label. Ship it.echo/frontend/src/components/project/ProjectPortalEditor.tsx (3)
59-70: Clean locale resolution pattern. 10x approved.Type-safe mapping from ISO 639-1 to full locale codes with a defensive
localeFromIsohelper. Minimal surface area.
202-206: LGTM, sensible fallback chain.UI language → project language →
en-US. Users get translations in their preferred language with graceful degradation.
216-220: Triple fallback for topic labels is chef's kiss. 👨🍳
translationLocale→en-US→topic.key. Bulletproof localization.echo/frontend/src/components/conversation/hooks/index.ts (1)
1024-1066: LGTM, solid hook addition. 🎯Proper handling of Directus aggregate which returns string numbers on line 1061. The
|| 0fallback protects againstNaNfromNumber(undefined). Query key is unique and descriptive. Based on learnings, this pattern is correct.echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx (4)
62-68: LGTM, smart query gating. 🧠Enabled condition stops polling once summary exists.
hasTranscriptcleanly derived from count.
70-104: Clean toast.promise integration.Differentiated messaging for generate vs. regenerate flows. The promise is returned correctly after toast.promise observes it.
106-113: useMutationState for cross-component tracking - big brain move. 💡This handles the case where mutation might be triggered elsewhere but loading state should still reflect here.
169-190: UX polish: disabled state with explanatory tooltip.Users understand why they can't generate without transcript. Ship it.
echo/frontend/src/components/participant/verify/VerifySelection.tsx (1)
10-10: Locale resolution pipeline (UI language → project language →en-US) looks correctHooking in
useLanguage()here to grabiso639_1and then feeding it throughlocaleFromLanguagebefore falling back to the project language and finallyLANGUAGE_TO_LOCALE.enis exactly the priority stack we want: user UI preference first, project config as backup, hard default last.A couple of nice properties:
- If either the UI or project language isn’t in
LANGUAGE_TO_LOCALE, we automatically slide to the next option oren-USwithout blowing up.- We don’t need to wait on
loadingfromuseLanguagesinceiso639_1is computed synchronously from the route / current locale, so this stays deterministic on first render.Net: behavior is sane and robust; I’d ship this as‑is once the mapping is centralized.
Also applies to: 61-67
| .map((conv: string | ProjectChatMessageConversation1) => | ||
| typeof conv === "object" && conv !== null | ||
| ? (conv as ProjectChatMessageConversation1).conversation_id | ||
| : null, | ||
| ) | ||
| .filter((conv) => conv != null); |
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
🏁 Script executed:
#!/bin/bash
# Search for added_conversations usage and context
rg -n "added_conversations" --type=ts --type=tsx -B 3 -A 3Repository: Dembrane/echo
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Search for added_conversations usage - try without file type restriction
rg -n "added_conversations" --type=typescript -B 3 -A 3Repository: Dembrane/echo
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for added_conversations without file type restriction
rg -n "added_conversations" -B 3 -A 3Repository: Dembrane/echo
Length of output: 5944
Strings are valid conversation IDs and should not be dropped—the current mapping logic has a bug.
The type definition confirms that added_conversations can contain either string[] (raw conversation IDs) or ProjectChatMessageConversation1[] (expanded objects). The current code at lines 159-164 maps strings to null and filters them out, which silently discards valid conversation IDs that should be included in the output.
The fix should preserve strings as-is:
.map((conv: string | ProjectChatMessageConversation1) =>
typeof conv === "object" && conv !== null
? (conv as ProjectChatMessageConversation1).conversation_id
: conv,
)
.filter((conv) => conv != null);This way, string IDs pass through unchanged while objects have their conversation_id extracted.
🤖 Prompt for AI Agents
In echo/frontend/src/components/chat/ChatHistoryMessage.tsx around lines 159 to
164, the mapping converts string conversation IDs to null and then filters them
out; preserve string IDs by returning the string unchanged and only extract
conversation_id from object entries, then filter out null/undefined; update the
map callback to return conv when it's a string and the object's conversation_id
when it's an object, keeping the subsequent .filter((conv) => conv != null) to
remove only truly null/undefined values.
| type LanguageCode = "de" | "en" | "es" | "fr" | "nl"; | ||
|
|
||
| const LANGUAGE_TO_LOCALE: Record<LanguageCode, string> = { | ||
| de: "de-DE", | ||
| en: "en-US", | ||
| es: "es-ES", | ||
| fr: "fr-FR", | ||
| nl: "nl-NL", | ||
| }; | ||
|
|
||
| const localeFromLanguage = (language?: string) => { | ||
| if (!language) return undefined; | ||
| const iso = language.includes("-") ? language.split("-")[0] : language; | ||
| return LANGUAGE_TO_LOCALE[iso as LanguageCode]; | ||
| }; |
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.
🧹 Nitpick | 🔵 Trivial
Centralize LanguageCode / LANGUAGE_TO_LOCALE / localeFromLanguage to stay DRY and future‑proof
Nice, clean helper and mapping; runtime behavior is solid. The only concern is duplication + drift: per the PR summary, similar language→locale logic now lives in other components as well. Once we add or tweak a language, we’ll have to remember to touch multiple maps and union types.
I’d pull this into a shared i18n util (e.g. @/lib/i18n/locale) and import it here, so we have a single source of truth for both LanguageCode and the mapping:
- type LanguageCode = "de" | "en" | "es" | "fr" | "nl";
-
- const LANGUAGE_TO_LOCALE: Record<LanguageCode, string> = {
- de: "de-DE",
- en: "en-US",
- es: "es-ES",
- fr: "fr-FR",
- nl: "nl-NL",
- };
-
- const localeFromLanguage = (language?: string) => {
- if (!language) return undefined;
- const iso = language.includes("-") ? language.split("-")[0] : language;
- return LANGUAGE_TO_LOCALE[iso as LanguageCode];
- };
+ import { LANGUAGE_TO_LOCALE, localeFromLanguage } from "@/lib/i18n/locale";That also lets you derive the union type from whatever source backs SUPPORTED_LANGUAGES instead of hand‑maintaining string literals here.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In echo/frontend/src/components/participant/verify/VerifySelection.tsx around
lines 19-33, the LanguageCode type, LANGUAGE_TO_LOCALE map, and
localeFromLanguage helper are duplicated elsewhere; extract them into a shared
i18n module (e.g. src/lib/i18n/locale.ts or @/lib/i18n/locale) that exports a
single SUPPORTED_LANGUAGES array, a derived LanguageCode union type, the
LANGUAGE_TO_LOCALE mapping and localeFromLanguage function, update this file to
import those exports, and replace the local definitions so all consumers use the
centralized source of truth.
Summary by CodeRabbit
New Features
Improvements
Changes
✏️ Tip: You can customize this high-level summary in your review settings.