docs: update for PR #1780 wrapper-layer architecture changes#465
docs: update for PR #1780 wrapper-layer architecture changes#465MervinPraison wants to merge 2 commits into
Conversation
- Add backward-compat constants section to framework-availability.mdx - Expand lazy constants mention and add ToolRegistry deprecation note to thread-safety.mdx - Add lazy singleton implementation note to async-bridge.mdx - Create migration guide for deprecated ToolRegistry AutoGen methods - Update docs.json navigation for new migration page 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Mervin Praison <MervinPraison@users.noreply.github.com>
|
Warning Review limit reached
More reviews will be available in 49 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR updates documentation to explain the deprecation of AutoGen-specific ChangesDocumentation Updates for ToolRegistry AutoGen Deprecation and Framework Availability
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Review Summary by QodoDocument PR #1780 wrapper-layer architecture and AutoGen migration
WalkthroughsDescription• Document wrapper-layer architecture changes from PR #1780 • Add backward-compat constants section to framework availability docs • Create migration guide for deprecated ToolRegistry AutoGen methods • Update thread-safety docs with lazy constant resolution details • Add lazy singleton pattern note to async-bridge documentation Diagramflowchart LR
A["PR #1780 Changes"] --> B["Lazy Constants"]
A --> C["ToolRegistry Deprecation"]
B --> D["framework-availability.mdx"]
B --> E["thread-safety.mdx"]
B --> F["async-bridge.mdx"]
C --> G["tool-registry-autogen-migration.mdx"]
D --> H["Updated docs.json"]
G --> H
File Changes1. docs.json
|
Code Review by Qodo
1.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation to reflect changes regarding lazy loading of framework availability constants, the async bridge background loop, and the deprecation of AutoGen-specific methods in ToolRegistry in favor of the new framework adapter module. However, several discrepancies exist between the updated documentation and the actual codebase implementation. Specifically, the framework availability constants are eagerly evaluated rather than lazily loaded, the documented deprecation warnings are not actually emitted by ToolRegistry, and several referenced import paths, methods (such as register_tool), and functions (such as get_available_tools and register_builtin_tools) do not exist in the referenced adapter modules.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| For historical reasons, six module-level constants are exposed on | ||
| `praisonai.agents_generator`. They mirror `is_available(...)` for the | ||
| matching framework and are resolved lazily (PEP 562 `__getattr__`), | ||
| so import-time cost is zero. |
There was a problem hiding this comment.
The documentation states that these constants are resolved lazily via PEP 562 __getattr__. However, in praisonai/agents_generator.py (lines 33-38), these constants are eagerly evaluated at import time:\n\npython\nPRAISONAI_TOOLS_AVAILABLE = is_available("praisonai_tools")\nCREWAI_AVAILABLE = is_available("crewai")\n...\n\n\nThis eager evaluation defeats the lazy loading goal and incurs import-time overhead. Please update the implementation in praisonai/agents_generator.py to use PEP 562 __getattr__ or correct the documentation.
| **`inbuilt_tools` lazy import** (PR #1681) now routes through `praisonai.auto._load_optional("inbuilt_autogen_tools", ...)` instead of a hand-rolled re-entry guard. Negative results are cached, so a missing `crewai` or `autogen` install no longer pays the `find_spec` cost on every attribute access. `PRAISONAI_TOOLS_AVAILABLE` is now resolved lazily via `__getattr__`. | ||
| **`inbuilt_tools` lazy import** (PR #1681) now routes through `praisonai.auto._load_optional("inbuilt_autogen_tools", ...)` instead of a hand-rolled re-entry guard. Negative results are cached, so a missing `crewai` or `autogen` install no longer pays the `find_spec` cost on every attribute access. | ||
|
|
||
| **Framework availability constants** (PR #1780) — All six module-level constants (`PRAISONAI_TOOLS_AVAILABLE`, `CREWAI_AVAILABLE`, `AUTOGEN_AVAILABLE`, `AG2_AVAILABLE`, `PRAISONAI_AVAILABLE`, `AGENTOPS_AVAILABLE`) on `praisonai.agents_generator` are now resolved lazily via `__getattr__` (PEP 562). This eliminates import-time framework probing, significantly reducing startup overhead. See [Framework Availability → Backward-compat constants](/docs/features/framework-availability#backward-compat-constants-on-praisonaiagents_generator) for migration guidance. |
There was a problem hiding this comment.
The documentation states that these constants are resolved lazily via PEP 562 __getattr__. However, in praisonai/agents_generator.py (lines 33-38), these constants are eagerly evaluated at import time. This eager evaluation defeats the lazy loading goal and incurs import-time overhead. Please update the implementation in praisonai/agents_generator.py to use PEP 562 __getattr__ or correct the documentation.
| # These methods now emit DeprecationWarning | ||
| registry.register_autogen_adapter("my_tool", my_adapter) | ||
| adapter = registry.get_autogen_adapter("my_tool") | ||
| adapters = registry.list_autogen_adapters() | ||
| registry.register_builtin_autogen_adapters() |
There was a problem hiding this comment.
The documentation states that these methods emit DeprecationWarning. However, the implementation of ToolRegistry in praisonai/tool_registry.py does not raise or emit any DeprecationWarning for these methods. Please add the deprecation warnings to the implementation or correct the documentation.
|
|
||
| <Step title="After (recommended)"> | ||
| ```python | ||
| from praisonai.framework_adapters.autogen import AutoGenAdapter |
There was a problem hiding this comment.
The import path praisonai.framework_adapters.autogen is incorrect. The actual file is named autogen_adapter.py, so the import should be from praisonai.framework_adapters.autogen_adapter.
from praisonai.framework_adapters.autogen_adapter import AutoGenAdapter
| adapter = AutoGenAdapter() | ||
| adapter.register_tool("my_tool", my_tool_function) |
There was a problem hiding this comment.
AutoGenAdapter in praisonai/framework_adapters/autogen_adapter.py does not have a register_tool method. Please verify the correct API for registering tools with the new framework adapters and update the documentation accordingly.
| autogen_tools = registry.list_autogen_adapters() # DeprecationWarning | ||
|
|
||
| # After (recommended) | ||
| from praisonai.framework_adapters.autogen import get_available_tools |
There was a problem hiding this comment.
The function get_available_tools is imported from praisonai.framework_adapters.autogen, but it is not defined in praisonai/framework_adapters/autogen_adapter.py. Please verify if this function exists or if the import path is correct.
| registry.register_builtin_autogen_adapters() # DeprecationWarning | ||
|
|
||
| # After (recommended) | ||
| from praisonai.framework_adapters.autogen import register_builtin_tools |
There was a problem hiding this comment.
The function register_builtin_tools is imported from praisonai.framework_adapters.autogen, but it is not defined in praisonai/framework_adapters/autogen_adapter.py. Please verify if this function exists or if the import path is correct.
| ```python | ||
| import warnings | ||
|
|
||
| warnings.filterwarnings("ignore", category=DeprecationWarning, module="praisonai.tool_registry") |
There was a problem hiding this comment.
Since praisonai.tool_registry does not actually emit any DeprecationWarning, this warning filter is currently redundant and will have no effect. Consider adding the warnings to the codebase first.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/tool-registry-autogen-migration.mdx`:
- Around line 33-43: The Python snippets reference undefined symbols (e.g.,
my_adapter, my_tool, my_tool_function, my_function, old_function, new_function,
setup_tools, adapter); update each example so it is runnable by adding minimal
imports or stub definitions for those symbols and using the correct names where
shown (for example define a simple my_adapter function/class before calling
registry.register_autogen_adapter and assign adapter when demonstrating
registry.get_autogen_adapter), and similarly add small stub implementations for
my_tool_function, my_function, old_function/new_function, and a simple
setup_tools function in the other examples so every snippet can be executed
as-is without external edits.
- Around line 64-93: Add a missing "## Configuration Options" section under "How
It Works" that follows the standard template: insert the heading "Configuration
Options" and a table with columns "Option", "Type", "Default", "Description",
and populate it with SDK config options relevant to AutoGen integration (include
entries for any options referenced by register_autogen_adapter,
get_autogen_adapter, list_autogen_adapters, and
register_builtin_autogen_adapters such as adapter registration paths, enable
flags, and default adapter names or module paths), ensuring each row specifies
the option name, its type, default value, and a concise description.
- Around line 8-27: Insert a runnable, agent-centric code example at the very
top of this document that demonstrates how to register or use an AutoGen tool
from the agent perspective (showing agent creation, tool registration via the
AutoGen framework adapter, and a minimal run invocation), placed immediately
before the existing Mermaid diagram so readers see agent usage first; reference
the AutoGen helper symbols like ToolRegistry.register_autogen_adapter and
framework_adapters.autogen in the example and ensure the snippet is concise and
runnable (no prose between the code block and the diagram).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bdbe11c1-63eb-4900-8718-549d855da2f9
📒 Files selected for processing (5)
docs.jsondocs/features/async-bridge.mdxdocs/features/framework-availability.mdxdocs/features/thread-safety.mdxdocs/features/tool-registry-autogen-migration.mdx
There was a problem hiding this comment.
Pull request overview
Updates the documentation set to reflect wrapper-layer architecture changes introduced in upstream PR #1780 (lazy framework availability probes, async-bridge lazy singleton behavior, and ToolRegistry AutoGen deprecations), and adds a dedicated migration guide for deprecated AutoGen-specific ToolRegistry methods.
Changes:
- Added a new migration guide page for ToolRegistry AutoGen deprecations and linked it from navigation.
- Updated thread-safety and framework-availability docs to document lazy resolution of framework availability constants via
__getattr__(PEP 562). - Added an implementation note to the async-bridge docs describing lazy background loop creation and safe no-op shutdown behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/features/tool-registry-autogen-migration.mdx | New migration guide page describing AutoGen-specific ToolRegistry deprecations and recommended replacements |
| docs/features/thread-safety.mdx | Adds PR #1780 notes: lazy framework availability constants + ToolRegistry AutoGen deprecation notice |
| docs/features/framework-availability.mdx | Documents backward-compat, lazily-resolved module-level availability constants on praisonai.agents_generator |
| docs/features/async-bridge.mdx | Adds note about lazy singleton/background loop creation and safe no-op shutdown() |
| docs.json | Adds the new migration guide to the docs navigation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from praisonai.framework_adapters.autogen import AutoGenAdapter | ||
| from praisonai.tool_registry import ToolRegistry |
| # After (recommended) | ||
| from praisonai.framework_adapters.autogen import AutoGenAdapter | ||
|
|
||
| adapter = AutoGenAdapter() |
| # After (recommended) | ||
| from praisonai.framework_adapters.autogen import get_available_tools | ||
|
|
||
| autogen_tools = get_available_tools() | ||
| ``` |
| # After (recommended) | ||
| from praisonai.framework_adapters.autogen import register_builtin_tools | ||
|
|
||
| register_builtin_tools() | ||
| ``` |
| from praisonai.tool_registry import ToolRegistry | ||
| from praisonai.framework_adapters.autogen import AutoGenAdapter | ||
|
|
|
|
||
| `ToolRegistry` now holds a `threading.Lock` around all reads and mutations, matching `PluginRegistry` / integration registry. Eliminates `RuntimeError: dictionary changed size during iteration` when registering tools concurrently with iteration. Reference: [PR #1673](https://github.com/MervinPraison/PraisonAI/pull/1673). | ||
|
|
||
| As of PR #1780, four AutoGen-specific methods (`register_autogen_adapter`, `get_autogen_adapter`, `list_autogen_adapters`, `register_builtin_autogen_adapters`) now emit `DeprecationWarning` and will be removed in a future version. AutoGen-specific functionality has been moved to the `framework_adapters.autogen` module. See [ToolRegistry AutoGen Migration](/docs/features/tool-registry-autogen-migration) for replacement patterns. |
| A[🔧 ToolRegistry.register_autogen_adapter] --> B[⚠️ DeprecationWarning] | ||
| B --> C[📦 framework_adapters.autogen] | ||
| D[🔧 Direct ToolRegistry methods] --> E[✅ Still Available] |
| User->>ToolRegistry: register_autogen_adapter() [DEPRECATED] | ||
| ToolRegistry-->>User: DeprecationWarning | ||
| ToolRegistry->>FrameworkAdapter: Forward to framework_adapters.autogen | ||
| FrameworkAdapter->>AutoGen: Configure AutoGen tools |
| | `register_autogen_adapter(tool_type_name, adapter)` | **Deprecated** | Use `framework_adapters.autogen` module | | ||
| | `get_autogen_adapter(tool_type_name)` | **Deprecated** | Use `framework_adapters.autogen` module | | ||
| | `list_autogen_adapters()` | **Deprecated** | Use `framework_adapters.autogen` module | | ||
| | `register_builtin_autogen_adapters()` | **Deprecated** | Use `framework_adapters.autogen` module | |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above first. Review areas:
|
|
@claude You are the FINAL architecture reviewer. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Final Architecture Review & Issue ResolutionPhase 1: Architecture Review per AGENTS.md
Phase 2: Fix Valid Issues
Phase 3: Final Verdict
✅ Final Verdict: APPROVED with Fixes AppliedAll valid issues identified by reviewers have been resolved. The documentation now accurately reflects the actual SDK implementation. Summary of Changes Applied1. Fixed
|
- Remove false lazy loading claims in framework-availability.mdx - Correct import paths from nonexistent praisonai.framework_adapters.autogen to praisonai.framework_adapters - Fix thread-safety.mdx deprecation warning claims to match actual SDK - Completely rewrite tool-registry-autogen-migration.mdx with accurate info - Add proper runnable code examples with all imports - Add missing Configuration Options section per AGENTS.md - Fix section intro formatting to one sentence per AGENTS.md - Add missing EOF newlines - Update all references to match actual AutoGenAdapter class location 🤖 Generated with Claude Code Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Completed a full pass over the current PR state (including all prior reviewer threads) and validated docs build locally (
from praisonai.agents_generator import AgentsGenerator
from praisonai.framework_adapters import AutoGenAdapter
# from praisonai.tool_registry import ToolRegistry # remove if unused
from praisonai.framework_adapters import AutoGenAdapter
adapter = AutoGenAdapter()
print(adapter.list_tools())If you want, I can apply these two doc-polish changes in a tiny follow-up commit. |
Fixes #462
Updates documentation to reflect the wrapper-layer architecture changes from PR #1780, including:
Changes Made
1. Framework Availability Constants (docs/features/framework-availability.mdx)
2. Thread Safety Updates (docs/features/thread-safety.mdx)
3. Async Bridge Implementation Note (docs/features/async-bridge.mdx)
4. New Migration Guide (docs/features/tool-registry-autogen-migration.mdx)
5. Navigation Updates (docs.json)
All changes follow AGENTS.md documentation standards and maintain the agent-centric, beginner-friendly approach.
Generated with Claude Code
Summary by CodeRabbit