Merge pull request #137 from ai-agent-assembly/v0.0.1/AAASM-3087/refactor/sonar_code_smells

[AAASM-3087] 🧹 (python-sdk): Clear 8 CRITICAL smells + exclude generated proto

Author: Chisanan232

agent_assembly/__init__.py

@@ -10,28 +10,34 @@
 
 __version__ = "0.0.2a1"
 
+_MODULE_CORE = "agent_assembly.core"
+_MODULE_ADAPTERS = "agent_assembly.adapters"
+_MODULE_EXCEPTIONS = "agent_assembly.exceptions"
+_MODULE_TYPES = "agent_assembly.types"
+_MODULE_NATIVE_CORE = "agent_assembly._core"
+
 # AAASM-1696: top-level exports are resolved lazily so that lightweight
 # submodules (e.g. `agent_assembly.runtime`, which is stdlib-only) can be
 # imported without dragging in the SDK's third-party dependency surface
 # (`httpx`, `pydantic`, …). See PEP 562.
 _LAZY_EXPORTS: dict[str, str] = {
-    "init_assembly": "agent_assembly.core",
-    "AssemblyContext": "agent_assembly.core",
-    "GovernanceInterceptor": "agent_assembly.adapters",
-    "FrameworkAdapter": "agent_assembly.adapters",
-    "AssemblyError": "agent_assembly.exceptions",
-    "AgentError": "agent_assembly.exceptions",
-    "PolicyError": "agent_assembly.exceptions",
-    "GatewayError": "agent_assembly.exceptions",
-    "ConfigurationError": "agent_assembly.exceptions",
-    "AdapterValidationError": "agent_assembly.exceptions",
-    "ToolExecutionBlockedError": "agent_assembly.exceptions",
-    "MCPToolBlockedError": "agent_assembly.exceptions",
-    "AuditEvent": "agent_assembly.types",
-    "CallStackNode": "agent_assembly.types",
-    "CallStackNodeKind": "agent_assembly.types",
-    "GovernanceEvent": "agent_assembly._core",
-    "RuntimeClient": "agent_assembly._core",
+    "init_assembly": _MODULE_CORE,
+    "AssemblyContext": _MODULE_CORE,
+    "GovernanceInterceptor": _MODULE_ADAPTERS,
+    "FrameworkAdapter": _MODULE_ADAPTERS,
+    "AssemblyError": _MODULE_EXCEPTIONS,
+    "AgentError": _MODULE_EXCEPTIONS,
+    "PolicyError": _MODULE_EXCEPTIONS,
+    "GatewayError": _MODULE_EXCEPTIONS,
+    "ConfigurationError": _MODULE_EXCEPTIONS,
+    "AdapterValidationError": _MODULE_EXCEPTIONS,
+    "ToolExecutionBlockedError": _MODULE_EXCEPTIONS,
+    "MCPToolBlockedError": _MODULE_EXCEPTIONS,
+    "AuditEvent": _MODULE_TYPES,
+    "CallStackNode": _MODULE_TYPES,
+    "CallStackNodeKind": _MODULE_TYPES,
+    "GovernanceEvent": _MODULE_NATIVE_CORE,
+    "RuntimeClient": _MODULE_NATIVE_CORE,
 }
 
 _ALWAYS_EXPORTED: list[str] = [
@@ -60,10 +66,10 @@
 
 
 def _core_available() -> bool:
-    if "agent_assembly._core" in sys.modules:
+    if _MODULE_NATIVE_CORE in sys.modules:
         return True
     try:
-        return importlib.util.find_spec("agent_assembly._core") is not None
+        return importlib.util.find_spec(_MODULE_NATIVE_CORE) is not None
     except (ModuleNotFoundError, ValueError):
         return False
 
@@ -80,7 +86,7 @@ def __getattr__(name: str) -> Any:
     try:
         module = importlib.import_module(module_name)
     except ImportError:
-        if module_name == "agent_assembly._core":
+        if module_name == _MODULE_NATIVE_CORE:
             raise AttributeError(
                 f"module 'agent_assembly' has no attribute {name!r}: the native '_core' extension is not built"
             ) from None

agent_assembly/adapters/crewai/patch.py

@@ -135,29 +135,29 @@ def _get_thread_local_agent_id() -> str | None:
     return None
 
 
+def _nonempty_str_agent_id(source: Any) -> str | None:
+    """Return ``source["agent_id"]`` when ``source`` is a dict holding a non-empty str."""
+    if not isinstance(source, dict):
+        return None
+    value = source.get("agent_id")
+    if isinstance(value, str) and value:
+        return value
+    return None
+
+
 def _extract_agent_id_from_inputs(args: tuple[Any, ...], kwargs: dict[str, Any]) -> str | None:
-    direct_agent_id = kwargs.get("agent_id")
-    if isinstance(direct_agent_id, str) and direct_agent_id:
+    direct_agent_id = _nonempty_str_agent_id(kwargs)
+    if direct_agent_id is not None:
         return direct_agent_id
 
     config = kwargs.get("config")
     if isinstance(config, dict):
-        configurable = config.get("configurable")
-        if isinstance(configurable, dict):
-            configurable_agent_id = configurable.get("agent_id")
-            if isinstance(configurable_agent_id, str) and configurable_agent_id:
-                return configurable_agent_id
-
-        metadata = config.get("metadata")
-        if isinstance(metadata, dict):
-            metadata_agent_id = metadata.get("agent_id")
-            if isinstance(metadata_agent_id, str) and metadata_agent_id:
-                return metadata_agent_id
-
-    if args and isinstance(args[0], dict):
-        state_agent_id = args[0].get("agent_id")
-        if isinstance(state_agent_id, str) and state_agent_id:
-            return state_agent_id
+        nested = _nonempty_str_agent_id(config.get("configurable")) or _nonempty_str_agent_id(config.get("metadata"))
+        if nested is not None:
+            return nested
+
+    if args:
+        return _nonempty_str_agent_id(args[0])
 
     return None
 

agent_assembly/adapters/langgraph/patch.py

@@ -15,6 +15,7 @@
 _ORIGINAL_COMPILE = "_agent_assembly_original_compile"
 _NODE_WRAPPED_FLAG = "_agent_assembly_node_wrapped"
 _INVOKE_WRAPPED_FLAG = "_agent_assembly_invoke_wrapped"
+_GRAPH_INVOKE_NODE_NAME = "graph.invoke"
 
 # Thread-local storage for the name of the most-recently-completed node so that
 # the next _record_node_enter can emit a directed Messages edge between them.
@@ -308,69 +309,86 @@ def sync_subgraph_wrapper(*args: Any, **kwargs: Any) -> Any:
         return sync_subgraph_wrapper
 
 
+def _wrap_callable_node_executor(node_map: Any, node_name: Any, node_executor: Any, callback_handler: Any) -> bool:
+    """Wrap a directly-callable node executor in the node map. Return True if replaced."""
+    wrapped_executor = _make_assembly_node_wrapper(str(node_name), node_executor, callback_handler)
+    if wrapped_executor is node_executor:
+        return False
+    try:
+        node_map[node_name] = wrapped_executor
+    except Exception:
+        return False
+    return True
+
+
+def _wrap_subgraph_spawn_node(node_map: Any, node_name: Any, node_executor: Any, process_agent_id: str | None) -> bool:
+    """Wrap a compiled-subgraph node (spawn point) for lineage. Return True when wrapped."""
+    node_delegation_reason = f"langgraph_node:{node_name}"
+    sync_wrapper = _make_subgraph_spawn_wrapper(
+        str(node_name),
+        node_executor,
+        process_agent_id,
+        spawned_by_tool=None,
+        delegation_reason=node_delegation_reason,
+    )
+    with contextlib.suppress(Exception):
+        node_map[node_name] = sync_wrapper
+    if hasattr(node_executor, "ainvoke") and not getattr(node_executor, "_agent_assembly_ainvoke_spawned", False):
+        async_wrapper = _make_subgraph_spawn_wrapper(
+            str(node_name),
+            node_executor,
+            process_agent_id,
+            async_=True,
+            spawned_by_tool=None,
+            delegation_reason=node_delegation_reason,
+        )
+        node_executor.ainvoke = async_wrapper
+        node_executor._agent_assembly_ainvoke_spawned = True
+    return True
+
+
+def _wrap_node_invoke_methods(node_name: Any, node_executor: Any, callback_handler: Any) -> bool:
+    """Wrap a node executor's invoke/ainvoke methods. Return True if either was wrapped."""
+    wrapped_any = False
+    invoke = getattr(node_executor, "invoke", None)
+    if callable(invoke):
+        node_executor.invoke = _make_assembly_node_wrapper(str(node_name), invoke, callback_handler)
+        wrapped_any = True
+
+    ainvoke = getattr(node_executor, "ainvoke", None)
+    if callable(ainvoke):
+        node_executor.ainvoke = _make_assembly_node_wrapper(str(node_name), ainvoke, callback_handler)
+        wrapped_any = True
+    return wrapped_any
+
+
+def _wrap_node_entry(
+    node_map: Any, node_name: Any, node_executor: Any, callback_handler: Any, process_agent_id: str | None
+) -> bool:
+    """Dispatch a single (node_name, node_executor) entry to the right wrapper. Return True if wrapped."""
+    # ToolNode: intercept any compiled-subgraph tools it holds.
+    # Must come before the callable() check since ToolNode is also callable.
+    if _is_tool_node(node_executor):
+        return _wrap_tool_node_subgraphs(str(node_name), node_executor, process_agent_id)
+
+    if callable(node_executor):
+        return _wrap_callable_node_executor(node_map, node_name, node_executor, callback_handler)
+
+    # Spawn point: node is itself a compiled subgraph — wrap for lineage.
+    if _is_compiled_subgraph(node_executor):
+        return _wrap_subgraph_spawn_node(node_map, node_name, node_executor, process_agent_id)
+
+    return _wrap_node_invoke_methods(node_name, node_executor, callback_handler)
+
+
 def _wrap_node_map(node_map: Any, callback_handler: Any, process_agent_id: str | None = None) -> bool:
     items_method = getattr(node_map, "items", None)
     if not callable(items_method):
         return False
 
     wrapped_any = False
     for node_name, node_executor in list(items_method()):
-        # ToolNode: intercept any compiled-subgraph tools it holds.
-        # Must come before the callable() check since ToolNode is also callable.
-        if _is_tool_node(node_executor):
-            if _wrap_tool_node_subgraphs(str(node_name), node_executor, process_agent_id):
-                wrapped_any = True
-            continue
-
-        if callable(node_executor):
-            wrapped_executor = _make_assembly_node_wrapper(str(node_name), node_executor, callback_handler)
-            if wrapped_executor is node_executor:
-                continue
-            try:
-                node_map[node_name] = wrapped_executor
-            except Exception:
-                continue
-            wrapped_any = True
-            continue
-
-        # Spawn point: node is itself a compiled subgraph — wrap for lineage.
-        if _is_compiled_subgraph(node_executor):
-            node_delegation_reason = f"langgraph_node:{node_name}"
-            sync_wrapper = _make_subgraph_spawn_wrapper(
-                str(node_name),
-                node_executor,
-                process_agent_id,
-                spawned_by_tool=None,
-                delegation_reason=node_delegation_reason,
-            )
-            with contextlib.suppress(Exception):
-                node_map[node_name] = sync_wrapper
-            if hasattr(node_executor, "ainvoke") and not getattr(
-                node_executor, "_agent_assembly_ainvoke_spawned", False
-            ):
-                async_wrapper = _make_subgraph_spawn_wrapper(
-                    str(node_name),
-                    node_executor,
-                    process_agent_id,
-                    async_=True,
-                    spawned_by_tool=None,
-                    delegation_reason=node_delegation_reason,
-                )
-                node_executor.ainvoke = async_wrapper
-                node_executor._agent_assembly_ainvoke_spawned = True
-            wrapped_any = True
-            continue
-
-        invoke = getattr(node_executor, "invoke", None)
-        if callable(invoke):
-            wrapped_invoke = _make_assembly_node_wrapper(str(node_name), invoke, callback_handler)
-            node_executor.invoke = wrapped_invoke
-            wrapped_any = True
-
-        ainvoke = getattr(node_executor, "ainvoke", None)
-        if callable(ainvoke):
-            wrapped_ainvoke = _make_assembly_node_wrapper(str(node_name), ainvoke, callback_handler)
-            node_executor.ainvoke = wrapped_ainvoke
+        if _wrap_node_entry(node_map, node_name, node_executor, callback_handler, process_agent_id):
             wrapped_any = True
 
     return wrapped_any
@@ -432,11 +450,11 @@ def _wrap_graph_invoke_fallback(compiled_graph: Any, callback_handler: Any) -> N
         async def wrapped_async_invoke(*invoke_args: Any, **invoke_kwargs: Any) -> Any:
             state = _extract_state(invoke_args, invoke_kwargs)
             config = _extract_config(invoke_args, invoke_kwargs)
-            _record_node_enter(callback_handler, node_name="graph.invoke", state=state, config=config)
+            _record_node_enter(callback_handler, node_name=_GRAPH_INVOKE_NODE_NAME, state=state, config=config)
             result = await invoke(*invoke_args, **invoke_kwargs)
             _record_node_exit(
                 callback_handler,
-                node_name="graph.invoke",
+                node_name=_GRAPH_INVOKE_NODE_NAME,
                 previous_state=state,
                 next_state=result,
                 config=config,
@@ -449,11 +467,11 @@ async def wrapped_async_invoke(*invoke_args: Any, **invoke_kwargs: Any) -> Any:
         def wrapped_sync_invoke(*invoke_args: Any, **invoke_kwargs: Any) -> Any:
             state = _extract_state(invoke_args, invoke_kwargs)
             config = _extract_config(invoke_args, invoke_kwargs)
-            _record_node_enter(callback_handler, node_name="graph.invoke", state=state, config=config)
+            _record_node_enter(callback_handler, node_name=_GRAPH_INVOKE_NODE_NAME, state=state, config=config)
             result = invoke(*invoke_args, **invoke_kwargs)
             _record_node_exit(
                 callback_handler,
-                node_name="graph.invoke",
+                node_name=_GRAPH_INVOKE_NODE_NAME,
                 previous_state=state,
                 next_state=result,
                 config=config,

agent_assembly/adapters/openai_agents/patch.py

@@ -27,6 +27,7 @@
 _EDGE_EMITTER: Any = None
 _MAX_AUDIT_RESULT_CHARS = 2000
 _MAX_DELEGATION_REASON_CHARS = 256
+_OPENAI_AGENTS_MODULE = "openai.agents"
 
 
 def set_edge_emitter(emitter: Any) -> None:
@@ -74,7 +75,7 @@ def revert(self) -> None:
 
 
 def _is_openai_agents_available() -> bool:
-    return importlib.util.find_spec("openai.agents") is not None
+    return importlib.util.find_spec(_OPENAI_AGENTS_MODULE) is not None
 
 
 def set_process_agent_id(agent_id: str | None) -> None:
@@ -90,7 +91,7 @@ def _get_process_agent_id() -> str | None:
 
 def _load_openai_agents_function_tool_class() -> type[Any] | None:
     try:
-        module = importlib.import_module("openai.agents")
+        module = importlib.import_module(_OPENAI_AGENTS_MODULE)
     except ImportError:
         return None
 
@@ -102,7 +103,7 @@ def _load_openai_agents_function_tool_class() -> type[Any] | None:
 
 def _load_openai_agents_runner_class() -> type[Any] | None:
     try:
-        module = importlib.import_module("openai.agents")
+        module = importlib.import_module(_OPENAI_AGENTS_MODULE)
     except ImportError:
         return None
     runner_cls = getattr(module, "Runner", None)
@@ -113,7 +114,7 @@ def _load_openai_agents_runner_class() -> type[Any] | None:
 
 def _load_openai_agents_handoff_class() -> type[Any] | None:
     try:
-        module = importlib.import_module("openai.agents")
+        module = importlib.import_module(_OPENAI_AGENTS_MODULE)
     except ImportError:
         return None
     handoff_cls = getattr(module, "Handoff", None)
@@ -330,7 +331,7 @@ def _build_tool_result_error(
     is_pending_rejection: bool,
 ) -> object:
     try:
-        module = importlib.import_module("openai.agents")
+        module = importlib.import_module(_OPENAI_AGENTS_MODULE)
     except ImportError:
         module = None