Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 34 additions & 34 deletions src/praisonai-agents/praisonaiagents/agent/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,14 @@ def _hook_runner(self):
return self.__hook_runner

@property
def stream_emitter(self):
def stream_emitter(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. A more specific type StreamEventEmitter is available, as it's being instantiated in this property. Using a forward reference Optional['StreamEventEmitter'] is recommended to align with the lazy loading pattern used in this file. You'll also need to add from ..streaming.events import StreamEventEmitter inside a TYPE_CHECKING block.

Suggested change
def stream_emitter(self) -> Optional[Any]:
def stream_emitter(self) -> Optional['StreamEventEmitter']:

"""Lazy-loaded StreamEventEmitter for real-time events (zero overhead when not used)."""
if self.__stream_emitter is None:
self.__stream_emitter = _get_stream_emitter()()
return self.__stream_emitter

@stream_emitter.setter
def stream_emitter(self, value):
def stream_emitter(self, value: Optional[Any]) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. A more specific type StreamEventEmitter is available. Using a forward reference Optional['StreamEventEmitter'] is recommended to align with the lazy loading pattern used in this file. You'll also need to add from ..streaming.events import StreamEventEmitter inside a TYPE_CHECKING block.

Suggested change
def stream_emitter(self, value: Optional[Any]) -> None:
def stream_emitter(self, value: Optional['StreamEventEmitter']) -> None:

"""Allow setting stream_emitter directly."""
self.__stream_emitter = value

Expand Down Expand Up @@ -1762,57 +1762,57 @@ def _cache_lock(self):
return self.__cache_lock

@property
def auto_memory(self):
def auto_memory(self) -> Optional[bool]:
"""AutoMemory instance for automatic memory extraction."""
return self._auto_memory

@auto_memory.setter
def auto_memory(self, value):
def auto_memory(self, value: Optional[bool]) -> None:
self._auto_memory = value

@property
def policy(self):
def policy(self) -> Optional[Any]:
"""PolicyEngine instance for execution control."""
return self._policy

@policy.setter
def policy(self, value):
def policy(self, value: Optional[Any]) -> None:
self._policy = value

@property
def background(self):
def background(self) -> Optional[bool]:
"""BackgroundRunner instance for async task execution."""
return self._background

Comment on lines 1782 to 1786
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background is annotated as bool, but the backing field self._background is set from a local background = None and (in this file) is never populated from any config/param. This means the property can be None at runtime, conflicting with the annotation. Consider making this Optional[bool] (or implement/initialize the intended background runner/flag) and update the docstring (it currently claims it is a BackgroundRunner instance).

Copilot uses AI. Check for mistakes.
@background.setter
def background(self, value):
def background(self, value: Optional[bool]) -> None:
self._background = value

@property
def checkpoints(self):
def checkpoints(self) -> Optional[bool]:
"""CheckpointService instance for file-level undo/restore."""
return self._checkpoints

Comment on lines 1791 to 1795
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkpoints is annotated as bool, but the backing field self._checkpoints is set from a local checkpoints = None and isn’t assigned elsewhere in this file, so it can be None at runtime. Either default it to False or change the annotation to Optional[bool]; also the docstring describes an instance, not a flag.

Copilot uses AI. Check for mistakes.
@checkpoints.setter
def checkpoints(self, value):
def checkpoints(self, value: Optional[bool]) -> None:
self._checkpoints = value

@property
def output_style(self):
def output_style(self) -> Optional[str]:
"""OutputStyle instance for response formatting."""
return self._output_style

@output_style.setter
def output_style(self, value):
def output_style(self, value: Optional[str]) -> None:
self._output_style = value

@property
def thinking_budget(self):
def thinking_budget(self) -> Optional[int]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[int] contradicts the docstring which states this property returns a ThinkingBudget instance. Also, self._thinking_budget is always None as it's not initialized elsewhere. If it's supposed to be an integer, the docstring should be updated. If it's an instance, the type hint should be Optional['ThinkingBudget'] and the initialization logic should be reviewed.

"""ThinkingBudget instance for extended thinking control."""
return self._thinking_budget
Comment on lines +1801 to 1812
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output_style is annotated as Optional[str], but it is sourced from OutputConfig.style (typed as Optional[Any] in config/feature_configs.py) and isn’t used elsewhere here, so constraining it to str may be incorrect. Similarly, thinking_budget is annotated as Optional[int], but the project’s thinking module documentation indicates this may be a ThinkingBudget object; consider loosening these to Optional[Any] or importing the concrete types.

Copilot uses AI. Check for mistakes.

@thinking_budget.setter
def thinking_budget(self, value):
def thinking_budget(self, value: Optional[int]) -> None:
self._thinking_budget = value

@property
Expand All @@ -1835,7 +1835,7 @@ def cost_summary(self) -> dict:
}

