Docs, discovery behavior, and dependency updates; add CONTRIBUTING.md#8
Conversation
📝 WalkthroughWalkthroughThis pull request consolidates OpenRTC's documentation, updates project dependencies to include Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
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📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openrtc/pool.py (1)
359-376:⚠️ Potential issue | 🔴 CriticalRemove the explicit
ctx.connect()call; AgentSession automatically connects via RoomIO whenroom=ctx.roomis passed.In livekit-agents,
AgentSessionconnects to LiveKit automatically duringstart()when passed theroomparameter. The explicitawait ctx.connect()on line 372 is redundant and violates the correct session lifecycle pattern. When using AgentSession withroom=ctx.room(as this code does), omit thectx.connect()call entirely—the framework handles it internally via RoomIO.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openrtc/pool.py` around lines 359 - 376, The explicit await ctx.connect() in _handle_session should be removed because AgentSession.start(..., room=ctx.room) already connects via RoomIO; update the _handle_session function to omit the ctx.connect() call (leave AgentSession(...) construction and await session.start(agent=config.agent_cls(), room=ctx.room) and the greeting logic intact) so the session lifecycle is handled solely by AgentSession/RoomIO.
🧹 Nitpick comments (2)
tests/test_discovery.py (1)
9-19: Minor: Template always importsagent_configeven when unused.The
_AGENT_TEMPLATEincludesfrom openrtc import agent_configeven when the decorator is not used (e.g., intest_discover_uses_filename_and_pool_defaults_without_decorator). This results in an unused import in the generated test modules, but it doesn't affect test correctness.💡 Optional: Conditionally include the import
_AGENT_TEMPLATE = """from __future__ import annotations from livekit.agents import Agent -from openrtc import agent_config {config_block} +{imports} {decorator}class {class_name}(Agent): def __init__(self) -> None: super().__init__(instructions={instructions!r}) """Then update
_write_agent_moduleto includeimports="from openrtc import agent_config\n"only when a decorator is specified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_discovery.py` around lines 9 - 19, The template always includes "from openrtc import agent_config" which produces an unused import when no decorator is provided; update the generation so the import is only added when a decorator is used: remove the hardcoded "from openrtc import agent_config" line from _AGENT_TEMPLATE and instead pass imports="from openrtc import agent_config\n" into _write_agent_module only when the {decorator} value is non-empty (or when a decorator flag is true), ensuring _write_agent_module receives the imports string conditionally and _AGENT_TEMPLATE only contains the necessary placeholders ({config_block}, {decorator}, {class_name}, {instructions!r}).src/openrtc/pool.py (1)
431-440: Unusedmoduleparameter after removing legacy metadata support.The
moduleparameter is no longer used in_resolve_discovery_metadatasince the legacyAGENT_*attribute reading was removed. The method now only checks for decorator metadata onagent_cls.♻️ Consider removing the unused parameter
def _resolve_discovery_metadata( self, - module: ModuleType, agent_cls: type[Agent], ) -> AgentDiscoveryConfig: metadata = getattr(agent_cls, _AGENT_METADATA_ATTR, None) if metadata is not None: return metadata return AgentDiscoveryConfig()And update the call site at line 240:
- metadata = self._resolve_discovery_metadata(module, agent_cls) + metadata = self._resolve_discovery_metadata(agent_cls)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openrtc/pool.py` around lines 431 - 440, The module parameter on _resolve_discovery_metadata is now unused; remove the module: ModuleType parameter from the function signature and its internal references, leaving def _resolve_discovery_metadata(self, agent_cls: type[Agent]) -> AgentDiscoveryConfig, and keep the existing logic that reads getattr(agent_cls, _AGENT_METADATA_ATTR, None) and returns AgentDiscoveryConfig() when missing; update every call site that passed a module argument (calls to _resolve_discovery_metadata) to call it with only agent_cls so the signature change is consistent across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/openrtc/pool.py`:
- Around line 359-376: The explicit await ctx.connect() in _handle_session
should be removed because AgentSession.start(..., room=ctx.room) already
connects via RoomIO; update the _handle_session function to omit the
ctx.connect() call (leave AgentSession(...) construction and await
session.start(agent=config.agent_cls(), room=ctx.room) and the greeting logic
intact) so the session lifecycle is handled solely by AgentSession/RoomIO.
---
Nitpick comments:
In `@src/openrtc/pool.py`:
- Around line 431-440: The module parameter on _resolve_discovery_metadata is
now unused; remove the module: ModuleType parameter from the function signature
and its internal references, leaving def _resolve_discovery_metadata(self,
agent_cls: type[Agent]) -> AgentDiscoveryConfig, and keep the existing logic
that reads getattr(agent_cls, _AGENT_METADATA_ATTR, None) and returns
AgentDiscoveryConfig() when missing; update every call site that passed a module
argument (calls to _resolve_discovery_metadata) to call it with only agent_cls
so the signature change is consistent across the codebase.
In `@tests/test_discovery.py`:
- Around line 9-19: The template always includes "from openrtc import
agent_config" which produces an unused import when no decorator is provided;
update the generation so the import is only added when a decorator is used:
remove the hardcoded "from openrtc import agent_config" line from
_AGENT_TEMPLATE and instead pass imports="from openrtc import agent_config\n"
into _write_agent_module only when the {decorator} value is non-empty (or when a
decorator flag is true), ensuring _write_agent_module receives the imports
string conditionally and _AGENT_TEMPLATE only contains the necessary
placeholders ({config_block}, {decorator}, {class_name}, {instructions!r}).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42b5efab-34c9-4a3c-8226-2feb87942b83
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
CONTRIBUTING.mdREADME.mdpyproject.tomlsrc/openrtc/pool.pytests/test_discovery.py
1 participant
Motivation
CONTRIBUTING.mdand expand the user-facing documentation inREADME.mdwith examples and behavioral details.livekit-agentsand optional extras, plus updated lockfile entries for reproducible installs.Description
CONTRIBUTING.mdwith local setup, commands, architecture notes, and contribution/testing expectations.README.mdto document purpose, quick-start examples forAgentPool.add()and discovery with@agent_config(...), routing rules, provider examples, CLI usage, and migration guidance.AgentPoolinsrc/openrtc/pool.pyto prefer decorator metadata for discovery, return an emptyAgentDiscoveryConfig()when no decorator is present (removing legacyAGENT_*module-level parsing), adjust docstrings, and remove_read_module_strhelper.tests/test_discovery.pyto reflect the new discovery behavior (filename fallback and pool defaults) and updated expectations.pyproject.tomlto addlivekit-agentsand optional extras, addpytest-asyncioto dev deps, and export package metadata, and replaceuv.lockwith a comprehensive regenerated lockfile including added dependencies and extras.Testing
uv run pytest(equivalent topytest) and the modified discovery tests intests/test_discovery.pypassed.uv run ruff checkand formatting where applicable; no blocking issues remained after changes.Codex Task
Summary by CodeRabbit
Release Notes
Documentation
Dependencies
livekit-agentsas a core dependency.Behavior Change
@agent_config(...)decorator; legacy module-level metadata configuration is no longer supported.