-
Notifications
You must be signed in to change notification settings - Fork 14
TY linting (replaces mypi) #210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughComprehensive refactoring replacing mypy with the ty type checker as the primary type validation tool. Updates CI/CD workflows, development scripts, documentation, and adds type safety improvements throughout the codebase, including validation guards in the video RTC subsystem and explicit type annotations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
getstream/video/rtc/peer_connection.py (1)
78-84: Duplicate variable declarations.
relayed_audioandrelayed_videoare initialized toNonetwice: on lines 78-79 and again on lines 82-83. Remove the duplicate declarations on lines 82-83.🔧 Proposed fix
relayed_audio = None relayed_video = None audio_info = None video_info = None - relayed_audio = None - relayed_video = None track_infos = []
🤖 Fix all issues with AI agents
In @.cursor/worktrees.json:
- Around line 1-5: The worktree setup currently runs "npm install" which is
incorrect for this Python project; update the "setup-worktree" entry in
.cursor/worktrees.json to use the appropriate Python install command (e.g.,
replace the "npm install" array element with "uv install" or "pip install -e .")
so the Cursor IDE worktree setup succeeds; ensure the "setup-worktree" array
only contains valid shell commands for Python environment setup.
In @DEVELOPMENT.md:
- Around line 44-55: The fenced code blocks that show CLI commands (e.g., the
block containing "uv run python dev.py ty" and the block containing "uvx ty
check getstream/ --exclude ...") lack language specifiers; update both opening
fences to use ```bash so the blocks become ```bash and enable proper syntax
highlighting for those command examples.
- Around line 32-36: Update the text "non integration tests" to use the
hyphenated form "non-integration tests" and add a language specifier to the
fenced code block containing `uv run python dev.py` (e.g., change ``` to
```bash) so the snippet is correctly marked as bash.
In @getstream/video/__init__.py:
- Line 43: The code passes an empty string when self.client.base_url is None
which leads to websocket_base_url remaining unset and a later RuntimeError;
before calling get_openai_realtime_client, assert/validate that
self.client.base_url is present and raise a ValueError with a clear message
(e.g. "base_url is required") if it is missing, then pass the validated base_url
into get_openai_realtime_client (refer to the client creation line and the
get_openai_realtime_client call to locate where to add the check).
🧹 Nitpick comments (3)
getstream/video/rtc/tracks.py (1)
299-306: Consider protecting_expected_trackswith the lock for consistency.
_expected_tracksis modified inhandle_track_published(line 215) and accessed/deleted here without lock protection, while_subscribed_track_detailsis consistently protected. Although Python's async model reduces immediate risk, for consistency and safety under concurrent task execution, consider extending lock usage to_expected_tracksoperations.♻️ Suggested fix
In
handle_track_published, wrap the write:if track_type == TrackType.TRACK_TYPE_AUDIO: track_key = f"{track_type}:{event.user_id}:{event.session_id}" - self._expected_tracks[track_key] = event.participant + async with self._lock: + self._expected_tracks[track_key] = event.participantMake
get_next_expected_audio_userasync and protect access:- def get_next_expected_audio_user(self) -> Optional[models_pb2.Participant]: + async def get_next_expected_audio_user(self) -> Optional[models_pb2.Participant]: """Get the next expected user for an incoming audio track.""" - # Find the first audio track expectation - for track_key, participant in list(self._expected_tracks.items()): - if track_key.startswith(f"{TrackType.TRACK_TYPE_AUDIO}:"): - # Remove this expectation since we're using it - del self._expected_tracks[track_key] - return participant - - return None + async with self._lock: + for track_key, participant in list(self._expected_tracks.items()): + if track_key.startswith(f"{TrackType.TRACK_TYPE_AUDIO}:"): + del self._expected_tracks[track_key] + return participant + return None.github/workflows/run_tests.yml (1)
32-61: Type check job looks good, but consider centralizing excludes.The typecheck job is well-structured. However, the exclude list is duplicated between this workflow and
dev.py(per the AI summary). Consider one of these approaches to avoid drift:
- Read excludes from a shared config file
- Use
dev.py typecheckin CI instead of duplicating the commandThis is optional since maintaining them in two places is acceptable for now.
getstream/base.py (1)
362-393: Consider whetherasyncio.to_threadis necessary for JSON operations.Using
asyncio.to_threadforjson.dumps(line 364) and_parse_response(lines 391-393) offloads CPU-bound work to a thread pool. However, for small JSON payloads typical in API calls, this adds overhead. This is a reasonable choice for large payloads but may be overkill for typical request sizes.
📜 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 ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.cursor/worktrees.json.github/workflows/run_tests.ymlDEVELOPMENT.mddev.pygetstream/base.pygetstream/common/telemetry.pygetstream/stream_response.pygetstream/utils/__init__.pygetstream/video/__init__.pygetstream/video/openai.pygetstream/video/rtc/connection_manager.pygetstream/video/rtc/connection_utils.pygetstream/video/rtc/coordinator/ws.pygetstream/video/rtc/network_monitor.pygetstream/video/rtc/peer_connection.pygetstream/video/rtc/recording.pygetstream/video/rtc/signaling.pygetstream/video/rtc/track_util.pygetstream/video/rtc/tracks.pyprotocolpyproject.tomltests/test_video_openai.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Plugins that work with audio, video, or WebRTC functionality should depend on
getstream[webrtc]instead of justgetstreamto access WebRTC-related dependencies likeaiortc,numpy,torch,torchaudio,soundfile,scipy,deepgram-sdk, andelevenlabs
Files:
getstream/utils/__init__.pygetstream/video/rtc/connection_utils.pygetstream/video/__init__.pygetstream/video/rtc/recording.pygetstream/video/rtc/connection_manager.pygetstream/video/rtc/signaling.pytests/test_video_openai.pygetstream/video/rtc/tracks.pygetstream/video/openai.pygetstream/video/rtc/peer_connection.pygetstream/video/rtc/coordinator/ws.pygetstream/stream_response.pygetstream/video/rtc/network_monitor.pygetstream/video/rtc/track_util.pydev.pygetstream/common/telemetry.pygetstream/base.py
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/test_*.py: Use fixtures to inject objects in tests; test client fixtures can use the Stream API client
Load credentials from .env file; the client fixture will load credentials automatically
Keep tests well organized and use test classes to group similar tests
Tests that rely on file assets should use files fromtests/assets/folder; add new files there and reuse existing ones when possible. Do not use files larger than 256 kilobytes
Do not use mocks or mock objects in tests unless directly requested
Files:
tests/test_video_openai.py
🧠 Learnings (4)
📚 Learning: 2025-12-02T12:16:57.243Z
Learnt from: CR
Repo: GetStream/stream-py PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T12:16:57.243Z
Learning: Use pytest for testing; run tests with `uv run pytest`
Applied to files:
DEVELOPMENT.mddev.py
📚 Learning: 2025-12-02T12:16:57.243Z
Learnt from: CR
Repo: GetStream/stream-py PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T12:16:57.243Z
Learning: Always run tests using `uv run pytest` from the root of the project; do not change directories to run tests
Applied to files:
DEVELOPMENT.md
📚 Learning: 2025-12-10T13:41:16.476Z
Learnt from: dangusev
Repo: GetStream/stream-py PR: 203
File: getstream/video/rtc/connection_manager.py:537-575
Timestamp: 2025-12-10T13:41:16.476Z
Learning: The `republish_tracks` method in `ConnectionManager` class in `getstream/video/rtc/connection_manager.py` is part of the public API and is intended to be called by users, not invoked internally by the SDK.
Applied to files:
getstream/video/rtc/peer_connection.py
📚 Learning: 2025-12-02T12:16:57.243Z
Learnt from: CR
Repo: GetStream/stream-py PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T12:16:57.243Z
Learning: Applies to **/*.py : Plugins that work with audio, video, or WebRTC functionality should depend on `getstream[webrtc]` instead of just `getstream` to access WebRTC-related dependencies like `aiortc`, `numpy`, `torch`, `torchaudio`, `soundfile`, `scipy`, `deepgram-sdk`, and `elevenlabs`
Applied to files:
pyproject.toml
🧬 Code graph analysis (5)
getstream/video/rtc/connection_utils.py (1)
getstream/video/rtc/pb/stream/video/sfu/event/events_pb2.pyi (1)
reconnect_details(730-730)
getstream/video/__init__.py (1)
getstream/video/openai.py (1)
get_openai_realtime_client(82-99)
getstream/video/rtc/connection_manager.py (2)
getstream/video/rtc/pb/stream/video/sfu/event/events_pb2.pyi (1)
join_response(101-104)getstream/stream_response.py (1)
data(22-24)
getstream/video/rtc/peer_connection.py (2)
getstream/video/rtc/connection_manager.py (2)
publisher_pc(493-494)publisher_pc(497-498)getstream/video/rtc/track_util.py (1)
id(2161-2162)
getstream/base.py (2)
getstream/stream_response.py (2)
data(22-24)headers(30-32)getstream/common/telemetry.py (1)
current_operation(214-216)
🪛 GitHub Actions: CI (unit)
getstream/base.py
[error] 42-42: StreamAPIException: GetOrCreateCall failed with error: """
[error] 42-42: StreamAPIException: UpdateCall failed with error: """
🪛 LanguageTool
DEVELOPMENT.md
[grammar] ~32-~32: Use a hyphen to join words.
Context: ...rtcut to ruff, ty (type checker) and non integration tests: ``` uv run python de...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
DEVELOPMENT.md
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (4)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (39)
getstream/video/rtc/tracks.py (1)
79-79: LGTM!The explicit type annotation for
_lockis a clean improvement that enhances type checker compatibility without changing behavior.protocol (1)
1-1: Verify the newly added protocol submodule aligns with the PR objectives.The protocol submodule is being added for the first time in this commit. Please verify that the submodule content aligns with the PR's objectives and does not introduce unexpected dependencies. The commit message "cleanup" does not provide context about why the protocol submodule was added or what it contains.
getstream/video/rtc/recording.py (1)
279-279: Type ignore is appropriate for this dynamic frame handling.The
track.recv()returns a generic frame type, but the code handles this safely by checking for theto_ndarraymethod at line 375 before processing. The# type: ignore[arg-type]correctly silences the false positive.getstream/video/rtc/connection_utils.py (1)
403-407: Type ignore is appropriate for protobuf field access.Protobuf-generated message classes have dynamically defined attributes that type checkers cannot fully introspect. The
# type: ignore[attr-defined]correctly silences this false positive for themigrating_fromfield assignment onReconnectDetails.getstream/utils/__init__.py (1)
79-85: Cast usage is correct given the type signature constraints.The
cast(datetime, ...)accurately reflects that when iterating over dict/list values, the type signature guarantees those values areint | float | str(not nested structures), so the recursive calls will returndatetime. The comments clarify this reasoning well.getstream/video/openai.py (3)
173-185: Type ignores are appropriate for monkey-patching patterns.The
# type: ignore[attr-defined]for adding_original_recvand# type: ignore[method-assign]for replacingrecvare the correct suppressions for this intentional monkey-patching. The added comments improve code clarity.
216-226: Consistent type suppression for URL preparation patching.Same appropriate pattern as above for storing the original method and replacing it dynamically.
252-258: Instance and class-level patching correctly suppressed.The instance-level patching in
patched_aenter(lines 252-253) and the class-level__aenter__replacement (line 258) use appropriate type ignores for this dynamic behavior.getstream/video/rtc/signaling.py (1)
106-112: LGTM! Defensive guard for type safety.The early return guards against calling
run_forever()on aNonewebsocket, which satisfies the type checker and provides runtime safety. Sinceself.wsis typed asOptional[websocket.WebSocketApp](initialized toNoneat line 45), this check is necessary before invoking methods on it.pyproject.toml (1)
74-74: Good documentation for retained dependency.The comment clarifying that
mypy-protobufis only forgenerate_webrtc.shprotobuf type stubs is helpful. This clearly distinguishes it from runtime type checking (now handled by ty).getstream/video/rtc/network_monitor.py (1)
8-8: LGTM! Explicit optional typing.The explicit
Optional[List[str]]annotation forconnectivity_hostsis a good improvement for type safety. The default behavior remains unchanged with the fallback to predefined hosts whenNoneis passed.Also applies to: 21-28
getstream/stream_response.py (1)
30-32: Good defensive copy, note the subtle behavioral change.Returning
dict(self.__headers)instead of the rawhttpx.Headersobject is a solid improvement:
- Prevents callers from mutating internal state
- Satisfies the
Dict[str, Any]return type annotationNote:
httpx.Headersis case-insensitive whiledictis not. If any callers relied on case-insensitive header access (e.g.,headers()["Content-Type"]matchingheaders()["content-type"]), they would now need exact case matching. Existing tests pass, suggesting this is not an issue in the test suite, but worth monitoring if external callers access headers case-insensitively.getstream/common/telemetry.py (2)
179-179: LGTM - Type ignore cleanup aligns with ty migration.The removal of
type: ignorecomments from OpenTelemetry span method calls (start_as_current_span,set_attributes,add_event,record_exception,set_status) is appropriate as thetytype checker handles these correctly without suppression.Also applies to: 193-193, 200-200, 207-208
375-375: Appropriate type ignore retained.The
type: ignore[arg-type]onotel_context.detach(token)is correctly retained. The token returned byotel_context.attach()has a type that doesn't perfectly match thedetach()parameter type in OpenTelemetry's type stubs, making this suppression necessary.tests/test_video_openai.py (1)
303-309: LGTM - Type narrowing pattern is correct.The assertion after
pytest.skipis a standard pattern for helping the type checker understand thatopenai_api_keycannot beNoneat this point. While redundant at runtime (sincepytest.skipraises an exception), it's necessary for static type analysis with ty.getstream/video/rtc/connection_manager.py (7)
87-89: LGTM - Public attribute aliases.Direct assignments creating public aliases for the private managers. This improves API discoverability while maintaining internal encapsulation.
188-189: Good defensive guard before signaling operation.Adding a runtime check before calling
SendAnswerensures the signaling client is properly initialized. This prevents a confusingAttributeErroronNoneand provides type narrowing for the type checker.
230-233: Good pre-condition validation for watch_call.These guards ensure
user_idandclient_idare set before callingwatch_call, preventing runtime errors and providing clear error messages. This also enables type narrowing for downstream usage.
278-279: Good pre-condition validation for join_call.Ensures
user_idis notNonebefore proceeding with the join operation. The guard is correctly placed inside theif not (ws_url or token)block whereuser_idis required.
288-290: LGTM - Correct use of.dataaccessor.The change from
join_responsetojoin_response.dataaligns with theStreamResponse.dataaccessor pattern shown in the relevant code snippets, ensuring proper access to the encapsulated response data.
302-303: Good guard before WebSocket connection.Validates that both
tokenandws_urlare set before attempting the WebSocket connection. This prevents cryptic errors from the underlying WebSocket library.
358-360: Good guard before creating SFU signaling client.Ensures
join_responseis set before accessingcredentials.server.url. This is necessary becausejoin_responseis only assigned within the conditional block on lines 277-294.getstream/video/__init__.py (1)
24-25: LGTM - Explicit Optional type hints.Making the optional nature of
call_idandcustom_dataexplicit in the type hints improves code clarity and aligns with Python typing best practices.dev.py (4)
16-40: Return type and error handling improvements look good.The
run()function now correctly returnssubprocess.CompletedProcess[str]and handles the error case by returning a validCompletedProcessobject whencheck=False. This is cleaner than returning the exception object.
43-48: Default behavior change approved.Running
checkby default when no subcommand is provided is a sensible developer experience improvement.
81-121: Type checking exclusion list and commands are well-structured.The
TY_EXCLUDESlist appropriately excludes generated code and files that may have complex typing. The_run_typecheck()helper centralizes the logic, and bothtypecheck()andty_alias()provide convenient access.
146-158: Bothgenerate.shandgenerate_webrtc.shexist at the repository root with proper execute permissions. The shell script paths in theregen()command are correct.getstream/base.py (3)
49-56: Dynamicfrom_dictinvocation is a good defensive pattern.Using
getattr(data_type, "from_dict")followed by explicit call ensures compatibility with various data types that may implementfrom_dictdifferently.
144-149: Empty string fallback forbase_urlpreventsNoneissues.Using
self.base_url or ""ensures a valid string is always passed tohttpx.Client, avoiding potential type errors.
326-334: Async context manager methods are now correctly async.The
__aenter__and__aexit__methods should beasyncfor proper async context manager behavior. Callingawait self.aclose()in__aexit__ensures proper cleanup.getstream/video/rtc/peer_connection.py (2)
102-107: Guard conditions for relayed tracks are a good addition.The added checks
if audio and relayed_audioandif video and relayed_videoensure tracks are only added to the peer connection when both the original and relayed tracks exist. This prevents potentialNonereference errors.
126-145: SDP matching guards improve robustness.Adding
relayed_audioandrelayed_videochecks to the SDP matching conditions ensures that track info is only appended when the corresponding relayed track exists, preventing potential issues during negotiation.getstream/video/rtc/coordinator/ws.py (3)
12-18: Proper use ofTYPE_CHECKINGfor forward references.Using
TYPE_CHECKINGto conditionally importWebSocketClientProtocolavoids runtime import overhead while enabling proper type hints.
111-118: Validation guard for requireduser_detailsis appropriate.When
user_tokenis not provided,user_detailsis required to generate a token. RaisingValueErrorwith a clear message helps catch configuration errors early.
203-209: Guard forcall.idprevents downstream errors.Raising
ValueErrorwhencall.idisNoneensures the path parameters are valid before constructing the reconnection URL. Thetype: ignorecomment is acceptable for the nested client header access where the type system can't fully track the attribute chain.getstream/video/rtc/track_util.py (4)
118-145: Improved type annotations forPcmData.__init__.Making
samplesparameterOptional[NDArray]with proper default handling and adding explicit type annotation forself.formatimproves type checker compatibility. The constructor properly handlesNonesamples by creating an empty array with the correct dtype.
591-611: Cleaner encoding string normalization.Extracting
encoding_strand normalizing it early makes the subsequent comparisons cleaner and more consistent. The logic correctly handles bothG711Encodingenum values and string literals.
865-888:type: ignoreannotations for PyAV codec context.PyAV's type stubs are incomplete for audio codec context attributes. The
type: ignoreannotations are appropriate here to suppress false positives while maintaining runtime correctness. The comments document the reason.
2077-2080: Guard preventsAttributeErrorwhen ICE parameters are missing.Adding the check
if reference_ice and media.iceensures ICE parameter updates only occur when both the reference and target media have ICE configuration. This prevents potentialAttributeErrorwhen either isNone.
| { | ||
| "setup-worktree": [ | ||
| "npm install" | ||
| ] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for package.json and Node.js indicators
fd -t f "package.json" --max-depth 3
# Check for Python project indicators
fd -t f -e "py|toml|txt" "(setup|pyproject|requirements)" --max-depth 2
# List key files in root to understand project type
git ls-files | head -20Repository: GetStream/stream-py
Length of output: 469
🏁 Script executed:
#!/bin/bash
# Check for npm references in the repository
rg -i "npm\s+(install|run)" --max-count=10
# Check workflows for any Node.js setup
rg -i "node|npm" .github/workflows/ --max-count=10
# Verify no package.json exists anywhere
find . -name "package.json" 2>/dev/null
# Check pre-commit config for Node.js hooks
cat .pre-commit-config.yaml 2>/dev/null | head -30Repository: GetStream/stream-py
Length of output: 453
Replace npm install with appropriate Python setup command.
This is a Python project with no Node.js dependencies. The worktree setup should use uv install or pip install -e . instead. Currently, npm install will fail since there's no package.json in the repository, breaking the Cursor IDE worktree setup for this project.
🤖 Prompt for AI Agents
In @.cursor/worktrees.json around lines 1 - 5, The worktree setup currently runs
"npm install" which is incorrect for this Python project; update the
"setup-worktree" entry in .cursor/worktrees.json to use the appropriate Python
install command (e.g., replace the "npm install" array element with "uv install"
or "pip install -e .") so the Cursor IDE worktree setup succeeds; ensure the
"setup-worktree" array only contains valid shell commands for Python environment
setup.
| Shortcut to ruff, ty (type checker) and non integration tests: | ||
|
|
||
| ``` | ||
| uv run python dev.py check | ||
| uv run python dev.py | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammar and markdown lint issues.
- "non integration tests" should be hyphenated as "non-integration tests"
- The code block on line 34 is missing a language specifier
Suggested fix
-Shortcut to ruff, ty (type checker) and non integration tests:
+Shortcut to ruff, ty (type checker) and non-integration tests:
-```
+```bash
uv run python dev.py</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
Shortcut to ruff, ty (type checker) and non-integration tests:
🧰 Tools
🪛 LanguageTool
[grammar] ~32-~32: Use a hyphen to join words.
Context: ...rtcut to ruff, ty (type checker) and non integration tests: ``` uv run python de...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @DEVELOPMENT.md around lines 32 - 36, Update the text "non integration tests"
to use the hyphenated form "non-integration tests" and add a language specifier
to the fenced code block containing `uv run python dev.py` (e.g., change ``` to
```bash) so the snippet is correctly marked as bash.
| ### Type checking (ty) | ||
|
|
||
| Type checking is run via the `ty` type checker, excluding generated code: | ||
|
|
||
| ``` | ||
| uv run python dev.py ty | ||
| ``` | ||
|
|
||
| Or manually (note: requires exclude flags for generated code - see dev.py for the full list): | ||
| ``` | ||
| uv run mypy --install-types --non-interactive --exclude 'getstream/models/.*' . | ||
| uvx ty check getstream/ --exclude "getstream/models/" --exclude "getstream/video/rtc/pb/" ... | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language specifiers to code blocks.
Static analysis indicates the code blocks are missing language specifiers for proper syntax highlighting.
Suggested fix
### Type checking (ty)
Type checking is run via the `ty` type checker, excluding generated code:
-```
+```bash
uv run python dev.py tyOr manually (note: requires exclude flags for generated code - see dev.py for the full list):
- +bash
uvx ty check getstream/ --exclude "getstream/models/" --exclude "getstream/video/rtc/pb/" ...
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @DEVELOPMENT.md around lines 44 - 55, The fenced code blocks that show CLI
commands (e.g., the block containing "uv run python dev.py ty" and the block
containing "uvx ty check getstream/ --exclude ...") lack language specifiers;
update both opening fences to use ```bash so the blocks become ```bash and
enable proper syntax highlighting for those command examples.
| from .openai import get_openai_realtime_client, ConnectionManagerWrapper | ||
|
|
||
| client = get_openai_realtime_client(openai_api_key, self.client.base_url) | ||
| client = get_openai_realtime_client(openai_api_key, self.client.base_url or "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Check base.py for base_url initialization
rg -n "base_url" getstream/base.py -A 2 -B 2 | head -100Repository: GetStream/stream-py
Length of output: 1674
🏁 Script executed:
# Check the context around line 43 in video/__init__.py
cat -n getstream/video/__init__.py | sed -n '30,60p'Repository: GetStream/stream-py
Length of output: 1541
🏁 Script executed:
# Check get_openai_realtime_client function
rg -n "def get_openai_realtime_client" getstream/ -A 15Repository: GetStream/stream-py
Length of output: 1100
🏁 Script executed:
# Check if there are any imports of WebRTC-related dependencies
rg -n "aiortc|numpy|torch|torchaudio|soundfile|scipy|deepgram|elevenlabs" getstream/video/Repository: GetStream/stream-py
Length of output: 9254
🏁 Script executed:
# Check what happens when websocket_base_url is not set
rg -n "websocket_base_url" getstream/video/openai.py -A 3 -B 3Repository: GetStream/stream-py
Length of output: 898
🏁 Script executed:
# Check setup.py or pyproject.toml for WebRTC dependencies
fd -e "setup.py" -o -e "pyproject.toml" -o -e "setup.cfg" | xargs catRepository: GetStream/stream-py
Length of output: 179
🏁 Script executed:
# Find and read dependency files
find . -maxdepth 2 -name "pyproject.toml" -o -name "setup.py" -o -name "setup.cfg" | head -5Repository: GetStream/stream-py
Length of output: 79
🏁 Script executed:
# Read pyproject.toml
cat pyproject.toml 2>/dev/null || echo "File not found"Repository: GetStream/stream-py
Length of output: 3307
🏁 Script executed:
# Read setup.py
cat setup.py 2>/dev/null | head -150Repository: GetStream/stream-py
Length of output: 45
Fail explicitly if base_url is missing.
The code currently passes an empty string when base_url is None, which doesn't match the http:// or https:// prefix checks, leaving websocket_base_url unset. This causes a RuntimeError later in patched_connect when it tries to access the undefined attribute.
Raise an explicit ValueError instead:
Suggested fix
def connect_openai(
self, openai_api_key, agent_user_id, model="gpt-4o-realtime-preview"
):
from .openai import get_openai_realtime_client, ConnectionManagerWrapper
- client = get_openai_realtime_client(openai_api_key, self.client.base_url or "")
+ if not self.client.base_url:
+ raise ValueError("client.base_url must be set to use connect_openai")
+ client = get_openai_realtime_client(openai_api_key, self.client.base_url)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client = get_openai_realtime_client(openai_api_key, self.client.base_url or "") | |
| def connect_openai( | |
| self, openai_api_key, agent_user_id, model="gpt-4o-realtime-preview" | |
| ): | |
| from .openai import get_openai_realtime_client, ConnectionManagerWrapper | |
| if not self.client.base_url: | |
| raise ValueError("client.base_url must be set to use connect_openai") | |
| client = get_openai_realtime_client(openai_api_key, self.client.base_url) |
🤖 Prompt for AI Agents
In @getstream/video/__init__.py at line 43, The code passes an empty string when
self.client.base_url is None which leads to websocket_base_url remaining unset
and a later RuntimeError; before calling get_openai_realtime_client,
assert/validate that self.client.base_url is present and raise a ValueError with
a clear message (e.g. "base_url is required") if it is missing, then pass the
validated base_url into get_openai_realtime_client (refer to the client creation
line and the get_openai_realtime_client call to locate where to add the check).
TY linting (replaces mypi)
Summary by CodeRabbit
Bug Fixes
Documentation
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.