feat(calls): voice capture announcement and mobile UI fixes#2277
Open
webguru-hypha wants to merge 10 commits into
Open
feat(calls): voice capture announcement and mobile UI fixes#2277webguru-hypha wants to merge 10 commits into
webguru-hypha wants to merge 10 commits into
Conversation
Play a short voice cue when space call capture begins so participants know the session is being recorded. Co-authored-by: Cursor <cursoragent@cursor.com>
Open the attach menu as a modal on mobile, hide the General memory tab on small screens, and stack treasury action buttons more cleanly. Co-authored-by: Cursor <cursoragent@cursor.com>
Add translated escrow banner copy for profile flows and refresh AI panel suggestion prompts with short tag labels in en, de, pt, es, and fr. Co-authored-by: Cursor <cursoragent@cursor.com>
Wire the personal escrow banner to Spaces translations instead of hardcoded English labels and body copy. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve specialists from each user question and render consistent chips above assistant messages without opaque avatar initials. Co-authored-by: Cursor <cursoragent@cursor.com>
Show large suggestion cards only before the first user message, then persist compact translated tags above the composer for follow-up prompts. Co-authored-by: Cursor <cursoragent@cursor.com>
Align Playwright constants with the refreshed translated suggestion set. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 8 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR converts AI suggestions to structured items, adds browser speech synthesis for call announcements, extracts/upserts signal descriptions into Matrix rooms, introduces mobilized-agent detection and rendering with improved markdown parsing, adds a compact-panels hook and mobile-responsive UI behaviors, and updates system prompt formatting and i18n across multiple locales. ChangesAI Suggestions Refactor & Voice Announcements
Signal Description & Coherence Management
Mobile & Responsive UX
AI System & Localization
Minor UI Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AI-created signals only persisted to Postgres; manual create/edit seeded Matrix from the form. Post the description when coherence chat opens, with a shared upsert helper. feat(ai-panel): fix nested list numbering and confidence as percent feat(chat-server): add space journey guidance for recommendations feat(ui): add compact panels mode and refine profile menu layout Unify specialist avatar outline styles and shared tag accent classes. Co-authored-by: Cursor <cursoragent@cursor.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (bullet !== '-' && bullet !== '*') return null; | ||
| if (line[1] !== ' ') return null; | ||
| const text = line.slice(2).trim(); | ||
| const bulletMatch = line.match(/^[-*•]\s+(.+)$/); |
Comment on lines
+166
to
+169
| return text.replace(CONFIDENCE_SCORE_PATTERN, (match, prefix, raw) => { | ||
| const formatted = formatConfidenceScore(raw); | ||
| return formatted ? `${prefix}${formatted}` : match; | ||
| }); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-e2e/src/ai-chat-panel.spec.ts (1)
83-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis E2E test no longer validates UI behavior.
The loop now checks hardcoded constants against a broad regex (Line 91), so the test can pass even if suggestion prompts never render. Please assert visible suggestion items in the panel (or explicitly scope this as a non-UI unit-style test elsewhere).
Suggested direction
test('suggestion prompts should match implemented capabilities', async ({ page, }) => { await chatPanel.openPanel(); - // Verify suggestion-like buttons are visible in the panel - // For unauthenticated users, suggestions may not show, so verify constants as fallback - for (const suggestion of EXPECTED_SUGGESTIONS) { - expect(suggestion.toLowerCase()).toMatch( - /space|signal|blind|discussion|memory|value|token/, - ); - } + for (const suggestion of EXPECTED_SUGGESTIONS) { + await expect(page.getByRole('button', { name: new RegExp(suggestion, 'i') })).toBeVisible(); + } expect(EXPECTED_WELCOME_MESSAGE).toContain('space details'); expect(EXPECTED_WELCOME_MESSAGE).toContain('member counts'); expect(EXPECTED_WELCOME_MESSAGE).toContain('agreements'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web-e2e/src/ai-chat-panel.spec.ts` around lines 83 - 93, The test "suggestion prompts should match implemented capabilities" currently only checks EXPECTED_SUGGESTIONS against a broad regex and doesn't validate UI; change it to assert the rendered suggestion elements after calling chatPanel.openPanel(): wait for the suggestion container/selector (the element used by chatPanel to render suggestions), query the visible suggestion items, assert there is at least one visible suggestion, and then validate each visible item's text matches EXPECTED_SUGGESTIONS (or the expected regex); if this test must run for unauthenticated flows where suggestions are not rendered, move the current constant-only assertions into a separate non-UI unit test and mark the UI test to skip or require authentication so it reliably checks UI rendering. Use the test name, chatPanel.openPanel(), and EXPECTED_SUGGESTIONS to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/chat-server/src/system-prompt.ts`:
- Around line 368-370: The two recommendation rules in the system prompt
conflict: the rule "End every recommendation-style reply with the single most
relevant next step" and the separate "Format recommendation answers as: ... 5)
Confidence" which implies a different ending; update the system prompt so there
is one deterministic structure by merging these into a single rule: change the
"Format recommendation answers" entry (the one starting with "Format
recommendation answers as:") to include the required final "single most relevant
next step" as an explicit final line (e.g., append "6) Next step (one short
line)") or remove the standalone ending rule, and ensure the terms
"recommendation replies" and the "Format recommendation answers" block are
consistent in wording and ordering so the model will always produce the same
sequence.
In `@packages/core/src/matrix/__tests__/call-capture-voice-announcement.test.ts`:
- Around line 55-70: Add a new test that exercises the async voice-loading path
for speakCallCaptureVoiceAnnouncement: stub global window.speechSynthesis so
getVoices() initially returns an empty array, provide spies for speak and
addEventListener, call speakCallCaptureVoiceAnnouncement('text'), capture the
callback registered for the 'voiceschanged' event via the addEventListener spy,
then simulate voices arriving by invoking that callback after replacing
getVoices() to return a non-empty array and assert that speak was called with
the expected utterance; use the same unique symbols
speakCallCaptureVoiceAnnouncement, speechSynthesis.getVoices,
speechSynthesis.addEventListener (event name 'voiceschanged'), and
speechSynthesis.speak to locate and implement the test.
In `@packages/core/src/matrix/client/hooks/call-capture-voice-announcement.ts`:
- Around line 14-29: The hardcoded PREFERRED_VOICE_NAME_PARTS list is
platform-specific; update the hook to accept an optional voice preference array
(e.g., add a parameter preferredVoiceNameParts?: string[] to the
call-capture-voice-announcement hook entry point) and fall back to the existing
PREFERRED_VOICE_NAME_PARTS constant when none is provided, so callers can inject
platform-specific preferences; alternatively (or additionally) change the
selection logic in the function that iterates SpeechSynthesisVoice objects to
prefer voices by generic characteristics (e.g., voice.lang, voice.default,
voice.localService) instead of exact name parts, and add a short inline comment
documenting that PREFERRED_VOICE_NAME_PARTS is a best-effort ranking and will be
overridden by the injected preference array.
- Around line 98-100: The hardcoded speech settings on the
SpeechSynthesisUtterance (utterance.rate = 0.93, utterance.pitch = 1.02,
utterance.volume = 0.92) should be made configurable or fall back to the
standard defaults; update the callCaptureVoiceAnnouncement hook to accept
optional parameters (e.g., speechRate, speechPitch, speechVolume) or read them
from a config, set utterance.rate = speechRate ?? 1.0, utterance.pitch =
speechPitch ?? 1.0, utterance.volume = speechVolume ?? 1.0, and add a short
comment documenting the default behavior so the values aren’t arbitrarily
hardcoded.
In `@packages/epics/src/coherence/components/memory-filters.tsx`:
- Around line 55-59: The useEffect currently lists onFilterChange in its
dependency array which can cause unnecessary re-runs if the parent doesn't
memoize that callback; update the effect that checks isMobile and activeFilter
(the useEffect block referencing isMobile, activeFilter, and calling
onFilterChange('proposals')) to only depend on [activeFilter, isMobile] and
remove onFilterChange from the deps so the effect only triggers when
activeFilter or isMobile change; leave the call to onFilterChange intact but do
not add it back to the dependency array.
In `@packages/epics/src/coherence/utils/signal-chat-description.ts`:
- Around line 56-73: The code is locating the description by taking the earliest
loaded message (existingMessages -> firstMessage) which can be wrong; instead,
when seeding the description, capture and persist the resulting event id (from
matrix.sendMessage) or add a unique tag/marker to that event, and on subsequent
runs look up that persisted id or search for the tagged event (rather than
sorting by timestamp) and pass that exact event id to matrix.editRoomMessage
(use canonicalRoomId, the stored seededEventId or tag lookup, and
nextDescription). Ensure sendMessage's returned event id is stored in durable
room metadata or state so edits always target the original seeded description.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 107-108: The client component human-right-panel.tsx must not
import/call the server-only getCoherenceBySlug; remove that import and any
direct calls in the client init flow (where getCoherenceBySlug is used around
the init logic) and instead fetch the coherence description from a server layer
(server action, API route, or RSC) and pass the resolved description into the
client component as a prop; inside the client use the existing
upsertSignalDescriptionInRoom to seed the room with the passed-in description.
Ensure the server module getCoherenceBySlug remains only in server code and that
human-right-panel.tsx only receives and consumes the description string via
props.
In `@packages/epics/src/people/components/button-profile.tsx`:
- Around line 145-355: Extract the duplicated menu body into a shared component
(e.g. ProfileMenuContent) and replace the three inline copies with that
component; ProfileMenuContent should accept props like variant
('dropdown'|'sheet') to pick item class (menuItemClass vs compactSheetItemClass)
and receive person, address, navItems, profileUrl, onboardingUrl,
notificationCentrePath, resolvedTheme, onChangeThemeMode, hasMfaMethods,
showMfaEnrollmentModal, onDelete, onLogout, handleAddressCopy, primaryLine,
displayName, and t so it can render the avatar header, EthAddress block,
navItems map, profile/onboarding/notification links, theme/MFA actions and
logout/delete items; then swap the original DropdownMenuContent/Sheet content
blocks to render <ProfileMenuContent ... /> so layout/trigger logic
(DropdownMenu/Sheet) remains in place while avoiding duplication.
In `@packages/epics/src/treasury/components/assets/assets-section.tsx`:
- Around line 87-115: Duplicate "New Token" button JSX appears in the isDisabled
branches; extract it into a single reusable piece (e.g., a local NewTokenButton
component or variable) that renders the Button with RadiobuttonIcon and
{tTreasury('newToken')} and accepts props like title, className and disabled;
then conditionally wrap that component with Link when !isDisabled using the
existing href `${basePath}/create/issue-new-token?hideBack=true` and preserve
tooltipMessage, colorVariant/variant, and any accessibility attributes so you
only change rendering logic where Link wrapping differs (refer to isDisabled,
Button, Link, RadiobuttonIcon, basePath, and tTreasury).
In `@packages/i18n/src/messages/fr.json`:
- Line 1035: Update the French message for the key acceptInvestmentBody to fix
the awkward double-preposition phrase: replace "l'investissement de votre espace
de {buyerAmountLabel}" with a clearer order such as "l'investissement de
{buyerAmountLabel} de votre espace" or use a colon "l'investissement de votre
espace : {buyerAmountLabel}" so the sentence reads naturally (keep the rest of
the string and placeholders {sellerLabel} and {sellerAmountLabel} unchanged).
---
Outside diff comments:
In `@apps/web-e2e/src/ai-chat-panel.spec.ts`:
- Around line 83-93: The test "suggestion prompts should match implemented
capabilities" currently only checks EXPECTED_SUGGESTIONS against a broad regex
and doesn't validate UI; change it to assert the rendered suggestion elements
after calling chatPanel.openPanel(): wait for the suggestion container/selector
(the element used by chatPanel to render suggestions), query the visible
suggestion items, assert there is at least one visible suggestion, and then
validate each visible item's text matches EXPECTED_SUGGESTIONS (or the expected
regex); if this test must run for unauthenticated flows where suggestions are
not rendered, move the current constant-only assertions into a separate non-UI
unit test and mark the UI test to skip or require authentication so it reliably
checks UI rendering. Use the test name, chatPanel.openPanel(), and
EXPECTED_SUGGESTIONS to locate and update the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad60e4c2-0c6d-4624-8546-73eea0aa570d
📒 Files selected for processing (34)
apps/web-e2e/src/ai-chat-panel.spec.tspackages/chat-server/src/stream-chat.tspackages/chat-server/src/system-prompt.tspackages/core/src/matrix/__tests__/call-capture-voice-announcement.test.tspackages/core/src/matrix/client/hooks/call-capture-voice-announcement.tspackages/core/src/matrix/client/hooks/use-space-group-call.tspackages/epics/src/coherence/components/coherence-block.tsxpackages/epics/src/coherence/components/create-signal-form.tsxpackages/epics/src/coherence/components/memory-filters.tsxpackages/epics/src/coherence/utils/signal-chat-description.tspackages/epics/src/common/ai-agent-competencies.tspackages/epics/src/common/ai-left-panel.tsxpackages/epics/src/common/ai-panel/ai-panel-message-bubble.tsxpackages/epics/src/common/ai-panel/ai-panel-messages.tsxpackages/epics/src/common/ai-panel/ai-panel-mobilized-agents.tsxpackages/epics/src/common/ai-panel/ai-panel-suggestions.tsxpackages/epics/src/common/ai-panel/index.tspackages/epics/src/common/ai-panel/mock-data.tspackages/epics/src/common/human-chat-panel-context.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-right-panel.tsxpackages/epics/src/common/panel-wrap-layout.tsxpackages/epics/src/people/components/button-profile.tsxpackages/epics/src/people/components/escrow-deposit-banner.tsxpackages/epics/src/people/components/members-section.tsxpackages/epics/src/treasury/components/assets/assets-section.tsxpackages/epics/src/treasury/components/assets/space-pending-rewards-section.tsxpackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.jsonpackages/ui/src/hooks/use-compact-panels-mode.tspackages/ui/src/index.ts
Comment on lines
+368
to
+370
| - End every recommendation-style reply with the single most relevant next step for right now (setup-focused when the space is young; purpose- and gap-focused as activity grows). | ||
| - Recommendation answers must be concise and action-driven, defaulting to 3 options max. | ||
| - Format recommendation answers as: 1) Action (one line), 2) Why now (one short line), 3) Expected impact (one short line), 4) First step (one short line), 5) Confidence (0.0-1.0). | ||
| - Format recommendation answers as: 1) Action (one line), 2) Why now (one short line), 3) Expected impact (one short line), 4) First step (one short line), 5) Confidence (percentage, e.g. 80% — never use a 0.0-1.0 decimal). |
Contributor
There was a problem hiding this comment.
Conflicting recommendation ending rules will produce inconsistent outputs.
Line 368 says recommendation replies must end with the next step, but Line 370’s required format ends with Confidence. Please make these rules consistent so the model has a single deterministic structure.
Proposed prompt fix
-- End every recommendation-style reply with the single most relevant next step for right now (setup-focused when the space is young; purpose- and gap-focused as activity grows).
+- Include exactly one most relevant next step for right now (setup-focused when the space is young; purpose- and gap-focused as activity grows).
-- Format recommendation answers as: 1) Action (one line), 2) Why now (one short line), 3) Expected impact (one short line), 4) First step (one short line), 5) Confidence (percentage, e.g. 80% — never use a 0.0-1.0 decimal).
+- Format recommendation answers as: 1) Action (one line), 2) Why now (one short line), 3) Expected impact (one short line), 4) Confidence (percentage, e.g. 80% — never use a 0.0-1.0 decimal), 5) First step (one short line).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/chat-server/src/system-prompt.ts` around lines 368 - 370, The two
recommendation rules in the system prompt conflict: the rule "End every
recommendation-style reply with the single most relevant next step" and the
separate "Format recommendation answers as: ... 5) Confidence" which implies a
different ending; update the system prompt so there is one deterministic
structure by merging these into a single rule: change the "Format recommendation
answers" entry (the one starting with "Format recommendation answers as:") to
include the required final "single most relevant next step" as an explicit final
line (e.g., append "6) Next step (one short line)") or remove the standalone
ending rule, and ensure the terms "recommendation replies" and the "Format
recommendation answers" block are consistent in wording and ordering so the
model will always produce the same sequence.
Comment on lines
+55
to
+70
| it('ignores empty announcements', () => { | ||
| const speak = vi.fn(); | ||
| vi.stubGlobal('window', { | ||
| speechSynthesis: { | ||
| speak, | ||
| cancel: vi.fn(), | ||
| getVoices: vi.fn(() => []), | ||
| addEventListener: vi.fn(), | ||
| removeEventListener: vi.fn(), | ||
| }, | ||
| }); | ||
|
|
||
| speakCallCaptureVoiceAnnouncement(' '); | ||
|
|
||
| expect(speak).not.toHaveBeenCalled(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider adding test for async voice loading path.
The current tests cover the happy path (voices available immediately) and empty input, but the voiceschanged event listener path (lines 115-122 in the implementation) is not tested. This is the scenario when getVoices() initially returns an empty array and voices load asynchronously.
🧪 Suggested test for async voice loading
+ it('waits for voices to load when initially unavailable', async () => {
+ const speak = vi.fn();
+ const cancel = vi.fn();
+ const addEventListener = vi.fn();
+ const removeEventListener = vi.fn();
+ const voices = [
+ { name: 'Samantha', lang: 'en-US', localService: true },
+ ];
+
+ vi.stubGlobal('SpeechSynthesisUtterance', MockSpeechSynthesisUtterance);
+ vi.stubGlobal('window', {
+ speechSynthesis: {
+ speak,
+ cancel,
+ getVoices: vi.fn(() => []),
+ addEventListener,
+ removeEventListener,
+ },
+ });
+
+ speakCallCaptureVoiceAnnouncement('Recording started', 'en-US');
+
+ expect(addEventListener).toHaveBeenCalledWith('voiceschanged', expect.any(Function));
+ expect(speak).not.toHaveBeenCalled(); // Not called yet
+
+ // Simulate voices becoming available
+ const handler = addEventListener.mock.calls[0][1];
+ window.speechSynthesis.getVoices = vi.fn(() => voices);
+ handler();
+
+ expect(removeEventListener).toHaveBeenCalledWith('voiceschanged', handler);
+ expect(speak).toHaveBeenCalledTimes(1);
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/matrix/__tests__/call-capture-voice-announcement.test.ts`
around lines 55 - 70, Add a new test that exercises the async voice-loading path
for speakCallCaptureVoiceAnnouncement: stub global window.speechSynthesis so
getVoices() initially returns an empty array, provide spies for speak and
addEventListener, call speakCallCaptureVoiceAnnouncement('text'), capture the
callback registered for the 'voiceschanged' event via the addEventListener spy,
then simulate voices arriving by invoking that callback after replacing
getVoices() to return a non-empty array and assert that speak was called with
the expected utterance; use the same unique symbols
speakCallCaptureVoiceAnnouncement, speechSynthesis.getVoices,
speechSynthesis.addEventListener (event name 'voiceschanged'), and
speechSynthesis.speak to locate and implement the test.
Comment on lines
+14
to
+29
| const PREFERRED_VOICE_NAME_PARTS = [ | ||
| 'samantha', | ||
| 'karen', | ||
| 'victoria', | ||
| 'google uk english female', | ||
| 'google us english', | ||
| 'microsoft zira', | ||
| 'microsoft aria', | ||
| 'moira', | ||
| 'fiona', | ||
| 'tessa', | ||
| 'amelie', | ||
| 'anna', | ||
| 'helena', | ||
| 'paulina', | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider platform-agnostic voice selection.
The hardcoded PREFERRED_VOICE_NAME_PARTS list contains platform-specific voice names (macOS "Samantha", Windows "Zira", Google TTS). While the fallback logic handles missing voices, this preference list may not work well across different operating systems and could favor certain platforms.
Consider either:
- Accepting a voice preference list as a parameter, or
- Using voice characteristics (gender, quality metadata) instead of names, or
- Documenting that this is a best-effort ranking that falls back gracefully
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/matrix/client/hooks/call-capture-voice-announcement.ts`
around lines 14 - 29, The hardcoded PREFERRED_VOICE_NAME_PARTS list is
platform-specific; update the hook to accept an optional voice preference array
(e.g., add a parameter preferredVoiceNameParts?: string[] to the
call-capture-voice-announcement hook entry point) and fall back to the existing
PREFERRED_VOICE_NAME_PARTS constant when none is provided, so callers can inject
platform-specific preferences; alternatively (or additionally) change the
selection logic in the function that iterates SpeechSynthesisVoice objects to
prefer voices by generic characteristics (e.g., voice.lang, voice.default,
voice.localService) instead of exact name parts, and add a short inline comment
documenting that PREFERRED_VOICE_NAME_PARTS is a best-effort ranking and will be
overridden by the injected preference array.
Comment on lines
+98
to
+100
| utterance.rate = 0.93; | ||
| utterance.pitch = 1.02; | ||
| utterance.volume = 0.92; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Hardcoded speech parameters lack justification.
The rate, pitch, and volume values (0.93, 1.02, 0.92) appear arbitrary and may not be optimal for all voices or user preferences. Default values for these properties are typically 1.0.
Consider:
- Documenting why these specific values were chosen, or
- Using standard defaults (
1.0), or - Making these configurable via parameters if there's a specific UX goal (e.g., slower rate for clarity)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/matrix/client/hooks/call-capture-voice-announcement.ts`
around lines 98 - 100, The hardcoded speech settings on the
SpeechSynthesisUtterance (utterance.rate = 0.93, utterance.pitch = 1.02,
utterance.volume = 0.92) should be made configurable or fall back to the
standard defaults; update the callCaptureVoiceAnnouncement hook to accept
optional parameters (e.g., speechRate, speechPitch, speechVolume) or read them
from a config, set utterance.rate = speechRate ?? 1.0, utterance.pitch =
speechPitch ?? 1.0, utterance.volume = speechVolume ?? 1.0, and add a short
comment documenting the default behavior so the values aren’t arbitrarily
hardcoded.
Comment on lines
+55
to
+59
| useEffect(() => { | ||
| if (isMobile && activeFilter === 'general') { | ||
| onFilterChange('proposals'); | ||
| } | ||
| }, [activeFilter, isMobile, onFilterChange]); |
Contributor
There was a problem hiding this comment.
Risk of unnecessary re-execution if onFilterChange is not memoized.
The effect depends on onFilterChange, which may cause it to run on every parent re-render if the callback isn't wrapped in useCallback. While this won't break functionality, it could lead to unexpected filter switches or performance overhead.
🛡️ Recommended fix: remove `onFilterChange` from dependencies
useEffect(() => {
if (isMobile && activeFilter === 'general') {
onFilterChange('proposals');
}
- }, [activeFilter, isMobile, onFilterChange]);
+ }, [activeFilter, isMobile]);Since onFilterChange is a prop callback, it's safe to omit from dependencies here — the effect only needs to react to changes in activeFilter and isMobile.
📝 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
| useEffect(() => { | |
| if (isMobile && activeFilter === 'general') { | |
| onFilterChange('proposals'); | |
| } | |
| }, [activeFilter, isMobile, onFilterChange]); | |
| useEffect(() => { | |
| if (isMobile && activeFilter === 'general') { | |
| onFilterChange('proposals'); | |
| } | |
| }, [activeFilter, isMobile, onFilterChange]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/epics/src/coherence/components/memory-filters.tsx` around lines 55 -
59, The useEffect currently lists onFilterChange in its dependency array which
can cause unnecessary re-runs if the parent doesn't memoize that callback;
update the effect that checks isMobile and activeFilter (the useEffect block
referencing isMobile, activeFilter, and calling onFilterChange('proposals')) to
only depend on [activeFilter, isMobile] and remove onFilterChange from the deps
so the effect only triggers when activeFilter or isMobile change; leave the call
to onFilterChange intact but do not add it back to the dependency array.
Comment on lines
+56
to
+73
| const existingMessages = matrix.getRoomMessages(canonicalRoomId) ?? []; | ||
| const firstMessage = [...existingMessages].sort( | ||
| (a, b) => a.timestamp.getTime() - b.timestamp.getTime(), | ||
| )[0]; | ||
|
|
||
| if (!firstMessage?.id) { | ||
| await matrix.sendMessage({ | ||
| roomId: canonicalRoomId, | ||
| message: nextDescription, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| await matrix.editRoomMessage({ | ||
| roomId: canonicalRoomId, | ||
| targetEventId: firstMessage.id, | ||
| message: nextDescription, | ||
| }); |
Contributor
There was a problem hiding this comment.
Don't identify the description row by “oldest message”.
When description seeding is retried after the room already has traffic, this edits whichever loaded event has the smallest timestamp, not necessarily the seeded description. Because getRoomMessages() only reflects the currently loaded timeline, pagination can also change the target. Persist the seeded event id, or tag that event and look it up explicitly before calling editRoomMessage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/epics/src/coherence/utils/signal-chat-description.ts` around lines
56 - 73, The code is locating the description by taking the earliest loaded
message (existingMessages -> firstMessage) which can be wrong; instead, when
seeding the description, capture and persist the resulting event id (from
matrix.sendMessage) or add a unique tag/marker to that event, and on subsequent
runs look up that persisted id or search for the tagged event (rather than
sorting by timestamp) and pass that exact event id to matrix.editRoomMessage
(use canonicalRoomId, the stored seededEventId or tag lookup, and
nextDescription). Ensure sendMessage's returned event id is stored in durable
room metadata or state so edits always target the original seeded description.
Comment on lines
+107
to
+108
| import { getCoherenceBySlug } from '@hypha-platform/core/coherence/server/web3'; | ||
| import { upsertSignalDescriptionInRoom } from '../coherence/utils/signal-chat-description'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Client-side import and call site:\n'
rg -n -C2 "getCoherenceBySlug|coherence/server/web3" \
packages/epics/src/common/human-right-panel.tsx
printf '\nServer-side implementation:\n'
sed -n '1,120p' packages/core/src/coherence/server/web3/get-coherence-by-slug.tsRepository: hypha-dao/hypha-web
Length of output: 1550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== human-right-panel.tsx header (check 'use client') ==="
sed -n '1,60p' packages/epics/src/common/human-right-panel.tsx
echo
echo "=== human-right-panel.tsx around getCoherenceBySlug call (confirm effect/handler) ==="
sed -n '2360,2470p' packages/epics/src/common/human-right-panel.tsxRepository: hypha-dao/hypha-web
Length of output: 5350
Keep coherence lookup out of this client component
packages/epics/src/common/human-right-panel.tsx is a 'use client' file, but it imports getCoherenceBySlug from @hypha-platform/core/coherence/server/web3 (line 107) and calls it in the client init flow (around lines 2418-2420). That server module (packages/core/src/coherence/server/web3/get-coherence-by-slug.ts contains 'use server') must not be imported/called from the browser. Fetch/resolve the coherence description in a server action/API/RSC layer and pass it into the client, then seed via upsertSignalDescriptionInRoom.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/epics/src/common/human-right-panel.tsx` around lines 107 - 108, The
client component human-right-panel.tsx must not import/call the server-only
getCoherenceBySlug; remove that import and any direct calls in the client init
flow (where getCoherenceBySlug is used around the init logic) and instead fetch
the coherence description from a server layer (server action, API route, or RSC)
and pass the resolved description into the client component as a prop; inside
the client use the existing upsertSignalDescriptionInRoom to seed the room with
the passed-in description. Ensure the server module getCoherenceBySlug remains
only in server code and that human-right-panel.tsx only receives and consumes
the description string via props.
Comment on lines
+145
to
+355
| if (!useSheetProfileMenu) { | ||
| return ( | ||
| <div className="flex items-center gap-2"> | ||
| {trailingBeforeProfile} | ||
| <DropdownMenu | ||
| open={profileMenuOpen} | ||
| onOpenChange={setProfileMenuOpen} | ||
| modal={false} | ||
| > | ||
| <DropdownMenuTrigger asChild> | ||
| <button | ||
| type="button" | ||
| className={cn( | ||
| 'box-border flex h-10 min-h-10 w-10 min-w-10 shrink-0 items-center justify-center', | ||
| 'isolate overflow-hidden rounded-md bg-neutral-1 p-0 text-neutral-12 outline-none', | ||
| 'shadow-sm transition-colors duration-150', | ||
| 'hover:text-foreground', | ||
| 'focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2', | ||
| 'data-[state=open]:shadow-md', | ||
| )} | ||
| aria-label={t('openProfileMenu')} | ||
| aria-haspopup="menu" | ||
| > | ||
| <PersonAvatar | ||
| size="toolbar" | ||
| avatarSrc={person?.avatarUrl} | ||
| userName={person?.nickname} | ||
| shape="rounded" | ||
| className="h-full w-full rounded-md ring-0" | ||
| /> | ||
| </button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent | ||
| align="end" | ||
| side="bottom" | ||
| sideOffset={6} | ||
| collisionPadding={12} | ||
| className={cn( | ||
| 'w-[min(17.5rem,calc(100vw-1.5rem))] border border-border/90 p-1', | ||
| 'bg-popover text-popover-foreground shadow-xl', | ||
| )} | ||
| > | ||
| <DropdownMenuLabel className="cursor-default px-2 pb-0 pt-1.5 font-normal"> | ||
| <div className="flex gap-3"> | ||
| <PersonAvatar | ||
| size="md" | ||
| avatarSrc={person?.avatarUrl} | ||
| userName={person?.nickname} | ||
| shape="rounded" | ||
| className="ring-1 ring-border/60" | ||
| /> | ||
| <div className="flex min-w-0 flex-1 flex-col gap-1"> | ||
| <span className="truncate text-2 font-semibold leading-snug text-foreground"> | ||
| {primaryLine} | ||
| </span> | ||
| {displayName.trim() && person?.nickname ? ( | ||
| <span className="truncate text-1 text-muted-foreground"> | ||
| {person.nickname} | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| </DropdownMenuLabel> | ||
| {address ? ( | ||
| <div className="mt-2 w-full border-t border-border/50 pt-2 pb-1"> | ||
| <div | ||
| className={cn( | ||
| 'w-full rounded-md border border-border/50 bg-muted/35 px-2 py-1.5', | ||
| 'text-1 text-muted-foreground', | ||
| )} | ||
| > | ||
| <EthAddress address={address} onClick={handleAddressCopy} /> | ||
| </div> | ||
| </div> | ||
| ) : null} | ||
| {navItems.length > 0 ? ( | ||
| <> | ||
| <DropdownMenuSeparator className="-mx-0 my-1" /> | ||
| <DropdownMenuGroup className="space-y-0.5"> | ||
| {navItems.map((item) => | ||
| item.href ? ( | ||
| <DropdownMenuItem | ||
| key={item.href} | ||
| className={menuItemClass} | ||
| asChild | ||
| > | ||
| <Link href={item.href}> | ||
| <span className="flex-1">{item.label}</span> | ||
| <ChevronRight | ||
| className="ml-auto size-4 opacity-60" | ||
| aria-hidden | ||
| /> | ||
| </Link> | ||
| </DropdownMenuItem> | ||
| ) : ( | ||
| <DropdownMenuItem | ||
| key={`nav-action-${item.label}`} | ||
| className={menuItemClass} | ||
| onClick={item.onClick} | ||
| > | ||
| <span className="flex-1">{item.label}</span> | ||
| </DropdownMenuItem> | ||
| ), | ||
| )} | ||
| </DropdownMenuGroup> | ||
| </> | ||
| ) : null} | ||
| {(profileUrl || onboardingUrl || notificationCentrePath) && ( | ||
| <> | ||
| <DropdownMenuSeparator className="-mx-0 my-1" /> | ||
| <DropdownMenuGroup className="space-y-0.5"> | ||
| {profileUrl ? ( | ||
| <DropdownMenuItem className={menuItemClass} asChild> | ||
| <Link href={profileUrl}> | ||
| <UserRound className="size-4 shrink-0" aria-hidden /> | ||
| <span className="flex-1">{t('viewProfile')}</span> | ||
| <ChevronRight | ||
| className="ml-auto size-4 opacity-60" | ||
| aria-hidden | ||
| /> | ||
| </Link> | ||
| </DropdownMenuItem> | ||
| ) : null} | ||
| {onboardingUrl ? ( | ||
| <DropdownMenuItem className={menuItemClass} asChild> | ||
| <Link href={onboardingUrl}> | ||
| <Compass className="size-4 shrink-0" aria-hidden /> | ||
| <span className="flex-1"> | ||
| {t('continueAdventure')} | ||
| </span> | ||
| <ChevronRight | ||
| className="ml-auto size-4 opacity-60" | ||
| aria-hidden | ||
| /> | ||
| </Link> | ||
| </DropdownMenuItem> | ||
| ) : null} | ||
| {notificationCentrePath ? ( | ||
| <DropdownMenuItem className={menuItemClass} asChild> | ||
| <Link href={notificationCentrePath}> | ||
| <Bell className="size-4 shrink-0" aria-hidden /> | ||
| <span className="flex-1"> | ||
| {t('notificationCentre')} | ||
| </span> | ||
| <ChevronRight | ||
| className="ml-auto size-4 opacity-60" | ||
| aria-hidden | ||
| /> | ||
| </Link> | ||
| </DropdownMenuItem> | ||
| ) : null} | ||
| </DropdownMenuGroup> | ||
| </> | ||
| )} | ||
| <DropdownMenuSeparator className="-mx-0 my-1" /> | ||
| <DropdownMenuGroup className="space-y-0.5"> | ||
| {onChangeThemeMode ? ( | ||
| <DropdownMenuItem | ||
| className={menuItemClass} | ||
| onClick={onChangeThemeMode} | ||
| > | ||
| <span className="flex-1"> | ||
| {resolvedTheme === 'dark' | ||
| ? t('switchToLightMode') | ||
| : t('switchToDarkMode')} | ||
| </span> | ||
| <Repeat className="size-4 shrink-0" aria-hidden /> | ||
| </DropdownMenuItem> | ||
| ) : null} | ||
| <DropdownMenuItem | ||
| className={menuItemClass} | ||
| onClick={showMfaEnrollmentModal} | ||
| > | ||
| <span className="flex-1"> | ||
| {hasMfaMethods ? t('updateMfa') : t('protectMfa')} | ||
| </span> | ||
| <Shield className="size-4 shrink-0" aria-hidden /> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuGroup> | ||
| {onDelete ? ( | ||
| <> | ||
| <DropdownMenuSeparator className="-mx-0 my-1" /> | ||
| <DropdownMenuItem | ||
| onClick={onDelete} | ||
| className={cn( | ||
| menuItemClass, | ||
| 'text-error-11 focus:text-error-11', | ||
| )} | ||
| disabled | ||
| > | ||
| <span className="flex-1">{t('delete')}</span> | ||
| <TrashIcon className="size-4 shrink-0" aria-hidden /> | ||
| </DropdownMenuItem> | ||
| </> | ||
| ) : null} | ||
| <DropdownMenuSeparator className="-mx-0 my-1" /> | ||
| <DropdownMenuItem | ||
| onClick={onLogout} | ||
| className={cn( | ||
| menuItemClass, | ||
| 'text-error-11 focus:bg-error-3 focus:text-error-12 data-[highlighted]:bg-error-3', | ||
| )} | ||
| > | ||
| <span className="flex-1">{t('logout')}</span> | ||
| <LogOutIcon className="size-4 shrink-0" aria-hidden /> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| </div> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider extracting shared menu structure to reduce duplication.
The new dropdown menu (lines 145-355) duplicates ~85% of the desktop dropdown menu structure (lines 686-871), including avatar display, address UI, nav items, profile/onboarding/notification links, theme/MFA actions, and logout logic. This same content also appears in the sheet menu (lines 357-576). Changes to menu items or actions now require updates in three places.
♻️ Possible refactor: extract a shared menu content component
Consider extracting the menu structure into a reusable component that accepts a render prop or layout variant:
function ProfileMenuContent({
variant: 'dropdown' | 'sheet',
person,
address,
navItems,
// ... other props
}) {
const itemClass = variant === 'dropdown'
? menuItemClass
: compactSheetItemClass;
return (
<>
{/* Avatar header */}
{/* Address */}
{/* Nav items */}
{/* Profile/onboarding/notifications */}
{/* Theme/MFA */}
{/* Delete/logout */}
</>
);
}Then each menu wrapper (dropdown/sheet) would just handle positioning and trigger behavior while delegating content to the shared component.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/epics/src/people/components/button-profile.tsx` around lines 145 -
355, Extract the duplicated menu body into a shared component (e.g.
ProfileMenuContent) and replace the three inline copies with that component;
ProfileMenuContent should accept props like variant ('dropdown'|'sheet') to pick
item class (menuItemClass vs compactSheetItemClass) and receive person, address,
navItems, profileUrl, onboardingUrl, notificationCentrePath, resolvedTheme,
onChangeThemeMode, hasMfaMethods, showMfaEnrollmentModal, onDelete, onLogout,
handleAddressCopy, primaryLine, displayName, and t so it can render the avatar
header, EthAddress block, navItems map, profile/onboarding/notification links,
theme/MFA actions and logout/delete items; then swap the original
DropdownMenuContent/Sheet content blocks to render <ProfileMenuContent ... /> so
layout/trigger logic (DropdownMenu/Sheet) remains in place while avoiding
duplication.
Comment on lines
+87
to
+115
| <div className="flex w-full flex-row flex-wrap items-center justify-end gap-2"> | ||
| {isDisabled ? ( | ||
| <Button colorVariant="accent" variant="outline" disabled> | ||
| <RadiobuttonIcon /> | ||
| {tTreasury('newToken')} | ||
| </Button> | ||
| </Link> | ||
| )} | ||
| ) : ( | ||
| <Link | ||
| href={`${basePath}/create/issue-new-token?hideBack=true`} | ||
| scroll={false} | ||
| title={tooltipMessage || ''} | ||
| > | ||
| <Button colorVariant="accent" variant="outline"> | ||
| <RadiobuttonIcon /> | ||
| {tTreasury('newToken')} | ||
| </Button> | ||
| </Link> | ||
| )} | ||
|
|
||
| <Button | ||
| className={cn(isDisabled && 'cursor-not-allowed')} | ||
| title={tooltipMessage || ''} | ||
| onClick={fundWallet} | ||
| disabled={isDisabled} | ||
| > | ||
| <CopyIcon /> | ||
| {tTreasury('depositFunds')} | ||
| </Button> | ||
| <Button | ||
| className={cn(isDisabled && 'cursor-not-allowed')} | ||
| title={tooltipMessage || ''} | ||
| onClick={fundWallet} | ||
| disabled={isDisabled} | ||
| > | ||
| <CopyIcon /> | ||
| {tTreasury('depositFunds')} | ||
| </Button> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider extracting the duplicated "New Token" button.
The button JSX is duplicated across the isDisabled conditional branches (lines 89-92 and 99-102), differing only by the Link wrapper. This creates maintenance overhead if button props need to change.
♻️ Proposed refactor to reduce duplication
- <div className="flex w-full flex-row flex-wrap items-center justify-end gap-2">
- {isDisabled ? (
- <Button colorVariant="accent" variant="outline" disabled>
- <RadiobuttonIcon />
- {tTreasury('newToken')}
- </Button>
- ) : (
- <Link
- href={`${basePath}/create/issue-new-token?hideBack=true`}
- scroll={false}
- title={tooltipMessage || ''}
- >
- <Button colorVariant="accent" variant="outline">
- <RadiobuttonIcon />
- {tTreasury('newToken')}
- </Button>
- </Link>
- )}
+ <div className="flex w-full flex-row flex-wrap items-center justify-end gap-2">
+ {React.createElement(
+ isDisabled ? 'div' : Link,
+ isDisabled
+ ? { title: tooltipMessage || '' }
+ : {
+ href: `${basePath}/create/issue-new-token?hideBack=true`,
+ scroll: false,
+ title: tooltipMessage || '',
+ },
+ <Button
+ colorVariant="accent"
+ variant="outline"
+ disabled={isDisabled}
+ >
+ <RadiobuttonIcon />
+ {tTreasury('newToken')}
+ </Button>,
+ )}
<ButtonAlternatively, keep the duplication if the pattern is used consistently across the codebase for clarity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/epics/src/treasury/components/assets/assets-section.tsx` around
lines 87 - 115, Duplicate "New Token" button JSX appears in the isDisabled
branches; extract it into a single reusable piece (e.g., a local NewTokenButton
component or variable) that renders the Button with RadiobuttonIcon and
{tTreasury('newToken')} and accepts props like title, className and disabled;
then conditionally wrap that component with Link when !isDisabled using the
existing href `${basePath}/create/issue-new-token?hideBack=true` and preserve
tooltipMessage, colorVariant/variant, and any accessibility attributes so you
only change rendering logic where Link wrapping differs (refer to isDisabled,
Button, Link, RadiobuttonIcon, basePath, and tTreasury).
| "investmentDepositProposalDescription": "Approuvez et déposez {payAmount} {paySymbol} dans l'escrow (#{escrowId}) pour finaliser l'investissement dans {otherParty}. En retour, {activeSpace} recevra {receiveAmount} {receiveSymbol}.", | ||
| "confirmInvestmentCta": "Confirmer l'investissement", | ||
| "acceptInvestmentTitle": "Accepter l'investissement", | ||
| "acceptInvestmentBody": "{sellerLabel} a accepté l'investissement de votre espace de {buyerAmountLabel} et propose {sellerAmountLabel} en retour.", |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Improve French word order for clarity.
The phrase "l'investissement de votre espace de {buyerAmountLabel}" has awkward double preposition structure in French. The natural word order would be:
"l'investissement de {buyerAmountLabel} de votre espace"
or with punctuation:
"l'investissement de votre espace : {buyerAmountLabel}"
Compare to line 1036 (Personal variant) which correctly uses "votre investissement de {buyerAmountLabel}".
📝 Suggested improvement
- "acceptInvestmentBody": "{sellerLabel} a accepté l'investissement de votre espace de {buyerAmountLabel} et propose {sellerAmountLabel} en retour.",
+ "acceptInvestmentBody": "{sellerLabel} a accepté l'investissement de {buyerAmountLabel} de votre espace et propose {sellerAmountLabel} en retour.",📝 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
| "acceptInvestmentBody": "{sellerLabel} a accepté l'investissement de votre espace de {buyerAmountLabel} et propose {sellerAmountLabel} en retour.", | |
| "acceptInvestmentBody": "{sellerLabel} a accepté l'investissement de {buyerAmountLabel} de votre espace et propose {sellerAmountLabel} en retour.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/i18n/src/messages/fr.json` at line 1035, Update the French message
for the key acceptInvestmentBody to fix the awkward double-preposition phrase:
replace "l'investissement de votre espace de {buyerAmountLabel}" with a clearer
order such as "l'investissement de {buyerAmountLabel} de votre espace" or use a
colon "l'investissement de votre espace : {buyerAmountLabel}" so the sentence
reads naturally (keep the rest of the string and placeholders {sellerLabel} and
{sellerAmountLabel} unchanged).
…ription Use safe upsert mode when opening a signal room from the human panel so only empty or matching description anchors are updated. Co-authored-by: Cursor <cursoragent@cursor.com>
| const decodeHtmlEntities = (value: string): string => { | ||
| if (typeof document === 'undefined') return value; | ||
| const textarea = document.createElement('textarea'); | ||
| textarea.innerHTML = value; |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/epics/src/common/human-right-panel.tsx (1)
107-108:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftRemove server-only coherence lookup from this client component.
Line 107 imports a server module and Line 2418 calls it inside a
'use client'component. Move coherence description resolution to a server layer (RSC/server action/API) and pass the description into this client/context.#!/bin/bash set -euo pipefail echo "Verify client directive + server import/call in HumanRightPanel" sed -n '1,130p' packages/epics/src/common/human-right-panel.tsx | nl -ba | sed -n '1,130p' echo rg -n --type=tsx -C2 "getCoherenceBySlug|`@hypha-platform/core/coherence/server/web3`" packages/epics/src/common/human-right-panel.tsxAs per coding guidelines:
Never import ./server paths from a client component.Also applies to: 2416-2422
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/epics/src/common/human-right-panel.tsx` around lines 107 - 108, This client component currently imports and calls server-only functions getCoherenceBySlug and upsertSignalDescriptionInRoom inside the 'use client' HumanRightPanel; remove those imports and the direct server call, and instead accept a pre-resolved coherence description via props or context. Move the coherence lookup/upsert logic into a server layer (RSC, server action or API route) that calls getCoherenceBySlug/upsertSignalDescriptionInRoom, then pass the resulting description string into HumanRightPanel (or provide it through a client-safe context provider) and replace the lines that invoke getCoherenceBySlug/upsertSignalDescriptionInRoom (around the HumanRightPanel usage) to consume the injected description instead.packages/epics/src/coherence/utils/signal-chat-description.ts (1)
107-111:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist and target a stable description-anchor event id.
Line 110 and Line 128 still pick
chatMessages[0]as the edit target insavemode. That target depends on currently loaded history and can overwrite unrelated chat messages. Store the seeded description event id (room metadata/state) and edit that exact id instead of “oldest loaded message”.Also applies to: 126-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/epics/src/coherence/utils/signal-chat-description.ts` around lines 107 - 111, The code currently picks chatMessages[0] (firstMessage) as the edit target which is unstable; instead persist and read a stable description-anchor event id in room state/metadata and use that id when saving edits. Update the save flow in signal-chat-description.ts to: 1) on initial seed/create, write the created description event id into room metadata/state (e.g., a "description_anchor_event_id" key) using matrix APIs; 2) when in save mode, read that persisted anchor id for canonicalRoomId and target that specific event id for edits instead of using getChatTimelineMessages(...)[0]; and 3) keep a fallback to the oldest loaded message only if no persisted anchor id exists, then persist the chosen id after creating/editing. Ensure you reference getChatTimelineMessages, matrix.getRoomMessages, canonicalRoomId and the variable firstMessage when implementing these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/epics/src/coherence/utils/signal-chat-description.ts`:
- Around line 107-111: The code currently picks chatMessages[0] (firstMessage)
as the edit target which is unstable; instead persist and read a stable
description-anchor event id in room state/metadata and use that id when saving
edits. Update the save flow in signal-chat-description.ts to: 1) on initial
seed/create, write the created description event id into room metadata/state
(e.g., a "description_anchor_event_id" key) using matrix APIs; 2) when in save
mode, read that persisted anchor id for canonicalRoomId and target that specific
event id for edits instead of using getChatTimelineMessages(...)[0]; and 3) keep
a fallback to the oldest loaded message only if no persisted anchor id exists,
then persist the chosen id after creating/editing. Ensure you reference
getChatTimelineMessages, matrix.getRoomMessages, canonicalRoomId and the
variable firstMessage when implementing these changes.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 107-108: This client component currently imports and calls
server-only functions getCoherenceBySlug and upsertSignalDescriptionInRoom
inside the 'use client' HumanRightPanel; remove those imports and the direct
server call, and instead accept a pre-resolved coherence description via props
or context. Move the coherence lookup/upsert logic into a server layer (RSC,
server action or API route) that calls
getCoherenceBySlug/upsertSignalDescriptionInRoom, then pass the resulting
description string into HumanRightPanel (or provide it through a client-safe
context provider) and replace the lines that invoke
getCoherenceBySlug/upsertSignalDescriptionInRoom (around the HumanRightPanel
usage) to consume the injected description instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7097f08d-7af6-43fe-9c60-037b9387c283
📒 Files selected for processing (2)
packages/epics/src/coherence/utils/signal-chat-description.tspackages/epics/src/common/human-right-panel.tsx
Replace plain "Loading..." text with an accent spinner and optional message skeleton preview while auth or the Matrix timeline initializes. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Commits
feat(calls): announce when call capture recording startsfix(ui): improve mobile layouts for chat, memory, and treasuryfix(i18n): add escrow and AI panel strings across localesfix(people): translate profile escrow deposit bannersfeat(ai-panel): show mobilized specialists on assistant repliesfeat(ai-panel): keep translated suggestion tags above composertest(e2e): update AI chat panel suggestion expectationsTest plan
Made with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Internationalization
Refactor