@property
def context_manager(self):
def context_manager(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. The ContextManager type is imported within this property's implementation. You can use a forward reference Optional['ContextManager'] for a more specific type hint. You'll also need to add from ..context import ContextManager inside a TYPE_CHECKING block.

Suggested change
def context_manager(self) -> Optional[Any]:
def context_manager(self) -> Optional['ContextManager']:

"""
ContextManager instance for unified context management.

Expand Down Expand Up @@ -1927,7 +1927,7 @@ def context_manager(self):
return self._context_manager

@context_manager.setter
def context_manager(self, value):
def context_manager(self, value: Optional[Any]) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. The ContextManager type is available. You can use a forward reference Optional['ContextManager'] for a more specific type hint.

Suggested change
def context_manager(self, value: Optional[Any]) -> None:
def context_manager(self, value: Optional['ContextManager']) -> None:

"""Set context manager directly."""
self._context_manager = value
self._context_manager_initialized = True
Expand Down Expand Up @@ -1978,7 +1978,7 @@ def llm_summarize(messages: List[Dict[str, Any]], max_tokens: int = 500) -> str:
return llm_summarize

@property
def console(self):
def console(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. The Console type from the rich library is used here. You can use Optional['Console'] for a more specific type hint. You'll also need to add from rich.console import Console inside a TYPE_CHECKING block.

Suggested change
def console(self) -> Optional[Any]:
def console(self) -> Optional['Console']:

"""Lazily initialize Rich Console only when needed AND verbose is True."""
# Only return console if verbose mode is enabled
# This prevents panels from being shown in status/silent modes
Expand All @@ -1990,7 +1990,7 @@ def console(self):
return self._console

@property
def skill_manager(self):
def skill_manager(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. The SkillManager type is imported within this property's implementation. You can use a forward reference Optional['SkillManager'] for a more specific type hint. You'll also need to add from ..skills import SkillManager inside a TYPE_CHECKING block.

Suggested change
def skill_manager(self) -> Optional[Any]:
def skill_manager(self) -> Optional['SkillManager']:

"""Lazily initialize SkillManager only when skills are accessed."""
if self._skill_manager is None and (self._skills or self._skills_dirs):
from ..skills import SkillManager
Expand Down Expand Up @@ -2074,7 +2074,7 @@ def _openai_client(self):
return self.__openai_client

@property
def agent_id(self):
def agent_id(self) -> str:
"""Lazily generate agent ID when first accessed."""
if self._agent_id is None:
import uuid
Expand Down Expand Up @@ -3499,7 +3499,7 @@ def _model_supports_prompt_caching(self) -> bool:
return supports_prompt_caching(model_name)

@property
def rules_manager(self):
def rules_manager(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. The RulesManager type is available. You can use a forward reference Optional['RulesManager'] for a more specific type hint. You'll also need to add from ..memory.rules_manager import RulesManager inside a TYPE_CHECKING block.

Suggested change
def rules_manager(self) -> Optional[Any]:
def rules_manager(self) -> Optional['RulesManager']:

"""
Lazy-initialized RulesManager for persistent rules/instructions.

Expand Down Expand Up @@ -3680,7 +3680,7 @@ def get_learn_context(self) -> str:

return ""

def store_memory(self, content: str, memory_type: str = "short_term", **kwargs):
def store_memory(self, content: str, memory_type: str = "short_term", **kwargs) -> None:
"""
Store content in memory.

Expand Down Expand Up @@ -3752,7 +3752,7 @@ def _display_memory_info(self):
))

@property
def llm_model(self):
def llm_model(self) -> Optional[str]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The return type hint Optional[str] is incorrect. According to the implementation and docstring, this property can return either a string (model name) or an LLM instance. The correct type hint should be Union[str, 'LLM']. You'll also need to add from ..llm.llm import LLM inside a TYPE_CHECKING block to avoid circular imports.

Suggested change
def llm_model(self) -> Optional[str]:
def llm_model(self) -> Union[str, 'LLM']:

"""Unified property to get the LLM model regardless of configuration type.

Returns:
Expand Down Expand Up @@ -3788,12 +3788,12 @@ def _ensure_knowledge_processed(self):
self._knowledge_processed = True

@property
def retrieval_config(self):
def retrieval_config(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. This property returns a RetrievalConfig instance. You can use Optional['RetrievalConfig'] for a more specific type hint. You'll also need to add from ..rag.retrieval_config import RetrievalConfig inside a TYPE_CHECKING block.

Suggested change
def retrieval_config(self) -> Optional[Any]:
def retrieval_config(self) -> Optional['RetrievalConfig']:

"""Get the unified retrieval configuration."""
return self._retrieval_config

@property
def rag(self):
def rag(self) -> Optional[Any]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type hint Optional[Any] is too generic. This property returns a RAG instance. You can use Optional['RAG'] for a more specific type hint. You'll also need to add from praisonaiagents.rag import RAG inside a TYPE_CHECKING block.

Suggested change
def rag(self) -> Optional[Any]:
def rag(self) -> Optional['RAG']:

"""
Lazy-loaded RAG instance for advanced retrieval with citations.

Expand Down Expand Up @@ -4721,7 +4721,7 @@ def _cast_arguments(self, func, arguments):
logging.debug(f"Type casting failed for {getattr(func, '__name__', 'unknown function')}: {e}")
return arguments

def execute_tool(self, function_name, arguments, tool_call_id=None):
def execute_tool(self, function_name: str, arguments: Dict[str, Any], tool_call_id: Optional[str] = None) -> Any:
"""
Execute a tool dynamically based on the function name and arguments.
Injects agent state for tools with Injected[T] parameters.
Expand Down Expand Up @@ -5268,7 +5268,7 @@ def _execute_mcp_tool(mcp_instance, func_name, args):
logging.error(error_msg)
return {"error": error_msg}

def clear_history(self):
def clear_history(self) -> None:
"""Clear all chat history.

Also resets _auto_save_last_index to prevent silent message loss
Expand Down Expand Up @@ -5362,7 +5362,7 @@ def get_history_size(self) -> int:
return len(self.chat_history)

@contextlib.contextmanager
def ephemeral(self):
def ephemeral(self) -> Generator[None, None, None]:
"""
Context manager for ephemeral conversations.

Expand Down Expand Up @@ -6085,7 +6085,7 @@ def as_tool(
config=HandoffConfig(context_policy=ContextPolicy.NONE),
)

def chat(self, prompt, temperature=1.0, tools=None, output_json=None, output_pydantic=None, reasoning_steps=False, stream=None, task_name=None, task_description=None, task_id=None, config=None, force_retrieval=False, skip_retrieval=False, attachments=None, tool_choice=None):
def chat(self, prompt: str, temperature: float = 1.0, tools: Optional[List[Any]] = None, output_json: Optional[Any] = None, output_pydantic: Optional[Any] = None, reasoning_steps: bool = False, stream: Optional[bool] = None, task_name: Optional[str] = None, task_description: Optional[str] = None, task_id: Optional[str] = None, config: Optional[Dict[str, Any]] = None, force_retrieval: bool = False, skip_retrieval: bool = False, attachments: Optional[List[str]] = None, tool_choice: Optional[str] = None) -> Optional[str]:
"""
Chat with the agent.

Expand Down Expand Up @@ -7251,7 +7251,7 @@ async def astart(self, prompt: str, **kwargs):
kwargs['stream'] = stream_requested
return await self.achat(prompt, **kwargs)

def run(self, prompt: str, **kwargs):
def run(self, prompt: str, **kwargs: Any) -> Optional[str]:
"""Execute agent silently and return structured result.

Production-friendly execution. Always uses silent mode with no streaming
Expand Down Expand Up @@ -7437,7 +7437,7 @@ def switch_model(self, new_model: str) -> None:

# Chat history is preserved in self.chat_history (no action needed)

def start(self, prompt: str = None, **kwargs):
def start(self, prompt: Optional[str] = None, **kwargs) -> Union[str, Generator[str, None, None]]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The return type hint is incomplete. When autonomy is enabled, this method calls self.run_autonomous which returns an AutonomyResult instance. The return type should be updated to include AutonomyResult. You'll also need to add from .autonomy import AutonomyResult inside a TYPE_CHECKING block.

Suggested change
def start(self, prompt: Optional[str] = None, **kwargs) -> Union[str, Generator[str, None, None]]:
def start(self, prompt: Optional[str] = None, **kwargs) -> Union[str, Generator[str, None, None], 'AutonomyResult']:

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start() can return an AutonomyResult when self.autonomy_enabled is true (both caller and iterative modes), but the annotated return type only includes str and Generator[str, None, None]. Consider including AutonomyResult in the return union (using a forward reference / TYPE_CHECKING import to avoid runtime import), and also account for None returns from non-autonomy paths where chat can fail/block.

Suggested change
def start(self, prompt: Optional[str] = None, **kwargs) -> Union[str, Generator[str, None, None]]:
def start(self, prompt: Optional[str] = None, **kwargs) -> Union[str, "AutonomyResult", Generator[str, None, None], None]:

Copilot uses AI. Check for mistakes.
"""Start the agent interactively with verbose output.

Beginner-friendly execution. Defaults to verbose output with streaming
Expand Down Expand Up @@ -7750,7 +7750,7 @@ def run_chat():
self.verbose = original_verbose
self.markdown = original_markdown

def iter_stream(self, prompt: str, **kwargs):
def iter_stream(self, prompt: str, **kwargs) -> Generator[str, None, None]:
"""Stream agent response as an iterator of chunks.

App-friendly streaming. Yields response chunks without terminal display.
Expand Down Expand Up @@ -8390,7 +8390,7 @@ def pending_approval_count(self) -> int:
"""Number of approval requests still waiting."""
return len(self._pending_approvals)

def execute(self, task, context=None):
def execute(self, task: Any, context: Optional[Any] = None) -> Optional[str]:
"""Execute a task synchronously - backward compatibility method"""
if hasattr(task, 'description'):
prompt = task.description
Expand All @@ -8400,7 +8400,7 @@ def execute(self, task, context=None):
prompt = str(task)
return self.chat(prompt)

async def aexecute(self, task, context=None):
async def aexecute(self, task: Any, context: Optional[Any] = None) -> Optional[str]:
"""Execute a task asynchronously - backward compatibility method"""
if hasattr(task, 'description'):
prompt = task.description
Expand Down Expand Up @@ -8469,7 +8469,7 @@ async def execute_tool_async(self, function_name: str, arguments: Dict[str, Any]
logging.error(f"Error in execute_tool_async: {str(e)}", exc_info=True)
return {"error": f"Error in execute_tool_async: {str(e)}"}

def launch(self, path: str = '/', port: int = 8000, host: str = '0.0.0.0', debug: bool = False, protocol: str = "http"):
def launch(self, path: str = '/', port: int = 8000, host: str = '0.0.0.0', debug: bool = False, protocol: str = "http") -> None:
"""
Launch the agent as an HTTP API endpoint or an MCP server.

Expand Down