-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: src/praisonai/praisonai: three concrete gaps in framework dispatch, async lifecycle, and tool resolu #2083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| from __future__ import annotations | ||
|
|
||
| from typing import Dict, Type, Optional | ||
| import inspect | ||
| import logging | ||
|
|
||
| from .base import FrameworkAdapter | ||
|
|
@@ -62,6 +63,28 @@ def __init__(self) -> None: | |
| entry_point_group="praisonai.framework_adapters", | ||
| builtins=_BUILTIN_ADAPTERS | ||
| ) | ||
|
|
||
| def _validate_adapter(self, name: str, adapter) -> None: | ||
| """Validate that adapter implements the required protocol signature.""" | ||
| _REQUIRED_KW = {"tools_dict", "agent_callback", "task_callback", "cli_config"} | ||
|
|
||
| sig = inspect.signature(type(adapter).run) | ||
| kw_only = { | ||
| p.name for p in sig.parameters.values() | ||
| if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD) | ||
| } | ||
| missing = _REQUIRED_KW - kw_only | ||
| if missing: | ||
| raise TypeError( | ||
| f"FrameworkAdapter {name!r} does not implement the protocol: " | ||
| f"missing keyword-only parameters {sorted(missing)}" | ||
|
Comment on lines
+71
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Demonstrate that the current inspect predicate accepts positional-or-keyword parameters.
# Expected: current check reports no missing params, while strict keyword-only validation flags all four.
python - <<'PY'
import inspect
_REQUIRED_KW = {"tools_dict", "agent_callback", "task_callback", "cli_config"}
class BadAdapter:
def run(
self,
config,
llm_config,
topic,
tools_dict=None,
agent_callback=None,
task_callback=None,
cli_config=None,
):
pass
sig = inspect.signature(BadAdapter.run)
accepted_by_current_check = {
p.name
for p in sig.parameters.values()
if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD)
}
not_keyword_only = {
name
for name in _REQUIRED_KW
if sig.parameters[name].kind != inspect.Parameter.KEYWORD_ONLY
}
print("missing under current check:", sorted(_REQUIRED_KW - accepted_by_current_check))
print("not keyword-only under strict check:", sorted(not_keyword_only))
PYRepository: MervinPraison/PraisonAI Length of output: 294 🏁 Script executed: cat -n src/praisonai/praisonai/framework_adapters/registry.py | sed -n '60,120p'Repository: MervinPraison/PraisonAI Length of output: 2721 🏁 Script executed: rg "_REQUIRED_KW" src/praisonai/praisonai/framework_adapters/registry.py -A 2 -B 2Repository: MervinPraison/PraisonAI Length of output: 651 🏁 Script executed: rg "class PluginRegistry" src/praisonai/ -A 20Repository: MervinPraison/PraisonAI Length of output: 1654 🏁 Script executed: rg "def create" src/praisonai/praisonai/plugin_base.py -A 15Repository: MervinPraison/PraisonAI Length of output: 240 🏁 Script executed: rg "def create" src/praisonai/praisonai/_registry.py -A 25Repository: MervinPraison/PraisonAI Length of output: 1785 🏁 Script executed: rg "ValueError\|TypeError" src/praisonai/praisonai/_registry.pyRepository: MervinPraison/PraisonAI Length of output: 161 🏁 Script executed: rg "def resolve" src/praisonai/praisonai/_registry.py -A 20Repository: MervinPraison/PraisonAI Length of output: 757 Enforce the exact keyword-only contract and distinguish protocol validation errors from constructor failures. Line 74 accepts Introduce a dedicated Proposed fix+class AdapterProtocolError(TypeError):
+ """Raised when a framework adapter violates the FrameworkAdapter protocol."""
+
+
class FrameworkAdapterRegistry(PluginRegistry[FrameworkAdapter]):
@@
- sig = inspect.signature(type(adapter).run)
- kw_only = {
- p.name for p in sig.parameters.values()
- if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD)
- }
- missing = _REQUIRED_KW - kw_only
- if missing:
- raise TypeError(
+ run = getattr(type(adapter), "run", None)
+ if run is None:
+ raise AdapterProtocolError(
+ f"FrameworkAdapter {name!r} does not implement the protocol: missing run()"
+ )
+
+ sig = inspect.signature(run)
+ params = sig.parameters
+ present = set(params)
+ missing = _REQUIRED_KW - present
+ not_keyword_only = {
+ param_name
+ for param_name in _REQUIRED_KW & present
+ if params[param_name].kind != inspect.Parameter.KEYWORD_ONLY
+ }
+ if missing or not_keyword_only:
+ problems = []
+ if missing:
+ problems.append(f"missing parameters {sorted(missing)}")
+ if not_keyword_only:
+ problems.append(f"not keyword-only parameters {sorted(not_keyword_only)}")
+ raise AdapterProtocolError(
f"FrameworkAdapter {name!r} does not implement the protocol: "
- f"missing keyword-only parameters {sorted(missing)}"
+ + "; ".join(problems)
)
@@
- except (ValueError, TypeError):
+ except (ValueError, AdapterProtocolError):
return False🤖 Prompt for AI Agents |
||
| ) | ||
|
|
||
| def create(self, name: str, *args, **kwargs): | ||
| """Create an adapter instance with protocol validation.""" | ||
| adapter = super().create(name, *args, **kwargs) | ||
| self._validate_adapter(name, adapter) | ||
| return adapter | ||
|
|
||
| # Backward compatibility aliases - delegate to parent methods | ||
| def list_registered(self) -> list[str]: | ||
|
|
@@ -85,7 +108,7 @@ def is_available(self, name: str) -> bool: | |
| """ | ||
| try: | ||
| adapter = self.create(name) | ||
| except ValueError: | ||
| except (ValueError, TypeError): | ||
| return False | ||
|
|
||
| try: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation strictly checks for
inspect.Parameter.KEYWORD_ONLY. However, parameters defined asPOSITIONAL_OR_KEYWORD(i.e., without the*separator in the function signature) can also be safely passed as keyword arguments. This strict check will cause false-positiveTypeErrors for perfectly valid custom adapters. Allowing bothKEYWORD_ONLYandPOSITIONAL_OR_KEYWORDmakes the validation more robust.