Feat: Add Decart restyling processor #202
Conversation
WalkthroughIntroduces a new Decart plugin for vision-agents that enables live video style restyling through a realtime API integration. Includes the RestylingProcessor class, DecartVideoTrack handler, example agent setup, tests, and workspace configuration updates. Adds a frame resize utility to core video utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent
participant RestylingProcessor
participant DecartAPI as Decart Realtime API
participant DecartVideoTrack
participant Publisher
Agent->>RestylingProcessor: process_video(incoming_track)
RestylingProcessor->>DecartAPI: connect (first call only)
RestylingProcessor->>DecartAPI: send initial_state(prompt, mirror)
loop Processing Loop
RestylingProcessor->>DecartAPI: recv transformed_frames
DecartAPI-->>RestylingProcessor: styled frames
RestylingProcessor->>DecartVideoTrack: add_frame(restyled_frame)
end
Agent->>RestylingProcessor: update_prompt(new_prompt)
RestylingProcessor->>DecartAPI: update prompt
Agent->>RestylingProcessor: publish_video_track()
RestylingProcessor-->>Publisher: VideoStreamTrack
Publisher->>DecartVideoTrack: recv()
DecartVideoTrack-->>Publisher: av.VideoFrame @ 30 FPS
sequenceDiagram
participant CLI
participant AgentLauncher
participant createAgent as create_agent()
participant Agent
participant LLM
participant joinCall as join_call()
CLI->>AgentLauncher: launch
AgentLauncher->>createAgent: call
createAgent->>Agent: new Agent(RestylingProcessor, LLM, STT/TTS)
createAgent->>LLM: register_callable(change_prompt)
createAgent-->>AgentLauncher: Agent
AgentLauncher->>joinCall: call (call_type, call_id)
joinCall->>Agent: create User
joinCall->>Agent: create Call
joinCall->>Agent: join Call
joinCall->>Agent: wait for call end
joinCall->>Agent: finish()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![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 (6)
plugins/decart/README.md (1)
1-4: Consider expanding README with basic usage detailsRight now this is a minimal stub; consider adding a short “Getting started” section (required env vars, how to enable the plugin, and a link to the example) so users can discover the new processor more easily.
plugins/decart/tests/test_decart_plugin.py (1)
1-16: Clarify async test execution and make the examples exercise real behavior
test_regularandtest_simplecurrently justassert True, so they don’t validate anything about the Decart plugin. Either flesh them out (even simple smoke checks on import/wiring) or drop them to avoid a false sense of coverage.test_simpleisasync defwith only@pytest.mark.integration. Unless you’ve configuredpytest-asynciofor automatic coroutine handling (asyncio_mode = "auto"or similar), this coroutine might not be awaited, and its body may never run.Consider either making
test_simplea synchronous test for now, or adding the appropriate asyncio marker/configuration so it’s actually executed as async.plugins/decart/tests/test_decart_restyling.py (1)
10-35: Tighten tests and align comments with actual behavior
test_regularonly doesassert Truewhile the comment claims it will fail ifDECART_API_KEYis not set. Either wire it to actually exerciseRestylingProcessor(or skip when the key is missing), or relax the comment so it matches reality.test_processor_initializationis declaredasync defwith@pytest.mark.integration, but the body is fully synchronous. It could just be a regulardef, or—if you plan to await async APIs later—ensure your pytest/pytest-asyncio configuration is set up so this coroutine is actually awaited (or mark it with@pytest.mark.asyncioas appropriate).- The skip logic on
ValueErroris fine for now, but if you ever change the error message, matching on"API key"may become brittle; checking forDECART_API_KEYin the message or using a dedicated exception type would be more robust.None of these are blockers, but cleaning them up will make the tests more truthful and maintainable.
plugins/decart/example/decart_example.py (1)
21-67: Double-checkjoincontext-manager usage and minor example polish
- In
join_call(Lines [50-64]), you’re doingwith await agent.join(call):inside an async function. This is correct only ifAgent.joinis an async function that returns a synchronous context manager. If insteadAgent.joinis implemented as an async context manager (common pattern), this should beasync with agent.join(call):instead. Worth confirming against theAgent.joinsignature to avoid subtle lifecycle bugs.create_agent(**kwargs)(Line [21]) doesn’t usekwargs. If the launcher never passes anything through, you could drop**kwargsfor clarity, or forward them intoAgent/components if that’s the intent.The rest of the example wiring (processor, LLM, TTS/STT, and
change_promptregistration) reads clearly for showcasing the plugin.plugins/decart/vision_agents/plugins/decart/restyling.py (2)
165-210: Connection / reconnection loop looks reasonable, with minor nit on duplicate reconnect pathsThe combination of
_on_errorscheduling_connect_to_decartand_processing_loopperiodically attempting reconnects whennot self._connected and not self._connectingis a reasonable resilience strategy. The_connectingflag should prevent concurrent connection attempts.If you ever see overlapping reconnect attempts in logs, consider centralizing reconnection into
_processing_loopand having_on_erroronly flip flags, but this is optional and not a blocker.Also applies to: 245-260
269-290: Close/cleanup logic is pragmatic; be aware ofget_event_loopbehaviorThe
close()implementation that cancels internal tasks and then either schedules_async_close()on a running loop orrun_until_complete()on a non‑running loop is pragmatic and should work in the typical agent lifecycle.Longer‑term, you might want to:
- Use
asyncio.get_running_loop()inside branches where you know you’re in an async context.- Expose an explicit async
aclose()API that callers canawait, to avoid synchronousrun_until_completecalls from arbitrary threads.Not required for this PR but worth considering as the processor gets more usage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (10)
plugins/decart/README.md(1 hunks)plugins/decart/example/decart_example.py(1 hunks)plugins/decart/example/pyproject.toml(1 hunks)plugins/decart/pyproject.toml(1 hunks)plugins/decart/tests/test_decart_plugin.py(1 hunks)plugins/decart/tests/test_decart_restyling.py(1 hunks)plugins/decart/vision_agents/plugins/decart/__init__.py(1 hunks)plugins/decart/vision_agents/plugins/decart/decart_video_track.py(1 hunks)plugins/decart/vision_agents/plugins/decart/restyling.py(1 hunks)pyproject.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
plugins/decart/vision_agents/plugins/decart/restyling.py (2)
agents-core/vision_agents/core/processors/base_processor.py (3)
AudioVideoProcessor(117-146)VideoProcessorMixin(66-73)VideoPublisherMixin(84-86)plugins/decart/vision_agents/plugins/decart/decart_video_track.py (4)
DecartVideoTrack(14-104)recv(80-100)add_frame(44-49)stop(102-104)
plugins/decart/example/decart_example.py (4)
agents-core/vision_agents/core/agents/agents.py (3)
Agent(74-1316)create_call(774-779)finish(588-621)agents-core/vision_agents/core/agents/agent_launcher.py (1)
AgentLauncher(18-125)plugins/decart/vision_agents/plugins/decart/restyling.py (1)
set_prompt(146-154)agents-core/vision_agents/core/llm/llm.py (2)
LLM(49-418)register_function(212-225)
plugins/decart/vision_agents/plugins/decart/decart_video_track.py (1)
agents-core/vision_agents/core/utils/video_queue.py (2)
VideoLatestNQueue(6-28)put_latest_nowait(22-28)
plugins/decart/vision_agents/plugins/decart/__init__.py (1)
plugins/decart/vision_agents/plugins/decart/restyling.py (1)
RestylingProcessor(41-295)
plugins/decart/tests/test_decart_restyling.py (1)
plugins/decart/vision_agents/plugins/decart/restyling.py (1)
RestylingProcessor(41-295)
🪛 GitHub Actions: CI (unit)
plugins/decart/vision_agents/plugins/decart/restyling.py
[error] 109-109: Mypy type error: Argument 1 to 'realtime' of 'Models' has incompatible type 'str'; expected Literal['mirage', 'mirage_v2', 'lucy_v2v_720p_rt'].
[error] 227-227: Mypy type error: Argument 1 to 'add_frame' of 'DecartVideoTrack' has incompatible type 'Frame | Packet'; expected 'VideoFrame'.
⏰ 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). (3)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Mypy
🔇 Additional comments (7)
pyproject.toml (1)
28-28: Decart workspace wiring looks consistentThe new
vision-agents-plugins-decartsource and"plugins/decart"workspace member align with how other plugins are registered; I don’t see issues here.Also applies to: 59-60
plugins/decart/pyproject.toml (1)
1-41: Decart plugin packaging looks reasonableBuild-system, project metadata, dependency-groups, and Hatch/UV configuration look consistent for a standalone plugin package; no blocking issues from my side.
plugins/decart/vision_agents/plugins/decart/__init__.py (1)
1-3: Public export ofRestylingProcessorlooks goodImporting
RestylingProcessorhere and exposing it via__all__matches how the tests and example import the processor; this is a clean export point.plugins/decart/vision_agents/plugins/decart/decart_video_track.py (1)
14-103: Overall DecartVideoTrack design looks solidThe track’s placeholder frame, latest‑N queue with capacity 2, and ~30 FPS
recv()loop with last‑frame fallback form a clean and predictable WebRTC source. The_resize_framehelper is well‑factored for potential future use.plugins/decart/vision_agents/plugins/decart/restyling.py (3)
221-228: Frame receive loop is fine; mypy error here is addressed viaDecartVideoTrack.add_frameThe
while not self._video_track._stoppedloop that callsframe = await transformed_stream.recv()and forwards toself._video_track.add_frame(frame)is structurally sound. The mypy error at Line 227 (Frame | Packet→VideoFrame) is best handled by relaxing the parameter type ofDecartVideoTrack.add_frameand guarding forav.VideoFramethere (see comment indecart_video_track.py).After updating
DecartVideoTrack.add_frameas suggested, please re‑run mypy to confirm that the CI error reported for Line 227 here disappears.
41-145: RestylingProcessor overall structure integrates cleanly with the existing processor mixinsThe processor wires Decart Realtime into the existing
AudioVideoProcessor/VideoProcessorMixin/VideoPublisherMixinstack in a clear way:process_videohandles incoming tracks,publish_video_trackexposes the transformed track, and prompt/mirror setters guard on connection state. Logging around connection lifecycle is also helpful for debugging.
5-5: Verify the proposed Literal type alias resolves the mypy error at Line 109The proposed fix is syntactically sound and follows best practices. The code currently passes
self.model_name(a plainstrfrom the constructor parameter) tomodels.realtime(), and the fix correctly narrows this to aLiteraltype alias. The model names you've included match the documented supported models.However, I cannot independently confirm the mypy error at Line 109 exists—I lack access to the Decart SDK type stubs to verify what
models.realtime()actually expects. The fix itself is correct and will resolve the issue if the SDK stubs definemodels.realtime()to accept aLiteraltype.Before merging, verify:
- Run
mypy plugins/decart/vision_agents/plugins/decart/restyling.pylocally to confirm the error is resolved after applying this change- If the SDK adds new models, update
DecartRealtimeModelaccordingly
| dependencies = [ | ||
| "vision-agents", | ||
| "python-dotenv", | ||
| "vision-agents-plugins-openai", | ||
| "vision-agents-plugins-decart", | ||
| "vision-agents-plugins-elvenlabs", | ||
| "vision-agents-plugins-getstream", | ||
| "vision-agents-plugins-deepgram", | ||
|
|
||
| ] | ||
|
|
||
| [tool.uv.sources] | ||
| vision-agents = { workspace = true } | ||
| vision-agents-plugins-getstream = { editable=true } | ||
| vision-agents-plugins-openai = { editable=true } | ||
| vision-agents-plugins-elevenlabs = { editable=true } | ||
| vision-agents-plugins-deepgram = { editable=true } | ||
| vision-agents-plugins-decart = { editable=true } |
There was a problem hiding this comment.
🧩 Analysis chain
Fix ElevenLabs plugin dependency name typo
Line [11] declares:
vision-agents-plugins-elvenlabs
but [tool.uv.sources] (Line [21]) and the rest of the repo use vision-agents-plugins-elevenlabs. This mismatch will prevent the example project from resolving the dependency correctly.
Recommend updating the dependency name:
dependencies = [
"vision-agents",
"python-dotenv",
"vision-agents-plugins-openai",
"vision-agents-plugins-decart",
- "vision-agents-plugins-elvenlabs",
+ "vision-agents-plugins-elevenlabs",
"vision-agents-plugins-getstream",
"vision-agents-plugins-deepgram",
]I'll verify this dependency name mismatch by searching the repository for package definitions and other references.
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of elevenlabs or elvenlabs (case-insensitive)
echo "=== Searching for 'elevenlabs' references ==="
rg -i "elevenlabs" -n --type toml --type py --type txt
echo ""
echo "=== Searching for 'elvenlabs' references ==="
rg -i "elvenlabs" -n --type toml --type py --type txt
echo ""
echo "=== Looking for elevenlabs package directory ==="
fd -i "elevenlabs|elvenlabs" -t d
echo ""
echo "=== Checking pyproject.toml files for package name patterns ==="
fd "pyproject.toml" -x grep -H "vision-agents-plugins-elven" {}Length of output: 14667
Fix ElevenLabs plugin dependency name typo
Line 11 declares "vision-agents-plugins-elvenlabs" but [tool.uv.sources] (Line 21) and the rest of the repository use vision-agents-plugins-elevenlabs. This mismatch will prevent the example project from resolving the dependency correctly.
Recommend updating the dependency name:
dependencies = [
"vision-agents",
"python-dotenv",
"vision-agents-plugins-openai",
"vision-agents-plugins-decart",
- "vision-agents-plugins-elvenlabs",
+ "vision-agents-plugins-elevenlabs",
"vision-agents-plugins-getstream",
"vision-agents-plugins-deepgram",
]📝 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.
| dependencies = [ | |
| "vision-agents", | |
| "python-dotenv", | |
| "vision-agents-plugins-openai", | |
| "vision-agents-plugins-decart", | |
| "vision-agents-plugins-elvenlabs", | |
| "vision-agents-plugins-getstream", | |
| "vision-agents-plugins-deepgram", | |
| ] | |
| [tool.uv.sources] | |
| vision-agents = { workspace = true } | |
| vision-agents-plugins-getstream = { editable=true } | |
| vision-agents-plugins-openai = { editable=true } | |
| vision-agents-plugins-elevenlabs = { editable=true } | |
| vision-agents-plugins-deepgram = { editable=true } | |
| vision-agents-plugins-decart = { editable=true } | |
| dependencies = [ | |
| "vision-agents", | |
| "python-dotenv", | |
| "vision-agents-plugins-openai", | |
| "vision-agents-plugins-decart", | |
| "vision-agents-plugins-elevenlabs", | |
| "vision-agents-plugins-getstream", | |
| "vision-agents-plugins-deepgram", | |
| ] | |
| [tool.uv.sources] | |
| vision-agents = { workspace = true } | |
| vision-agents-plugins-getstream = { editable=true } | |
| vision-agents-plugins-openai = { editable=true } | |
| vision-agents-plugins-elevenlabs = { editable=true } | |
| vision-agents-plugins-deepgram = { editable=true } | |
| vision-agents-plugins-decart = { editable=true } |
🤖 Prompt for AI Agents
plugins/decart/example/pyproject.toml lines 6-23: the dependency list contains a
typo "vision-agents-plugins-elvenlabs" (line 11) which doesn't match the correct
package name used elsewhere ("vision-agents-plugins-elevenlabs"); update the
dependency entry to "vision-agents-plugins-elevenlabs" so it matches the
[tool.uv.sources] and the rest of the repo, and ensure there are no other
mismatched names or duplicate entries in the dependencies array.
| @@ -0,0 +1,104 @@ | |||
| import asyncio | |||
| import logging | |||
| from typing import Optional | |||
There was a problem hiding this comment.
🧩 Analysis chain
Broaden add_frame parameter type to match aiortc/av typings and fix mypy error
CI reports a type mismatch at the call site where frame = await transformed_stream.recv() is passed into DecartVideoTrack.add_frame: recv() is typed as returning Frame | Packet, but add_frame currently accepts only av.VideoFrame. To keep things type‑safe and robust while still only processing video frames, you can relax the parameter type and add a runtime guard:
-import logging
-from typing import Optional
+import logging
+from typing import Any, Optional
@@
- async def add_frame(self, frame: av.VideoFrame) -> None:
- if self._stopped:
- return
- # if frame.width != self.width or frame.height != self.height:
- # frame = await asyncio.to_thread(self._resize_frame, frame)
- self.frame_queue.put_latest_nowait(frame)
+ async def add_frame(self, frame: Any) -> None:
+ if self._stopped:
+ return
+
+ # Only process video frames; ignore any other payload types (e.g., packets).
+ if not isinstance(frame, av.VideoFrame):
+ logger.debug("Ignoring non-video frame in DecartVideoTrack.add_frame: %r", frame)
+ return
+
+ # if frame.width != self.width or frame.height != self.height:
+ # frame = await asyncio.to_thread(self._resize_frame, frame)
+ self.frame_queue.put_latest_nowait(frame)This should satisfy mypy at the call site in restyling._receive_frames_from_decart while ensuring non‑video data won’t accidentally enter the track. Please re‑run mypy to confirm the CI error at Line 227 in restyling.py disappears.
Also applies to: 44-50
I'll verify the review comment by examining the relevant code and type signatures.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine the DecartVideoTrack implementation
fd "decart_video_track\.py" -x cat {}
echo "---"
echo "Checking restyling.py for the call site around line 227:"
echo "---"
# Search for restyling.py and show context around recv() call
fd "restyling\.py" -x cat {} | head -250 | tail -50
Length of output: 5309
🏁 Script executed:
#!/bin/bash
# Search for where add_frame is called to understand the type mismatch
rg -n "add_frame" -B 3 -A 3 --type pyLength of output: 32368
🏁 Script executed:
#!/bin/bash
# Check what aiortc.MediaStreamTrack is and what recv returns
rg -n "MediaStreamTrack\|recv" --type py -B 2 -A 2 | head -100Length of output: 0
I need to verify the type signature of aiortc's MediaStreamTrack.recv() to confirm the review comment's claims.
---
🌐 Web query:
```web
aiortc MediaStreamTrack recv return type Frame Packet
💡 Result:
Short answer: MediaStreamTrack.recv() returns either a decoded AV Frame (av.audio.frame.AudioFrame or av.video.frame.VideoFrame) or an av.packet.Packet (raw/encoded), i.e. Union[Frame, Packet]. Use isinstance to branch on which you received.
Why: aiortc tracks can deliver decoded frames (common for VideoStreamTrack/AudioStreamTrack or when a player/decoder is used) or raw encoded packets (when the source/MediaPlayer is configured not to decode). Example check:
- if isinstance(obj, av.video.frame.VideoFrame) / av.audio.frame.AudioFrame: handle decoded frame
- elif isinstance(obj, av.packet.Packet): handle encoded packet
References: aiortc API (MediaStreamTrack.recv docs) and implementation. [1][2][3]
Sources
[1] aiortc API — MediaStreamTrack.recv return type.
[2] aiortc mediastreams.py (recv docstring & signature).
[3] aiortc helpers/MediaPlayer notes on decoded frames vs packets.
Broaden add_frame parameter type to resolve type mismatch with aiortc.MediaStreamTrack.recv()
The type mismatch is real: aiortc's MediaStreamTrack.recv() is documented to return Union[Frame, Packet] (which includes av.VideoFrame, av.AudioFrame, or av.Packet), but add_frame currently accepts only av.VideoFrame. This causes mypy to flag the call site in restyling.py:227. Relax the parameter type and guard with an isinstance check:
-import logging
-from typing import Optional
+import logging
+from typing import Any, Optional
@@
- async def add_frame(self, frame: av.VideoFrame) -> None:
- if self._stopped:
- return
- # if frame.width != self.width or frame.height != self.height:
- # frame = await asyncio.to_thread(self._resize_frame, frame)
- self.frame_queue.put_latest_nowait(frame)
+ async def add_frame(self, frame: Any) -> None:
+ if self._stopped:
+ return
+
+ # Only process video frames; ignore any other payload types (e.g., packets).
+ if not isinstance(frame, av.VideoFrame):
+ logger.debug("Ignoring non-video frame in DecartVideoTrack.add_frame: %r", frame)
+ return
+
+ # if frame.width != self.width or frame.height != self.height:
+ # frame = await asyncio.to_thread(self._resize_frame, frame)
+ self.frame_queue.put_latest_nowait(frame)This resolves the mypy error at restyling.py:227 while safely filtering non-video payloads.
Also applies to: 44-50
🤖 Prompt for AI Agents
In plugins/decart/vision_agents/plugins/decart/decart_video_track.py around
lines 3 and also affecting lines 44-50, the add_frame parameter is typed too
narrowly as av.VideoFrame while aiortc.MediaStreamTrack.recv() can return
av.VideoFrame, av.AudioFrame or av.Packet; change the add_frame signature to
accept the broader union type (e.g., av.VideoFrame | av.AudioFrame | av.Packet
or typing.Union[...] ) and inside the function add an isinstance guard that only
processes av.VideoFrame values (ignore or return early for non-video frames) so
mypy is satisfied and non-video payloads are safely filtered out.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/decart/vision_agents/plugins/decart/restyling.py (3)
215-234: Frame forwarding loop is straightforward but could log non-cancellation errorsThe
_receive_frames_from_decartloop correctly bails onCancelledErrorand forwards onlyav.VideoFrameinstances intoDecartVideoTrack. If you ever need to diagnose dropped frames or upstream failures, you may want to also log unexpected exceptions here instead of letting them silently kill the task.async def _receive_frames_from_decart( self, transformed_stream: MediaStreamTrack ) -> None: - try: - while not self._video_track._stopped: - frame = await transformed_stream.recv() - if frame and isinstance(frame, av.VideoFrame): - await self._video_track.add_frame(frame) - except asyncio.CancelledError: - logger.debug("Frame receiving from Decart cancelled") + try: + while not self._video_track._stopped: + frame = await transformed_stream.recv() + if frame and isinstance(frame, av.VideoFrame): + await self._video_track.add_frame(frame) + except asyncio.CancelledError: + logger.debug("Frame receiving from Decart cancelled") + except Exception as e: + logger.error("Error while receiving frames from Decart: %s", e) --- `273-298`: **close() behavior is mostly safe but async cleanup is fire-and-forget when loop is running** The `close()` method correctly stops the local video track and cancels background tasks, then runs `_async_close()` either via `run_until_complete` (no running loop) or `asyncio.create_task` (running loop), swallowing `RuntimeError` if no loop is present. This is generally fine, but note that in the “loop is running” case cleanup of `_realtime_client` / `_decart_client` is fire‑and‑forget; callers can’t know when the underlying connections are actually closed. If you need stronger lifecycle guarantees (e.g., tests waiting for full shutdown), consider exposing an async `async_close()` public method that awaits `_async_close()` directly, and have `close()` remain the best‑effort sync wrapper. --- `66-116`: **Literal is currently accurate; optional refactor suggestion remains valid** The verification confirms that the current `Literal["mirage", "mirage_v2", "lucy_v2v_720p_rt"]` matches the currently documented valid realtime model names in the Decart Python SDK. The code is functionally correct. However, the original suggestion to either drop the `Literal` or derive it from Decart's type hints remains sound from a maintainability perspective, since the Literal will need manual updates if Decart adds new models in the future. The refactor is optional but recommended to reduce future maintenance burden. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **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. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5b1eca0acc19b223c093fa1b5e05a7662eeb448b and 9edb8988b0457c1ebdf68afed07bd36c0d0ec3d9. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `plugins/decart/example/decart_example.py` (1 hunks) * `plugins/decart/vision_agents/plugins/decart/restyling.py` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * plugins/decart/example/decart_example.py </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>plugins/decart/vision_agents/plugins/decart/restyling.py (2)</summary><blockquote> <details> <summary>agents-core/vision_agents/core/processors/base_processor.py (3)</summary> * `AudioVideoProcessor` (117-146) * `VideoProcessorMixin` (66-73) * `VideoPublisherMixin` (84-86) </details> <details> <summary>plugins/decart/vision_agents/plugins/decart/decart_video_track.py (4)</summary> * `DecartVideoTrack` (14-104) * `recv` (80-100) * `add_frame` (44-49) * `stop` (102-104) </details> </blockquote></details> </details> </details> <details> <summary>⏰ 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). (5)</summary> * GitHub Check: unit / Mypy * GitHub Check: unit / Test "not integration" * GitHub Check: unit / Ruff * GitHub Check: unit / Test "not integration" * GitHub Check: unit / Ruff & mypy </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>plugins/decart/vision_agents/plugins/decart/restyling.py (3)</summary><blockquote> `26-39`: **Reconnect heuristic is clear and focused on transient failures** The `_should_reconnect` helper cleanly separates retry decisions and focuses on connection/timeout‑style failures, which is a good default for avoiding pointless retries on auth/config errors. --- `129-147`: **Video processing + publishing flow looks consistent with core mixins** `process_video` wiring (`_current_track` + `_connect_to_decart`) and `publish_video_track` returning the dedicated `DecartVideoTrack` align with the `AudioVideoProcessor` / `VideoPublisherMixin` patterns and keep state in one place (`_current_track`, `_video_track`). --- `149-167`: **Prompt/mirror setters behave defensively when not connected** Guarding `set_prompt` and `set_mirror` on `_realtime_client` presence avoids accidental calls before connection and gives clear logging, which is a nice, low‑surprise behavior for callers. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
agents-core/vision_agents/core/utils/video_utils.py (1)
53-61: Consider extracting common aspect-ratio scaling logic.The aspect ratio calculation logic (lines 53-61) is duplicated in
resize_frame(lines 96-106). Consider extracting this into a helper function to improve maintainability.Example helper function:
def _calculate_scaled_dimensions( src_width: int, src_height: int, target_width: int, target_height: int ) -> tuple[int, int]: """Calculate scaled dimensions maintaining aspect ratio.""" scale = min(target_width / src_width, target_height / src_height) return int(src_width * scale), int(src_height * scale)Also applies to: 96-106
plugins/decart/vision_agents/plugins/decart/decart_restyling_processor.py (2)
110-112: Consider validating model name before casting.The
castto aLiteraltype doesn't provide runtime validation. If an invalid model name is passed, the error will occur later when the Decart SDK validates it, potentially making debugging harder.Consider adding explicit validation or at least documenting the valid model names in the docstring.
+ # Valid model names + valid_models = ["mirage", "mirage_v2", "lucy_v2v_720p_rt"] + if model not in valid_models: + raise ValueError( + f"Invalid model '{model}'. Valid options: {', '.join(valid_models)}" + ) + self.model = models.realtime( cast(Literal["mirage", "mirage_v2", "lucy_v2v_720p_rt"], self.model_name) )
298-304: Event loop handling in close() could be simplified.The conditional logic for handling running vs non-running event loops is necessary but somewhat complex. The duplicate check for client existence on lines 300 and 303 could be simplified.
Consider this refactoring:
loop = asyncio.get_event_loop() + has_clients = self._realtime_client or self._decart_client + if not has_clients: + return + if loop.is_running(): - if self._realtime_client or self._decart_client: - asyncio.create_task(self._async_close()) + asyncio.create_task(self._async_close()) else: - if self._realtime_client or self._decart_client: - loop.run_until_complete(self._async_close()) + loop.run_until_complete(self._async_close())
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (7)
agents-core/vision_agents/core/utils/video_utils.py(2 hunks)plugins/decart/example/decart_example.py(1 hunks)plugins/decart/tests/test_decart_restyling.py(1 hunks)plugins/decart/tests/test_decart_video_track.py(1 hunks)plugins/decart/vision_agents/plugins/decart/__init__.py(1 hunks)plugins/decart/vision_agents/plugins/decart/decart_restyling_processor.py(1 hunks)plugins/decart/vision_agents/plugins/decart/decart_video_track.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/decart/vision_agents/plugins/decart/init.py
- plugins/decart/example/decart_example.py
- plugins/decart/vision_agents/plugins/decart/decart_video_track.py
🧰 Additional context used
🧬 Code graph analysis (3)
plugins/decart/tests/test_decart_restyling.py (3)
plugins/decart/vision_agents/plugins/decart/decart_restyling_processor.py (13)
RestylingProcessor(42-311)close(288-304)publish_video_track(140-141)process_video(129-138)update_prompt(143-172)set_mirror(174-181)_connect_to_decart(183-228)_on_error(263-267)_on_connection_change(249-261)_disconnect_from_decart(281-286)_processing_loop(270-279)_receive_frames_from_decart(239-247)_on_remote_stream(230-237)plugins/decart/vision_agents/plugins/decart/decart_video_track.py (3)
DecartVideoTrack(15-76)recv(52-72)stop(74-76)plugins/decart/tests/test_decart_video_track.py (1)
sample_frame(17-19)
plugins/decart/vision_agents/plugins/decart/decart_restyling_processor.py (2)
agents-core/vision_agents/core/processors/base_processor.py (3)
AudioVideoProcessor(117-146)VideoProcessorMixin(66-73)VideoPublisherMixin(84-86)plugins/decart/vision_agents/plugins/decart/decart_video_track.py (1)
DecartVideoTrack(15-76)
plugins/decart/tests/test_decart_video_track.py (2)
plugins/decart/vision_agents/plugins/decart/decart_video_track.py (1)
DecartVideoTrack(15-76)agents-core/vision_agents/core/utils/audio_queue.py (2)
qsize(40-42)get(119-136)
⏰ 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 / Mypy
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (4)
plugins/decart/tests/test_decart_restyling.py (1)
1-446: Well-structured test suite with comprehensive coverage.The test suite effectively covers the RestylingProcessor functionality:
- Connection lifecycle and state management
- Prompt and mirror updates with connection state checks
- Frame handling and forwarding
- Reconnection logic for various error types
- Proper use of mocks to isolate unit behavior
The consistent use of
processor.close()for cleanup and appropriate async patterns demonstrate good testing practices.plugins/decart/tests/test_decart_video_track.py (1)
1-124: Thorough test coverage for DecartVideoTrack.The test suite comprehensively validates DecartVideoTrack behavior:
- Initialization with default and custom dimensions
- Frame adding with and without resize requirements
- Frame queue management (2-frame capacity)
- Placeholder frame behavior when queue is empty
- Stop state preventing further operations
- Proper pts and time_base assignment on received frames
The tests effectively exercise the public API and verify expected behavior.
plugins/decart/vision_agents/plugins/decart/decart_restyling_processor.py (2)
183-228: Robust connection management with proper guards.The connection logic correctly:
- Prevents duplicate connection attempts with the
_connectingflag- Cleans up existing connections before reconnecting
- Initializes the realtime client with proper state (prompt, mirror)
- Registers necessary event handlers
- Handles errors by resetting state and re-raising
The pattern ensures connection integrity and prevents resource leaks.
230-248: Clean frame handling with proper task lifecycle management.The frame receiving logic:
- Correctly cancels previous frame receiving tasks before starting new ones (preventing duplicates)
- Continuously forwards frames from Decart's transformed stream to the output video track
- Handles cancellation gracefully with try/except for CancelledError
- Respects the video track's stopped state
| def resize_frame(self, frame: av.VideoFrame) -> av.VideoFrame: | ||
| logger.debug( | ||
| f"Resizing frame from {frame.width}x{frame.height} to {self.width}x{self.height}" | ||
| ) | ||
| img = frame.to_image() | ||
|
|
||
| # Calculate scaling to maintain aspect ratio | ||
| src_width, src_height = img.size | ||
| target_width, target_height = self.width, self.height | ||
|
|
||
| # Calculate scale factor (fit within target dimensions) | ||
| scale = min(target_width / src_width, target_height / src_height) | ||
| new_width = int(src_width * scale) | ||
| new_height = int(src_height * scale) | ||
|
|
||
| # Resize with aspect ratio maintained | ||
| resized = img.resize((new_width, new_height), Image.Resampling.LANCZOS) | ||
|
|
||
| # Create black background at target resolution | ||
| result = Image.new("RGB", (target_width, target_height), (0, 0, 0)) | ||
|
|
||
| # Paste resized image centered | ||
| x_offset = (target_width - new_width) // 2 | ||
| y_offset = (target_height - new_height) // 2 | ||
| result.paste(resized, (x_offset, y_offset)) | ||
|
|
||
| return av.VideoFrame.from_image(result) |
There was a problem hiding this comment.
Critical: Fix misleading function signature for module-level function.
This function is defined at module level with a self parameter, which is misleading since self conventionally indicates a class method. The function is called from DecartVideoTrack as asyncio.to_thread(resize_frame, self, frame), passing the track instance as the first argument.
The parameter should be renamed and properly typed to clarify that it expects an object with width and height attributes.
Apply this diff to fix the signature:
-def resize_frame(self, frame: av.VideoFrame) -> av.VideoFrame:
+def resize_frame(target_dimensions: object, frame: av.VideoFrame) -> av.VideoFrame:
+ """
+ Resize a video frame to target dimensions while preserving aspect ratio.
+
+ Args:
+ target_dimensions: Object with width and height attributes.
+ frame: Input video frame.
+
+ Returns:
+ Resized video frame centered on black background.
+ """
logger.debug(
- f"Resizing frame from {frame.width}x{frame.height} to {self.width}x{self.height}"
+ f"Resizing frame from {frame.width}x{frame.height} to {target_dimensions.width}x{target_dimensions.height}"
)
img = frame.to_image()
# Calculate scaling to maintain aspect ratio
src_width, src_height = img.size
- target_width, target_height = self.width, self.height
+ target_width, target_height = target_dimensions.width, target_dimensions.height
# Calculate scale factor (fit within target dimensions)
scale = min(target_width / src_width, target_height / src_height)
new_width = int(src_width * scale)
new_height = int(src_height * scale)
# Resize with aspect ratio maintained
- resized = img.resize((new_width, new_height), Image.Resampling.LANCZOS)
+ resized = img.resize((new_width, new_height), Resampling.LANCZOS)
# Create black background at target resolution
result = Image.new("RGB", (target_width, target_height), (0, 0, 0))
# Paste resized image centered
x_offset = (target_width - new_width) // 2
y_offset = (target_height - new_height) // 2
result.paste(resized, (x_offset, y_offset))
return av.VideoFrame.from_image(result)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In agents-core/vision_agents/core/utils/video_utils.py around lines 90-116, the
function is defined at module level but uses the misleading parameter name
"self"; rename that parameter to something like "track" and add a proper type
annotation (e.g., track: Any or define a small Protocol with width: int and
height: int) to clarify it expects an object with width and height attributes,
then update the function signature and any internal "self" references to use
"track" while leaving call sites (asyncio.to_thread(resize_frame, self, frame))
unchanged.
decartpluginSummary by CodeRabbit
New Features
Documentation
Tests