feat(realtime): land LiveKit dead code + LK-based subscribe (PR 2 of 3)#138
Conversation
commit: |
| * totalInterFrameDelay + totalSquaredInterFrameDelay. | ||
| */ | ||
| interFrameDelayStdDevMs: number | null; | ||
| interFrameDelayVarianceMs: number | null; |
There was a problem hiding this comment.
Field named "variance" actually computes standard deviation
Medium Severity
The field interFrameDelayStdDevMs was renamed to interFrameDelayVarianceMs, but the computation still calculates the standard deviation (square root of variance), not variance. The comment even still states "Report std-dev in ms." Consumers relying on this field as a variance value will get incorrect results — variance and standard deviation have different units and magnitudes (variance is in ms², std-dev is in ms).
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a7a41d2. Configure here.
Brings in the LiveKit transport layer as importable-but-unwired code. No live caller on the publish or subscribe path reaches any of the new modules — they sit alongside the existing aiortc transport until PR 3 performs the final swap. What lands: - New modules: stream-session, signaling-channel, media-channel, remote-stream-exposure, observability/livekit-stats-provider, realtime/config-realtime - New utils: utils/media (imageToBase64), utils/platform - RealtimeObservability gains setLiveKitRoom() that wires a LiveKit-room stats provider into the same stats pipeline introduced in PR 1. setLiveKitRoom is only invoked from within MediaChannel (also dead), so the orchestrator's behavior on the live aiortc path is unchanged. - types.ts becomes a superset: keeps every aiortc message type (still consumed by webrtc-connection/manager) and adds LK message + domain types (LiveKitJoin/RoomInfo, QueuePosition, ConnectionStatus, SessionStarted, InitialState, ServerError, ImageSetOptions, ...). No new types are re-exported from index.ts — public surface unchanged. - Adds livekit-client@^2 to packages/sdk deps (required for the new modules to typecheck even with no live caller). Notes for PR 3: - subscribe-client.ts and createDecartClient still expose the aiortc subscribe path unchanged. PR 3 swaps the implementation behind it. - diagnostics keeps the LK-incompatible event types (IceCandidateEvent, IceStateEvent, PeerConnectionStateEvent, SignalingStateEvent, SelectedCandidatePairEvent) because aiortc publish still emits them; PR 3 drops them along with aiortc. - New tests: realtime-observability-unit.test.ts (covers the orchestrator including setLiveKitRoom indirectly via setStatsProvider). Dead-code invariant: every reference to stream-session, signaling-channel, media-channel, remote-stream-exposure, livekit-stats-provider, and livekit-client lives inside one of the new LK files. No aiortc-path file imports any of them. Validation: typecheck clean, 193/193 tests pass, biome diagnostics unchanged vs origin/main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
a7a41d2 to
0b24f12
Compare
Refresh the LK transport modules landed in PR 2 to match their current state on alon/livekit-prod so PR 3 reduces to wiring + aiortc removal. - stream-session, media-channel, signaling-channel, observability/realtime-observability, utils/platform, tests/realtime-observability-unit.test.ts: taken from prod verbatim. - initial-state-gate.ts: pulled from prod (now imported by stream-session). - remote-stream-exposure.ts: deleted (no longer used in prod). - types.ts and observability/diagnostics.ts: prod content + aiortc-only types/events retained so webrtc-connection/webrtc-manager still compile. PR 3 will drop those superset entries with the aiortc files. Validation: typecheck clean, 196/196 tests pass, biome clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| private async request<TAck extends IncomingRealtimeMessage>( | ||
| message: OutgoingRealtimeMessage, | ||
| matchAck: (msg: IncomingRealtimeMessage) => boolean, | ||
| timeoutMs: number, | ||
| label: string, | ||
| ): Promise<TAck> { |
There was a problem hiding this comment.
lets make it accept a configuration obj? having 4 different params (and those one specifically) makes it too vague on the caller side
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7269fa0. Configure here.
| } | ||
|
|
||
| this.stopStats(); | ||
| this.resetStallDetection(); |
There was a problem hiding this comment.
Redundant resetStallDetection call after stopStats
Low Severity
In setStatsProvider, this.stopStats() is called on line 159 and then this.resetStallDetection() is called immediately after on line 160. However, stopStats() already calls this.resetStallDetection() internally (line 191), making the second call redundant.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7269fa0. Configure here.


