New speaker assignment in transcript editor#1452
Conversation
📝 WalkthroughWalkthroughParticipants chip refactor: several functions were made internal, a new exported ParticipantList and exported ParticipentItem API were added, and per-item removal and selection props were introduced. Transcript view: adds a candidate-based speaker-assignment flow, integrates ParticipantList into the popover, and replaces the radio range picker with a Select-based SpeakerRangeSelector and UI for applying the candidate+range. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TV as TranscriptView
participant SS as Speaker Selector Popover
participant PL as ParticipantList
participant SRS as SpeakerRangeSelector
User->>TV: Open speaker selector
TV->>SS: Render popover
SS->>PL: Render participants (allowMutate depends on context)
rect rgb(240,248,255)
note right of SS: Candidate-based assignment flow
User->>PL: Click a participant
PL-->>SS: set candidate (popover stays open)
SS->>SRS: Render Select-based range picker
User->>SRS: Choose range
SRS-->>SS: onChange(range)
User->>SS: Click "Apply Speaker Change"
SS->>TV: onSpeakerChange(candidate, range)
TV-->>User: Transcript updated
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (2)
396-403: Fix react-query cache key: results depend on sessionId.
queryFnfilters by session participants, butqueryKeyomitssessionId, causing stale/incorrect caching across sessions.Apply:
- const participants = useQuery({ - queryKey: ["search-participants", query], + const participants = useQuery({ + queryKey: ["search-participants", sessionId, query], + enabled: !!query.trim(), queryFn: async () => { const humans = await dbCommands.listHumans({ search: [4, query] }); const participants = await dbCommands.sessionListParticipants(sessionId); return humans.filter((human) => !participants.some((participant) => participant.id === human.id)); }, });
1-1: Fix Lingui imports andtusage (compile/runtime issue).
useLinguidoesn’t exposet, andtshould come from@lingui/macro. Current code risks runtime errors and unused import.Apply:
-import { Trans, useLingui } from "@lingui/react/macro"; +import { Trans, t } from "@lingui/macro";- const { t } = useLingui(); + // use t macro from @lingui/macro (no hook needed)Also applies to: 278-289
apps/desktop/src/components/right-panel/views/transcript-view.tsx (1)
409-436: Candidate never applied—selecting a person doesn’t change the speaker.Clicking a human only sets
candidate; choosing a range setsspeakerRangebut doesn’t applycandidate. Nothing callsonSpeakerChange(candidate, range), so the assignment never happens.Apply:
- const handleClickHuman = (human: Human) => { - setCandidate(human); - }; + const handleClickHuman = (human: Human) => { + setCandidate(human); + };- {candidate?.id && ( + {candidate?.id && ( <div className="flex items-center gap-1 text-sm"> <span>Assign</span> - <span className="font-semibold border rounded-md py-1 px-2 text-xs truncate"> - {candidate.full_name} + <span className="font-semibold border rounded-md py-1 px-2 text-xs truncate"> + {getDisplayName(candidate)} </span> <span className="text-neutral-500">→</span> - <SpeakerRangeSelector onChange={setSpeakerRange} /> + <SpeakerRangeSelector + onChange={(value) => { + setSpeakerRange(value); + if (candidate) { + setHuman(candidate); // triggers effect to apply change + setCandidate(null); // clear candidate UI + setIsOpen(false); // close popover after assign + } + }} + /> </div> )}This relies on the existing
useEffect([human, speakerRange])to callonSpeakerChange(human, speakerRange)after state updates. No new error handling introduced.Also applies to: 482-491
🧹 Nitpick comments (2)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
133-140: Typo: rename ParticipentItem → ParticipantItem.Improves readability and avoids future confusion.
Apply:
- <ParticipentItem + <ParticipantItem key={member.id} member={member} sessionId={sessionId} isLast={index === (participants ?? []).length - 1} handleClickHuman={handleClickHuman} />-function ParticipentItem({ +function ParticipantItem({ member, sessionId, isLast = false, handleClickHuman, }: {Also applies to: 151-161
apps/desktop/src/components/right-panel/views/transcript-view.tsx (1)
439-444: Return null instead of empty paragraph.Avoids extra DOM nodes in non-interactive states.
Apply:
- if (!sessionId) { - return <p></p>; - } + if (!sessionId) return null;- if (!inactive) { - return <p></p>; - } + if (!inactive) return null;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx(1 hunks)apps/desktop/src/components/right-panel/views/transcript-view.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsxapps/desktop/src/components/right-panel/views/transcript-view.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/right-panel/views/transcript-view.tsx (2)
packages/tiptap/src/transcript/views.tsx (1)
SpeakerChangeRange(70-70)packages/ui/src/components/ui/select.tsx (5)
Select(174-174)SelectTrigger(174-174)SelectValue(174-174)SelectContent(174-174)SelectItem(174-174)
⏰ 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). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (1)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
17-17: Safe to remove export—no external references found
No occurrences ofuseParticipantsWithOrgoutside its defining file; it can be made internal.
apps/desktop/src/components/right-panel/views/transcript-view.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/right-panel/views/transcript-view.tsx (1)
430-435: Don’t auto-apply speaker changes when range changes.This effect triggers on speakerRange change and calls onSpeakerChange immediately, bypassing the new candidate + Apply flow. Remove it.
Apply:
-useEffect(() => { - if (human) { - onSpeakerChange(human, speakerRange); - } -}, [human, speakerRange]);
🧹 Nitpick comments (4)
apps/desktop/src/components/right-panel/views/transcript-view.tsx (1)
531-571: Isolate radio groups to avoid cross-component coupling.Multiple SpeakerRangeSelector instances share name="speaker-range". Use a unique name per instance to prevent native radio grouping collisions.
Apply:
-function SpeakerRangeSelector({ value, onChange }: SpeakerRangeSelectorProps) { +function SpeakerRangeSelector({ value, onChange }: SpeakerRangeSelectorProps) { + const groupName = useId(); const options = [ { value: "current" as const, label: "Only this" }, { value: "fromHere" as const, label: "From here" }, { value: "all" as const, label: "All" }, ]; @@ <label key={option.value} className="flex-1 cursor-pointer" > <input type="radio" - name="speaker-range" + name={groupName} value={option.value} className="sr-only" checked={value === option.value} onChange={() => onChange(option.value)} />And update the React import:
-import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useMemo, useRef, useState, useId } from "react";apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (3)
17-51: Avoid O(n²) lookups when grouping by organization.Build a map of organizations once and index by id instead of finding per group during mapping.
Apply:
function useParticipantsWithOrg(sessionId: string) { const { data: participants = [] } = useQuery({ queryKey: ["participants", sessionId], queryFn: async () => { const participants = await dbCommands.sessionListParticipants(sessionId); - const orgs = await Promise.all( + const orgs = await Promise.all( participants .map((p) => p.organization_id) .filter((id) => id !== null) .map((id) => dbCommands.getOrganization(id)), ).then((orgs) => orgs.filter((o) => o !== null)); + const orgMap = new Map(orgs.map((o) => [o.id, o])); const grouped = participants.reduce((acc, participant) => { const orgId = participant.organization_id ?? NO_ORGANIZATION_ID; acc[orgId] = [...(acc[orgId] || []), participant]; return acc; }, {} as Record<string, Human[]>); - return Object.entries(grouped).map(([orgId, participants]) => ({ - organization: orgs.find((o) => o.id === orgId) ?? null, + return Object.entries(grouped).map(([orgId, participants]) => ({ + organization: orgId === NO_ORGANIZATION_ID ? null : orgMap.get(orgId) ?? null, participants, })).sort((a, b) => { if (!a.organization && b.organization) { return 1; } if (a.organization && !b.organization) { return -1; } return (a.organization?.name || "").localeCompare(b.organization?.name || ""); }); }, });
178-190: Fix typo: “ParticipentItem” → “ParticipantItem”.Pure readability; avoids propagating misspelling in future references.
Apply:
-function ParticipentItem({ +function ParticipantItem({ member, sessionId, handleClickHuman, allowRemove = true, selected = false, }: {And replace usages:
- <ParticipentItem + <ParticipantItem key={member.id} member={member} sessionId={sessionId} handleClickHuman={handleClickHuman} allowRemove={allowMutate} selected={member.id === selectedHuman?.id} />
210-212: Minor a11y: label the remove control.Add aria-label for screen readers; keep stopPropagation behavior.
Apply:
- {allowRemove && ( + {allowRemove && ( <div role="button" tabIndex={0} + aria-label="Remove participant" onClick={(e) => { e.stopPropagation(); handleRemoveParticipant(member.id); }} onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); e.stopPropagation(); handleRemoveParticipant(member.id); } }}Also applies to: 218-253
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/desktop/src/locales/en/messages.pois excluded by!**/*.poapps/desktop/src/locales/ko/messages.pois excluded by!**/*.po
📒 Files selected for processing (2)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx(4 hunks)apps/desktop/src/components/right-panel/views/transcript-view.tsx(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/right-panel/views/transcript-view.tsxapps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (1)
packages/utils/src/string.ts (1)
getInitials(1-12)
⏰ 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 (windows, windows-latest)
🔇 Additional comments (7)
apps/desktop/src/components/right-panel/views/transcript-view.tsx (5)
18-18: Import looks correct.ParticipantList export path matches usage.
419-447: Candidate-based flow: reset-on-open is good.Resetting candidate when the popover opens avoids stale selections.
448-451: Click-to-candidate wiring looks right.Selection is deferred until Apply; no side effects here.
495-501: ParticipantList integration is appropriate.allowMutate={false} prevents destructive actions in this context; selectedHuman highlights the candidate.
501-518: Apply button flow is coherent.Apply calls onSpeakerChange(candidate, range) and closes the popover.
apps/desktop/src/components/editor-area/note-header/chips/participants-chip.tsx (2)
115-130: Inner chip refactor LGTM.Delegating to ParticipantList simplifies composition and reuse.
132-177: ParticipantList API and rendering look solid.selectedHuman and allowMutate cover both read-only and interactive contexts.
1 participant
No description provided.