Skip to content

Fix/module pickle import#16

Merged
mahimairaja merged 7 commits intomainfrom
fix/module-pickle-import
Mar 21, 2026
Merged

Fix/module pickle import#16
mahimairaja merged 7 commits intomainfrom
fix/module-pickle-import

Conversation

@mahimairaja
Copy link
Copy Markdown
Collaborator

@mahimairaja mahimairaja commented Mar 21, 2026

Summary by CodeRabbit

  • Documentation
    • Clarified agent discovery works with livekit dev (including spawn-based worker runtimes) and guidance that agents registered directly must be defined at module scope.
  • New Features
    • Example now loads environment variables from a .env file.
  • Chores
    • Added python-dotenv to dev dependencies.
  • Tests
    • Added tests for agent discovery/serialization and class validation.

@mahimairaja mahimairaja self-assigned this Mar 21, 2026
@mahimairaja mahimairaja added the bug Something isn't working label Mar 21, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 21, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ff538234-2a97-4b8e-9197-4e7b1d6833b8

📥 Commits

Reviewing files that changed from the base of the PR and between 5305698 and 684fea2.

📒 Files selected for processing (2)
  • src/openrtc/pool.py
  • tests/test_pool.py

📝 Walkthrough

Walkthrough

This PR makes discovered agent configs pickle-safe across process boundaries by storing resolvable class references, reworks module loading to support file-backed discovery, adds validation to reject worker-unsafe agent classes, updates docs and examples (dotenv), and adds tests covering serialization and validation.

Changes

Cohort / File(s) Summary
Documentation
README.md, docs/api/pool.md
Clarified that discovered agents work with livekit dev (file-backed loading) and that AgentPool.add() requires module-scope Agent subclasses for spawn-based runtimes.
Core: agent loading & serialization
src/openrtc/pool.py
Added _AgentClassRef; changed AgentConfig pickling (__getstate__/__setstate__) to use class refs; introduced deterministic discovered module naming, _load_module_from_path, _resolve_agent_class, and stricter validation rejecting <locals> and unsafe __main__ classes.
Examples & deps
examples/main.py, pyproject.toml
Removed from __future__ import annotations in example, added python-dotenv usage via load_dotenv() and added python-dotenv>=1.2.2 to dev deps.
Tests
tests/test_discovery.py, tests/test_pool.py
Added tests ensuring discovered AgentConfig is pickle-serializable and restorable after module unload; added tests for _load_module_from_path, _resolve_agent_class, _try_get_module_path, and AgentPool.add() rejection rules.

Sequence Diagram(s)

sequenceDiagram
    participant Discover as Discovery CLI
    participant Pool as AgentPool
    participant Config as AgentConfig
    participant Loader as Module Loader
    participant Worker as Worker Process

    Discover->>Pool: discover(paths)
    activate Pool
    Pool->>Loader: _load_agent_module(file_path)
    Loader->>Loader: _load_module_from_path(resolved_path)
    Loader-->>Pool: module
    Pool->>Config: create AgentConfig(agent_cls, ...)
    activate Config
    Config->>Config: __post_init__ builds _AgentClassRef
    Config-->>Pool: AgentConfig
    deactivate Config
    Pool-->>Discover: return AgentConfig list
    deactivate Pool

    Discover->>Worker: pickle(AgentConfig)
    activate Worker
    Worker->>Worker: unpickle -> __setstate__
    Worker->>Loader: _resolve_agent_class(from _AgentClassRef)
    Loader->>Loader: import or load module from path
    Loader-->>Worker: agent_cls
    Worker->>Worker: restore AgentConfig.agent_cls
    deactivate Worker
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

codex

Poem

🐰 I hopped through modules, sniffed each file and name,
Packed class-refs snug to cross the worker's lane,
No locals left behind, no main left adrift,
Pickled and restored — a neat, resilient gift,
A carrot-coded hop toward modular fame! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix/module pickle import' is vague and generic, using non-descriptive terminology that doesn't clearly convey what the changeset addresses. Consider a more specific title that describes the actual change, such as 'Enable agent discovery to work with spawn-based workers and add module-scope validation for AgentPool.add()' or 'Implement serializable agent class references for worker process compatibility'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/module-pickle-import

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 98.79518% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.64%. Comparing base (c7db36a) to head (684fea2).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/openrtc/pool.py 98.79% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   88.05%   90.64%   +2.58%     
==========================================
  Files           3        3              
  Lines         268      342      +74     
==========================================
+ Hits          236      310      +74     
  Misses         32       32              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openrtc/pool.py`:
- Around line 1-5: Ruff flagged formatting issues in the module imports at the
top of src/openrtc/pool.py; run the formatter (e.g., ruff format
src/openrtc/pool.py) and commit the resulting changes so imports (from
__future__ and standard lib imports like importlib, importlib.util, inspect) are
properly grouped, sorted, and the file conforms to ruff's style (trailing
newline, spacing, and any other minor reformatting).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09cba6fc-c729-41b4-bd60-b6293aeeb4f8

📥 Commits

Reviewing files that changed from the base of the PR and between c7db36a and 5305698.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • README.md
  • docs/api/pool.md
  • examples/main.py
  • pyproject.toml
  • src/openrtc/pool.py
  • tests/test_discovery.py
  • tests/test_pool.py

Comment thread src/openrtc/pool.py
@mahimairaja mahimairaja merged commit 0a9c9c4 into main Mar 21, 2026
8 of 9 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Mar 21, 2026
@mahimairaja mahimairaja deleted the fix/module-pickle-import branch March 23, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant