fix: Wrapper layer: stub framework adapters, orchestrator-side version dispatch, and unconditional autoge#2086
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR implements the three issues from linked issue ChangesAutoGen Family Adapter Refactor and Generator Wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the framework adapter resolution mechanism, introducing an AutoGenFamilyAdapter to act as a router that selects the concrete AutoGen version (v2, v4, or AG2) based on environment variables and availability. It also adds a resolve_alias method to the base adapter interface and marks unimplemented adapters explicitly. The review feedback suggests raising a clear NotImplementedError instead of a misleading ImportError for unimplemented adapters, ensuring that autogen_v2 is included in the autogen-family checks for tool registration, and respecting explicit AUTOGEN_VERSION pins directly without silent fallbacks.
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.
Comment on lines
+234
to
+241
| # Get framework adapter first | ||
| self.framework_adapter = self._get_framework_adapter(framework) | ||
| if not self.framework_adapter.is_available(): | ||
| raise ImportError(f"Framework '{framework}' is not available. Please install the required dependencies.") | ||
|
|
||
| # Only autogen-family adapters need the autogen tool shim | ||
| if framework in {"autogen", "autogen_v4", "ag2"}: | ||
| self.tool_registry.register_builtin_autogen_adapters() |
Contributor
There was a problem hiding this comment.
When a framework is explicitly marked as unimplemented (such as autogen_v4 or ag2), calling is_available() returns False. However, raising a generic ImportError asking the user to install dependencies is misleading because the adapter itself is not implemented in the codebase.
Additionally, if framework is directly set to "autogen_v2", the built-in autogen adapters should still be registered.
Let's update this check to raise NotImplementedError for unimplemented adapters and include "autogen_v2" in the autogen-family check.
Suggested change
| # Get framework adapter first | |
| self.framework_adapter = self._get_framework_adapter(framework) | |
| if not self.framework_adapter.is_available(): | |
| raise ImportError(f"Framework '{framework}' is not available. Please install the required dependencies.") | |
| # Only autogen-family adapters need the autogen tool shim | |
| if framework in {"autogen", "autogen_v4", "ag2"}: | |
| self.tool_registry.register_builtin_autogen_adapters() | |
| # Get framework adapter first | |
| self.framework_adapter = self._get_framework_adapter(framework) | |
| if not self.framework_adapter.is_available(): | |
| if getattr(self.framework_adapter, "implemented", True) is False: | |
| raise NotImplementedError(f"Framework '{framework}' is not yet implemented in PraisonAI.") | |
| raise ImportError(f"Framework '{framework}' is not available. Please install the required dependencies.") | |
| # Only autogen-family adapters need the autogen tool shim | |
| if framework in {"autogen", "autogen_v2", "autogen_v4", "ag2"}: | |
| self.tool_registry.register_builtin_autogen_adapters() |
Comment on lines
+577
to
+579
| # Final availability check | ||
| if not adapter.is_available(): | ||
| raise ImportError(f"Framework '{framework}' is not available. Please install the required dependencies.") |
Contributor
There was a problem hiding this comment.
If the framework is resolved to an autogen-family framework (e.g., via YAML configuration or version resolution) during execution, we should ensure that the built-in autogen tool adapters are registered.
Also, we should handle the implemented check here to raise a clear NotImplementedError instead of a misleading ImportError when an unimplemented framework is selected.
# Final availability check
if not adapter.is_available():
if getattr(adapter, "implemented", True) is False:
raise NotImplementedError(f"Framework '{framework}' is not yet implemented in PraisonAI.")
raise ImportError(f"Framework '{framework}' is not available. Please install the required dependencies.")
# Only autogen-family adapters need the autogen tool shim
if framework in {"autogen", "autogen_v2", "autogen_v4", "ag2"}:
self.tool_registry.register_builtin_autogen_adapters()
Comment on lines
+197
to
+200
| if requested == "v0.2" and v2_available: | ||
| return "autogen_v2" | ||
| if requested == "v0.4" and v4_available: | ||
| return "autogen_v4" |
Contributor
There was a problem hiding this comment.
If the user explicitly pins AUTOGEN_VERSION to "v0.2" or "v0.4", we should respect that choice and dispatch to the corresponding adapter directly, even if it is currently unavailable or unimplemented. This ensures that the user receives a clear error message (e.g., NotImplementedError for v0.4) rather than silently falling back to v2.
Suggested change
| if requested == "v0.2" and v2_available: | |
| return "autogen_v2" | |
| if requested == "v0.4" and v4_available: | |
| return "autogen_v4" | |
| if requested == "v0.2": | |
| return "autogen_v2" | |
| if requested == "v0.4": | |
| return "autogen_v4" |
Greptile SummaryThis PR introduces a wrapper/routing layer for the AutoGen adapter family:
Confidence Score: 3/5The routing layer works correctly for the common path (framework=autogen, no AUTOGEN_VERSION env var), but two narrower paths produce incorrect behavior that will confuse users. The direct-access path (framework=autogen_v2) breaks whenever AUTOGEN_VERSION=v0.4 is set in the environment — the adapter raises ImportError claiming AutoGen is unavailable even when v0.2 is fully installed. Separately, the family adapter's resolve() accepts but never reads the YAML autogen_version field, silently discarding an explicit user preference. src/praisonai/praisonai/framework_adapters/autogen_adapter.py — AutoGenAdapter.resolve() and AutoGenFamilyAdapter.resolve() both need fixes before the v2 direct-access and YAML-config paths are reliable. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant O as Orchestrator (_prepare_for_run)
participant R as FrameworkAdapterRegistry
participant FA as AutoGenFamilyAdapter (name=autogen)
participant AA as AutoGenAdapter (name=autogen_v2)
participant V4 as AutoGenV4Adapter (stub)
Note over O,V4: framework=autogen path
O->>R: create(autogen)
R-->>O: AutoGenFamilyAdapter
O->>FA: "resolve(config=config)"
FA->>FA: resolve_alias() reads only AUTOGEN_VERSION env
FA-->>O: AutoGenAdapter (autogen_v2)
O->>AA: run(config, llm_config, topic)
Note over O,V4: framework=autogen_v2 path (direct)
O->>R: create(autogen_v2)
R-->>O: AutoGenAdapter
O->>AA: "resolve(config=config)"
AA->>AA: reads AUTOGEN_VERSION env
alt "AUTOGEN_VERSION=v0.4"
AA->>V4: is_available()
V4-->>AA: "False (implemented=False)"
AA-->>O: raises ImportError even if v0.2 is installed
else "AUTOGEN_VERSION=auto or v0.2"
AA-->>O: self (AutoGenAdapter)
end
O->>AA: run(config, llm_config, topic)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant O as Orchestrator (_prepare_for_run)
participant R as FrameworkAdapterRegistry
participant FA as AutoGenFamilyAdapter (name=autogen)
participant AA as AutoGenAdapter (name=autogen_v2)
participant V4 as AutoGenV4Adapter (stub)
Note over O,V4: framework=autogen path
O->>R: create(autogen)
R-->>O: AutoGenFamilyAdapter
O->>FA: "resolve(config=config)"
FA->>FA: resolve_alias() reads only AUTOGEN_VERSION env
FA-->>O: AutoGenAdapter (autogen_v2)
O->>AA: run(config, llm_config, topic)
Note over O,V4: framework=autogen_v2 path (direct)
O->>R: create(autogen_v2)
R-->>O: AutoGenAdapter
O->>AA: "resolve(config=config)"
AA->>AA: reads AUTOGEN_VERSION env
alt "AUTOGEN_VERSION=v0.4"
AA->>V4: is_available()
V4-->>AA: "False (implemented=False)"
AA-->>O: raises ImportError even if v0.2 is installed
else "AUTOGEN_VERSION=auto or v0.2"
AA-->>O: self (AutoGenAdapter)
end
O->>AA: run(config, llm_config, topic)
Reviews (5): Last reviewed commit: "fix: wrapper layer framework adapter iss..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
Copilot stopped work on behalf of
MervinPraison due to an error
June 19, 2026 22:38
Owner
Author
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. 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 |
Contributor
|
Claude finished @MervinPraison's task —— View job Architecture Review & Fix Implementation ✅Todo List:
Architecture ReviewProtocol-driven ✅: Correctly uses adapter pattern with protocol interface. Backward compatibility ✅: Fixed critical bugs that would have broken runtime. Performance ✅: No hot-path regressions, proper lazy imports maintained. Fixes AppliedFixed all critical issues identified by reviewers:
Files Modified
Final Verdict ✅All critical issues have been resolved. The PR is now ready for merge:
The wrapper layer now properly handles stub framework adapters, orchestrator-side version dispatch, and unconditional autogen tool registration. |
Owner
Author
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Maintainer triage (22 Jun 2026):
Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
Contributor
|
Claude encountered an error —— View job Architecture Review & Conflict Resolution
|
Owner
Author
|
Automated merge readiness report (local git rebase origin/main scan, 2026-06-22 22:04 UTC) GitHub status
CI (key jobs: test-core, smoke, test-windows, main (3.11), review)
Conflicts (git rebase origin/main)Conflict files (4):
|
Contributor
|
Claude finished @MervinPraison's task —— View job Architecture Review & Conflict Resolution ✅Todo List:
SummarySuccessfully rebased PR #2086 onto latest main, resolving all conflicts while preserving the critical fixes:
Architecture Compliance ✅
The PR is now ready for merge with all conflicts resolved and critical bugs fixed. |
praisonai-triage-agent
Bot
force-pushed
the
claude/issue-1590-20260430-0810
branch
from
1694029 to
55448b6
Compare
Contributor
|
Claude finished @MervinPraison's task —— View job Architecture Review & Conflict Resolution ✅Todo List:
SummarySuccessfully rebased PR #2086 onto latest main and fixed all critical issues identified by reviewers: Fixes Applied
Architecture Compliance✅ Protocol-driven: Adapter pattern with protocol interface properly maintained Files Modified
Final Verdict✅ PR is ready for merge. All critical issues have been resolved:
The wrapper layer now properly handles stub framework adapters, orchestrator-side version dispatch, and unconditional autogen tool registration. |
Owner
Author
|
@claude too many changes here, rebase, remove unwanted, and focus only on the changes that are asked to do so |
Contributor
|
Claude finished @MervinPraison's task —— View job Cleaning up PR #2086 ✅I've successfully cleaned up the PR to focus only on the essential changes for fixing issue #1590. Todo List:
Changes AppliedThe PR now contains only the minimal changes needed to fix the wrapper layer framework adapter issues:
SummaryThe PR has been reduced from 72 changed files to just 4 essential files:
Total changes: +146 insertions, -344 deletions (net reduction due to cleanup of stub implementations) The changes now properly address all three issues from #1590:
--- | |
…h, fail-fast stubs, conditional tool registration - Add resolve_alias() to FrameworkAdapter protocol for version selection - Add AutoGenFamilyAdapter as router for autogen version dispatch - Add resolve() override to properly create concrete adapter instances - Mark unimplemented adapters (v0.4, AG2) with NotImplementedError - Fix autogen_v2 dispatch by setting correct name attribute - Move autogen tool registration to be conditional on framework family - Include autogen_v2 in autogen family tool registration guard Fixes #1590 Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
praisonai-triage-agent
Bot
force-pushed
the
claude/issue-1590-20260430-0810
branch
from
0d5346e to
03d43a4
Compare
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1590
Auto-opened from
claude/issue-1590-20260430-0810.Summary by CodeRabbit
Release Notes
New Features
Improvements