Close idle agents after some timeout & Agent clean up#279
Conversation
📝 WalkthroughWalkthroughConverts Agent.join to an async context manager; removes AgentSessionContextManager; adds idle/participant tracking and wait/idle APIs to GetStream transports; introduces AgentLauncher lifecycle and idle-cleanup; adds cancel_and_wait utility; updates examples, tests, and workspace config. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Agent
participant StreamEdge
participant MCP as "MCP/Edge"
participant Tracing
participant Cleanup
Client->>Agent: async with agent.join(call)
activate Agent
Agent->>Tracing: _start_tracing(call_id, agent_id)
Agent->>Agent: create/attach user
Agent->>StreamEdge: connect() / join_edge()
activate StreamEdge
StreamEdge->>MCP: establish connection
MCP-->>StreamEdge: connection ready
StreamEdge-->>Agent: joined + participant updates
StreamEdge->>StreamEdge: _on_participant_change() (idle tracking/event)
Agent->>Agent: publish_tracks()
Agent-->>Client: context entered (yield)
deactivate Agent
Client->>Agent: exit context
activate Agent
Agent->>Agent: set _call_ended_event
Agent->>Cleanup: close() (stop consumers, tear down tracks)
activate Cleanup
Cleanup->>StreamEdge: close() / reset connection
StreamEdge-->>Cleanup: closed
Cleanup->>Tracing: _end_tracing()
Cleanup-->>Agent: cleanup complete
deactivate Cleanup
Agent-->>Client: context exited
deactivate Agent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 |
![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
🤖 Fix all issues with AI Agents
In @plugins/getstream/tests/test_stream_edge_transport.py:
- Line 3: The test imports and uses unittest.mock.Mock (from Mock and
call=Mock()) which violates the guideline "Never mock in tests"; remove the Mock
import and replace the mocked call with a real Call object or a pytest fixture
that constructs a minimal real/stub Call instance satisfying ConnectionManager
requirements (e.g., use an existing Call class or create a test Call fixture)
and update occurrences of call=Mock() (and any other Mock usage) in
test_stream_edge_transport.py to use that fixture or real Call object instead.
In @plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py:
- Around line 79-80: Replace the broad "except Exception as e" around the call
to self._connection.leave() with explicit handlers for the specific errors that
can be raised (e.g., ConnectionError, OSError, asyncio.TimeoutError, and
asyncio.CancelledError); log each explicitly via logger.error or
logger.exception including context about the connection, re-raise
asyncio.CancelledError immediately, and allow any truly unexpected exceptions to
propagate instead of catching them silently—update imports as needed and remove
the catch-all to comply with the "no except Exception" guideline.
🧹 Nitpick comments (8)
plugins/inworld/example/inworld_tts_example.py (1)
50-50: Consider removing redundantcreate_user()call.The
agent.join()method internally callscreate_user()at the start of its execution (seeagents.pyline 541), making this explicit call redundant.🔎 Proposed simplification
- # Ensure the agent user is created - await agent.create_user() # Create a call call = await agent.create_call(call_type, call_id)plugins/sample_plugin/example/my_example.py (1)
39-44: Remove redundantcreate_user()call.Line 42 explicitly calls
await agent.create_user(), but thejoin()method already invokescreate_user()internally (agents-core/vision_agents/core/agents/agents.py, line 535). This duplication is unnecessary and could cause confusion.🔎 Proposed refactor
async def join_call(agent: Agent, call_type: str, call_id: str, **kwargs) -> None: """Join the call and start the agent.""" - # Ensure the agent user is created - await agent.create_user() # Create a call call = await agent.create_call(call_type, call_id)agents-core/vision_agents/core/utils/utils.py (1)
105-121: Clarify the intent ofasyncio.shieldor consider simplification.The use of
asyncio.shieldafter directly cancellingfutcreates potentially confusing behavior:
asyncio.shieldprotects the inner future from cancellation propagating inward from the caller, but herefutis already cancelled directly before shielding- The
CancelledErrorfromfutwill still propagate outward through the shield and be suppressed- If the current task (caller) is cancelled while waiting, the shield prevents immediate termination, which may or may not be desirable
If the intent is to ensure cleanup completes even when the caller is cancelled, this should be explicitly documented in the docstring. Otherwise, a simpler implementation would suffice:
async def cancel_and_wait(fut: asyncio.Future) -> None: if fut.done(): return fut.cancel() with contextlib.suppress(asyncio.CancelledError): await futplugins/getstream/tests/test_stream_edge_transport.py (1)
65-66: Double-wrapping of timeout logic.
conn.wait_for_participant(timeout=2.0)already wraps the internal wait withasyncio.wait_for. The outerasyncio.wait_for(..., timeout=0.5)creates nested timeouts. If the intent is to test a 0.5s timeout, pass it directly towait_for_participant.🔎 Proposed fix
# This should timeout since only agent is present with pytest.raises(asyncio.TimeoutError): - await asyncio.wait_for(conn.wait_for_participant(timeout=2.0), timeout=0.5) + await conn.wait_for_participant(timeout=0.5)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
262-262:hasattrusage violates coding guidelines.The guidelines state: "Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access." Consider using
isinstancechecks or try/except.🔎 Proposed fix
- if hasattr(event.payload, "type") and event.payload is not None: + if isinstance(event, sfu_events.TrackUnpublishedEvent) and event.payload is not None:Use
isinstanceto distinguish betweenTrackUnpublishedEventandParticipantLeftEventrather than probing for attributes.agents-core/vision_agents/core/agents/agents.py (3)
670-673: Multiplegetattrusages violate coding guidelines.The coding guidelines state: "Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access." The
_applymethod usesgetattrtwice.🔎 Proposed fix
Consider defining an interface or protocol that subclasses must implement, then use normal attribute access:
# Define a protocol with optional lifecycle methods class LifecyclePlugin(Protocol): def start(self) -> Awaitable[None] | None: ... def stop(self) -> Awaitable[None] | None: ... def close(self) -> Awaitable[None] | None: ...Alternatively, if dynamic dispatch is required, document why
getattris necessary here as an exception to the guidelines.
207-210:hasattrusage violates coding guidelines.Line 208 uses
hasattr(plugin, "events"). Per guidelines, prefer normal attribute access orisinstancechecks with protocols.🔎 Proposed fix
for plugin in [stt, tts, turn_detection, llm, edge, profiler]: - if plugin and hasattr(plugin, "events"): + if plugin is not None: + try: + plugin_events = plugin.events + except AttributeError: + continue self.logger.debug(f"Register events from plugin {plugin}") - self.events.merge(plugin.events) + self.events.merge(plugin_events)
740-745: Broad exception handling violates coding guidelines.Line 743 uses
except Exception as e:which the guidelines prohibit. Identify specific exceptions thatforwarder.stop()can raise.🔎 Proposed fix
for forwarder in self._video_forwarders: try: await forwarder.stop() - except Exception as e: + except (asyncio.CancelledError, RuntimeError) as e: self.logger.error(f"Error stopping video forwarder: {e}")Identify the actual exception types that
stop()can raise and handle them specifically.
📜 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 (41)
DEVELOPMENT.mdagents-core/vision_agents/core/agents/agent_session.pyagents-core/vision_agents/core/agents/agents.pyagents-core/vision_agents/core/utils/utils.pyexamples/01_simple_agent_example/README.mdexamples/01_simple_agent_example/simple_agent_example.pyexamples/01_simple_agent_example/test.pyexamples/02_golf_coach_example/golf_coach_example.pyexamples/03_football_commentator_example/football_commentator_example.pyplugins/aws/example/aws_realtime_function_calling_example.pyplugins/aws/example/aws_realtime_nova_example.pyplugins/cartesia/example/main.pyplugins/cartesia/example/narrator-example.pyplugins/decart/example/decart_example.pyplugins/deepgram/example/deepgram_tts_example.pyplugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.pyplugins/elevenlabs/example/elevenlabs_example.pyplugins/elevenlabs/vision_agents/plugins/elevenlabs/stt.pyplugins/fast_whisper/example/fast_whisper_example.pyplugins/fish/example/fish_example.pyplugins/getstream/tests/test_stream_edge_transport.pyplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.pyplugins/heygen/README.mdplugins/heygen/example/avatar_example.pyplugins/heygen/example/avatar_realtime_example.pyplugins/huggingface/example/main.pyplugins/inworld/example/inworld_tts_example.pyplugins/kokoro/example/kokoro_example.pyplugins/moondream/README.mdplugins/moondream/example/moondream_vlm_example.pyplugins/openai/examples/qwen_vl_example/README.mdplugins/openai/examples/qwen_vl_example/qwen_vl_example.pyplugins/openrouter/example/openrouter_example.pyplugins/qwen/example/README.mdplugins/qwen/example/qwen_realtime_example.pyplugins/roboflow/example/roboflow_example.pyplugins/sample_plugin/example/my_example.pyplugins/smart_turn/example/smart_turn_example.pyplugins/vogent/example/vogent_example.pyplugins/wizper/example/wizper_example.pytests/test_agent.py
💤 Files with no reviewable changes (2)
- agents-core/vision_agents/core/agents/agent_session.py
- tests/test_agent.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
agents-core/vision_agents/core/utils/utils.pyplugins/elevenlabs/vision_agents/plugins/elevenlabs/stt.pyplugins/openrouter/example/openrouter_example.pyplugins/cartesia/example/narrator-example.pyplugins/wizper/example/wizper_example.pyplugins/sample_plugin/example/my_example.pyplugins/aws/example/aws_realtime_function_calling_example.pyplugins/kokoro/example/kokoro_example.pyplugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.pyplugins/aws/example/aws_realtime_nova_example.pyplugins/qwen/example/qwen_realtime_example.pyplugins/inworld/example/inworld_tts_example.pyplugins/cartesia/example/main.pyplugins/elevenlabs/example/elevenlabs_example.pyplugins/deepgram/example/deepgram_tts_example.pyplugins/smart_turn/example/smart_turn_example.pyplugins/huggingface/example/main.pyplugins/openai/examples/qwen_vl_example/qwen_vl_example.pyplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.pyplugins/heygen/example/avatar_example.pyexamples/03_football_commentator_example/football_commentator_example.pyplugins/roboflow/example/roboflow_example.pyplugins/decart/example/decart_example.pyexamples/02_golf_coach_example/golf_coach_example.pyexamples/01_simple_agent_example/simple_agent_example.pyplugins/heygen/example/avatar_realtime_example.pyplugins/fast_whisper/example/fast_whisper_example.pyagents-core/vision_agents/core/agents/agents.pyplugins/fish/example/fish_example.pyplugins/getstream/tests/test_stream_edge_transport.pyplugins/moondream/example/moondream_vlm_example.pyplugins/vogent/example/vogent_example.py
**/*test*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*test*.py: Never mock in tests; use pytest for testing
Mark integration tests with @pytest.mark.integration decorator
@pytest.mark.asyncio is not needed - it is automatic
Files:
plugins/getstream/tests/test_stream_edge_transport.py
🧠 Learnings (2)
📚 Learning: 2025-10-13T22:00:34.300Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 98
File: plugins/deepgram/vision_agents/plugins/deepgram/stt.py:135-150
Timestamp: 2025-10-13T22:00:34.300Z
Learning: In the Deepgram STT plugin (plugins/deepgram/vision_agents/plugins/deepgram/stt.py), the `started()` method is designed to wait for the connection attempt to complete, not to guarantee a successful connection. It's acceptable for the connection attempt to fail, and downstream code handles the case where `self.dg_connection` is `None`. The `_connected_once` event is set in the `finally` block intentionally to signal attempt completion.
Applied to files:
plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py
📚 Learning: 2025-12-10T19:35:39.238Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 249
File: agents-core/vision_agents/core/agents/agents.py:1032-1042
Timestamp: 2025-12-10T19:35:39.238Z
Learning: In `agents-core/vision_agents/core/agents/agents.py`, when using `video_track_override_path`, creating a new `VideoFileTrack` for each participant (each call to `_on_track_added`) is intentional to maintain proper track lifecycle semantics tied to each participant.
Applied to files:
agents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (25)
plugins/elevenlabs/vision_agents/plugins/elevenlabs/stt.py (4)
agents-core/vision_agents/core/events/base.py (1)
AudioFormat(23-30)agents-core/vision_agents/core/stt/events.py (1)
TranscriptResponse(7-13)agents-core/vision_agents/core/utils/utils.py (1)
cancel_and_wait(105-121)agents-core/vision_agents/core/stt/stt.py (1)
close(164-165)
plugins/openrouter/example/openrouter_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/cartesia/example/narrator-example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/sample_plugin/example/my_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/aws/example/aws_realtime_function_calling_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/kokoro/example/kokoro_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py (2)
agents-core/vision_agents/core/stt/events.py (1)
TranscriptResponse(7-13)agents-core/vision_agents/core/utils/utils.py (1)
cancel_and_wait(105-121)
plugins/aws/example/aws_realtime_nova_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/inworld/example/inworld_tts_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(318-373)
plugins/cartesia/example/main.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/elevenlabs/example/elevenlabs_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/deepgram/example/deepgram_tts_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/smart_turn/example/smart_turn_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/huggingface/example/main.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/openai/examples/qwen_vl_example/qwen_vl_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(318-373)
plugins/heygen/example/avatar_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(318-373)
examples/03_football_commentator_example/football_commentator_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(318-373)
plugins/roboflow/example/roboflow_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/decart/example/decart_example.py (1)
agents-core/vision_agents/core/agents/agents.py (2)
Agent(90-1367)join(510-588)
examples/01_simple_agent_example/simple_agent_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)agents-core/vision_agents/core/edge/edge_transport.py (1)
join(46-47)
plugins/heygen/example/avatar_realtime_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/fast_whisper/example/fast_whisper_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/fish/example/fish_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(318-373)
plugins/getstream/tests/test_stream_edge_transport.py (2)
plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (3)
StreamConnection(37-95)idle_since(53-61)wait_for_participant(63-67)agents-core/vision_agents/core/agents/agents.py (1)
wait_for_participant(590-609)
plugins/moondream/example/moondream_vlm_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(318-373)
⏰ 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). (8)
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Mypy
🔇 Additional comments (52)
plugins/deepgram/example/deepgram_tts_example.py (1)
56-56: LGTM! Correct async context manager usage.The change from
with awaittoasync withproperly aligns with theagent.join()method's async context manager pattern. Based on the relevant code snippet,agent.join()returns anAsyncIterator[None]and must be used withasync with.plugins/heygen/example/avatar_example.py (1)
65-65: Correct usage of async context manager.The change from
with awaittoasync withis the proper syntax for async context managers in Python. Sinceagent.join()returns anAsyncIterator[None](async generator), it must be used withasync withto properly handle the async enter/exit protocol.plugins/moondream/example/moondream_vlm_example.py (1)
44-44: LGTM! Critical fix for async context manager usage.The change from
with awaittoasync withcorrects the async context manager syntax. The previous syntax would incorrectly attempt to await the async generator first, then use the result (None) as a synchronous context manager, which would fail. The new syntax properly invokes the async context manager protocol, ensuring the cleanup logic in thefinallyblock ofagent.join()executes correctly.plugins/inworld/example/inworld_tts_example.py (1)
57-64: Correct usage of async context manager.The change to
async with agent.join(call):is the proper way to use thejoin()method, which returns anAsyncIterator[None]and functions as an async context manager. This ensures proper agent lifecycle management and cleanup.plugins/huggingface/example/main.py (2)
54-54: Correct async context manager pattern.The change from
with await agent.join(call):toasync with agent.join(call):is the proper pattern for async context managers. Sincejoinreturns anAsyncIterator[None], this ensures the async context manager protocol is followed correctly, allowing proper setup and teardown within the join lifecycle.
49-49: Remove the redundantcreate_user()call.The
joinmethod internally callsawait self.create_user()at the start of its execution. Whilecreate_user()is idempotent and safe to call multiple times (it checksif self._agent_user_initializedand returns early on subsequent calls), the call on line 49 is unnecessary and should be removed.plugins/cartesia/example/main.py (1)
54-54: Correct async context manager usage.This change properly converts the join pattern to use
async withfor the async context manager, aligning with theAgent.join()method signature that returnsAsyncIterator[None].plugins/aws/example/aws_realtime_function_calling_example.py (1)
90-90: Correct async context manager usage.The conversion to
async withproperly handles the async context manager protocol forAgent.join().examples/03_football_commentator_example/football_commentator_example.py (1)
67-67: Correct async context manager usage.The change properly uses
async withfor the async context manager, consistent with theAgent.join()implementation.plugins/roboflow/example/roboflow_example.py (1)
55-55: Correct async context manager usage.The change properly adopts
async withfor the async context manager returned byAgent.join().plugins/kokoro/example/kokoro_example.py (1)
51-51: Correct async context manager usage.The change properly uses
async withfor the async context manager, ensuring proper lifecycle management of the join operation.plugins/elevenlabs/example/elevenlabs_example.py (1)
52-54: LGTM! Correct migration to async context manager.The change from
with awaittoasync withcorrectly reflects the updatedjoin()method signature that returns anAsyncIterator[None].plugins/sample_plugin/example/my_example.py (1)
49-56: LGTM! Correct async context manager usage.The migration to
async with agent.join(call):aligns with the updatedjoin()signature returning anAsyncIterator[None].plugins/qwen/example/README.md (1)
62-64: LGTM! Documentation reflects the new API.The example code correctly demonstrates the async context manager pattern for
join().examples/01_simple_agent_example/simple_agent_example.py (1)
57-72: LGTM! Async context manager correctly applied.The change to
async with agent.join(call):properly reflects the new async context manager pattern for managing the agent's join lifecycle.plugins/aws/example/aws_realtime_nova_example.py (1)
39-45: LGTM! Async context manager correctly implemented.The migration to
async with agent.join(call):is consistent with the newjoin()method that returns anAsyncIterator[None]and manages the agent lifecycle through proper async context management.plugins/smart_turn/example/smart_turn_example.py (1)
31-31: Correct async context manager usage.This change properly fixes the async context manager usage. The
agent.join()method returns anAsyncIterator[None]and must be used withasync with, notwith await. The old pattern was incorrect.plugins/wizper/example/wizper_example.py (1)
71-71: Correct async context manager usage.This properly fixes the async context manager pattern for
agent.join(). The change fromwith awaittoasync withis essential for correct async operation.plugins/openrouter/example/openrouter_example.py (1)
116-116: Correct async context manager usage.The change to
async with agent.join(call):properly implements the async context manager protocol for the join operation.plugins/heygen/README.md (1)
59-59: Documentation correctly updated.Both code examples in the README now demonstrate the proper
async with agent.join(call):pattern, ensuring users implement the correct async context manager usage.Also applies to: 111-111
plugins/moondream/README.md (1)
155-155: Documentation correctly updated.The example now demonstrates the proper async context manager usage with
async with agent.join(call):, consistent with the correct implementation pattern.examples/02_golf_coach_example/golf_coach_example.py (1)
32-32: LGTM! Correct async context manager usage.The change to
async with agent.join(call):properly aligns with the async context manager protocol, as thejoinmethod returns anAsyncIterator.plugins/heygen/example/avatar_realtime_example.py (1)
55-55: LGTM! Proper async context manager pattern.Correctly uses
async withfor the async context manager returned byagent.join().plugins/vogent/example/vogent_example.py (1)
31-31: LGTM! Consistent with async context manager protocol.plugins/fast_whisper/example/fast_whisper_example.py (1)
49-49: LGTM! Correct async context manager usage.agents-core/vision_agents/core/utils/utils.py (1)
2-2: LGTM! Required import for the new utility function.plugins/decart/example/decart_example.py (2)
10-10: LGTM! Good practice to load environment variables early.Loading environment variables via
load_dotenv()right after logger initialization ensures configuration is available before agent creation.
49-49: Correct async context manager syntax.The change from
with await agent.join(call):toasync with agent.join(call):is the proper syntax for async context managers. TheAgent.joinmethod is decorated with@asynccontextmanager, so it must be entered withasync with.plugins/qwen/example/qwen_realtime_example.py (1)
30-30: Correct async context manager syntax.This aligns with the async context manager pattern implemented in
Agent.join. The change ensures proper async/await semantics.plugins/cartesia/example/narrator-example.py (1)
56-56: Correct async context manager syntax.Consistent with the async context manager pattern for
Agent.join. The change ensures proper resource management and cleanup via the finally block in the join implementation.plugins/openai/examples/qwen_vl_example/qwen_vl_example.py (1)
39-39: Correct async context manager syntax.The transition to
async withensures proper async/await semantics and aligns with the@asynccontextmanagerdecorator onAgent.join.plugins/openai/examples/qwen_vl_example/README.md (1)
47-47: Documentation correctly updated.The example code in the documentation now accurately reflects the proper async context manager usage.
DEVELOPMENT.md (1)
472-472: LGTM! Async context manager usage is correct.The migration from
with await agent.join(call):toasync with agent.join(call):properly reflects the new async context manager pattern for the join operation.plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py (2)
4-4: LGTM! Import updates support the refactored close method.The addition of
Anytype and the new imports forTranscriptResponseandcancel_and_waitproperly support the improved cleanup logic in the close() method.Also applies to: 13-14
276-291: Excellent cleanup improvements with robust error handling.The refactored close() method demonstrates several improvements:
- Replaces manual task cancellation with the
cancel_and_waitutility for safer cancellation semantics- Wraps the connection close sequence in try/except/finally to ensure state reset even if closing fails
- Logs exceptions for visibility while continuing cleanup
The finally block guarantees that connection state is reset in all cases, preventing potential resource leaks.
examples/01_simple_agent_example/README.md (1)
86-86: LGTM! Example code updated correctly.The example properly demonstrates the async context manager pattern for joining calls, consistent with the new API design.
plugins/fish/example/fish_example.py (1)
58-58: LGTM! Fish example updated with async context manager.The join call correctly uses the async context manager pattern, aligning with the broader refactoring across all examples.
plugins/elevenlabs/vision_agents/plugins/elevenlabs/stt.py (3)
5-5: LGTM! Import reorganization and additions are appropriate.The import updates properly support the refactored close() method:
- Type imports reordered for clarity
- Specific ElevenLabs types imported directly
- New
cancel_and_waitutility imported for task cancellationAlso applies to: 10-10, 12-12, 14-14, 18-18, 20-20
390-395: Excellent refactoring using cancel_and_wait utility.The close() method now uses the
cancel_and_waitutility for both_send_taskand_listen_task, providing consistent and robust task cancellation semantics. This eliminates manual cancellation logic and ensures tasks are properly awaited.
402-402: LGTM! Minor wording improvement in error message.The error message now reads more naturally while maintaining the same functionality.
plugins/getstream/tests/test_stream_edge_transport.py (2)
18-40: LGTM!The idle detection test correctly exercises the state transitions: no participants → idle, self only → idle, other participant joins → not idle, participant leaves → idle again.
68-90: LGTM!The event-driven completion test correctly exercises the async flow: creates wait task, verifies it blocks, simulates participant join, and confirms task completion.
plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (3)
42-47: LGTM!The initialization of idle tracking state (
_idle_since,_participant_joinedevent, and participant subscription) is well-structured for the async event-driven design.
53-67: LGTM!The
idle_since()andwait_for_participant()methods are cleanly implemented. The docstrings follow Google style as required.
82-95: The event never being cleared is not a practical issue in this design.
wait_for_participant()is called only once during agent initialization (agents.py:576), and eachStreamConnectionis created fresh for its lifecycle. The method is not designed to be called multiple times on the same connection instance. The callback_on_participant_changeis used for passive monitoring of idleness, not for triggering re-waits. No test case or usage in the codebase attempts to re-wait after participants leave, so the proposed fix addresses a non-existent use case.Likely an incorrect or invalid review comment.
agents-core/vision_agents/core/agents/agents.py (7)
509-588: LGTM!The refactor to an async context manager is well-designed. The lock acquisition and release in try/finally ensures proper cleanup. The
_call_ended_eventcheck prevents re-joining, and the finally block guarantees resource cleanup.
611-632: LGTM!The
idle_for()implementation correctly handles edge cases: no connection, not joined yet, and adjustsidle_sinceto account for participant wait time before the agent fully joined.
693-718: LGTM!The close/shield pattern ensures cleanup completes even if the outer task is cancelled. The
_closedflag prevents duplicate execution, and the shielded task pattern is correctly implemented.
328-334: LGTM!The
on_call_endedhandler correctly signals the event and triggers agent cleanup when the call ends.
908-961: LGTM!The audio consumer loop correctly uses
_call_ended_eventas the termination signal and handles timeout/empty queue exceptions appropriately.
979-1001: LGTM!The track removal handling correctly filters for video track types and includes improved logging with track type name and participant info.
1051-1053: LGTM!Improved logging with track type name and participant user ID provides better observability.
plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py
Show resolved
Hide resolved
- Removed outdaded `AgentSessionContextManager` - `Agent.join` now invoked via `async with` instead of `with await` - Updated all examples - Fixed opentelemetry context issues - `Agent.close()` now completes even on cancellation.
dangusev
force-pushed
the
feature/clean-idle-agents
branch
from
ce264e9 to
2058913
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
agents-core/vision_agents/core/warmup.py (1)
28-32: Critical: Missing async handling in cache hit path.Line 31 invokes
on_warmed_upsynchronously while line 44 usesawait await_or_run. Ifon_warmed_upis async, cached resources will trigger an unawaited coroutine warning and fail to initialize properly.🐛 Proposed fix
async def warmup(self, warmable: "Warmable"): warmable_cls = type(warmable) if (resource := self._cache.get(warmable_cls)) is not None: # The resource is already loaded. # Set it to the warmable instance and exit - warmable.on_warmed_up(resource) + await await_or_run(warmable.on_warmed_up, resource) returnplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
497-503: Browser open exception handling is too broad.The
except Exception as eon line 501 catches all exceptions when opening the browser. Per coding guidelines, use specific exception handling.🔧 Proposed fix
try: # Run webbrowser.open in a separate thread to avoid blocking the event loop await asyncio.to_thread(webbrowser.open, url) logger.info("✅ Browser opened successfully!") - except Exception as e: - logger.error(f"❌ Failed to open browser: {e}") + except OSError as e: + logger.error(f"❌ Failed to open browser (OS error): {e}") logger.warning(f"Please manually open this URL: {url}")Based on coding guidelines.
agents-core/vision_agents/core/agents/agents.py (1)
857-867: Broad exception handling in _on_agent_say.The
except Exception as eon line 857 violates coding guidelines. Consider catching specific exceptions thattts.send()can raise.🔧 Proposed fix
- except Exception as e: + except (RuntimeError, ValueError, OSError) as e: # Emit say error event self.events.send( events.AgentSayErrorEvent(Based on coding guidelines.
🤖 Fix all issues with AI agents
In @agents-core/vision_agents/core/agents/agent_launcher.py:
- Around line 36-37: The docstring for the agent_idle_timeout parameter is
wrong; update the description to state the actual default of 10.0 seconds (or
change the function signature if you intended 30.0) so the docstring and the
parameter default for agent_idle_timeout in agent_launcher.py match; reference
the agent_idle_timeout parameter in the function signature and adjust the
docstring text to "Default - `10.0`" (or set agent_idle_timeout=30.0 if you
prefer the docstring value).
In @tests/test_agents/test_agent_launcher.py:
- Around line 161-163: Update the incorrect comment that says "The agents must
be closed" to accurately reflect the assertions verifying agents remain open;
change it to something like "The agents must remain open" or "The agents must
not be closed" near the assertions for agent1.closed and agent2.closed so the
comment matches the test intent and the checks on agent1 and agent2.
- Around line 36-40: The fixture stream_edge_mock incorrectly sets mock.events
as if it were a callable by using mock.events.return_value; treat events as an
attribute like on the StreamEdge type and assign the EventManager instance
directly: replace the mock.events.return_value = EventManager() with a direct
attribute assignment so mock.events refers to the EventManager object (keep
MagicMock usage and the fixture name stream_edge_mock unchanged).
🧹 Nitpick comments (2)
agents-core/vision_agents/core/warmup.py (1)
86-97: Update signature and docstring to reflect async support.The abstract method signature declares a synchronous return (
-> None), yetawait_or_runnow permits async implementations. Update the type hint to-> None | Awaitable[None]and document this flexibility in the docstring so implementers know both patterns are valid.♻️ Proposed enhancement
+from typing import Any, Generic, Type, TypeVar, Awaitable + ... @abc.abstractmethod - def on_warmed_up(self, resource: T) -> None: + def on_warmed_up(self, resource: T) -> None | Awaitable[None]: """ A method to set the loaded resource back to the object instance. This method is called every time an Agent starts. + + This method can be either synchronous or asynchronous. Args: - resource: + resource: The loaded resource to set on the instance. Returns: + None or an awaitable that resolves to None. """tests/test_agents/test_agent_launcher.py (1)
3-3: Use of mocking violates coding guidelines.The coding guidelines specify: "Never mock in tests; use pytest for testing." This file imports
MagicMockandpatchfromunittest.mockand uses them extensively. Consider using real implementations or pytest-specific fixtures instead of mocks.Based on coding guidelines.
📜 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 (46)
DEVELOPMENT.mdagents-core/vision_agents/core/agents/__init__.pyagents-core/vision_agents/core/agents/agent_launcher.pyagents-core/vision_agents/core/agents/agent_session.pyagents-core/vision_agents/core/agents/agents.pyagents-core/vision_agents/core/cli/cli_runner.pyagents-core/vision_agents/core/utils/utils.pyagents-core/vision_agents/core/warmup.pyexamples/01_simple_agent_example/README.mdexamples/01_simple_agent_example/simple_agent_example.pyexamples/02_golf_coach_example/golf_coach_example.pyexamples/04_football_commentator_example/football_commentator_example.pyplugins/aws/example/aws_realtime_function_calling_example.pyplugins/aws/example/aws_realtime_nova_example.pyplugins/cartesia/example/main.pyplugins/cartesia/example/narrator-example.pyplugins/decart/example/decart_example.pyplugins/deepgram/example/deepgram_tts_example.pyplugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.pyplugins/elevenlabs/example/elevenlabs_example.pyplugins/elevenlabs/vision_agents/plugins/elevenlabs/stt.pyplugins/fast_whisper/example/fast_whisper_example.pyplugins/fish/example/fish_example.pyplugins/getstream/tests/test_stream_edge_transport.pyplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.pyplugins/heygen/README.mdplugins/heygen/example/avatar_example.pyplugins/heygen/example/avatar_realtime_example.pyplugins/huggingface/example/main.pyplugins/inworld/example/inworld_tts_example.pyplugins/kokoro/example/kokoro_example.pyplugins/moondream/README.mdplugins/moondream/example/moondream_vlm_example.pyplugins/openai/examples/qwen_vl_example/README.mdplugins/openai/examples/qwen_vl_example/qwen_vl_example.pyplugins/openrouter/example/openrouter_example.pyplugins/qwen/example/README.mdplugins/qwen/example/qwen_realtime_example.pyplugins/roboflow/example/roboflow_example.pyplugins/sample_plugin/example/my_example.pyplugins/smart_turn/example/smart_turn_example.pyplugins/vogent/example/vogent_example.pyplugins/wizper/example/wizper_example.pytests/test_agent.pytests/test_agents/__init__.pytests/test_agents/test_agent_launcher.py
💤 Files with no reviewable changes (2)
- agents-core/vision_agents/core/agents/agent_session.py
- tests/test_agent.py
🚧 Files skipped from review as they are similar to previous changes (26)
- plugins/qwen/example/README.md
- DEVELOPMENT.md
- examples/01_simple_agent_example/simple_agent_example.py
- agents-core/vision_agents/core/utils/utils.py
- plugins/heygen/README.md
- plugins/cartesia/example/narrator-example.py
- plugins/roboflow/example/roboflow_example.py
- plugins/moondream/example/moondream_vlm_example.py
- plugins/fast_whisper/example/fast_whisper_example.py
- plugins/smart_turn/example/smart_turn_example.py
- examples/02_golf_coach_example/golf_coach_example.py
- plugins/cartesia/example/main.py
- plugins/getstream/tests/test_stream_edge_transport.py
- plugins/elevenlabs/example/elevenlabs_example.py
- plugins/sample_plugin/example/my_example.py
- plugins/openai/examples/qwen_vl_example/README.md
- plugins/qwen/example/qwen_realtime_example.py
- plugins/decart/example/decart_example.py
- plugins/heygen/example/avatar_example.py
- plugins/moondream/README.md
- plugins/kokoro/example/kokoro_example.py
- examples/01_simple_agent_example/README.md
- plugins/aws/example/aws_realtime_function_calling_example.py
- plugins/fish/example/fish_example.py
- plugins/deepgram/vision_agents/plugins/deepgram/deepgram_stt.py
- plugins/elevenlabs/vision_agents/plugins/elevenlabs/stt.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
plugins/deepgram/example/deepgram_tts_example.pyplugins/heygen/example/avatar_realtime_example.pyagents-core/vision_agents/core/cli/cli_runner.pyagents-core/vision_agents/core/agents/__init__.pyplugins/openai/examples/qwen_vl_example/qwen_vl_example.pyplugins/wizper/example/wizper_example.pyplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.pyplugins/inworld/example/inworld_tts_example.pyplugins/huggingface/example/main.pytests/test_agents/test_agent_launcher.pyagents-core/vision_agents/core/agents/agent_launcher.pyexamples/04_football_commentator_example/football_commentator_example.pyplugins/openrouter/example/openrouter_example.pyagents-core/vision_agents/core/agents/agents.pyplugins/vogent/example/vogent_example.pyagents-core/vision_agents/core/warmup.pyplugins/aws/example/aws_realtime_nova_example.py
**/*test*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*test*.py: Never mock in tests; use pytest for testing
Mark integration tests with @pytest.mark.integration decorator
@pytest.mark.asyncio is not needed - it is automatic
Files:
tests/test_agents/test_agent_launcher.py
🧠 Learnings (2)
📚 Learning: 2025-11-24T17:04:43.030Z
Learnt from: CR
Repo: GetStream/Vision-Agents PR: 0
File: .cursor/rules/python.mdc:0-0
Timestamp: 2025-11-24T17:04:43.030Z
Learning: Applies to **/*.py : Never write `except Exception as e` - use specific exception handling
Applied to files:
plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py
📚 Learning: 2025-12-10T19:35:39.238Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 249
File: agents-core/vision_agents/core/agents/agents.py:1032-1042
Timestamp: 2025-12-10T19:35:39.238Z
Learning: In `agents-core/vision_agents/core/agents/agents.py`, when using `video_track_override_path`, creating a new `VideoFileTrack` for each participant (each call to `_on_track_added`) is intentional to maintain proper track lifecycle semantics tied to each participant.
Applied to files:
agents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (16)
plugins/deepgram/example/deepgram_tts_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/heygen/example/avatar_realtime_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
agents-core/vision_agents/core/cli/cli_runner.py (1)
agents-core/vision_agents/core/agents/agent_launcher.py (2)
start(59-66)stop(68-73)
agents-core/vision_agents/core/agents/__init__.py (1)
agents-core/vision_agents/core/agents/agent_types.py (1)
AgentOptions(15-25)
plugins/openai/examples/qwen_vl_example/qwen_vl_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(326-381)
plugins/wizper/example/wizper_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(326-381)
plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (3)
agents-core/vision_agents/core/edge/sfu_events.py (7)
Participant(229-270)user_id(489-493)user_id(856-860)user_id(901-905)user_id(1186-1190)user_id(2093-2097)user_id(2142-2146)agents-core/vision_agents/core/edge/types.py (3)
Participant(22-24)Connection(27-35)OutputAudioTrack(39-49)agents-core/vision_agents/core/events/manager.py (1)
wait(483-496)
plugins/huggingface/example/main.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
tests/test_agents/test_agent_launcher.py (5)
agents-core/vision_agents/core/agents/agent_launcher.py (3)
AgentLauncher(15-185)warmup(75-94)launch(96-109)agents-core/vision_agents/core/edge/types.py (1)
User(15-18)agents-core/vision_agents/core/events/manager.py (1)
EventManager(56-561)agents-core/vision_agents/core/llm/llm.py (2)
LLM(50-396)LLMResponseEvent(39-43)agents-core/vision_agents/core/warmup.py (1)
Warmable(47-107)
agents-core/vision_agents/core/agents/agent_launcher.py (3)
agents-core/vision_agents/core/utils/utils.py (2)
await_or_run(92-102)cancel_and_wait(105-121)agents-core/vision_agents/core/warmup.py (4)
warmup(26-44)warmup(98-107)Warmable(47-107)WarmupCache(15-44)agents-core/vision_agents/core/agents/agents.py (2)
Agent(90-1371)idle_for(611-632)
examples/04_football_commentator_example/football_commentator_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
plugins/openrouter/example/openrouter_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
agents-core/vision_agents/core/agents/agents.py (2)
agents-core/vision_agents/core/edge/types.py (5)
OutputAudioTrack(39-49)Participant(22-24)User(15-18)close(34-35)stop(47-47)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (11)
StreamConnection(37-95)StreamEdge(98-505)_connection(132-135)close(69-80)close(421-423)join(326-381)create_user(314-316)publish_tracks(400-409)create_conversation(305-312)wait_for_participant(63-67)idle_since(53-61)
plugins/vogent/example/vogent_example.py (2)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (1)
join(326-381)
agents-core/vision_agents/core/warmup.py (1)
agents-core/vision_agents/core/utils/utils.py (1)
await_or_run(92-102)
plugins/aws/example/aws_realtime_nova_example.py (1)
agents-core/vision_agents/core/agents/agents.py (1)
join(510-588)
⏰ 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). (8)
- GitHub Check: unit / Mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Mypy
- GitHub Check: unit / Ruff
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
🔇 Additional comments (30)
plugins/heygen/example/avatar_realtime_example.py (1)
55-55: LGTM! Correct async context manager usage.The change from
with await agent.join(call):toasync with agent.join(call):correctly uses the async context manager pattern, as thejoinmethod returns anAsyncIterator[None]designed for this usage.agents-core/vision_agents/core/agents/__init__.py (1)
10-10: LGTM! Clean public API addition.The import and export of
AgentOptionsfollows the established pattern in this module and properly exposes it as part of the public API for lifecycle management and agent configuration.Also applies to: 16-16
plugins/aws/example/aws_realtime_nova_example.py (1)
39-39: Correct async context manager usage.The update to use
async withinstead ofwith awaitproperly aligns with the refactoredAgent.join()method, which now returns anAsyncIterator[None]and should be used as an async context manager.plugins/inworld/example/inworld_tts_example.py (1)
57-57: Correct async context manager usage.The change to
async withproperly handles the async context manager returned byAgent.join().examples/04_football_commentator_example/football_commentator_example.py (1)
67-67: Correct async context manager usage.The update to
async withcorrectly uses the refactoredAgent.join()async context manager.plugins/openai/examples/qwen_vl_example/qwen_vl_example.py (1)
39-39: Correct async context manager usage.The change to
async withproperly uses the async context manager pattern forAgent.join().plugins/openrouter/example/openrouter_example.py (1)
116-116: Correct async context manager usage.The update to
async withcorrectly handles the async context manager returned byAgent.join(), aligning with the broader refactoring in this PR.plugins/deepgram/example/deepgram_tts_example.py (1)
56-63: LGTM - Correct async context manager usage.The change from
with await agent.join(call):toasync with agent.join(call):correctly uses the async context manager pattern foragent.join(), which returns anAsyncIterator[None]. The cleanup logic (call close, tracing end, lock release) is properly handled in the context manager'sfinallyblock.plugins/huggingface/example/main.py (1)
54-62: LGTM - Consistent async context manager pattern.The update correctly adopts the async context manager syntax for joining calls. This aligns with the framework's lifecycle management where
agent.join()now properly manages entry/exit semantics asynchronously.plugins/vogent/example/vogent_example.py (1)
31-35: LGTM - Proper async context manager adoption.The change correctly reflects the updated
agent.join()API, ensuring the agent lifecycle (connection, cleanup, lock management) is properly handled through async context management.plugins/wizper/example/wizper_example.py (1)
71-73: LGTM - Async context manager correctly applied.The change properly uses the async context manager protocol for
agent.join(), ensuring proper resource cleanup and lifecycle management.agents-core/vision_agents/core/cli/cli_runner.py (2)
104-105: LGTM - Proper lifecycle initialization.The change from
launcher.warmup()tolauncher.start()correctly adopts the new lifecycle management wherestart()internally callswarmup()and additionally creates the idle agent cleanup task (_cleanup_idle_agents()). This aligns with the PR's objective of managing agent lifecycle and idle timeout.
138-139: LGTM - Ensures cleanup in all code paths.Adding the
finallyblock withawait launcher.stop()ensures the launcher is properly stopped and the cleanup task is cancelled regardless of whether the operation succeeds, fails, or is interrupted. Thestop()implementation safely handles being called even ifstart()never completed (it checksif self._cleanup_task:before cancelling).tests/test_agents/test_agent_launcher.py (1)
44-58: Tests cover key launcher lifecycle scenarios adequately.The test suite exercises warmup, launch, idle cleanup with timeout, zero-timeout behavior, and active agent preservation. The test logic correctly patches
idle_forto simulate idle vs. active states and uses appropriate async patterns.Also applies to: 60-74, 76-104, 106-133, 135-163
agents-core/vision_agents/core/agents/agent_launcher.py (4)
45-57: Validation and state initialization are well-structured.The validation for
agent_idle_timeout >= 0andagent_idle_cleanup_interval > 0with clear error messages is appropriate. Usingweakref.WeakSetfor tracking active agents ensures automatic cleanup when agents are garbage collected.
59-73: Lifecycle methods implement proper start/stop semantics.The
start()method correctly enforces single-start constraint and spawns the cleanup task after warmup. Thestop()method properly usescancel_and_waitto gracefully terminate the cleanup task. The debug logging aids troubleshooting.
150-178: Idle agent cleanup logic is robust with proper shielding.The
_cleanup_idle_agentsmethod correctly:
- Early-returns when idle timeout is disabled (
not self._agent_idle_timeout)- Uses
asyncio.shieldto protect close operations from cancellation- Handles exceptions per-agent with
return_exceptions=True- Logs failures without crashing the cleanup loop
180-184: Async context manager implementation is correct.The
__aenter__and__aexit__properly delegate tostart()andstop(), providing clean resource management for users.plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (3)
42-47: Idle tracking initialization is well-designed.The initialization of
_idle_sinceto0.0and use ofasyncio.Eventfor participant waiting provides a clean synchronization mechanism. Subscribing toparticipants_state.map()ensures reactive updates to participant changes.
53-67: Idle detection and participant waiting APIs are correctly implemented.The
idle_since()method returns0.0for active connections and a timestamp otherwise, matching the documented semantics. Thewait_for_participant()method properly wraps the event wait with an optional timeout.
82-95: Participant change handler correctly manages idle state.The
_on_participant_changemethod properly:
- Filters out the agent's own user_id when detecting other participants
- Resets
_idle_sinceto0.0when participants are present- Sets the event to signal waiting coroutines
- Only records idle timestamp when transitioning from active to idle (guard:
elif not self._idle_since)agents-core/vision_agents/core/agents/agents.py (9)
145-149: Agent user ID initialization is properly guarded.The initialization flag
_agent_user_initializedand the fallback to generate a UUID-based ID when none is provided ensures consistent agent identification.
246-254: New synchronization primitives properly initialized.The addition of
_call_ended_event,_joined_at,_join_lock,_close_lock, and_closedprovides the necessary state for coordinating the join/close lifecycle and preventing race conditions.
328-333: Call ended handler correctly triggers cleanup.The
on_call_endedhandler sets the event and callsclose(), ensuring proper cleanup when the call ends externally. This integrates well with thefinish()method which waits on the same event.
509-588: Async context manager join() implementation is well-structured.The refactored
join()method:
- Prevents double-join with
_call_ended_eventcheck- Uses
_join_lockfor serialization- Properly initializes tracing, user, MCP, and connection
- Sets up audio/video tracks before yielding
- Ensures cleanup in
finallyblock with proper lock releaseThe
participant_wait_timeoutparameter with default of10.0and special handling for0(skip wait) andNone(wait forever) is intuitive.
590-609: wait_for_participant handles timeout gracefully.The method logs when no participants join after timeout and continues rather than raising, which is appropriate for the default flow where the agent can proceed regardless.
611-632: idle_for calculation correctly adjusts for join time.The adjustment
max(idle_since, self._joined_at)ensures that time spent waiting for participants before actually joining is not counted as idle time. This prevents premature agent cleanup when using participant wait timeouts.
665-680: Dynamic method dispatch uses getattr pattern.The
_applymethod usesgetattrto dynamically invoke lifecycle methods on subclasses. While coding guidelines discouragegetattr, this is a common pattern for plugin-style architecture where method existence isn't guaranteed at compile time.Consider whether a protocol-based approach (defining explicit lifecycle interfaces) would be more maintainable than dynamic dispatch.
697-721: Close method uses shielded task for guaranteed cleanup.The pattern of wrapping
_close()in a shielded task ensures cleanup completes even ifclose()is cancelled. The re-raising ofCancelledErrorafter cleanup maintains proper cancellation semantics.
917-917: Audio consumer loop condition is correct.The condition
while self._call_ended_event and not self._call_ended_event.is_set()properly terminates when the call ends or when the event is not yet initialized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @agents-core/vision_agents/core/agents/agent_launcher.py:
- Around line 150-178: The _cleanup_idle_agents loop can terminate if operations
like agent.idle_for(), accessing agent.agent_user.id, asyncio.shield/gather, or
asyncio.sleep raise; wrap the main while self._running loop body in a try/except
that catches targeted exceptions (at minimum AttributeError and
asyncio.TimeoutError, optionally RuntimeError) to log the error via logger.error
including exc_info and continue the loop so the cleanup task doesn't stop;
ensure you still handle per-agent close results as before and do not swallow
exceptions from individual close() calls (keep existing return_exceptions
handling).
🧹 Nitpick comments (3)
agents-core/vision_agents/core/agents/agent_launcher.py (3)
30-37: Document theagent_idle_cleanup_intervalparameter.The docstring Args section is missing documentation for the
agent_idle_cleanup_intervalparameter added on line 28.📝 Proposed addition to docstring
Args: create_agent: A function that creates and returns an Agent instance join_call: Optional function that handles joining a call with the agent agent_idle_timeout: Optional timeout in seconds for agent to stay alone on the call. Default - `30.0`. `0` means idle agents won't leave the call until it's ended. + agent_idle_cleanup_interval: Interval in seconds between idle agent cleanup checks. Default - `5.0`.
82-83: Consider whether non-blocking warmup early-return is the desired behavior.When
self._warmup_lock.locked()is True, this returns immediately without waiting for the concurrent warmup to complete. Callers might expectawait warmup()to guarantee warmup is finished, but this early-return means they get control back while warmup is still in progress elsewhere.If all callers should wait for warmup completion, consider removing the
self._warmup_lock.locked()condition and letting concurrent callers await the lock naturally.
167-170: Redundant shield on individual close operations.Shielding the
gather()(lines 168-170) already protects all inner coroutines from cancellation, making the individualasyncio.shield(a.close())calls on line 167 redundant.♻️ Simplified shield usage
if idle_agents: - coros = [asyncio.shield(a.close()) for a in idle_agents] + coros = [a.close() for a in idle_agents] result = await asyncio.shield( asyncio.gather(*coros, return_exceptions=True) )
📜 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 (3)
agents-core/vision_agents/core/agents/agent_launcher.pypyproject.tomltests/test_agents/test_agent_launcher.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_agents/test_agent_launcher.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
agents-core/vision_agents/core/agents/agent_launcher.py
🧬 Code graph analysis (1)
agents-core/vision_agents/core/agents/agent_launcher.py (2)
agents-core/vision_agents/core/utils/utils.py (2)
await_or_run(92-102)cancel_and_wait(105-121)agents-core/vision_agents/core/warmup.py (4)
warmup(26-44)warmup(98-107)Warmable(47-107)WarmupCache(15-44)
⏰ 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). (8)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Mypy
- GitHub Check: unit / Ruff
- GitHub Check: unit / Ruff
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Mypy
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
🔇 Additional comments (1)
pyproject.toml (1)
32-33: Configuration is correct and verified.Both plugin directories exist with proper package structure (README.md, pyproject.toml, tests, and vision_agents modules), and the entries in pyproject.toml are correctly configured in both [tool.uv.sources] and [tool.uv.workspace].members sections.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @agents-core/pyproject.toml:
- Around line 91-92: Add individual optional-dependency entries for the two new
plugins so users can install them separately; in the
[project.optional-dependencies] section add entries named
"vision-agents-plugins-twilio" and "vision-agents-plugins-turbopuffer" with the
appropriate package lists (matching the packages referenced by those plugins)
and also ensure these keys are included in the "all-plugins" aggregated list
(they already are), following the same pattern used for existing plugins like
"vision-agents-plugins-twilio" and "vision-agents-plugins-turbopuffer"
counterparts elsewhere in the file.
📜 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 (1)
agents-core/pyproject.toml
⏰ 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). (8)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Mypy
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Mypy
examples/04_football_commentator_example/football_commentator_example.py
Show resolved
Hide resolved
|
Also remember to update visionagents.ai |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @agents-core/vision_agents/core/agents/agent_launcher.py:
- Around line 150-178: The _cleanup_idle_agents loop can fail if
agent.idle_for() or attribute access on agent.agent_user.id raises; wrap the
body of the while self._running loop (the logic that iterates
self._active_agents, computes agent_idle_for, appends to idle_agents, and shuts
them down) in a try/except that catches relevant exceptions (e.g.,
AttributeError, asyncio.TimeoutError, RuntimeError, Exception) and logs the
error with context (including agent identity when available) while continuing
the loop; ensure the await asyncio.sleep(self._agent_idle_cleanup_interval)
remains outside or in a finally clause so the cleanup loop keeps running even
after an error.
🧹 Nitpick comments (1)
agents-core/vision_agents/core/agents/agent_launcher.py (1)
167-169: Consider simplifying the shield usage.The code shields both individual
close()calls (line 167) and thegather(line 168-169). Sinceasyncio.shield(asyncio.gather(...))already prevents cancellation of the entire operation, the individual shields are redundant.♻️ Simplified approach
-coros = [asyncio.shield(a.close()) for a in idle_agents] +coros = [a.close() for a in idle_agents] result = await asyncio.shield( asyncio.gather(*coros, return_exceptions=True) )The outer shield is sufficient to protect all close operations from cancellation.
📜 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 (1)
agents-core/vision_agents/core/agents/agent_launcher.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Never adjust sys.path in Python code
Never writeexcept Exception as e- use specific exception handling
Avoid using getattr, hasattr, delattr and setattr; prefer normal attribute access in Python
Docstrings should follow the Google style guide for docstrings
Files:
agents-core/vision_agents/core/agents/agent_launcher.py
⏰ 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). (8)
- GitHub Check: unit / Ruff
- GitHub Check: unit / Mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Ruff
- GitHub Check: unit / Mypy
- GitHub Check: unit / Validate extra dependencies in "agents-core/pyproject.toml"
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (4)
agents-core/vision_agents/core/agents/agent_launcher.py (4)
27-37: LGTM! Configuration and validation are well-structured.The idle timeout configuration is clear and well-documented. The validation correctly enforces that
agent_idle_timeout >= 0(allowing 0 to disable cleanup) andagent_idle_cleanup_interval > 0(ensuring periodic checks happen). The docstring now accurately reflects the default of 60.0 seconds.Also applies to: 45-51
53-53: Excellent lifecycle management and agent tracking.The use of
weakref.WeakSetat line 53 prevents memory leaks by allowing agents to be garbage-collected when they close. The lifecycle methods provide clean startup/shutdown semantics, and the async context manager at lines 180-185 enables elegant resource management. The double-start guard at line 60-61 and the use ofcancel_and_waitat line 72 demonstrate careful attention to edge cases.Also applies to: 55-57, 59-73, 180-185
82-92: Well-implemented warmup guard.The guard at line 82 efficiently prevents redundant warmup operations. By checking both
_warmed_upandlocked(), it handles concurrent calls correctly: if the lock is held by another caller, subsequent callers return immediately without blocking. Setting_warmed_upat line 92 after successful warmup ensures future calls skip the expensive operation.
108-108: Correct placement of agent tracking.Adding the agent to
_active_agentsafter warmup ensures it's fully initialized before idle monitoring begins.
2 participants
What's changed
Main changes
AgentLaunchernow keeps track of the active agents and closes the idle ones after a specifiedagent_idle_timeout(default 30s).Agent.join()is complete (the agent joins the RTC connection earlier).Agent.join()now operates as an async context manager for improved resource management and cleanupAll examples have been updated to use the new API.
Other changes
Agent.wait_for_participant()now accepts a timeout (default 10s) to limit the waiting time.When timeout expires, it will proceed and join the call.
wait_for_participant()logic is moved toStreamConnectionsince it's tightly coupled withgetstreamimplementation.Agent.close()getting cancelled in the middle of a cleanup.elevenlabs.STTanddeepgram.STTcancelled in the middle of a cleanup, too.Agent.join()twilioandturbopufferintroduced in 0.2.7 along the waySummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.