Summary
Brings in the LiveKit transport layer as importable-but-mostly-unwired code, and rewires the subscribe path onto it. Publish (
createRealTimeClient) is untouched and still uses the existing aiortcWebRTCManager— the final swap happens in PR 3 (the currentalon/livekit-prodHEAD).This PR is structured so that every line either belongs to the LK transport (new files) or to a strictly additive change on the existing surface. Nothing aiortc was modified for the sake of giving this PR meaning.
What lands
stream-session.ts,signaling-channel.ts,media-channel.ts,remote-stream-exposure.ts,observability/livekit-stats-provider.ts,realtime/config-realtime.tsutils/media.ts(sharedimageToBase64),utils/platform.tsRealtimeObservabilitygainssetLiveKitRoom()that wires a LK-room stats provider into the same stats pipeline introduced in PR 1types.tsbecomes a superset:OfferMessage/AnswerMessage/IceCandidateMessage/ReadyMessage/SessionIdMessage/etc.) — still consumed bywebrtc-connection/webrtc-managerLiveKitJoinMessage,LiveKitRoomInfoMessage,QueuePosition/QueuePositionMessage,ConnectionStatus,SessionStarted,InitialState,ServerError,ImageSetOptions, …)subscribe-client.tsis rewritten ontolivekit-client+RealtimeObservabilitywith the newroom_nametoken format (wassid/ip/port). The aiortc-form encoder/decoder previously living here and used by the publish path'ssubscribeTokenis inlined intorealtime/client.tsso it dies cleanly whenclient.tsis rewritten in PR 3createDecartClientnow exposesrealtime.{ connect, subscribe }wheresubscribecomes fromcreateRealTimeSubscribeClientlivekit-client@^2topackages/sdkdepsIntentionally deferred to PR 3
noopLogger(PR 3 flips tocreateConsoleLogger(\"info\"))diagnostics.tskeeps the LK-incompatible event types (IceCandidateEvent,IceStateEvent,PeerConnectionStateEvent,SignalingStateEvent,SelectedCandidatePairEvent) because aiortc publish still emits them; PR 3 drops them along with aiortcclient.ts→StreamSession, deletion ofwebrtc-manager.ts/webrtc-connection.ts,methods.tsrewrite, aiortc tests removal)Tests
tests/realtime-observability-unit.test.tsandtests/realtime.unit.test.ts(theset()block from the LK branch is excluded — it depends on PR 3'smethods.ts)tests/unit.test.tsassertions adjusted to the new shape:room_nametests inrealtime.unit.test.ts)session_idpopulatessubscribeTokentestTelemetryReporter"warns on non-2xx" test (telemetry is now silent on non-2xx by design —.catch(() => {}))Test plan
pnpm typecheckcleanpnpm test— 217/217 passingpnpm exec biome check .cleancreateDecartClient().realtime.connect) still works against existing aiortc backendcreateDecartClient().realtime.subscribe) connects to a LK-backed/watch-streamtoken🤖 Generated with Claude Code
Note
Medium Risk
Introduces a new LiveKit-based realtime transport layer (WebSocket signaling + LiveKit media) plus new retry/handshake logic and telemetry diagnostics; although mostly additive, it touches connection and observability paths that can affect reliability and reporting.
Overview
Adds a new LiveKit-based realtime transport scaffold:
SignalingChannelmanages WebSocket join/ack flows (including queue position and generation events),MediaChannelmanages LiveKit room connection, remote stream aggregation, and local track publishing, andStreamSessionties them together with retry/reconnect behavior and initial-state gating.Enhances observability/telemetry by centralizing timeouts/thresholds in
REALTIME_CONFIG, adding aclient-session-connection-breakdowndiagnostic (phase timings + initial image size), and wiring LiveKit rooms into the existing stats pipeline viasetLiveKitRoom()+ a LiveKit RTC stats provider.Adds utility helpers (
utils/media,utils/platform), unit tests for the new observability breakdown/stats behavior and signaling close handling, and introduces thelivekit-clientdependency (lockfile updates included).Reviewed by Cursor Bugbot for commit 7269fa0. Bugbot is set up for automated code reviews on this repo. Configure here.