Fix transcript speaker editing and persistence#6348
Conversation
Greptile SummaryThis PR fixes the transcript speaker rename workflow end-to-end on macOS: Confidence Score: 5/5Safe to merge — all findings are P2 style/hardening concerns with no runtime impact under current code paths The two-phase commit (server then local) is sound and the new regression tests verify the backendId round-trip. The isSaving flag is never reset, but the sheet is always dismissed anyway. The tappedSegmentIndex fallback to 0 cannot fire in the current call graph. The Firestore race condition is pre-existing and not worsened by this PR. NameSpeakerSheet.swift (isSaving / tappedSegmentIndex fallback) and firestore.rs (non-atomic segment PATCH) Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant NS as NameSpeakerSheet
participant CDV as ConversationDetailView
participant AS as AppState
participant API as Rust Backend
participant FS as Firestore
participant TS as TranscriptionStorage (SQLite)
U->>NS: Tap speaker label → sheet opens
U->>NS: Select person / You, tap Save
NS->>CDV: onSave(personId, isUser, segmentIndices)
CDV->>CDV: assignmentMetadata(segmentIndices)
Note over CDV: targets = backendIds + #index:N fallbacks
CDV->>AS: onAssignSpeaker(conversationId, targets, personId, isUser)
AS->>API: PATCH /v1/conversations/:id/segments/assign-bulk
API->>FS: read conv → mutate segments → PATCH transcript_segments
FS-->>API: 200 OK
API-->>AS: 200 OK
AS-->>CDV: success = true
CDV->>TS: updateSpeakerAssignmentByBackendId(backendIds, fallbackOrders)
CDV->>CDV: updateDisplayedConversation() — optimistic UI update
CDV->>CDV: selectedSegmentForNaming = nil → dismiss sheet
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private func save() { | ||
| isSaving = true | ||
| let indices = tagAllFromSpeaker ? sameSpeakerIndices : [tappedSegmentIndex] | ||
| onSave(selectedPersonId, isUserSelected, indices) | ||
| let segmentIndices = tagAllFromSpeaker ? sameSpeakerIndices : [tappedSegmentIndex] | ||
| onSave(selectedPersonId, isUserSelected, segmentIndices) | ||
| } |
There was a problem hiding this comment.
isSaving never reset to false
save() sets isSaving = true synchronously but the flag is never cleared. The sheet is always dismissed by the parent (via selectedSegmentForNaming = nil), so the stuck spinner is invisible today. If any future code path keeps the sheet open after a failed save (e.g., a retry UX), the Save button will be permanently disabled.
| private func save() { | |
| isSaving = true | |
| let indices = tagAllFromSpeaker ? sameSpeakerIndices : [tappedSegmentIndex] | |
| onSave(selectedPersonId, isUserSelected, indices) | |
| let segmentIndices = tagAllFromSpeaker ? sameSpeakerIndices : [tappedSegmentIndex] | |
| onSave(selectedPersonId, isUserSelected, segmentIndices) | |
| } | |
| private func save() { | |
| isSaving = true | |
| let segmentIndices = tagAllFromSpeaker ? sameSpeakerIndices : [tappedSegmentIndex] | |
| onSave(selectedPersonId, isUserSelected, segmentIndices) | |
| // Note: isSaving is reset implicitly when the sheet is dismissed. | |
| // If you add a stay-open error path, reset isSaving = false there. | |
| } |
## Summary - preserve stable backend transcript segment ids through desktop decoding, local storage, and Firestore speaker assignment updates - fix the transcript speaker rename flow by centering dismissable sheets reliably and exposing speaker labels as real accessible buttons - add desktop automation hooks and regression tests used to verify the speaker rename flow on the Mac mini ## Testing - xcrun swift test -c debug --package-path Desktop --filter TranscriptSpeakerAssignmentTests - Mac mini manual verification: clicked transcript speaker label in Omi Dev, confirmed `Name Speaker` sheet opened, saved a speaker assignment, reopened the conversation, restarted Omi Dev, and confirmed the renamed speaker persisted instead of reverting to `Speaker 1`
1 participant
Summary
Testing
Name Speakersheet opened, saved a speaker assignment, reopened the conversation, restarted Omi Dev, and confirmed the renamed speaker persisted instead of reverting toSpeaker 1