Skip to content

Commit 8b917da

Browse files
fix: wrapper layer gaps - async path observability, sync/async duplication, tool caching (fixes #1860)
- Gap 1: Extract shared _prepare_adapter() method used by both sync/async paths Ensures observability init and adapter setup are consistent between code paths - Gap 2: Remove duplicate arun() method definitions in Protocol and BaseFrameworkAdapter - Gap 3: Fix ToolResolver to distinguish cacheable vs non-cacheable failures Transient import errors no longer permanently cached, allowing deps installed later Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
1 parent a9f4bd5 commit 8b917da

3 files changed

Lines changed: 74 additions & 131 deletions

File tree

src/praisonai/praisonai/agents_generator.py

Lines changed: 44 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,47 @@ def _get_framework_adapter(self, framework: str) -> FrameworkAdapter:
253253
"""
254254
return self._adapter_registry.create(framework)
255255

256+
def _prepare_adapter(self, framework: str, config: dict, tools_dict: dict):
257+
"""
258+
Single source of truth for adapter resolution + setup + observability.
259+
260+
Called by both sync and async entry points so they cannot drift.
261+
262+
Args:
263+
framework: Framework name to prepare
264+
config: Configuration dictionary
265+
tools_dict: Tools dictionary
266+
267+
Returns:
268+
Prepared and configured framework adapter
269+
"""
270+
initial_adapter = self._get_framework_adapter(framework)
271+
adapter = initial_adapter.resolve() # autogen v0.2/v0.4, etc.
272+
273+
from .framework_adapters.validators import assert_framework_available
274+
assert_framework_available(adapter.name)
275+
276+
from .observability.hooks import init_observability
277+
init_observability(adapter.name)
278+
279+
adapter.setup(framework_tag=adapter.name)
280+
281+
self._validate_cli_backend_compatibility(config, adapter.name)
282+
283+
# AgentOps init lives here too -- once, not in two places.
284+
api_key = os.getenv("AGENTOPS_API_KEY")
285+
if api_key:
286+
try:
287+
import agentops
288+
agentops.init(api_key, default_tags=[adapter.name])
289+
except ImportError:
290+
pass
291+
292+
self.framework = adapter.name
293+
self.framework_adapter = adapter
294+
self.logger.info(f"Using framework: {adapter.name}")
295+
return adapter
296+
256297
def _merge_cli_config(self, config, cli_config):
257298
"""
258299
Merge CLI configuration with YAML configuration.
@@ -594,27 +635,7 @@ def generate_crew_and_kickoff(self):
594635
self.logger.debug("tools folder exists in the root directory")
595636

596637
framework = self.framework or config.get('framework', 'crewai')
597-
598-
# Get initial adapter and resolve to concrete variant
599-
initial_adapter = self._get_framework_adapter(framework)
600-
adapter = initial_adapter.resolve()
601-
602-
# Validate framework availability early
603-
from .framework_adapters.validators import assert_framework_available
604-
assert_framework_available(adapter.name)
605-
606-
# Initialize observability hooks
607-
from .observability.hooks import init_observability
608-
init_observability(adapter.name)
609-
610-
# Run adapter setup hooks
611-
adapter.setup(framework_tag=adapter.name)
612-
613-
# Update framework reference if resolution changed it
614-
self.framework = adapter.name
615-
self.framework_adapter = adapter
616-
617-
self.logger.info(f"Using framework: {adapter.name}")
638+
adapter = self._prepare_adapter(framework, config, tools_dict)
618639
return adapter.run(
619640
config,
620641
self.config_list,
@@ -740,51 +761,8 @@ async def _arun_framework(self, config):
740761
self.logger.debug("tools folder exists in the root directory")
741762

742763
framework = self.framework or config.get('framework', 'crewai')
743-
744-
# AutoGen version selection logic
745-
if framework == "autogen":
746-
autogen_v4_adapter = self._get_framework_adapter("autogen_v4")
747-
autogen_v2_adapter = self._get_framework_adapter("autogen")
748-
749-
autogen_version = str(
750-
config.get('autogen_version', os.environ.get("AUTOGEN_VERSION", "auto"))
751-
).lower()
752-
use_v4 = False
753-
754-
if autogen_version == "v0.4" and autogen_v4_adapter.is_available():
755-
use_v4 = True
756-
elif autogen_version == "v0.2" and autogen_v2_adapter.is_available():
757-
use_v4 = False
758-
elif autogen_version == "auto":
759-
use_v4 = autogen_v4_adapter.is_available()
760-
else:
761-
use_v4 = autogen_v4_adapter.is_available() and not autogen_v2_adapter.is_available()
762-
763-
framework = "autogen_v4" if use_v4 else "autogen"
764-
765-
# Initialize AgentOps if configured
766-
agentops_api_key = os.getenv("AGENTOPS_API_KEY")
767-
if agentops_api_key:
768-
try:
769-
import agentops
770-
agentops.init(agentops_api_key, default_tags=[framework])
771-
except ImportError:
772-
pass
773-
774-
# Update framework adapter if framework changed
775-
if framework != self.framework:
776-
self.framework = framework
777-
self.framework_adapter = self._get_framework_adapter(framework)
778-
779-
# Validate framework availability
780-
from .framework_adapters.validators import assert_framework_available
781-
assert_framework_available(framework)
782-
783-
# Validate cli_backend compatibility
784-
self._validate_cli_backend_compatibility(config, framework)
785-
786-
self.logger.info(f"Using framework: {framework}")
787-
return await self.framework_adapter.arun(
764+
adapter = self._prepare_adapter(framework, config, tools_dict)
765+
return await adapter.arun(
788766
config,
789767
self.config_list,
790768
topic,

src/praisonai/praisonai/framework_adapters/base.py

Lines changed: 3 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -64,34 +64,6 @@ def run(
6464
"""
6565
...
6666

67-
async def arun(
68-
self,
69-
config: Dict[str, Any],
70-
llm_config: List[Dict],
71-
topic: str,
72-
*,
73-
tools_dict: Optional[Dict[str, Any]] = None,
74-
agent_callback: Optional[Callable] = None,
75-
task_callback: Optional[Callable] = None,
76-
cli_config: Optional[Dict[str, Any]] = None,
77-
) -> str:
78-
"""
79-
Async-native execution. Default = offload sync run() to a thread.
80-
81-
Args:
82-
config: Framework configuration
83-
llm_config: LLM configuration list
84-
topic: Topic for the tasks
85-
tools_dict: Available tools dictionary
86-
agent_callback: Callback for agent events
87-
task_callback: Callback for task events
88-
cli_config: CLI configuration
89-
90-
Returns:
91-
Execution result as string
92-
"""
93-
...
94-
9567
async def arun(
9668
self,
9769
config: Dict[str, Any],
@@ -157,29 +129,6 @@ def _sub(m):
157129
# Only substitute simple variable names like {topic}, not JSON like {"level":2}
158130
return re.sub(r'\{([a-zA-Z_][a-zA-Z0-9_]*)\}', _sub, template)
159131

160-
async def arun(
161-
self,
162-
config: Dict[str, Any],
163-
llm_config: List[Dict],
164-
topic: str,
165-
*,
166-
tools_dict: Optional[Dict[str, Any]] = None,
167-
agent_callback: Optional[Callable] = None,
168-
task_callback: Optional[Callable] = None,
169-
cli_config: Optional[Dict[str, Any]] = None,
170-
) -> str:
171-
"""
172-
Default async implementation that falls back to thread-offloaded sync.
173-
174-
Framework adapters with native async support should override this method.
175-
"""
176-
import asyncio
177-
return await asyncio.to_thread(
178-
self.run, config, llm_config, topic,
179-
tools_dict=tools_dict, agent_callback=agent_callback,
180-
task_callback=task_callback, cli_config=cli_config
181-
)
182-
183132
def resolve(self) -> "FrameworkAdapter":
184133
"""Default implementation returns self."""
185134
return self
@@ -200,8 +149,9 @@ async def arun(
200149
cli_config: Optional[Dict[str, Any]] = None,
201150
) -> str:
202151
"""
203-
Safe default for sync-only adapters (crewai, autogen v0.2):
204-
run the sync implementation in a worker thread, freeing the loop.
152+
Safe default: run sync implementation in a worker thread.
153+
154+
Framework adapters with native async support should override this method.
205155
"""
206156
import asyncio
207157
return await asyncio.to_thread(

src/praisonai/praisonai/tool_resolver.py

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@
3434

3535
logger = logging.getLogger(__name__)
3636

37+
38+
class _ResolveResult:
39+
"""Internal result wrapper to distinguish cacheable vs non-cacheable failures."""
40+
__slots__ = ("tool", "cacheable")
41+
42+
def __init__(self, tool, cacheable=True):
43+
self.tool = tool
44+
self.cacheable = cacheable
45+
46+
3747
# Sentinel for cache - needed because None is a valid cached result (tool not found)
3848
_SENTINEL = object()
3949

@@ -123,7 +133,7 @@ def _load_local_tools(self) -> Mapping[str, Callable]:
123133
self._local_tools_loaded = True
124134
return self._local_tools_cache
125135

126-
def _resolve_from_praisonaiagents(self, name: str) -> Optional[Callable]:
136+
def _resolve_from_praisonaiagents(self, name: str) -> _ResolveResult:
127137
"""Resolve tool from praisonaiagents.tools.TOOL_MAPPINGS.
128138
129139
Uses lazy loading via __getattr__ in praisonaiagents.tools.
@@ -132,7 +142,7 @@ def _resolve_from_praisonaiagents(self, name: str) -> Optional[Callable]:
132142
name: Tool name to resolve
133143
134144
Returns:
135-
Callable if found, None otherwise
145+
_ResolveResult with tool and cacheable flag
136146
"""
137147
try:
138148
from praisonaiagents import tools as agent_tools
@@ -149,25 +159,29 @@ def _resolve_from_praisonaiagents(self, name: str) -> Optional[Callable]:
149159
logger.warning(
150160
f"Tool '{name}' exists in TOOL_MAPPINGS but failed to load: {e}"
151161
)
152-
return None
162+
# IMPORTANT: do NOT cache. The dep may be installed later.
163+
return _ResolveResult(None, cacheable=False)
153164
if tool is not None:
154165
logger.debug(f"Resolved '{name}' from praisonaiagents.tools")
155-
return tool
166+
return _ResolveResult(tool)
156167

157168
# Also try direct attribute access (for non-TOOL_MAPPINGS items)
158169
tool = getattr(agent_tools, name, None)
159170
if tool is not None and callable(tool):
160171
logger.debug(f"Resolved '{name}' from praisonaiagents.tools (direct)")
161-
return tool
172+
return _ResolveResult(tool)
162173

163174
except ImportError:
164175
logger.debug("praisonaiagents not available")
176+
# SDK can be installed later
177+
return _ResolveResult(None, cacheable=False)
165178
except AttributeError:
166179
pass
167180
except Exception as e:
168181
logger.debug(f"Error resolving '{name}' from praisonaiagents: {e}")
169182

170-
return None
183+
# Genuinely not present
184+
return _ResolveResult(None)
171185

172186
def _resolve_from_praisonai_tools(self, name: str) -> Optional[Callable]:
173187
"""Resolve tool from praisonai-tools package (external).
@@ -304,12 +318,13 @@ def resolve(self, name: str, instantiate: bool = False) -> Optional[Callable]:
304318
return tool
305319

306320
# 3. Check praisonaiagents.tools
307-
tool = self._resolve_from_praisonaiagents(name)
308-
if tool is not None:
309-
self._resolve_cache[name] = tool
310-
if instantiate and self._is_class(tool):
311-
return tool()
312-
return tool
321+
result = self._resolve_from_praisonaiagents(name)
322+
if result.tool is not None:
323+
if result.cacheable:
324+
self._resolve_cache[name] = result.tool
325+
if instantiate and self._is_class(result.tool):
326+
return result.tool()
327+
return result.tool
313328

314329
# 4. Check praisonai-tools package
315330
tool = self._resolve_from_praisonai_tools(name)

0 commit comments

Comments
 (0)