chore(repo): fixes required for pana #1152
Conversation
|
Warning Rate limit exceeded@Brazol has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 45 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRemoves multiple Google protobuf well‑known types and generated files, upgrades protobuf toolchain, adds RTP metrics models and a SendMetrics RPC (clients, JSON/protobuf/Twirp clients, server dispatch), removes many generated Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SignalServer
participant MetricsProcessor
Client->>SignalServer: SendMetricsRequest (sessionId, unifiedSessionId, inbounds/outbounds...)
SignalServer->>MetricsProcessor: validate & deserialize metrics (InboundRtp/OutboundRtp...)
MetricsProcessor-->>SignalServer: ack/processing result
SignalServer-->>Client: SendMetricsResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_video_noise_cancellation/lib/noise_cancellation_audio_processor.dart (1)
29-31: Bug: Missing parentheses ontoStringcall.
err.toStringpasses the method reference to the logger instead of the actual error string. This will log the method signature rather than the error message.Proposed fix
} catch (err) { - _logger.e(err.toString); + _logger.e(() => err.toString()); }Note: Wrapping in a lambda
() =>matches the pattern used elsewhere in this file (e.g., line 27).
🤖 Fix all issues with AI agents
In
@packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation.dart:
- Around line 9-15: The documentation comment above the
StreamVideoNoiseCancellation class contains an orphaned code block marker (```)
on line 14; remove the stray ``` or replace it with a complete usage example so
the doc comment is properly terminated and not left open—update the doc comment
surrounding the StreamVideoNoiseCancellation class to either delete the orphaned
marker or add a small fenced code example showing class usage.
In @packages/stream_video_push_notification/example/pubspec.yaml:
- Line 1: The pubspec package name has a typo: change the name value
"stream_video_push_notificaiton_example" to the correct
"stream_video_push_notification_example" in the pubspec.yaml so the package
identifier is spelled correctly (update the name field in the file).
🧹 Nitpick comments (4)
packages/stream_video_push_notification/example/lib/main.dart (1)
6-21: Consider addingWidgetsFlutterBinding.ensureInitialized()for a more complete example.Flutter plugins typically require the binding to be initialized before use. Users copying this example might encounter runtime issues without it.
💡 Suggested improvement
+import 'package:flutter/widgets.dart'; import 'package:stream_video/stream_video.dart'; import 'package:stream_video_push_notification/stream_video_push_notification.dart'; void main() { + WidgetsFlutterBinding.ensureInitialized(); + // Initialize StreamVideo with push notification support final client = StreamVideo(packages/stream_video_noise_cancellation/example/pubspec.yaml (1)
1-12: Consider addingpublish_to: nonefor the example package.Since this is an example package that shouldn't be published to pub.dev, consider explicitly marking it as such. This prevents accidental publishing and clarifies intent.
📦 Suggested addition
name: stream_video_noise_cancellation_example +publish_to: none environment: sdk: ^3.8.0 flutter: ">=3.32.0"packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation_method_channel.dart (1)
41-51: Inconsistentasyncmodifier across methods.
setEnabledanddeviceSupportsAdvancedAudioProcessinglack theasyncmodifier whileregisterProcessorandisEnabledhave it. This causes theUnimplementedErrorto be thrown synchronously rather than returned as aFuture.error.While functionally equivalent when awaited in a try-catch, being consistent improves maintainability.
Suggested fix
@override - Future<void> setEnabled(bool enabled) { + Future<void> setEnabled(bool enabled) async { if (!CurrentPlatform.isAndroid && !CurrentPlatform.isIos) { throw UnimplementedError( 'The current platform (${CurrentPlatform.name}) does not support audio processing.', ); } - return methodChannel.invokeMethod<void>('setEnabled', {'enabled': enabled}); + await methodChannel.invokeMethod<void>('setEnabled', {'enabled': enabled}); }Apply the same pattern to
deviceSupportsAdvancedAudioProcessing.packages/stream_video_noise_cancellation/lib/noise_cancellation_audio_processor.dart (1)
53-68: Inconsistent log level for unsupported platform.
setEnableduses_logger.e(error) forPlatformExceptionwhileisEnabledanddeviceSupportsAdvancedAudioProcessinguse_logger.w(warning) for the same scenario. Consider using warning level consistently since unsupported platforms are an expected condition, not an error.Suggested fix
} on PlatformException catch (e) { - _logger.e( + _logger.w( () => 'Noise cancellation is not supported on this platform: $e', );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (36)
packages/stream_video/lib/protobuf/google/protobuf/duration.pb.dartpackages/stream_video/lib/protobuf/google/protobuf/duration.pbenum.dartpackages/stream_video/lib/protobuf/google/protobuf/duration.pbjson.dartpackages/stream_video/lib/protobuf/google/protobuf/duration.protopackages/stream_video/lib/protobuf/google/protobuf/struct.pb.dartpackages/stream_video/lib/protobuf/google/protobuf/struct.pbenum.dartpackages/stream_video/lib/protobuf/google/protobuf/struct.pbjson.dartpackages/stream_video/lib/protobuf/google/protobuf/struct.protopackages/stream_video/lib/protobuf/google/protobuf/timestamp.pb.dartpackages/stream_video/lib/protobuf/google/protobuf/timestamp.pbenum.dartpackages/stream_video/lib/protobuf/google/protobuf/timestamp.pbjson.dartpackages/stream_video/lib/protobuf/google/protobuf/timestamp.protopackages/stream_video/lib/protobuf/google/protobuf/wrappers.pb.dartpackages/stream_video/lib/protobuf/google/protobuf/wrappers.pbenum.dartpackages/stream_video/lib/protobuf/google/protobuf/wrappers.pbjson.dartpackages/stream_video/lib/protobuf/google/protobuf/wrappers.protopackages/stream_video/lib/protobuf/video/sfu/event/events.pb.dartpackages/stream_video/lib/protobuf/video/sfu/event/events.pbenum.dartpackages/stream_video/lib/protobuf/video/sfu/event/events.pbjson.dartpackages/stream_video/lib/protobuf/video/sfu/models/models.pb.dartpackages/stream_video/lib/protobuf/video/sfu/models/models.pbenum.dartpackages/stream_video/lib/protobuf/video/sfu/models/models.pbjson.dartpackages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pb.dartpackages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbenum.dartpackages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbjson.dartpackages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbserver.dartpackages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbtwirp.dartpackages/stream_video/pubspec.yamlpackages/stream_video_noise_cancellation/example/lib/main.dartpackages/stream_video_noise_cancellation/example/pubspec.yamlpackages/stream_video_noise_cancellation/lib/noise_cancellation_audio_processor.dartpackages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation.dartpackages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation_method_channel.dartpackages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation_platform_interface.dartpackages/stream_video_push_notification/example/lib/main.dartpackages/stream_video_push_notification/example/pubspec.yaml
💤 Files with no reviewable changes (16)
- packages/stream_video/lib/protobuf/google/protobuf/timestamp.proto
- packages/stream_video/lib/protobuf/google/protobuf/struct.pbenum.dart
- packages/stream_video/lib/protobuf/google/protobuf/struct.proto
- packages/stream_video/lib/protobuf/google/protobuf/wrappers.pbjson.dart
- packages/stream_video/lib/protobuf/google/protobuf/duration.pb.dart
- packages/stream_video/lib/protobuf/google/protobuf/wrappers.pbenum.dart
- packages/stream_video/lib/protobuf/google/protobuf/timestamp.pb.dart
- packages/stream_video/lib/protobuf/google/protobuf/duration.proto
- packages/stream_video/lib/protobuf/google/protobuf/duration.pbjson.dart
- packages/stream_video/lib/protobuf/google/protobuf/wrappers.proto
- packages/stream_video/lib/protobuf/google/protobuf/timestamp.pbjson.dart
- packages/stream_video/lib/protobuf/google/protobuf/struct.pbjson.dart
- packages/stream_video/lib/protobuf/google/protobuf/struct.pb.dart
- packages/stream_video/lib/protobuf/google/protobuf/wrappers.pb.dart
- packages/stream_video/lib/protobuf/google/protobuf/duration.pbenum.dart
- packages/stream_video/lib/protobuf/google/protobuf/timestamp.pbenum.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: iOS Build (Dogfooding)
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_push_notification
- GitHub Check: build
- GitHub Check: analyze
🔇 Additional comments (31)
packages/stream_video/lib/protobuf/video/sfu/event/events.pbenum.dart (1)
11-11: LGTM!Adding
prefer_relative_importsto the ignore list is appropriate for this generated protobuf file. Generated code typically uses absolute imports, and suppressing this linter warning avoids unnecessary noise without impacting functionality.packages/stream_video_push_notification/example/lib/main.dart (1)
1-4: LGTM!The example file provides a clear and concise demonstration of initializing StreamVideo with push notification support for both iOS (APN) and Android (Firebase) platforms. The placeholder values and structure are appropriate for documentation purposes.
packages/stream_video_noise_cancellation/example/pubspec.yaml (1)
3-5: Version constraints are appropriate.Dart SDK 3.8.0 and Flutter 3.32.0 are stable releases with established use in the field, making them suitable choices for an example package. The constraints (^3.8.0 and >=3.32.0) are neither overly restrictive nor problematic.
packages/stream_video_push_notification/example/pubspec.yaml (2)
10-10: No action needed. Thestream_video: ^1.2.2dependency is compatible with the specified Dart SDK (^3.8.0) and Flutter (>=3.32.0) versions—the package constraints match exactly.Likely an incorrect or invalid review comment.
4-5: No issues found. Dart SDK^3.8.0(released May 20, 2025) and Flutter>=3.32.0(released May 20, 2025) are both valid released versions and appropriate for this example project.packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation.dart (1)
16-37: Documentation additions look good.Clear and concise method documentation for the public API.
packages/stream_video_noise_cancellation/example/lib/main.dart (1)
1-16: LGTM!Clear example demonstrating how to wire up the
NoiseCancellationAudioProcessorwithStreamVideo. The placeholder credentials andunused_local_variableignore are appropriate for example code.packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation_method_channel.dart (1)
17-39: Platform guards implementation looks good.The platform checks with clear error messages are appropriate for guarding against unsupported platforms.
packages/stream_video_noise_cancellation/lib/noise_cancellation_audio_processor.dart (2)
36-49: LGTM!Graceful fallback to
falseonPlatformExceptionis appropriate for unsupported platforms.
70-88: LGTM!Error handling with graceful fallback is consistent with
isEnabled.packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation_platform_interface.dart (1)
32-52: LGTM!The platform interface follows standard Flutter plugin conventions with clear documentation and appropriate default
UnimplementedErrorimplementations.packages/stream_video/pubspec.yaml (2)
49-49: protoc_plugin version aligns with protobuf 6.0.0 upgrade.The protoc_plugin 25.0.0 upgrade is consistent with the protobuf 6.0.0 dependency change and ensures compatible code generation.
30-30: Protobuf 6.0.0 upgrade is correctly configured with compatible versions.The major version bump from protobuf 5.x to 6.x is properly paired with
protoc_plugin: ^25.0.0. The generated code reflects expected breaking changes: well-known types are imported as libraries, factory constructors use.addAll()pattern, and lint directives align with protobuf 6 output format.packages/stream_video/lib/protobuf/video/sfu/event/events.pbjson.dart (1)
11-12: Generated file update - lint directives aligned with protobuf 6.0.0.This is auto-generated code from protoc_plugin. The updated
ignore_for_filedirectives addingprefer_relative_importsare consistent with the newer code generator output.packages/stream_video/lib/protobuf/video/sfu/event/events.pb.dart (1)
11-11: Generated protobuf message classes updated for protobuf 6.0.0.This is auto-generated code. The visible change is the lint directive update. Per the AI summary, this regeneration also removes
createRepeated()static factory methods which were deprecated/removed in protobuf 6.x. The message class definitions and field accessors remain intact.packages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbenum.dart (1)
11-11: Generated enum file updated with new lint directives.Auto-generated protobuf enum definitions. The lint directive update is consistent with other generated files in this PR.
packages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbserver.dart (2)
39-40: New SendMetrics RPC method added to SignalServer service.The new
sendMetricsRPC method follows the established pattern of other service methods. This is generated code that adds metrics collection capability to the SFU signal server interface.
62-63: SendMetrics dispatch logic correctly wired.The
createRequestandhandleCallswitch cases forSendMetricsare properly implemented, matching the pattern used by existing RPC methods likeSendStats.Also applies to: 91-92
packages/stream_video/lib/protobuf/video/sfu/models/models.pbjson.dart (1)
1-1277: Generated protobuf descriptors look correct.This is a generated file with properly structured additions:
- New RTP-related type descriptors (RtpBase, InboundRtp, OutboundRtp, RemoteInboundRtp, RemoteOutboundRtp) follow standard protobuf-dart patterns
- ClientCapability enum properly includes the new
CLIENT_CAPABILITY_COORDINATOR_STATSvalue- Lint directive updates are appropriate for generated code
No manual modifications should be made to this file.
packages/stream_video/lib/protobuf/video/sfu/models/models.pbenum.dart (1)
531-559: Generated enum extension is correct.The new
CLIENT_CAPABILITY_COORDINATOR_STATSvalue (2) is properly added with:
- Correct value assignment
- Updated
_byValuelist initialization to reflect the new maximum value (2)- Appropriate documentation comment
No manual modifications should be made to this generated file.
packages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbjson.dart (2)
145-211: Generated SendMetrics descriptors are properly structured.The new
SendMetricsRequestincludes appropriate fields for RTP metrics:
session_idandunified_session_idfor identification- Repeated fields for
inbounds,outbounds,remote_inbounds,remote_outboundsreferencing the new RTP model types
SendMetricsResponseis empty, which is appropriate for a metrics submission endpoint.
713-773: Service registration is complete.The SendMetrics RPC is properly registered in:
SignalServerServiceBase$jsonservice definitionSignalServerServiceBase$messageJsonmessage type map (including all referenced RTP types)This ensures proper reflection and serialization support.
packages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pbtwirp.dart (3)
42-46: Abstract interface properly extended.The
sendMetricsmethod is correctly added to theSignalServerabstract class with appropriate signature.
293-321: JSON client implementation follows established patterns.The
sendMetricsimplementation inSignalServerJSONClientcorrectly:
- Sets context metadata (package, service, method)
- Routes through the interceptor
- Constructs the proper URL endpoint
- Uses
mergeFromProto3Jsonfor response parsing
625-653: Protobuf client implementation follows established patterns.The
sendMetricsimplementation inSignalServerProtobufClientcorrectly:
- Sets context metadata (package, service, method)
- Routes through the interceptor
- Constructs the proper URL endpoint
- Uses
mergeFromBufferfor binary response parsingpackages/stream_video/lib/protobuf/video/sfu/signal_rpc/signal.pb.dart (3)
414-507: SendMetricsRequest class is properly generated.The message class correctly implements:
- Factory constructor with optional parameters for all fields
fromBufferandfromJsonfactory methods for deserializationBuilderInfowith proper field definitions usingpPMfor packed repeated message fields- Getters for repeated fields (
inbounds,outbounds,remoteInbounds,remoteOutbounds) returningPbList<T>- Standard protobuf-dart lifecycle methods (
clone,copyWith,create,getDefault)
509-546: SendMetricsResponse class is properly generated.The empty response message follows standard protobuf-dart patterns with no fields defined, appropriate for a metrics submission endpoint that doesn't require response data.
1960-1963: SignalServerApi properly extended.The
sendMetricsmethod correctly invokes the RPC client with proper service name, method name, and response type.packages/stream_video/lib/protobuf/video/sfu/models/models.pb.dart (3)
11-19: LGTM - Import and ignore list changes are appropriate for generated protobuf code.The added imports for
fixnum(Int64 support) and well-known protobuf types (Struct,Timestamp) are required by the new RTP message classes and existing message types. Theprefer_relative_importsaddition to the ignore list is standard practice for generated protobuf files.
2563-2642: LGTM - RtpBase class follows correct protobuf generation patterns.The base class correctly defines the shared RTP fields (
ssrc,kind,timestampMs) with appropriate protobuf field types and follows standard Dart protobuf conventions.
2647-3110: LGTM - RTP direction classes are correctly generated.The four RTP message classes (
InboundRtp,OutboundRtp,RemoteInboundRtp,RemoteOutboundRtp) follow proper protobuf Dart patterns:
InboundRtpcorrectly uses$fixnum.Int64for packet counters (packetsReceived,packetsLost) which can exceed 32-bit limits in long-running sessions- Field tag numbers have intentional gaps (e.g., 1, 10, 11...) allowing future proto field additions without breaking wire compatibility
- Nested
RtpBasecomposition provides clean separation of shared fields
packages/stream_video_noise_cancellation/lib/stream_video_noise_cancellation.dart
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1152 +/- ##
========================================
- Coverage 6.31% 6.28% -0.03%
========================================
Files 595 592 -3
Lines 41110 41246 +136
========================================
- Hits 2595 2594 -1
- Misses 38515 38652 +137 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/actions/pana/action.yml:
- Around line 51-55: Add the missing dependency override for
stream_video_flutter in the stream_video_noise_cancellation example pubspec
adjustments: update the yq commands that set dependencies/dependency_overrides
for packages/stream_video_noise_cancellation/example/pubspec.yaml to also add
entries for "stream_video_flutter" (matching the pattern used for the push
notification example), ensuring both .dependencies.stream_video_flutter and
.dependency_overrides.stream_video_flutter are set to the appropriate relative
path so pana resolves the noise cancellation example consistently.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/actions/pana/action.ymlpackages/stream_video_push_notification/example/pubspec.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: stream_video_push_notification
- GitHub Check: build
- GitHub Check: analyze
🔇 Additional comments (2)
.github/actions/pana/action.yml (1)
38-43: LGTM!The dependency overrides for the push notification example are complete and follow the correct pattern, including the transitive
stream_video_flutteroverride.packages/stream_video_push_notification/example/pubspec.yaml (1)
1-12: Example pubspec structure follows standard Flutter conventions.The configuration is sound. The hardcoded
stream_video: ^1.2.2version serves as a fallback for standalone use. The specified SDK constraints (Dart 3.8.0 and Flutter 3.32.0) are valid and available on the stable channel.
catinstead ofyqfor dependency manipulationSummary by CodeRabbit
New Features
Documentation
Stability
Chores
✏️ Tip: You can customize this high-level summary in your review settings.