refactor: SDK Architecture - Centralize Logging and Retry Logic#1212
Conversation
|
Important Review skippedToo many files! This PR contains 166 files, which is 16 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (166)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoSDK Architecture Refactor - Centralize Logging and Retry Logic
WalkthroughsDescription• **Centralized logging infrastructure**: Created new _logging.py module providing get_logger() function to replace direct logging.getLogger() calls across the entire codebase • **Centralized resilience policies**: Implemented new utils/resilience.py module with @retry_with_backoff and @async_retry_with_backoff decorators supporting exponential backoff and jitter • **Comprehensive refactoring**: Migrated 150+ modules to use centralized logging, improving maintainability and enabling unified logging configuration • **Simplified retry logic**: Refactored web_search.py and duckduckgo_tools.py to use resilience decorators instead of manual retry loops • **Integration test suite**: Added comprehensive test_agent_decomposition_real.py validating all 34 Agent __init__ parameters and 142 original methods/properties after refactoring • **Automation script**: Created refactor_logging.py utility for automated logging migration across codebase • **Code cleanup**: Removed redundant logger configuration lines and extra blank lines throughout codebase Diagramflowchart LR
A["Direct logging.getLogger<br/>calls across codebase"] -->|"Centralize"| B["_logging.py<br/>get_logger function"]
C["Manual retry loops<br/>in tools"] -->|"Delegate to"| D["utils/resilience.py<br/>@retry_with_backoff"]
B -->|"Used by"| E["150+ modules<br/>migrated"]
D -->|"Used by"| F["web_search.py<br/>duckduckgo_tools.py"]
E -->|"Validated by"| G["test_agent_decomposition_real.py<br/>Integration tests"]
File Changes1. src/praisonai-agents/tests/integration/test_agent_decomposition_real.py
|
Code Review by Qodo
1. New retry_with_backoff decorators
|
There was a problem hiding this comment.
Code Review
This pull request implements a centralized logging system using a new get_logger utility and introduces a resilience module for standardized retry logic. While the architectural direction is sound, the PR is currently in a broken state because the _logging.py module was not included in the file set, which will trigger immediate import errors. Furthermore, the automated refactoring script used for this migration has introduced multiple syntax errors by leaving conditional blocks empty after removing setLevel calls, and the script itself contains hardcoded local directory paths that limit its portability.
| import time | ||
| import json | ||
| import logging | ||
| from praisonaiagents._logging import get_logger |
| if _verbose >= 5: | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
| elif _verbose >= 3: | ||
| logger.setLevel(logging.DEBUG) | ||
|
|
||
| else: | ||
| logger.setLevel(logging.WARNING) | ||
|
|
||
|
|
| if debug: | ||
| logger.setLevel(logging.DEBUG) | ||
|
|
||
| else: | ||
| # Set to WARNING by default to hide INFO messages | ||
| logger.setLevel(logging.WARNING) | ||
|
|
||
|
|
| if debug: | ||
| logger.setLevel(logging.DEBUG) | ||
|
|
||
|
|
| if debug: | ||
| logger.setLevel(logging.DEBUG) | ||
|
|
||
| else: | ||
| # Set to WARNING by default to hide INFO messages | ||
| logger.setLevel(logging.WARNING) | ||
|
|
||
|
|
| if verbose >= 5: | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
| else: | ||
| logger.setLevel(logging.WARNING) | ||
|
|
||
|
|
| if verbose >= 5: | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
| else: | ||
| logger.setLevel(logging.WARNING) | ||
|
|
||
|
|
| if verbose >= 5: | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
| else: | ||
| logger.setLevel(logging.WARNING) | ||
|
|
| import os | ||
| import re | ||
|
|
||
| target_dir = '/Users/praison/praisonai-package/src/praisonai-agents/praisonaiagents' |
| content = re.sub(r'logging\.getLogger\(', 'get_logger(', content) | ||
|
|
||
| # Strip manual setLevel unless it's a specific logic block that explicitly changes it. | ||
| content = re.sub(r'^[ \t]*logger\.setLevel\(logging\.[A-Z]+\)\s*$', '', content, flags=re.MULTILINE) |
There was a problem hiding this comment.
| def retry_with_backoff( | ||
| retries: int = 3, | ||
| backoff_in_seconds: float = 1.0, | ||
| max_backoff_in_seconds: float = 30.0, | ||
| exceptions: Union[Type[Exception], Tuple[Type[Exception], ...]] = (Exception,), | ||
| jitter_factor: float = 0.25, | ||
| ) -> Callable: | ||
| """ | ||
| Synchronous exponential backoff decorator with jitter to prevent thundering herd. | ||
|
|
||
| Args: | ||
| retries: Maximum number of retry attempts. | ||
| backoff_in_seconds: Initial backoff base in seconds. | ||
| max_backoff_in_seconds: Maximum allowed backoff delay in seconds. | ||
| exceptions: Exceptions to catch and retry (can be tuple of types). | ||
| jitter_factor: Randomness factor (e.g., 0.25 = ±25% random jitter). | ||
| """ | ||
|
|
||
| def decorator(func: Callable) -> Callable: | ||
| @wraps(func) | ||
| def wrapper(*args: Any, **kwargs: Any) -> Any: | ||
| attempt = 0 | ||
| while True: | ||
| try: | ||
| return func(*args, **kwargs) | ||
| except exceptions as e: | ||
| if attempt >= retries: | ||
| logger.error( | ||
| f"Execution failed permanently after {attempt} retries " | ||
| f"in '{func.__name__}': {str(e)}", | ||
| exc_info=True | ||
| ) | ||
| raise | ||
|
|
||
| # Calculate exponential backoff | ||
| delay = backoff_in_seconds * (2 ** attempt) | ||
| delay = min(delay, max_backoff_in_seconds) | ||
|
|
||
| # Add random jitter | ||
| jitter_range = delay * jitter_factor | ||
| delay = delay + random.uniform(-jitter_range, jitter_range) | ||
| delay = max(0.0, delay) | ||
|
|
||
| attempt += 1 | ||
| logger.warning( | ||
| f"Transient error '{e.__class__.__name__}' in '{func.__name__}'. " | ||
| f"Retrying in {delay:.2f}s (Attempt {attempt}/{retries})..." | ||
| ) | ||
| time.sleep(delay) | ||
|
|
||
| return wrapper | ||
|
|
||
| return decorator | ||
|
|
||
|
|
||
| def async_retry_with_backoff( | ||
| retries: int = 3, | ||
| backoff_in_seconds: float = 1.0, | ||
| max_backoff_in_seconds: float = 30.0, | ||
| exceptions: Union[Type[Exception], Tuple[Type[Exception], ...]] = (Exception,), | ||
| jitter_factor: float = 0.25, | ||
| ) -> Callable: | ||
| """ | ||
| Asynchronous exponential backoff decorator with jitter. | ||
|
|
||
| Identical semantics to `retry_with_backoff` but uses asyncio.sleep | ||
| for non-blocking concurrent multi-agent executions. | ||
| """ | ||
|
|
||
| def decorator(func: Callable) -> Callable: | ||
| @wraps(func) | ||
| async def wrapper(*args: Any, **kwargs: Any) -> Any: | ||
| attempt = 0 | ||
| while True: | ||
| try: | ||
| return await func(*args, **kwargs) | ||
| except exceptions as e: | ||
| if attempt >= retries: | ||
| logger.error( | ||
| f"Async execution failed permanently after {attempt} retries " | ||
| f"in '{func.__name__}': {str(e)}", | ||
| exc_info=True | ||
| ) | ||
| raise | ||
|
|
||
| # Calculate exponential backoff | ||
| delay = backoff_in_seconds * (2 ** attempt) | ||
| delay = min(delay, max_backoff_in_seconds) | ||
|
|
||
| # Add random jitter | ||
| jitter_range = delay * jitter_factor | ||
| delay = delay + random.uniform(-jitter_range, jitter_range) | ||
| delay = max(0.0, delay) | ||
|
|
||
| attempt += 1 | ||
| logger.warning( | ||
| f"Transient async error '{e.__class__.__name__}' in '{func.__name__}'. " | ||
| f"Retrying in {delay:.2f}s (Attempt {attempt}/{retries})..." | ||
| ) | ||
| await asyncio.sleep(delay) |
There was a problem hiding this comment.
1. New retry_with_backoff decorators 📎 Requirement gap ⚙ Maintainability
A new retry/backoff implementation is introduced in praisonaiagents/utils/resilience.py using bespoke retry loops with jitter and time.sleep/asyncio.sleep. This violates the requirement to centralize retry/backoff logic in the shared retry utility (or a dedicated core retry module) rather than creating additional implementations elsewhere.
Agent Prompt
## Issue description
`praisonaiagents/utils/resilience.py` introduces a new ad-hoc retry/backoff implementation (sync + async) instead of using/expanding the centralized retry utility.
## Issue Context
The repository already has `praisonaiagents/tools/retry.py`; the compliance requirement is to avoid multiple retry implementations scattered across modules.
## Fix Focus Areas
- src/praisonai-agents/praisonaiagents/utils/resilience.py[17-116]
- src/praisonai-agents/praisonaiagents/tools/retry.py[1-94]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # Look for logging.getLogger( | ||
| if 'logging.getLogger' in content: | ||
| print(f"Refactoring: {path}") | ||
|
|
||
| # Safely insert the new import if it hasn't been added yet | ||
| if 'from praisonaiagents._logging import get_logger' not in content: | ||
| # Find a good place to insert it: Replace "import logging" if exists | ||
| if 'import logging' in content: | ||
| content = re.sub(r'import logging\n', 'import logging\nfrom praisonaiagents._logging import get_logger\n', content, count=1) | ||
| else: | ||
| # Fallback injection at top | ||
| content = 'from praisonaiagents._logging import get_logger\n' + content | ||
|
|
||
| # Replace logging.getLogger( -> get_logger( | ||
| content = re.sub(r'logging\.getLogger\(', 'get_logger(', content) | ||
|
|
||
| # Strip manual setLevel unless it's a specific logic block that explicitly changes it. | ||
| content = re.sub(r'^[ \t]*logger\.setLevel\(logging\.[A-Z]+\)\s*$', '', content, flags=re.MULTILINE) | ||
| content = re.sub(r'^[ \t]*logger\.addHandler\(logging\.StreamHandler\(\)\)\s*$', '', content, flags=re.MULTILINE) | ||
|
|
||
| if content != original_content: | ||
| # Final cleanup for empty lines generated by strip | ||
| content = content.replace('\n\n\n', '\n\n') | ||
| with open(path, 'w') as f: | ||
| f.write(content) | ||
| changed_files += 1 | ||
|
|
||
| print(f"\nSuccessfully refactored {changed_files} files!") |
There was a problem hiding this comment.
2. refactor_logging.py uses print() 📎 Requirement gap ✧ Quality
The newly added refactor_logging.py emits output via print(), bypassing standardized logging. This violates the requirement to replace console debugging with the standardized logger so output is centrally formatted/routed.
Agent Prompt
## Issue description
The new `src/praisonai-agents/refactor_logging.py` uses `print()` for status output, which bypasses centralized logging.
## Issue Context
This PR is refactoring logging usage across the codebase and should not introduce new direct console output paths.
## Fix Focus Areas
- src/praisonai-agents/refactor_logging.py[20-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| def _configure_logging(cls): | ||
| """Configure logging settings once for all agent instances.""" | ||
| # Configure logging to suppress unwanted outputs | ||
| logging.getLogger("litellm").setLevel(logging.WARNING) | ||
| get_logger("litellm").setLevel(logging.WARNING) | ||
|
|
||
| # Allow httpx logging when LOGLEVEL=debug, otherwise suppress it | ||
| loglevel = os.environ.get('LOGLEVEL', 'INFO').upper() | ||
| if loglevel == 'DEBUG': | ||
| logging.getLogger("httpx").setLevel(logging.INFO) | ||
| logging.getLogger("httpcore").setLevel(logging.INFO) | ||
| get_logger("httpx").setLevel(logging.INFO) | ||
| get_logger("httpcore").setLevel(logging.INFO) | ||
| else: | ||
| logging.getLogger("httpx").setLevel(logging.WARNING) | ||
| logging.getLogger("httpcore").setLevel(logging.WARNING) | ||
| get_logger("httpx").setLevel(logging.WARNING) | ||
| get_logger("httpcore").setLevel(logging.WARNING) |
There was a problem hiding this comment.
3. External log levels ignored 🐞 Bug ≡ Correctness
Calls like Agent._configure_logging() now use get_logger("httpx"/"litellm"/"httpcore"), but
get_logger() prefixes these names to "praisonaiagents.<name>", so setLevel() no longer applies to
the real third‑party loggers and the intended noise suppression / debug enabling won’t work.
Agent Prompt
## Issue description
`get_logger(name)` currently prefixes any non-`praisonaiagents.*` name to `praisonaiagents.<name>`. After this refactor, many call sites pass explicit third-party logger names (e.g., `"httpx"`, `"httpcore"`, `"litellm"`) into `get_logger()` and then call `.setLevel(...)`. Because the name is rewritten, these calls no longer affect the real third-party loggers.
## Issue Context
The goal of this PR is centralized logging control. Current behavior prevents runtime suppression/enabling of third-party logs.
## Fix Focus Areas
- src/praisonai-agents/praisonaiagents/_logging.py[241-299]
- src/praisonai-agents/praisonaiagents/agent/agent.py[255-267]
- src/praisonai-agents/praisonaiagents/mcp/mcp.py[213-238]
## Expected fix
Adjust `get_logger()` so that when a **name is explicitly provided**, it returns `logging.getLogger(name)` without rewriting/prefixing (or add an opt-out flag and update third-party call sites). Keep the auto-prefix behavior only for the `name is None` auto-detected module-name path (or when the name already starts with `praisonaiagents.`). Then verify the affected call sites actually target the intended third-party logger names.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| """ | ||
| Comprehensive real agentic tests validating that the Agent class decomposition | ||
| (tool_execution.py, chat_handler.py, session_manager.py) did not remove any | ||
| features. Tests every single __init__ parameter with real LLM calls. | ||
|
|
||
| 34 Agent __init__ parameters verified: | ||
| Core identity: name, role, goal, backstory, instructions | ||
| LLM config: llm, model, base_url, api_key | ||
| Tools: tools, allow_delegation, allow_code_execution, code_execution_mode, handoffs | ||
| Session: auto_save, rate_limiter | ||
| Consolidated: memory, knowledge, planning, reflection, guardrails, web, | ||
| context, autonomy, verification_hooks, output, execution, | ||
| templates, caching, hooks, skills, approval, tool_timeout, learn | ||
| """ | ||
| import os | ||
| import pytest | ||
| import time | ||
| from typing import Any, Dict, Optional | ||
| from praisonaiagents import Agent, tool | ||
| from praisonaiagents.config.feature_configs import ( | ||
| OutputConfig, ExecutionConfig, ReflectionConfig, | ||
| TemplateConfig, CachingConfig, | ||
| ) | ||
|
|
||
|
|
||
| # ──────────────────────────────────────────────────────────────── | ||
| # Test Tools | ||
| # ──────────────────────────────────────────────────────────────── | ||
|
|
||
| @tool | ||
| def calculate_vault_code(base_number: int, multiplier: int) -> int: | ||
| """Calculates the secret vault code by multiplying base_number by multiplier.""" | ||
| return base_number * multiplier | ||
|
|
||
|
|
||
| @tool | ||
| def get_weather(city: str) -> str: | ||
| """Get current weather for a city.""" | ||
| return f"The weather in {city} is sunny and 22°C." | ||
|
|
||
|
|
||
| @tool | ||
| def echo_tool(message: str) -> str: | ||
| """Echoes back the message.""" | ||
| return f"Echo: {message}" | ||
|
|
||
|
|
||
| # ──────────────────────────────────────────────────────────────── | ||
| # Fixtures | ||
| # ──────────────────────────────────────────────────────────────── | ||
|
|
||
| @pytest.fixture | ||
| def api_key_check(): | ||
| """Ensure an API key is available for real LLM testing.""" | ||
| if not os.environ.get("OPENAI_API_KEY") and not os.environ.get("ANTHROPIC_API_KEY"): | ||
| pytest.skip("Requires OPENAI_API_KEY or ANTHROPIC_API_KEY") | ||
|
|
There was a problem hiding this comment.
4. Live tests not gated 🐞 Bug ☼ Reliability
tests/integration/test_agent_decomposition_real.py is collected by default and is explicitly designed for real LLM calls, but it is not marked with pytest’s "live" marker, so conftest.py’s PRAISONAI_LIVE_TESTS gating will not skip it when API keys are present.
Agent Prompt
## Issue description
A new integration test module intended for real LLM calls is not gated by the repo’s `PRAISONAI_LIVE_TESTS` mechanism because it lacks the `@pytest.mark.live` marker.
## Issue Context
`pytest_collection_modifyitems` in `tests/conftest.py` only skips items with the `live` keyword unless `PRAISONAI_LIVE_TESTS=1`.
## Fix Focus Areas
- src/praisonai-agents/tests/integration/test_agent_decomposition_real.py[1-60]
- src/praisonai-agents/tests/conftest.py[70-77]
## Expected fix
Add module-level gating, e.g.:
- `pytestmark = [pytest.mark.live, pytest.mark.network, pytest.mark.slow]`
Optionally also require the existing `live_test_enabled` fixture for real-call tests (in addition to API-key presence), so the module never makes outbound calls unless explicitly opted in.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above first. Review areas:
|
There was a problem hiding this comment.
Pull request overview
This PR aims to standardize logging across the SDK by routing modules through praisonaiagents._logging.get_logger, and to reduce duplicated retry/backoff logic by introducing a shared retry decorator and applying it to select tools.
Changes:
- Replaced many
logging.getLogger(...)usages with centralizedget_logger(...)across agents, tools, and subsystems. - Added
praisonaiagents/utils/resilience.pyproviding sync/async retry-with-backoff decorators. - Refactored DuckDuckGo search retry logic (and
web_search’s DuckDuckGo provider) to use the new decorator.
Reviewed changes
Copilot reviewed 152 out of 152 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/praisonai-agents/refactor_logging.py | Adds a refactor helper script (currently non-portable due to absolute paths). |
| src/praisonai-agents/praisonaiagents/workflows/workflows.py | Switch workflow module logging to get_logger. |
| src/praisonai-agents/praisonaiagents/utils/task_execution.py | Switch utility logging to get_logger. |
| src/praisonai-agents/praisonaiagents/utils/resilience.py | New retry/backoff decorators (sync + async). |
| src/praisonai-agents/praisonaiagents/ui/agui/conversion.py | Switch UI conversion logging to get_logger. |
| src/praisonai-agents/praisonaiagents/ui/agui/agui.py | Switch AGUI logging to get_logger. |
| src/praisonai-agents/praisonaiagents/ui/a2a/a2a.py | Switch A2A logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/web_search.py | Switch logging to get_logger; DuckDuckGo provider uses retry decorator. |
| src/praisonai-agents/praisonaiagents/tools/web_crawl_tools.py | Switch crawler tool logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/train/data/generatecot.py | Switch training script logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/subagent_tool.py | Switch tool logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/skill_bridge.py | Switch skill bridge logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/schedule_tools.py | Switch schedule tools logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/rules_tools.py | Switch rules tools logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/mentions.py | Switch mentions parser logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/health_monitor.py | Switch health monitor logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/exa_tools.py | Switch Exa tools logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/email_tools.py | Switch email tools logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/duckduckgo_tools.py | Refactor DuckDuckGo retry logic to decorator (but currently ignores retries argument). |
| src/praisonai-agents/praisonaiagents/tools/crawl4ai_tools.py | Switch Crawl4AI tools logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/circuit_breaker_integrations.py | Switch integrations logging to get_logger. |
| src/praisonai-agents/praisonaiagents/tools/ast_grep_tool.py | Switch ast-grep tool logging to get_logger. |
| src/praisonai-agents/praisonaiagents/telemetry/token_telemetry.py | Switch telemetry bridge logging to get_logger. |
| src/praisonai-agents/praisonaiagents/telemetry/telemetry.py | Switch telemetry logging to get_logger. |
| src/praisonai-agents/praisonaiagents/telemetry/performance_utils.py | Switch performance utils logging to get_logger. |
| src/praisonai-agents/praisonaiagents/telemetry/performance_monitor.py | Switch performance monitor logging to get_logger. |
| src/praisonai-agents/praisonaiagents/telemetry/performance_cli.py | Switch performance CLI logging to get_logger. |
| src/praisonai-agents/praisonaiagents/task/task.py | Switch task logging to get_logger and adjust logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/streaming/logging.py | Switch stream logger default to get_logger. |
| src/praisonai-agents/praisonaiagents/storage/base.py | Switch storage base logging to get_logger. |
| src/praisonai-agents/praisonaiagents/storage/backends.py | Switch storage backends logging to get_logger. |
| src/praisonai-agents/praisonaiagents/snapshot/snapshot.py | Switch snapshot logging to get_logger. |
| src/praisonai-agents/praisonaiagents/session/store.py | Switch session store logging to get_logger. |
| src/praisonai-agents/praisonaiagents/session/hierarchy.py | Switch session hierarchy logging to get_logger. |
| src/praisonai-agents/praisonaiagents/server/server.py | Switch server logging to get_logger. |
| src/praisonai-agents/praisonaiagents/scheduler/store.py | Switch scheduler store logging to get_logger. |
| src/praisonai-agents/praisonaiagents/scheduler/runner.py | Switch scheduler runner logging to get_logger. |
| src/praisonai-agents/praisonaiagents/scheduler/loop.py | Switch scheduler loop logging to get_logger. |
| src/praisonai-agents/praisonaiagents/scheduler/config_store.py | Switch scheduler config store logging to get_logger. |
| src/praisonai-agents/praisonaiagents/rag/pipeline.py | Switch RAG pipeline logging to get_logger. |
| src/praisonai-agents/praisonaiagents/policy/engine.py | Switch policy engine logging to get_logger. |
| src/praisonai-agents/praisonaiagents/policy/config.py | Switch policy config logging to get_logger. |
| src/praisonai-agents/praisonaiagents/plugins/plugin.py | Switch plugin base logging to get_logger. |
| src/praisonai-agents/praisonaiagents/plugins/manager.py | Switch plugin manager logging to get_logger. |
| src/praisonai-agents/praisonaiagents/plugins/loop_detection_plugin.py | Switch loop detection plugin logging to get_logger. |
| src/praisonai-agents/praisonaiagents/plugins/discovery.py | Switch plugin discovery logging to get_logger. |
| src/praisonai-agents/praisonaiagents/planning/todo.py | Switch todo logging to get_logger. |
| src/praisonai-agents/praisonaiagents/planning/storage.py | Switch planning storage logging to get_logger. |
| src/praisonai-agents/praisonaiagents/planning/planner.py | Switch planner logging to get_logger. |
| src/praisonai-agents/praisonaiagents/planning/plan.py | Switch plan logging to get_logger. |
| src/praisonai-agents/praisonaiagents/planning/approval.py | Switch planning approval logging to get_logger. |
| src/praisonai-agents/praisonaiagents/permissions/manager.py | Switch permissions manager logging to get_logger. |
| src/praisonai-agents/praisonaiagents/permissions/doom_loop.py | Switch doom loop logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/workflows.py | Switch memory workflows logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/rules_manager.py | Switch rules manager logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/memory.py | Switch memory logging + logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/memory/mcp_config.py | Switch MCP config logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/hooks.py | Switch memory hooks logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/file_memory.py | Switch file memory logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/docs_manager.py | Switch docs manager logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/auto_memory.py | Switch auto memory logging to get_logger. |
| src/praisonai-agents/praisonaiagents/memory/adapters/sqlite_adapter.py | Switch adapter logging + logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/mcp/mcp.py | Switch MCP logging configuration to get_logger. |
| src/praisonai-agents/praisonaiagents/mcp/mcp_websocket.py | Switch logging + logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/mcp/mcp_sse.py | Switch logging + logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/mcp/mcp_server.py | Switch logging + logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/mcp/mcp_http_stream.py | Switch logging + logger-level handling (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/main.py | Switch debug-level checks to use get_logger. |
| src/praisonai-agents/praisonaiagents/lsp/client.py | Switch LSP client logging to get_logger. |
| src/praisonai-agents/praisonaiagents/llm/rate_limiter.py | Switch rate limiter logging to get_logger. |
| src/praisonai-agents/praisonaiagents/llm/openai_client.py | Switch OpenAI client logging to get_logger. |
| src/praisonai-agents/praisonaiagents/llm/model_router.py | Switch model router logging to get_logger. |
| src/praisonai-agents/praisonaiagents/llm/llm.py | Switch LLM logging + configure external logger levels (currently using get_logger for third-party names). |
| src/praisonai-agents/praisonaiagents/llm/failover.py | Switch failover logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/vector_store.py | Switch knowledge logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/retrieval.py | Switch retrieval logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/rerankers.py | Switch reranker logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/readers.py | Switch readers logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/query_engine.py | Switch query engine logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/knowledge.py | Switch knowledge module logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/index.py | Switch index logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/adapters/mongodb_adapter.py | Switch adapter logging to get_logger. |
| src/praisonai-agents/praisonaiagents/knowledge/adapters/mem0_adapter.py | Switch adapter logging to get_logger. |
| src/praisonai-agents/praisonaiagents/hooks/runner.py | Switch hook runner logging to get_logger. |
| src/praisonai-agents/praisonaiagents/hooks/registry.py | Switch hook registry logging to get_logger. |
| src/praisonai-agents/praisonaiagents/guardrails/llm_guardrail.py | Switch guardrail logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/utils.py | Switch eval utils logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/tokens.py | Switch token eval logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/reliability.py | Switch reliability evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/performance.py | Switch performance evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/media.py | Switch media evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/loop.py | Switch eval loop logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/judge.py | Switch judge logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/grader.py | Switch grader logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/criteria.py | Switch criteria evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/base.py | Switch base evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/eval/accuracy.py | Switch accuracy evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/escalation/pipeline.py | Switch escalation pipeline logging to get_logger. |
| src/praisonai-agents/praisonaiagents/escalation/observability.py | Switch observability logging to get_logger. |
| src/praisonai-agents/praisonaiagents/escalation/doom_loop.py | Switch doom loop logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/store.py | Switch context store logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/session_tracker.py | Switch session tracker logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/manager.py | Adjust logging in dedup/token mismatch paths (currently missing get_logger import). |
| src/praisonai-agents/praisonaiagents/context/instrumentation.py | Switch instrumentation logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/search_tools.py | Switch fast search tools logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/search_strategy.py | Switch fast search strategy logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/search_backends.py | Switch search backends logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/parallel_executor.py | Switch parallel executor logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/indexer/symbol_indexer.py | Switch symbol indexer logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/indexer/file_watcher.py | Switch file watcher logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/indexer/file_indexer.py | Switch file indexer logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/index_manager.py | Switch index manager logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/fast_context.py | Switch fast context logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/fast_context_agent.py | Switch fast context agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/context_injector.py | Switch context injector logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/compressor.py | Switch compressor logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/fast/async_file_ops.py | Switch async file ops logging to get_logger. |
| src/praisonai-agents/praisonaiagents/context/aggregator.py | Switch context aggregator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/conditions/evaluator.py | Switch condition evaluator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/checkpoints/service.py | Switch checkpoint service logging to get_logger. |
| src/praisonai-agents/praisonaiagents/bus/bus.py | Switch event bus logging to get_logger. |
| src/praisonai-agents/praisonaiagents/background/task.py | Switch background task logging to get_logger. |
| src/praisonai-agents/praisonaiagents/background/runner.py | Switch background runner logging to get_logger. |
| src/praisonai-agents/praisonaiagents/approval/registry.py | Switch approval registry logging to get_logger. |
| src/praisonai-agents/praisonaiagents/approval/backends.py | Switch approval backends logging to get_logger. |
| src/praisonai-agents/praisonaiagents/approval/init.py | Switch approval module logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agents/delegator.py | Switch delegator logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agents/autoagents.py | Switch autoagents debug logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agents/auto_rag_agent.py | Switch auto RAG logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agents/agents.py | Switch manager logging + verbosity logic (currently introduces empty blocks). |
| src/praisonai-agents/praisonaiagents/agent/vision_agent.py | Switch vision agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/video_agent.py | Switch video agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/summarization.py | Switch summarization logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/router_agent.py | Switch router agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/realtime_agent.py | Switch realtime agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/query_rewriter_agent.py | Switch query rewriter logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/prompt_expander_agent.py | Switch prompt expander logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/ocr_agent.py | Switch OCR agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/message_queue.py | Switch message queue logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/loop_detection.py | Switch loop detection logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/image_agent.py | Switch image agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/heartbeat.py | Switch heartbeat logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/handoff.py | Switch handoff logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/embedding_agent.py | Switch embedding agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/deep_research_agent.py | Switch deep research agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/cost_persistence.py | Switch cost persistence logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/context_agent.py | Switch context agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/code_agent.py | Switch code agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/autonomy.py | Switch autonomy logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/audio_agent.py | Switch audio agent logging to get_logger. |
| src/praisonai-agents/praisonaiagents/agent/agent.py | Switch agent logging + attempts to reconfigure external logger levels via get_logger. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import os | ||
| import re | ||
|
|
||
| target_dir = '/Users/praison/praisonai-package/src/praisonai-agents/praisonaiagents' | ||
|
|
||
| changed_files = 0 |
There was a problem hiding this comment.
This script hard-codes a local absolute path ("/Users/praison/..."), and performs in-place refactors across the repo. It looks like a one-off migration helper and is not portable or safe to ship as part of the package. Consider removing it from the repo (or moving it to internal dev tooling with a configurable target_dir / CLI args and clear safeguards).
| attempt = 0 | ||
| while True: | ||
| try: | ||
| return func(*args, **kwargs) | ||
| except exceptions as e: | ||
| if attempt >= retries: | ||
| logger.error( | ||
| f"Execution failed permanently after {attempt} retries " |
There was a problem hiding this comment.
The retry counter is off by one relative to the docstring/typical semantics. With attempt starting at 0 and incremented after computing the delay, retries=3 results in 4 total executions (initial + 3 sleeps) before the permanent failure branch triggers. Either rename the parameter to max_retries (retries after the initial attempt) and update messaging, or change the condition/increment so retries truly caps total attempts as documented.
| attempt = 0 | ||
| while True: | ||
| try: | ||
| return await func(*args, **kwargs) | ||
| except exceptions as e: | ||
| if attempt >= retries: | ||
| logger.error( | ||
| f"Async execution failed permanently after {attempt} retries " |
There was a problem hiding this comment.
Same off-by-one retry semantics issue exists in the async decorator: retries=3 yields 4 total attempts. Align the implementation with the documented meaning of retries (or update naming/docs to reflect 'number of retries after the first attempt').
| # Set logger level based on config verbose level | ||
| verbose = self.config.get("verbose", 0) | ||
| if verbose >= 5: | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
| else: | ||
| logger.setLevel(logging.WARNING) | ||
|
|
There was a problem hiding this comment.
This if verbose >= 5: ... else: ... block is now empty, which makes the module invalid Python (SyntaxError) and will break imports. If logger-level overrides are intentionally removed in favor of centralized config, delete the conditional entirely; otherwise add the intended statements (or at least pass).
| # Also set third-party loggers to WARNING | ||
| logging.getLogger('chromadb').setLevel(logging.WARNING) | ||
| logging.getLogger('openai').setLevel(logging.WARNING) | ||
| logging.getLogger('httpx').setLevel(logging.WARNING) | ||
| logging.getLogger('httpcore').setLevel(logging.WARNING) | ||
| get_logger('chromadb').setLevel(logging.WARNING) | ||
| get_logger('openai').setLevel(logging.WARNING) | ||
| get_logger('httpx').setLevel(logging.WARNING) | ||
| get_logger('httpcore').setLevel(logging.WARNING) |
There was a problem hiding this comment.
get_logger('chromadb'/'openai'/'httpx'/'httpcore') will prefix these names to praisonaiagents.* (per _logging.get_logger), so it will NOT configure the actual third-party loggers and will leave their output unsuppressed. Use logging.getLogger('chromadb') (etc.) for external libraries, or update get_logger to support returning un-prefixed external loggers when explicitly requested.
| self._debug = debug | ||
|
|
||
| if debug: | ||
| logger.setLevel(logging.DEBUG) | ||
|
|
||
|
|
||
| # Register initial tools if provided |
There was a problem hiding this comment.
if debug: now has an empty body, which is a SyntaxError and will prevent this module from importing. Either remove the conditional or restore the intended debug log-level configuration (or add pass).
| # Set logger level based on verbose | ||
| if verbose >= 5: | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
| else: | ||
| logger.setLevel(logging.WARNING) | ||
|
|
||
|
|
There was a problem hiding this comment.
The if verbose >= 5: ... else: ... block is empty now, which is a SyntaxError and will prevent this adapter from importing. Remove the conditional or restore the intended logger.setLevel(...) statements (or add pass).
| # Always suppress litellm's internal debug messages | ||
| logging.getLogger("litellm.utils").setLevel(logging.WARNING) | ||
| logging.getLogger("litellm.main").setLevel(logging.WARNING) | ||
| logging.getLogger("litellm.litellm_logging").setLevel(logging.WARNING) | ||
| logging.getLogger("litellm.transformation").setLevel(logging.WARNING) | ||
| get_logger("litellm.utils").setLevel(logging.WARNING) | ||
| get_logger("litellm.main").setLevel(logging.WARNING) | ||
| get_logger("litellm.litellm_logging").setLevel(logging.WARNING) | ||
| get_logger("litellm.transformation").setLevel(logging.WARNING) | ||
|
|
There was a problem hiding this comment.
get_logger("litellm.*") will prefix these to praisonaiagents.litellm.*, so these setLevel(...) calls won't affect the actual LiteLLM loggers. For third-party logger configuration, use logging.getLogger("litellm.utils") (etc.) or provide a way for get_logger to bypass prefixing for external names.
| # Keep asyncio at WARNING unless explicitly in high debug mode | ||
| logging.getLogger("asyncio").setLevel(logging.WARNING) | ||
| logging.getLogger("selector_events").setLevel(logging.WARNING) | ||
| get_logger("asyncio").setLevel(logging.WARNING) | ||
| get_logger("selector_events").setLevel(logging.WARNING) |
There was a problem hiding this comment.
Same issue for these external loggers: get_logger("asyncio") / get_logger("selector_events") will not target the real stdlib loggers and therefore won't suppress them as intended.
| # Configure logging to suppress unwanted outputs | ||
| logging.getLogger("litellm").setLevel(logging.WARNING) | ||
| get_logger("litellm").setLevel(logging.WARNING) | ||
|
|
||
| # Allow httpx logging when LOGLEVEL=debug, otherwise suppress it | ||
| loglevel = os.environ.get('LOGLEVEL', 'INFO').upper() | ||
| if loglevel == 'DEBUG': | ||
| logging.getLogger("httpx").setLevel(logging.INFO) | ||
| logging.getLogger("httpcore").setLevel(logging.INFO) | ||
| get_logger("httpx").setLevel(logging.INFO) | ||
| get_logger("httpcore").setLevel(logging.INFO) | ||
| else: | ||
| logging.getLogger("httpx").setLevel(logging.WARNING) | ||
| logging.getLogger("httpcore").setLevel(logging.WARNING) | ||
| get_logger("httpx").setLevel(logging.WARNING) | ||
| get_logger("httpcore").setLevel(logging.WARNING) |
There was a problem hiding this comment.
get_logger("litellm"/"httpx"/"httpcore") prefixes names to praisonaiagents.*, so these setLevel(...) calls won't affect the real third-party loggers and may reintroduce noisy output. Use logging.getLogger("httpx") / logging.getLogger("litellm") for external libraries, or add an explicit way to bypass prefixing in get_logger for external logger names.
Updated logger level settings to use consistent DEBUG and WARNING levels based on the verbosity of the application. This enhances logging clarity and ensures that third-party loggers are set to WARNING by default. Adjusted imports to maintain consistent logging behavior across various modules for better maintainability. Additionally, bumped version in pyproject.toml to 1.5.89.
- Remove unittest.yml (517-line debug file, always failed exit 5) - Fix test-core.yml: deprecated codecov 'file:' -> 'files:' input - Fix test-optimized.yml: move secrets from inline shell expressions to env: blocks to resolve linter context access warnings - Fix test_serve_integration.py: replace 6x hardcoded /Users/praison paths with dynamic pathlib resolution (portable on any CI runner) - Fix test_tracker_complex_tasks.py: DEFAULT_TOOLS -> AUTONOMY_DEFAULT_TOOLS (symbol was renamed in tracker.py, causing ImportError in CI) Resolves GitHub Actions lint IDs: 7c7e1668 1d881864 aa30b182 a28bb9f7 ca2c8cc1 132d4548 a2735061 d3729ccb 333ae3f5 f3aca47c 7353e2c3 6d3216de
Fixes #1131, Fixes #1132. @[/review-chain] please review.