[Enhancement]Add startCall high-scale livestream publisher hint#1134
Merged
ipavlidakis merged 4 commits intodevelopfrom Apr 24, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAn optional join-only hint, Changes
Sequence DiagramsequenceDiagram
participant UI as Demo App UI
participant Env as AppEnvironment
participant VM as CallViewModel
participant CC as CallController
participant API as Backend API
UI->>Env: Read highScaleLivestreamPublisherHint.value
UI->>VM: startCall(..., highScaleLivestreamPublisherHint)
VM->>VM: Forward hint into CreateCallOptions
VM->>CC: call.join(create: true, options: CreateCallOptions)
CC->>CC: Cache hint in context
CC->>API: POST /calls/join (hintHighScaleLivestreamPublisher)
API-->>CC: Join response
CC-->>VM: Join complete
VM-->>UI: Update call state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
ipavlidakis
changed the title
ipavlidakis
force-pushed
the
iliaspavlidakis/ios-1623-enhancementimplement-high-tier-sfu-hinting
branch
from
d52570b to
114b37d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
DemoApp/Sources/Views/CallView/DemoCallsViewModel.swift (1)
70-75: Wrap the feature-flag lookup to keep this call readable.Line 75 exceeds the Swift line-length guideline.
♻️ Proposed wrap
+ let publisherHint = AppEnvironment + .highScaleLivestreamPublisherHint + .value + callViewModel.startCall( callType: .default, callId: UUID().uuidString, members: members, ring: true, - highScaleLivestreamPublisherHint: AppEnvironment.highScaleLivestreamPublisherHint.value + highScaleLivestreamPublisherHint: publisherHint )As per coding guidelines,
**/*.swift: “Use 80 characters as the maximum line length”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DemoApp/Sources/Views/CallView/DemoCallsViewModel.swift` around lines 70 - 75, The call to callViewModel.startCall is exceeding the Swift 80-char line limit because AppEnvironment.highScaleLivestreamPublisherHint.value is inlined; extract that feature-flag into a local let (e.g. let highScaleLivestreamPublisherHint = AppEnvironment.highScaleLivestreamPublisherHint.value) above the startCall invocation and pass the local variable as the highScaleLivestreamPublisherHint parameter to keep the call readable and within line-length guidelines.DemoApp/Sources/Views/CallView/CallingView/SimpleCallingView.swift (1)
250-257: Wrap the high-scale hint lookup before passing it.Line 256 is over the configured Swift line length.
♻️ Proposed wrap
await setPreferredVideoCodec(for: callId) try? await setAudioSessionPolicyOverride(for: callId) await setClientCapabilities(for: callId) + let publisherHint = AppEnvironment + .highScaleLivestreamPublisherHint + .value + viewModel.startCall( callType: callType, callId: callId, members: [], ring: false, maxDuration: AppEnvironment.callExpiration.duration, - highScaleLivestreamPublisherHint: AppEnvironment.highScaleLivestreamPublisherHint.value, + highScaleLivestreamPublisherHint: publisherHint, video: viewModel.callSettings.videoOn )As per coding guidelines,
**/*.swift: “Use 80 characters as the maximum line length”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DemoApp/Sources/Views/CallView/CallingView/SimpleCallingView.swift` around lines 250 - 257, The long call-site expression exceeds the 80-char line limit; extract AppEnvironment.highScaleLivestreamPublisherHint.value into a local let (e.g., let highScaleHint = AppEnvironment.highScaleLivestreamPublisherHint.value) above the viewModel.startCall call and pass highScaleHint into viewModel.startCall (referencing startCall and AppEnvironment.highScaleLivestreamPublisherHint.value) so the call invocation lines wrap under the configured line length.StreamVideoTests/Call/Call_Tests.swift (1)
641-667: Align the new test with the test naming andsubjectconventions.Line 641 is over 80 characters, and the SUT local should be named
subject.♻️ Proposed test cleanup
- func test_join_withHighScaleLivestreamPublisherHint_optionsWerePassedToCallController() async throws { + func test_highScaleHint_join_passesOptionsToCallController() async throws { let mockCallController = MockCallController() - let call = MockCall(.dummy(callController: mockCallController)) - call.stub(for: \.state, with: .init(.dummy())) + let subject = MockCall(.dummy(callController: mockCallController)) + subject.stub(for: \.state, with: .init(.dummy())) mockCallController.stub(for: .join, with: JoinCallResponse.dummy()) let options = CreateCallOptions( highScaleLivestreamPublisherHint: true ) - _ = try await call.join(options: options) + _ = try await subject.join(options: options)As per coding guidelines,
**/*.swift: “Use 80 characters as the maximum line length”. As per coding guidelines,**/*Tests.swift: “Usesubjectas the name of the subject under test in test files”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@StreamVideoTests/Call/Call_Tests.swift` around lines 641 - 667, The test function name test_join_withHighScaleLivestreamPublisherHint_optionsWerePassedToCallController exceeds the 80-character limit and the SUT local is not using the required name; rename the test to a shorter descriptive name (e.g., test_join_passesHighScaleLivestreamHintToCallController) and rename the local variable call to subject (update usages of subject.join(...) and subject.stub(...) as needed), ensuring any wrapped lines (like CreateCallOptions initialization) are broken to stay within 80 characters.DemoApp/Sources/Views/CallView/CallingView/DetailedCallingView.swift (1)
171-177: Avoid the overlongstartCallargument line.The new argument makes this call exceed the 80-character Swift limit.
♻️ Proposed wrap
} else { + let publisherHint = AppEnvironment + .highScaleLivestreamPublisherHint + .value + viewModel.startCall( callType: callType, callId: text, members: members, ring: callFlow == .ringEvents, - highScaleLivestreamPublisherHint: AppEnvironment.highScaleLivestreamPublisherHint.value, + highScaleLivestreamPublisherHint: publisherHint, video: viewModel.callSettings.videoOn ) }As per coding guidelines,
**/*.swift: “Use 80 characters as the maximum line length”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DemoApp/Sources/Views/CallView/CallingView/DetailedCallingView.swift` around lines 171 - 177, The call to viewModel.startCall exceeds the 80-character limit; break the call into multiple lines so each parameter is on its own indented line (e.g., viewModel.startCall( on its own line, then callType:, callId:, members:, ring: callFlow == .ringEvents, highScaleLivestreamPublisherHint: AppEnvironment.highScaleLivestreamPublisherHint.value, video: viewModel.callSettings.videoOn each on separate lines) and keep line lengths under 80 characters while preserving trailing commas and current argument order.StreamVideoSwiftUITests/CallViewModel_Tests.swift (1)
788-788: Shorten the test name to stay within the Swift line limit.The current declaration exceeds 80 characters.
♻️ Proposed rename
- func test_startCall_withHighScaleLivestreamPublisherHint_forwardsHintInJoinOptions() async throws { + func test_highScaleHint_startCall_forwardsJoinOptions() async throws {As per coding guidelines,
**/*.swift: “Use 80 characters as the maximum line length”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@StreamVideoSwiftUITests/CallViewModel_Tests.swift` at line 788, The test method name test_startCall_withHighScaleLivestreamPublisherHint_forwardsHintInJoinOptions is over the 80 char Swift line limit; rename it to a shorter, descriptive name (for example: test_startCall_forwardsHighScalePublisherHint) and update any references to that symbol in the test file (e.g., XCTest discovery or call sites) so the test still runs; ensure the new name remains descriptive and under 80 characters and rebuild tests to confirm no references are missed.DemoApp/Sources/Views/CallView/DemoCallContainerView.swift (1)
58-64: Wrap the high-scale hint expression before thestartCallcall.Line 63 exceeds the repository’s Swift line-length limit.
♻️ Proposed wrap
+ let publisherHint = AppEnvironment + .highScaleLivestreamPublisherHint + .value + viewModel.startCall( callType: callType, callId: .unique, members: [.init(user: .init(id: name))], ring: true, - highScaleLivestreamPublisherHint: AppEnvironment.highScaleLivestreamPublisherHint.value + highScaleLivestreamPublisherHint: publisherHint )As per coding guidelines,
**/*.swift: “Use 80 characters as the maximum line length”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DemoApp/Sources/Views/CallView/DemoCallContainerView.swift` around lines 58 - 64, The long argument expression AppEnvironment.highScaleLivestreamPublisherHint.value causes the startCall invocation to exceed the Swift 80-char line limit; fix this by extracting that expression into a short local variable (e.g., highScaleHint) just before calling viewModel.startCall and then pass that variable as the highScaleLivestreamPublisherHint argument to viewModel.startCall to keep the call lines under the limit.DemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swift (1)
10-32: Document the new debug flag semantics.Please add DocC comments here, especially because
.disabledmaps tonilrather thanfalse, and wrap the long static property declaration while touching it.📝 Proposed fix
+ /// Debug-only toggle for forwarding the high-scale livestream publisher + /// hint during call joins. enum HighScaleLivestreamPublisherHintToggle: Hashable, Debuggable { case enabled, disabled @@ + /// Backend join hint value. `nil` omits the hint from the request. var value: Bool? { switch self { @@ - nonisolated(unsafe) static var highScaleLivestreamPublisherHint: HighScaleLivestreamPublisherHintToggle = .disabled + /// Default to disabled so regular demo joins do not send the publisher hint. + nonisolated(unsafe) static var highScaleLivestreamPublisherHint: + HighScaleLivestreamPublisherHintToggle = .disabledAs per coding guidelines, "Use 80 characters as the maximum line length" and "Use docC with
///for doc comments on non-private APIs".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swift` around lines 10 - 32, Add DocC comments explaining the semantics of HighScaleLivestreamPublisherHintToggle (noting that .disabled maps to nil rather than false) on the enum and its computed properties (title, value) and also document the nonisolated static property highScaleLivestreamPublisherHint; update the long static declaration to wrap across multiple lines for readability (e.g., break before the type annotation and initializer) so it adheres to the 80-character line length guideline. Ensure comments use triple-slash DocC style (///) and reference the enum cases (.enabled, .disabled) and the static var name in the same declaration block.StreamVideoTests/Controllers/CallController_Tests.swift (1)
203-225: Replace fixed sleeps with state-based fulfillment.The new tests wait for 0.5 seconds before connecting the SFU. Please wait for the coordinator state/context instead so the tests remain deterministic.
🧪 Proposed fix
- await wait(for: 0.5) + await fulfillment { + webRTCCoordinatorFactory + .mockCoordinatorStack + .coordinator + .stateMachine + .currentStage + .id == .joining + } webRTCCoordinatorFactory .mockCoordinatorStack .sfuStack .setConnectionState(to: .connected(healthCheckInfo: .init()))Apply the same replacement in both new tests.
As per coding guidelines, "Use XCTest with async/await and expectations; avoid time-based sleeps in tests".
Also applies to: 268-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@StreamVideoTests/Controllers/CallController_Tests.swift` around lines 203 - 225, Replace the hard-coded wait(for: 0.5) sleeps with deterministic, state-based waits that observe the coordinator/context becoming ready: instead of waiting a fixed duration in the test around the joinTask (which calls subject.joinCall), await an expectation or use your existing fulfillment helper to wait for the webRTCCoordinatorFactory.mockCoordinatorStack (or its sfuStack) to reach the connected/ready state (e.g., observe setConnectionState or a join-ready callback) before calling mockCoordinatorStack.joinResponse([]); apply the same change to the second test block around lines 268-290 so both tests wait for the coordinator state/context rather than sleeping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 7-8: The changelog entry for the newly exposed API should be moved
from the "Changed" section to the "Added" section: relocate the line mentioning
CreateCallOptions.highScaleLivestreamPublisherHint (and its note about
Call.join(options: ...)) into the "### 🔥 Added" section so the doc reflects a
new API addition rather than a behavior change; keep the PR link and wording but
update the section header placement accordingly.
In `@Sources/StreamVideo/Controllers/CallController.swift`:
- Line 140: When building the JoinCallRequest, prefer the per-attempt hint from
options (options?.highScaleLivestreamPublisherHint) instead of reading
controller-wide cachedHighScaleLivestreamPublisherHint; use the cache only as a
fallback for recovery/reconnect flows and stop unconditionally overwriting
cachedHighScaleLivestreamPublisherHint during a normal explicit join.
Concretely: in the code paths that construct JoinCallRequest (referencing
JoinCallRequest and the join entrypoint where
cachedHighScaleLivestreamPublisherHint is currently assigned), read the hint
from options first and fall back to cachedHighScaleLivestreamPublisherHint only
if options is nil, and move any assignment to
cachedHighScaleLivestreamPublisherHint into reconnect/recovery handlers rather
than the regular join initialization.
---
Nitpick comments:
In
`@DemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swift`:
- Around line 10-32: Add DocC comments explaining the semantics of
HighScaleLivestreamPublisherHintToggle (noting that .disabled maps to nil rather
than false) on the enum and its computed properties (title, value) and also
document the nonisolated static property highScaleLivestreamPublisherHint;
update the long static declaration to wrap across multiple lines for readability
(e.g., break before the type annotation and initializer) so it adheres to the
80-character line length guideline. Ensure comments use triple-slash DocC style
(///) and reference the enum cases (.enabled, .disabled) and the static var name
in the same declaration block.
In `@DemoApp/Sources/Views/CallView/CallingView/DetailedCallingView.swift`:
- Around line 171-177: The call to viewModel.startCall exceeds the 80-character
limit; break the call into multiple lines so each parameter is on its own
indented line (e.g., viewModel.startCall( on its own line, then callType:,
callId:, members:, ring: callFlow == .ringEvents,
highScaleLivestreamPublisherHint:
AppEnvironment.highScaleLivestreamPublisherHint.value, video:
viewModel.callSettings.videoOn each on separate lines) and keep line lengths
under 80 characters while preserving trailing commas and current argument order.
In `@DemoApp/Sources/Views/CallView/CallingView/SimpleCallingView.swift`:
- Around line 250-257: The long call-site expression exceeds the 80-char line
limit; extract AppEnvironment.highScaleLivestreamPublisherHint.value into a
local let (e.g., let highScaleHint =
AppEnvironment.highScaleLivestreamPublisherHint.value) above the
viewModel.startCall call and pass highScaleHint into viewModel.startCall
(referencing startCall and
AppEnvironment.highScaleLivestreamPublisherHint.value) so the call invocation
lines wrap under the configured line length.
In `@DemoApp/Sources/Views/CallView/DemoCallContainerView.swift`:
- Around line 58-64: The long argument expression
AppEnvironment.highScaleLivestreamPublisherHint.value causes the startCall
invocation to exceed the Swift 80-char line limit; fix this by extracting that
expression into a short local variable (e.g., highScaleHint) just before calling
viewModel.startCall and then pass that variable as the
highScaleLivestreamPublisherHint argument to viewModel.startCall to keep the
call lines under the limit.
In `@DemoApp/Sources/Views/CallView/DemoCallsViewModel.swift`:
- Around line 70-75: The call to callViewModel.startCall is exceeding the Swift
80-char line limit because AppEnvironment.highScaleLivestreamPublisherHint.value
is inlined; extract that feature-flag into a local let (e.g. let
highScaleLivestreamPublisherHint =
AppEnvironment.highScaleLivestreamPublisherHint.value) above the startCall
invocation and pass the local variable as the highScaleLivestreamPublisherHint
parameter to keep the call readable and within line-length guidelines.
In `@StreamVideoSwiftUITests/CallViewModel_Tests.swift`:
- Line 788: The test method name
test_startCall_withHighScaleLivestreamPublisherHint_forwardsHintInJoinOptions is
over the 80 char Swift line limit; rename it to a shorter, descriptive name (for
example: test_startCall_forwardsHighScalePublisherHint) and update any
references to that symbol in the test file (e.g., XCTest discovery or call
sites) so the test still runs; ensure the new name remains descriptive and under
80 characters and rebuild tests to confirm no references are missed.
In `@StreamVideoTests/Call/Call_Tests.swift`:
- Around line 641-667: The test function name
test_join_withHighScaleLivestreamPublisherHint_optionsWerePassedToCallController
exceeds the 80-character limit and the SUT local is not using the required name;
rename the test to a shorter descriptive name (e.g.,
test_join_passesHighScaleLivestreamHintToCallController) and rename the local
variable call to subject (update usages of subject.join(...) and
subject.stub(...) as needed), ensuring any wrapped lines (like CreateCallOptions
initialization) are broken to stay within 80 characters.
In `@StreamVideoTests/Controllers/CallController_Tests.swift`:
- Around line 203-225: Replace the hard-coded wait(for: 0.5) sleeps with
deterministic, state-based waits that observe the coordinator/context becoming
ready: instead of waiting a fixed duration in the test around the joinTask
(which calls subject.joinCall), await an expectation or use your existing
fulfillment helper to wait for the webRTCCoordinatorFactory.mockCoordinatorStack
(or its sfuStack) to reach the connected/ready state (e.g., observe
setConnectionState or a join-ready callback) before calling
mockCoordinatorStack.joinResponse([]); apply the same change to the second test
block around lines 268-290 so both tests wait for the coordinator state/context
rather than sleeping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69a736b6-2619-40b7-a8dd-ab81f80c45ed
📒 Files selected for processing (14)
CHANGELOG.mdDemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swiftDemoApp/Sources/Components/Debug/Items/FeatureFlags/DebugMenu+FeatureFlagsView.swiftDemoApp/Sources/Views/CallView/CallingView/DetailedCallingView.swiftDemoApp/Sources/Views/CallView/CallingView/SimpleCallingView.swiftDemoApp/Sources/Views/CallView/DemoCallContainerView.swiftDemoApp/Sources/Views/CallView/DemoCallsViewModel.swiftSources/StreamVideo/Controllers/CallController.swiftSources/StreamVideo/Models/CoordinatorModels.swiftSources/StreamVideoSwiftUI/CallViewModel.swiftStreamVideoSwiftUITests/CallViewModel_Tests.swiftStreamVideoTests/Call/Call_JoinRecovery_Tests.swiftStreamVideoTests/Call/Call_Tests.swiftStreamVideoTests/Controllers/CallController_Tests.swift
martinmitrevski
approved these changes
Apr 22, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/StreamVideo/Controllers/CallController.swift (1)
59-59:⚠️ Potential issue | 🟠 MajorPrefer the per-attempt hint when building the join request.
cachedHighScaleLivestreamPublisherHintis controller-wide mutable state on an@unchecked Sendableclass. If another explicit join mutates the cache before this attempt buildsJoinCallRequest, Line 656 can send the wrong hint. Keep the cache for recovery/reconnect fallback, but useoptionsas the source of truth for the current join attempt.🐛 Proposed fix
let joinCall = JoinCallRequest( create: create, data: callRequest, - hintHighScaleLivestreamPublisher: cachedHighScaleLivestreamPublisherHint, + hintHighScaleLivestreamPublisher: options?.highScaleLivestreamPublisherHint + ?? cachedHighScaleLivestreamPublisherHint, location: location, migratingFrom: migratingFrom, migratingFromList: migratingFromList?.isEmpty == false ? migratingFromList : nil,Verification script to confirm the cache remains mutable controller state and is used as the request value:
#!/bin/bash # Description: Inspect the hint cache declaration, assignment, and request usage. rg -n -C3 'class CallController: `@unchecked` Sendable|cachedHighScaleLivestreamPublisherHint|hintHighScaleLivestreamPublisher:' --type swiftAlso applies to: 138-138, 653-657
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamVideo/Controllers/CallController.swift` at line 59, The join request is using controller-wide mutable state cachedHighScaleLivestreamPublisherHint instead of the per-attempt options; update the code that builds JoinCallRequest (see JoinCallRequest creation and the hintHighScaleLivestreamPublisher assignment) to prefer options.hintHighScaleLivestreamPublisher for the current join attempt and only fall back to cachedHighScaleLivestreamPublisherHint when options is nil/unspecified, while keeping cachedHighScaleLivestreamPublisherHint for reconnect/recovery paths and preserving its existing read/write sites in CallController.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/StreamVideo/Controllers/CallController.swift`:
- Line 59: The join request is using controller-wide mutable state
cachedHighScaleLivestreamPublisherHint instead of the per-attempt options;
update the code that builds JoinCallRequest (see JoinCallRequest creation and
the hintHighScaleLivestreamPublisher assignment) to prefer
options.hintHighScaleLivestreamPublisher for the current join attempt and only
fall back to cachedHighScaleLivestreamPublisherHint when options is
nil/unspecified, while keeping cachedHighScaleLivestreamPublisherHint for
reconnect/recovery paths and preserving its existing read/write sites in
CallController.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4e1ed63-5a13-4d83-95ef-57973bf67739
📒 Files selected for processing (1)
Sources/StreamVideo/Controllers/CallController.swift
ipavlidakis
force-pushed
the
iliaspavlidakis/ios-1623-enhancementimplement-high-tier-sfu-hinting
branch
from
2741b1d to
cf72477
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
StreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_ConnectingStageTests.swift (1)
386-426: Good coverage; minor suggestion to tighten the assertion.The new test confirms hint forwarding on the rejoining path. Consider also asserting
input.coordinator,input.currentSFU, andinput.migratingFromListhere (as the sibling tests do) so regressions narrowing only the hint don't slip through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@StreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_ConnectingStageTests.swift` around lines 386 - 426, In test_transition_fromRejoiningWithHighScaleHint_forwardsReceivedOptions add the same structural assertions used in sibling tests: after unwrapping input from mockCoordinatorStack.webRTCAuthenticator.recordedInputPayload(callType, for: .authenticate)?.first assert that input.coordinator is WebRTCCoordinator (type equality), input.currentSFU is nil, and input.migratingFromList is nil (or empty as per the other tests) in addition to the existing checks on create/ring/notify/options so the test validates the full payload shape as well as the hint.StreamVideoTests/Call/Call_JoinRecovery_Tests.swift (1)
477-479: Visibility relaxation is intentional and scoped.Dropping
privatesoCallController_Tests.swiftcan reuseCallAuthenticationBackedWebRTCAuthenticatoris reasonable for test-only shared infrastructure. Consider moving the helper to a dedicated test-support file if more suites start depending on it, to avoid implicit cross-file test coupling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@StreamVideoTests/Call/Call_JoinRecovery_Tests.swift` around lines 477 - 479, The class CallAuthenticationBackedWebRTCAuthenticator was made non-private so CallController_Tests can reuse it across files; leave the visibility change but, to avoid implicit cross-file test coupling, move CallAuthenticationBackedWebRTCAuthenticator into a dedicated test-support file (e.g., TestSupport/CallAuthenticators.swift) or a shared test helpers module and update imports accordingly so other suites can depend on it explicitly; ensure the type signature (CallAuthenticationBackedWebRTCAuthenticator: WebRTCAuthenticating, `@unchecked` Sendable) and any helpers it uses are kept accessible to tests but not promoted to production targets.DemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swift (1)
44-56: The@State+didSetpattern works here but is fragile; use an explicit side-effect in the updater instead.The pattern is used consistently across the debug menu (15+ similar views), and the updater closure directly assigns to the
@Statevariable (updater: { value = $0 }), which does trigger thedidSetobserver. However, this approach is not idiomatic and relies on internal@Stateproperty observer behavior that could break with SwiftUI version changes or if the updater mechanism ever changes to use a binding.Apply the suggested safer alternative to be explicit about the side-effect:
♻️ Safer alternative
- `@State` private var value = AppEnvironment.highScaleLivestreamPublisherHint { - didSet { AppEnvironment.highScaleLivestreamPublisherHint = value } - } + `@State` private var value = AppEnvironment.highScaleLivestreamPublisherHint var body: some View { ItemMenuView( items: [.enabled, .disabled], currentValue: value, label: "High-Scale Livestream Hint", availableAfterLogin: true, - updater: { value = $0 } + updater: { + value = $0 + AppEnvironment.highScaleLivestreamPublisherHint = $0 + } ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swift` around lines 44 - 56, The current fragile pattern uses `@State` private var value = AppEnvironment.highScaleLivestreamPublisherHint with a didSet to sync back to AppEnvironment and relies on the updater triggering that didSet; remove the didSet from the `@State` declaration and move the environment update into the ItemMenuView updater closure so it explicitly sets AppEnvironment.highScaleLivestreamPublisherHint and then updates the local state (e.g., updater: { newValue in AppEnvironment.highScaleLivestreamPublisherHint = newValue; value = newValue }), referencing the symbols value, AppEnvironment.highScaleLivestreamPublisherHint, and ItemMenuView/updater to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Sources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator`+Connecting.swift:
- Around line 16-17: The .rejoining → .connecting transition currently accepts
and forwards the incoming options parameter instead of enforcing recovery-safe
options; locate the transition handler in WebRTCCoordinator+Connecting.swift
(the method that handles the ".rejoining -> .connecting" transition and takes an
options parameter) and replace use of the raw options with a recovery-safe set
derived from the coordinator context (e.g., call context.recoveryJoinOptions()
or use a JoinOptions.recoverySafe(...) helper) so the transition always rebuilds
only recovery-safe join options before proceeding.
In `@StreamVideoTests/WebRTC/v2/WebRTCCoorindator_Tests.swift`:
- Around line 135-178: The test
test_connect_rejectedTransition_keepsHighScaleHint leaves the authenticator
waiting on canAuthenticate if the second subject.connect throws, causing slow
noisy failures; modify the test to ensure canAuthenticate.send(true) is always
executed by adding a defer block (or equivalent finally) immediately after
setting up mockWebRTCAuthenticator.onAuthenticate so that
canAuthenticate.send(true) runs regardless of exceptions, ensuring the
mockWebRTCAuthenticator.onAuthenticate wait is unblocked even when the second
connect throws.
---
Nitpick comments:
In
`@DemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swift`:
- Around line 44-56: The current fragile pattern uses `@State` private var value =
AppEnvironment.highScaleLivestreamPublisherHint with a didSet to sync back to
AppEnvironment and relies on the updater triggering that didSet; remove the
didSet from the `@State` declaration and move the environment update into the
ItemMenuView updater closure so it explicitly sets
AppEnvironment.highScaleLivestreamPublisherHint and then updates the local state
(e.g., updater: { newValue in AppEnvironment.highScaleLivestreamPublisherHint =
newValue; value = newValue }), referencing the symbols value,
AppEnvironment.highScaleLivestreamPublisherHint, and ItemMenuView/updater to
locate and change the code.
In `@StreamVideoTests/Call/Call_JoinRecovery_Tests.swift`:
- Around line 477-479: The class CallAuthenticationBackedWebRTCAuthenticator was
made non-private so CallController_Tests can reuse it across files; leave the
visibility change but, to avoid implicit cross-file test coupling, move
CallAuthenticationBackedWebRTCAuthenticator into a dedicated test-support file
(e.g., TestSupport/CallAuthenticators.swift) or a shared test helpers module and
update imports accordingly so other suites can depend on it explicitly; ensure
the type signature (CallAuthenticationBackedWebRTCAuthenticator:
WebRTCAuthenticating, `@unchecked` Sendable) and any helpers it uses are kept
accessible to tests but not promoted to production targets.
In
`@StreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_ConnectingStageTests.swift`:
- Around line 386-426: In
test_transition_fromRejoiningWithHighScaleHint_forwardsReceivedOptions add the
same structural assertions used in sibling tests: after unwrapping input from
mockCoordinatorStack.webRTCAuthenticator.recordedInputPayload(callType, for:
.authenticate)?.first assert that input.coordinator is WebRTCCoordinator (type
equality), input.currentSFU is nil, and input.migratingFromList is nil (or empty
as per the other tests) in addition to the existing checks on
create/ring/notify/options so the test validates the full payload shape as well
as the hint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 379743d1-7c4d-44a3-acc2-bddbbb13fa18
📒 Files selected for processing (20)
CHANGELOG.mdDemoApp/Sources/Components/Debug/Items/FeatureFlags/Components/HighScaleLivestreamPublisherHint.swiftDemoApp/Sources/Components/Debug/Items/FeatureFlags/DebugMenu+FeatureFlagsView.swiftDemoApp/Sources/Views/CallView/CallingView/DetailedCallingView.swiftDemoApp/Sources/Views/CallView/CallingView/SimpleCallingView.swiftDemoApp/Sources/Views/CallView/DemoCallContainerView.swiftDemoApp/Sources/Views/CallView/DemoCallsViewModel.swiftSources/StreamVideo/Controllers/CallController.swiftSources/StreamVideo/Models/CoordinatorModels.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Connecting.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Migrated.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Rejoining.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Stage.swiftSources/StreamVideoSwiftUI/CallViewModel.swiftStreamVideoSwiftUITests/CallViewModel_Tests.swiftStreamVideoTests/Call/Call_JoinRecovery_Tests.swiftStreamVideoTests/Call/Call_Tests.swiftStreamVideoTests/Controllers/CallController_Tests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_ConnectingStageTests.swiftStreamVideoTests/WebRTC/v2/WebRTCCoorindator_Tests.swift
✅ Files skipped from review due to trivial changes (2)
- StreamVideoSwiftUITests/CallViewModel_Tests.swift
- StreamVideoTests/Controllers/CallController_Tests.swift
🚧 Files skipped from review as they are similar to previous changes (8)
- StreamVideoTests/Call/Call_Tests.swift
- DemoApp/Sources/Components/Debug/Items/FeatureFlags/DebugMenu+FeatureFlagsView.swift
- DemoApp/Sources/Views/CallView/DemoCallsViewModel.swift
- Sources/StreamVideo/Models/CoordinatorModels.swift
- DemoApp/Sources/Views/CallView/DemoCallContainerView.swift
- DemoApp/Sources/Views/CallView/CallingView/DetailedCallingView.swift
- DemoApp/Sources/Views/CallView/CallingView/SimpleCallingView.swift
- Sources/StreamVideoSwiftUI/CallViewModel.swift
Comment on lines
+16
to
+17
| /// - Important: Rejoining transitions ignore ringing side effects. | ||
| /// They rebuild only recovery-safe join options from context. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect WebRTCCoordinator connecting-stage call sites and
# confirm rejoining paths do not forward unsanitized CreateCallOptions.
rg -n -C 6 --glob '*.swift' '\.connecting\s*\(' Sources/StreamVideo
rg -n -C 8 --glob '*.swift' 'highScaleLivestreamPublisherHint|rejoining' Sources/StreamVideoRepository: GetStream/stream-video-swift
Length of output: 41864
🏁 Script executed:
# Search for all direct instantiations or calls to .connecting state
rg -n '\.connecting\s*\(' Sources/StreamVideo/WebRTC --type swift -A 5 | head -100
# Check if there are any other places that may pass unsanitized options to .connecting
rg -n 'case \.rejoining' Sources/StreamVideo/WebRTC --type swift -A 15Repository: GetStream/stream-video-swift
Length of output: 5373
Enforce recovery-safe options in the .rejoining → .connecting transition.
The documentation at lines 73–74 states the transition "rebuilds only recovery-safe join options from context," but line 104 accepts and forwards the options parameter directly without filtering. While the current sole caller in WebRTCCoordinator+Rejoining.swift:117 properly passes context.recoveryJoinOptions(), this delegates the contract to the caller rather than enforcing it at the transition boundary.
Implement defensive narrowing to honor the documented contract:
Defensive narrowing
case .rejoining:
if ring || notify {
log.assert(ring == false, "Ring cannot be true when rejoining.")
log.assert(notify == false, "Notify cannot be true when rejoining.")
}
+ let recoveryOptions = context.recoveryJoinOptions()
execute(
create: false,
ring: false,
notify: false,
- options: options,
+ options: recoveryOptions,
updateSession: true,
onErrorDisconnect: true
)Also applies to: 73-74, 95-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Sources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator`+Connecting.swift
around lines 16 - 17, The .rejoining → .connecting transition currently accepts
and forwards the incoming options parameter instead of enforcing recovery-safe
options; locate the transition handler in WebRTCCoordinator+Connecting.swift
(the method that handles the ".rejoining -> .connecting" transition and takes an
options parameter) and replace use of the raw options with a recovery-safe set
derived from the coordinator context (e.g., call context.recoveryJoinOptions()
or use a JoinOptions.recoverySafe(...) helper) so the transition always rebuilds
only recovery-safe join options before proceeding.
Collaborator
StreamVideo XCSize
|
Collaborator
StreamVideoSwiftUI XCSize
|
Public Interface @MainActor open class CallViewModel: ObservableObject
- public func startCall(callType: String,callId: String,members: [Member],team: String? = nil,ring: Bool = false,maxDuration: Int? = nil,maxParticipants: Int? = nil,startsAt: Date? = nil,backstage: BackstageSettingsRequest? = nil,customData: [String: RawJSON]? = nil,video: Bool? = nil)
+ public func startCall(callType: String,callId: String,members: [Member],team: String? = nil,ring: Bool = false,maxDuration: Int? = nil,maxParticipants: Int? = nil,startsAt: Date? = nil,backstage: BackstageSettingsRequest? = nil,highScaleLivestreamPublisherHint: Bool? = nil,customData: [String: RawJSON]? = nil,video: Bool? = nil)
public struct CreateCallOptions: Sendable, Hashable
-
+ public var highScaleLivestreamPublisherHint: Bool?
-
+
- public init(memberIds: [String]? = nil,members: [MemberRequest]? = nil,custom: [String: RawJSON]? = nil,settings: CallSettingsRequest? = nil,startsAt: Date? = nil,team: String? = nil)
+
+ public init(memberIds: [String]? = nil,members: [MemberRequest]? = nil,custom: [String: RawJSON]? = nil,settings: CallSettingsRequest? = nil,startsAt: Date? = nil,team: String? = nil,highScaleLivestreamPublisherHint: Bool? = nil) |
Collaborator
SDK Size
|
|
ipavlidakis
deleted the
iliaspavlidakis/ios-1623-enhancementimplement-high-tier-sfu-hinting
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Issue Links
Resolves https://linear.app/stream/issue/IOS-1623/enhancementimplement-high-tier-sfu-hinting
🎯 Goal
Add support for the high-scale livestream publisher join hint across the iOS SDK so clients can explicitly mark publisher joins that should use the backend’s high-scale livestream routing behavior, and expose a Demo app toggle to exercise that end-to-end.
📝 Summary
CreateCallOptions.highScaleLivestreamPublisherHintJoinCallRequest.hintHighScaleLivestreamPublisherCallControllerso recovery joins reuse itCallViewModel.startCall(...)🛠 Implementation
On the core SDK side, the high-scale livestream publisher hint is now part of
CreateCallOptionsand is mapped toJoinCallRequest.hintHighScaleLivestreamPublisherbefore the backendjoinCallrequest is sent.CallControllercaches the value from the explicit join invocation so recovery joins continue to send the same hint even when reconnection paths no longer carryCreateCallOptions.On the SwiftUI side,
CallViewModel.startCall(...)now accepts the optional hint and forwards it through the create-and-join path. This keeps the feature explicit at the call site rather than storing it as mutable view model state.joinAndRingCall(...)was intentionally left unchanged.In the Demo app, a new feature flag was added under the existing debug feature-flags menu in a dedicated file. Demo
startCall(...)call sites now pass the flag explicitly so the end-to-end behavior can be toggled without changing code.The change is covered with focused tests in the core SDK for request shape and recovery propagation, and in SwiftUI for
startCall(...)forwarding.🧪 Manual Testing Notes
Feature Flags, toggleHigh-Scale Livestream Hint.startCall(...)flow with the flag disabled and confirm the call joins normally.hint_high_scale_livestream_publisher = truewhen enabled.☑️ Contributor Checklist
Summary by CodeRabbit
New Features
Debug
Documentation
Tests