fix desktop speaker selection dialog and transcript name persistence

[krushnarout]

Bugs Fixed

Bug 1 — Speaker selection dialog doesn't open on first click

Clicking a speaker label in the transcript darkens the window but the dialog doesn't appear. Repeated clicks on the same segment repeat the problem. Clicking a different segment's label opens it correctly, after which the original segment works too.

Demo:

CleanShot.2026-04-04.at.21.06.23.mp4

Bug 2 — Speaker name changes not saved

After assigning names to unrecognized speakers, the changes are lost on next open and revert to generic labels (e.g. "Speaker 1").

Demo:

CleanShot.2026-04-04.at.21.07.37.mp4

After Fix

Screen.Recording.2026-04-06.at.3.53.03.PM.mov

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes two bugs in the macOS desktop app's transcript speaker-naming workflow: (1) the speaker-selection dialog not showing on the first tap of a label, and (2) speaker name assignments reverting to generic labels on the next navigation. The changes span four files: AppState.swift (speaker-person map wiring), ConversationDetailView.swift (in-memory segment update after a successful assignment), TranscriptionStorage.swift (segment-level speaker persistence in SQLite), and RewindPage.swift (item-based sheet presentation for the live transcript).

• ConversationDetailView.swift now updates loadedConversation in-memory immediately after a successful onAssignSpeaker API call, giving instant UI feedback while the backend persists the change.
• RewindPage.swift switches to item-based dismissableSheet(item:), resolving the first-click issue by leveraging Identifiable-keyed overlay state rather than a detached Bool flag.
• TranscriptionStorage.swift introduces updateSegmentSpeakerAssignment to persist per-segment personId values in SQLite so speaker assignments survive app restarts for locally-recorded conversations.
• AppState.swift correctly wires the liveSpeakerPersonMap updates into the live transcript UI.

Three concerns are flagged below: an empty-sheet scenario when appState is nil in RewindPage, a potential data-loss race in upsertSegmentsFromServerConversation where a background server sync can overwrite locally-saved personId values before the server acknowledges the assignment, and an animation value keyed on a Bool instead of the actual item identity in DismissableSheetItemModifier.

Confidence Score: 3/5

Merge with caution — the core dialog and persistence fixes are sound, but a data-loss race and a potential empty-overlay regression need addressing.

The in-memory update path in ConversationDetailView and the item-based sheet presentation are well-implemented and directly address both bugs. However, the full segment delete+reinsert in upsertSegmentsFromServerConversation can silently wipe locally-saved personId values if a background sync fires during the API round-trip; the appState nil guard in RewindPage leaves the overlay triggerable with no content (reproducing Bug 1 in an edge path); and the animation Bool value in DismissableSheetItemModifier produces broken transitions on rapid taps.

TranscriptionStorage.swift (upsertSegmentsFromServerConversation race condition), RewindPage.swift (nil appState guard missing at tap site)

Important Files Changed

Filename	Overview
desktop/Desktop/Sources/MainWindow/Pages/ConversationDetailView.swift	Adds immediate in-memory update of transcript segment personIds after assignment succeeds; logic is clean and correct.
desktop/Desktop/Sources/Rewind/UI/RewindPage.swift	Switches live speaker dialog to item-based presentation; risk of empty overlay when appState is nil.
desktop/Desktop/Sources/Rewind/Core/TranscriptionStorage.swift	Full delete+reinsert in upsertSegmentsFromServerConversation can race with locally-saved personId from speaker assignment.
desktop/Desktop/Sources/AppState.swift	liveSpeakerPersonMap wiring looks correct; map is cleared appropriately on session boundaries.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant RV as RewindPage / ConvDetailView
    participant DM as DismissableSheetItemModifier
    participant AS as AppState
    participant TS as TranscriptionStorage
    participant API as Backend API

    U->>RV: Tap speaker label
    RV->>RV: selectedSegment = segment
    RV->>DM: item binding set (non-nil)
    DM->>DM: Show overlay + sheet content
    U->>DM: Enter name, press Save
    alt Live transcript
        DM->>AS: liveSpeakerPersonMap[speaker] = personId
    else History conversation
        DM->>API: onAssignSpeaker(convId, segIds, personId)
        API-->>RV: success = true
        RV->>RV: loadedConversation updated in-memory
        RV->>TS: updateSegmentSpeakerAssignment persists to SQLite
    end
    DM->>DM: selectedSegment = nil, overlay dismissed
    Note over TS,API: Race risk: background upsertSegmentsFromServerConversation
    Note over TS,API: can delete local personId before server confirms

Loading

Comments Outside Diff (3)

1. desktop/Desktop/Sources/Rewind/UI/RewindPage.swift, line 1228-1247 (link)

   Empty sheet when appState is nil

   The dismissableSheet content is wrapped in if let appState = appState { ... }. If appState is nil for any reason — notably, the expandRewindTranscript notification can set isTranscriptExpanded = true independently of whether appState is assigned — tapping a speaker label will set selectedSpeakerSegment, the dimmed overlay will appear, but no dialog content renders. This exactly matches the "darkens the window but the dialog doesn't appear" symptom from Bug 1.

   Guard the assignment at the tap site instead, so the sheet is never triggered without a valid appState:

   onSpeakerTapped: appState == nil ? nil : { segment in
       selectedSpeakerSegment = segment
   }

2. desktop/Desktop/Sources/Rewind/Core/TranscriptionStorage.swift, line 585-603 (link)

   Full segment deletion can race with local speaker assignments

   upsertSegmentsFromServerConversation deletes all local segments then re-inserts from server data. If a background sync fires after updateSegmentSpeakerAssignment has written a personId to the local DB, those values are wiped before the server has a chance to acknowledge the assignment.

   The guard in syncServerConversation checks session.updatedAt >= serverTimestamp, but updateSegmentSpeakerAssignment only touches transcription_segments, not transcription_sessions.updatedAt, so the guard never fires for speaker-only changes and the locally-stored personId can be silently lost.

   Consider either:

   1. Bumping session.updatedAt when speaker assignments are persisted locally so the guard fires, or
   2. Doing a selective merge (update in-place) that preserves existing non-null personId/isUser values rather than a full delete+reinsert.

3. desktop/Desktop/Sources/MainWindow/Pages/AppsPage.swift, line 1929 (link)

   Animation keyed on Bool instead of item identity

   item?.id != nil collapses to a Bool. When the user dismisses sheet A and quickly opens sheet B (or the same segment), if both states are non-nil the value stays true→true and SwiftUI does not replay the transition — the sheet content swaps without animation. Using the item's actual identity as the animation key fixes this:

_{Reviews (1): Last reviewed commit: "fix desktop speaker selection dialog and..." | Re-trigger Greptile}