Skip to content

fix(session): make PTT holds idempotent and observable - #218

Merged
scotej merged 26 commits into
mainfrom
fix/ptt-idempotent-diagnostics
Aug 11, 2026
Merged

fix(session): make PTT holds idempotent and observable#218
scotej merged 26 commits into
mainfrom
fix/ptt-idempotent-diagnostics

Conversation

@scotej

@scotej scotej commented Aug 10, 2026

Copy link
Copy Markdown
Owner

What changed

  • Make PTT press() idempotent while already active. A repeated/native duplicate Pressed edge can no longer mean "mute".
  • Drop duplicate Pressed edges at the Tauri-event listener as well, so a repeat burst does not cause redundant renders or getStats() work. The store keeps the same invariant independently as defence in depth.
  • Preserve the original 120-second stuck-key deadline across duplicate presses instead of clearing or extending it.
  • Add a macOS physical-key reconciliation watcher using CoreGraphics. Carbon/global-hotkey remains the low-latency press/release path, but while a session is active StudyVis also watches the current configured PTT shortcut and emits a recovery release after it has observed that combo physically held and then released. This covers genuinely dropped/delayed Carbon Released edges without hard-coding the default binding.
  • Keep the physical watcher generation-scoped to the live session. It cannot emit on session start, does nothing for unsupported/unobservable keys, follows runtime rebinding on the next poll, and is disabled entirely on non-macOS targets.
  • Distinguish normal shortcut releases from physical-watch recovery releases in diagnostics and suppress whichever duplicate release arrives second.
  • Add structured PTT edge diagnostics with monotonic per-run edge sequence, before/after state, edge source, hold duration, and explicit duplicate detection.
  • Add privacy-safe WebRTC media snapshots after each accepted local PTT edge: aggregate audio sender counts, MediaStreamTrack.enabled/readyState, and outbound RTP bytesSent / packetsSent counters and deltas.
  • Observe remote sessionStore.peers[*].ptt transitions and capture the matching receiver-side audio track state plus inbound RTP bytesReceived / packetsReceived counters. This lets two diagnostic logs distinguish sender mute/state failure, missing RTP transport, and receive/playback-side failure.
  • Emit high-signal warnings/errors only for deterministic media-state contradictions (active PTT with no/disabled audio sender, inactive PTT with an enabled sender, active peer PTT with no/live-broken audio receiver). Duplicate native edges and ordinary RTP snapshots are persisted at debug level, avoiding warning noise while preserving the exact timeline.
  • Keep Storybook/plain-web behavior quiet: a missing Tauri event bridge is logged only when a real Tauri runtime exists.

Root cause

PR #168 made a second press() while PTT was active an emergency mute, based on the assumption that the global-shortcut backend could never deliver another Pressed during one physical hold. Issue #209 reports a deterministic ~0.2 s hold threshold on macOS and, after checking with the other participant, no usable audio reached the peer. That makes Pressed -> active=false unsafe: a duplicate/repeated edge can disable the actual audio track while the key is still held.

This change restores the correct hold-to-talk invariant: Pressed can activate PTT or be ignored; only Released, native physical-release recovery, reset, or the failsafe can deactivate it.

Diagnostics / privacy

The added logs intentionally do not contain peer IDs, session topics, device names, track IDs, codecs, IP/ICE addresses, SDP, microphone contents, or captured audio. They record only booleans, counts, durations, event sequence numbers, edge source, and aggregate RTP byte/packet counters through the existing structured/redacted logger. Debug records are written to studyvis.log regardless of the developer-console debug preference, so exported diagnostics contain the PTT timeline without requiring a special reproduction mode.

Media snapshots keep their nested sender/receiver counters below the logger's per-object key cap, so bytesSent/bytesReceived and their deltas are not silently truncated.

Regression coverage

  • repeated press() remains active
  • a burst of repeated Pressed-style calls remains active until Release
  • duplicate presses do not re-arm/extend MAX_HOLD_MS
  • a new hold works cleanly after the failsafe fires
  • macOS virtual-key map covers the shipped default binding
  • outbound sender aggregation and track state
  • inbound receiver aggregation and track state
  • sender/receiver getStats() failures are non-fatal and counted
  • RTP counter deltas, including reconnect/reset handling
  • diagnostics degrade safely when the peer map cannot be inspected

Manual verification target

On macOS Apple Silicon with a Windows/macOS peer:

  1. Hold PTT for >1 second and speak continuously; the local indicator must remain active for the whole hold and the peer must hear the speech.
  2. Release; indicator and sender track must mute immediately.
  3. Repeat several short/long holds and test a rebound PTT shortcut.
  4. Export diagnostics from both participants. Expected local records: ptt.edge.received, optional ptt.edge.duplicate_pressed, and ptt.media.snapshot with enabled/live sender + outbound counters. edgeSource=physical-watch means the native watcher recovered a release before Carbon did. Expected receiver records: ptt.peer.edge.received and ptt.media.snapshot with live receiver + inbound counters.

Fixes #209

Copilot AI lite review requested due to automatic review settings August 10, 2026 23:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@scotej, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cdf39caa-a1db-498e-b0ee-9d886d4df10e

📥 Commits

Reviewing files that changed from the base of the PR and between 7fbc5d6 and 958e4e4.

📒 Files selected for processing (10)
  • src-tauri/src/commands/system.rs
  • src/features/system/PttListener.tsx
  • src/features/system/pttDiagnostics.ts
  • src/features/system/pttEdgeReconciler.ts
  • src/features/system/pttEventBridge.ts
  • src/stores/pttStore.ts
  • tests/unit/ptt-diagnostics.test.ts
  • tests/unit/ptt-edge-reconciler.test.ts
  • tests/unit/ptt-event-bridge.test.ts
  • tests/unit/ptt-store.test.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@scotej
scotej merged commit e7b229d into main Aug 11, 2026
19 checks passed
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.

Push to talk issue

2 participants