Skip to content

Update voice memo discard icon and hide transcript errors#684

Merged
yewreeka merged 14 commits intodevfrom
onefovrth/voice-memo-sharing
Apr 15, 2026
Merged

Update voice memo discard icon and hide transcript errors#684
yewreeka merged 14 commits intodevfrom
onefovrth/voice-memo-sharing

Conversation

@onefovrth
Copy link
Copy Markdown
Contributor

@onefovrth onefovrth commented Apr 10, 2026

Summary

  • Replace the voice memo discard button icon from X (xmark) to a red trash can (trash) for clearer destructive intent
  • Hide all transcript error states from the UI — errors are now marked as permanently failed (row hidden) instead of showing raw error strings and a retry button

Why hide transcript errors?

Transcript failures fall into two categories, neither of which benefits from showing errors to users:

  • Transcription errors (speech recognition, decoding): deterministic failures where retrying produces the same result
  • Attachment loading errors (network, decryption): surface raw developer-facing localizedDescription strings that aren't meaningful to users

In both cases the transcript row is now simply hidden rather than showing confusing error text.

Test plan

  • Record a voice memo, stop recording, verify the discard button shows a red trash icon
  • Send a voice memo, verify playback works normally
  • Verify transcript "Tap to transcribe" flow still works for successful transcriptions

🤖 Generated with Claude Code

Note

Move voice memo transcript into the bubble and hide transcript errors

  • Transcript content is moved from a separate row below the voice memo bubble into the bubble itself, with states for pending, completed (truncated with tap-to-expand sheet), and not-yet-requested (with a 'View transcript' button).
  • Failed transcripts no longer show error details or a retry affordance in the UI; all non-cancellation transcription failures are now recorded as permanently failed.
  • Reply composer bar and reply reference views now show transcript text when replying to an audio message.
  • The voice memo discard button changes from an xmark icon to a trash.fill icon in caution color.
  • Duration labels across recording, review, and playback views now use zero-padded MM:SS format.

Macroscope summarized 34d80e5.

Change the discard button from an X to a red trash icon for clearer
intent. Mark all transcript failures as permanent instead of showing
error text and retry — transcription errors (speech recognition,
decoding) are deterministic and retrying won't help, and attachment
loading errors surface raw developer-facing strings that aren't
useful to users.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

Claude finished @onefovrth's task —— View job


Code Review Complete ✅

Reviewed PR #684: Update voice memo discard icon and hide transcript errors


Summary

This PR successfully implements two improvements to voice memo UX:

  1. Discard icon change: Replaces xmark with trash.fill (red) for clearer destructive intent
  2. Error hiding: Treats all transcript errors as permanent failures, hiding error states from users

The implementation is sound with good test coverage. The macroscopeapp concern about debug code has been resolved in commit d661eff.


Code Quality ✅

Strengths:

  • Clean separation of concerns between service layer and UI
  • Proper error handling with distinct error types
  • Good use of SwiftUI patterns (Button action extraction, @observable, proper state management)
  • Exhaustive switch statements prevent missed cases
  • Thread-safe actor usage in service layer

Adherence to CLAUDE.md:

  • ✅ Uses guard with early return appropriately
  • ✅ Button actions extracted to avoid closure compilation errors
  • ✅ Proper SwiftUI observation patterns
  • ✅ No forced unwrapping
  • ✅ No implicitly unwrapped optionals

Test Coverage ✅

Excellent test coverage in VoiceMemoTranscriptionServiceTests.swift:

  • ✅ Happy path transcription (line 8)
  • ✅ Skips existing completed transcripts (line 45)
  • ✅ Does not auto-retry failed transcripts (line 90)
  • ✅ Retry bypasses failed-skip logic (line 129)
  • ✅ Transcriber failures → permanentlyFailed (line 170)
  • ✅ Empty transcript → permanentlyFailed (line 242)
  • ✅ Attachment loader failures → permanentlyFailed (line 308)
  • ✅ Concurrent enqueue deduplication (line 339)

All existing tests updated to reflect new permanentlyFailed behavior.


Architecture & Error Handling ✅

Service Layer (VoiceMemoTranscriptionService.swift):

  • Both attachment loading errors (line 173-186) and transcription errors (line 210-221) now call markPermanentlyFailed
  • Stores error.localizedDescription in the database but UI hides these rows entirely
  • Scheduler skips permanentlyFailed entries (line 119), preventing infinite retry loops

UI Layer:

  • VoiceMemoBubble.swift:246-250: Both .failed and .permanentlyFailed render EmptyView()
  • VoiceMemoTranscriptRow.swift:144: Headers hidden for both error states
  • MessagesListRepository.swift:163-165: Permanently failed transcripts filtered out before rendering
  • Fallback for nil transcript defaults to .permanentlyFailed (VoiceMemoBubble.swift:87) — prevents showing errors when transcript data is missing

Potential Issues & Suggestions

1. ⚠️ Lost retry functionality for transient failures

Issue: Previously, attachment loading errors (network issues, temporary decryption failures) were marked as recoverable .failed, allowing explicit user retry. Now all errors are permanent, making network-related failures unrecoverable.

Example scenario: User receives voice memo while offline → attachment fails to load → marked permanentlyFailed → when user comes back online, they cannot retry transcription.

Suggestion: Consider distinguishing between:

  • Permanent failures: Speech recognition unavailable, empty transcript, audio file corrupted
  • Transient failures: Network errors, temporary decryption issues
// Potential approach - retain retry for network errors
catch let error as RemoteAttachmentLoaderError where error.isRetryable {
    try await transcriptWriter.saveFailed(...)  // Allow retry
} catch {
    try await transcriptWriter.markPermanentlyFailed(...)  // No retry
}

Rationale from PR description: "Attachment loading errors surface raw developer-facing localizedDescription strings that aren't meaningful to users" — but hiding errors entirely removes user control. A middle ground might be showing a generic "Unable to transcribe" with retry button for transient errors.

2. ℹ️ New color assets lack documentation

Files:

  • colorFillInvertedMinimal.colorset (light: #141414, dark: #FAFAFA)
  • colorFillInvertedSubtle.colorset (light: #333333, dark: #F5F5F5)

Observation: These colors are used for voice memo UI (VoiceMemoBubble.swift:135, MessagesBottomBar.swift:202) but there's no comment explaining when to use Minimal vs Subtle.

Suggestion: Add brief documentation to distinguish use cases, or consolidate if they're too similar.

3. ✅ Defensive nil handling

VoiceMemoBubble.swift:87:

private var transcriptStatus: VoiceMemoTranscriptStatus {
    guard let transcript else { return .permanentlyFailed }
    ...
}

Good practice: Defaulting to .permanentlyFailed when transcript is nil prevents showing error UI. This is a safe fallback given the UI hides .permanentlyFailed.

Question: Should this log a warning? If transcript is unexpectedly nil, it might indicate a data sync issue worth investigating.

4. ✅ Preview data cleanup verified

The macroscopeapp review flagged hardcoded mock transcript data. Verified resolved in commit d661eff:

  • Mock "Blame it all on my roots..." lyrics removed
  • Mock "Callin Baton Rouge" text removed
  • permanentlyFailed guard restored (MessagesListRepository.swift:163)

Performance Considerations ✅

  • ✅ Actor-based state management prevents race conditions
  • ✅ Deduplication prevents duplicate transcription jobs (line 112-113 in service)
  • ✅ Efficient transcript filtering at repository layer before UI rendering
  • ✅ Temp file cleanup with orphan purging (line 241-272)
  • ✅ Waveform analysis cached and reused

Security Considerations ✅

  • ✅ No secrets or API keys exposed
  • ✅ Error messages stored in DB but not surfaced to UI (prevents info leakage)
  • ✅ Proper file cleanup with defer blocks
  • ✅ Atomic file writes for temp audio files

SwiftLint Compliance

No obvious violations detected:

  • ✅ No force unwrapping
  • ✅ No implicitly unwrapped optionals
  • ✅ Proper function length and complexity
  • ✅ Explicit types on properties
  • ✅ Consistent naming conventions

Recommendation: Run /lint locally to verify before merging.


Final Recommendation

Approve with suggestion: The code is well-written, tested, and follows project conventions. However, consider the trade-off of losing retry functionality for transient failures (Issue #1 above).

Options:

  1. Merge as-is if the product decision is to never show transcript errors (simplest UX)
  2. Add retry for network errors if users should be able to retry failed transcriptions when connectivity is restored
  3. Add ADR documenting the decision to treat all failures as permanent and the rationale

The technical implementation is solid either way — this is primarily a product/UX decision.


Test Plan Verification

From PR description:

  • ✅ Discard button shows red trash icon — verified in MessagesBottomBar.swift:196-198
  • ✅ Playback works normally — no changes to playback logic
  • ✅ "Tap to transcribe" still works — verified in VoiceMemoTranscriptRow.swift:110-112

onefovrth and others added 10 commits April 10, 2026 17:24
Refine layout: 48pt button containers, subtle play bg, caption duration
font, 00:XX format, tighter waveform-duration spacing, 44pt trash button.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Soften stop button to red icon on translucent red circle, switch duration
to caption font in caution color, and use asymmetric pill padding.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move transcript display inside the voice memo bubble instead of as a
separate row, supporting three states: no transcription, completed
transcript with sheet, and transcription approval prompt.

Add GesturePassthroughBackground marker so transcript buttons receive
taps through the UIKit gesture overlay.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use a hidden unclamped Text overlay to measure full height and only
show the chevron and sheet tap when the transcript is truncated.
Also add mock transcript data for simulator testing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add colorFillInvertedMinimal (#141414) and colorFillInvertedSubtle
(#333333) tokens. Use correct fill colors per bubble direction:
outgoing uses invertedSubtle bg, incoming uses invertedPrimary bg.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Flip dark mode values so inverted tokens mirror their non-inverted
counterparts: invertedMinimal #141414/#FAFAFA, invertedSubtle
#333333/#F5F5F5. Remove temporary mock transcript data.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Compact pill layout with waveform + duration row, optional 2-line
transcript below, 160pt max width, 24pt corner radius, subtle border.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pass allVoiceMemoTranscripts through CellConfig and MessagesListView
so reply references can display parent message transcript text.
Analyze audio to extract duration instead of showing "Audio" fallback.
Update waveform color to colorFillTertiary in reply reference.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
onefovrth and others added 3 commits April 10, 2026 21:52
When replying to a voice memo with a completed transcript, the
composer reply bar now shows the transcript text instead of
"Voice Memo". Also updates the waveform icon to 32x32 with a
rounded subtle background.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove hardcoded mock transcripts and TODO comments that were
used for simulator testing. Restore the permanentlyFailed guard
and use real stored transcript status and text.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@onefovrth onefovrth marked this pull request as ready for review April 11, 2026 23:38
@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp bot commented Apr 11, 2026

Approvability

Verdict: Needs human review

Beyond UI styling updates, this PR changes voice memo transcription error handling behavior - failures are now permanently hidden from users with no retry option. This user-facing behavior change to error handling warrants human review.

You can customize Macroscope's approvability policy. Learn more.

@yewreeka yewreeka merged commit 5f571c0 into dev Apr 15, 2026
8 of 9 checks passed
@yewreeka yewreeka deleted the onefovrth/voice-memo-sharing branch April 15, 2026 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants