fix: implement lazy loading for CLI command modules#2001
Conversation
All ~85 CLI command modules are now loaded only when invoked, not eagerly on startup. This reduces CLI cold-start latency by deferring imports until dispatch. - Created LazyCommandGroup class extending TyperGroup - Moved command registry to _LAZY_COMMANDS dictionary - Commands listed in --help without importing modules - Special handling for app, standardise, TUI, and retrieval commands - No user-facing changes; all commands remain available Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Action performedReview finished.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough
ChangesLazy CLI Command Loading
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/praisonai/praisonai/cli/app.py`:
- Around line 204-218: The list_commands method in the LazyCommandGroup class
hardcodes the wrong retrieval command name. Replace "delete" with "search" in
the commands list since retrieval.py actually registers "search" rather than
"delete". This hardcoded list should accurately reflect the actual command names
defined in retrieval.py to prevent lookup failures when users try to access the
correct commands.
- Around line 375-444: The method _get_app_command() declares a return type of
Optional[click.Command] but currently returns the raw app_cmd function from the
global app.command() decorator, and also registers duplicate commands on the
global app instance with each invocation. Create a local Typer instance inside
_get_app_command() instead of using the global app decorator, apply the command
decorator to app_cmd using this local instance, then use typer_get_command() to
convert the resulting decorated command to a click.Command object before
returning it, making it consistent with other similar methods in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18cfe5ed-e613-429f-b94d-d862af09042a
📒 Files selected for processing (1)
src/praisonai/praisonai/cli/app.py
| def list_commands(self, ctx: click.Context) -> List[str]: | ||
| """Return list of available commands without importing them.""" | ||
| commands = list(_LAZY_COMMANDS.keys()) | ||
| commands.extend(_SPECIAL_COMMANDS.keys()) | ||
|
|
||
| # Add special inline commands | ||
| commands.append("app") | ||
|
|
||
| # Add retrieval commands | ||
| commands.extend(["index", "query", "delete"]) | ||
|
|
||
| # Add standardise/standardize | ||
| commands.extend(["standardise", "standardize"]) | ||
|
|
||
| return sorted(commands) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify retrieval registration names and resolver parity.
rg -n -C3 'def register_commands\(main_app: typer\.Typer\)|main_app\.command\(' src/praisonai/praisonai/cli/commands/retrieval.py
rg -n -C3 'list_commands|get_command|index|query|search|delete' src/praisonai/praisonai/cli/app.pyRepository: MervinPraison/PraisonAI
Length of output: 3791
Lazy resolver hardcodes wrong retrieval command names and skips parent resolution.
LazyCommandGroup.list_commands() and get_command() both hardcode delete as a retrieval command, but retrieval.py actually registers search instead. This causes lookup failures for both delete (non-existent) and search (not listed). Additionally, get_command() never delegates to super().get_command(ctx, name), so commands registered directly on the app via register_commands() are shadowed by the lazy resolver.
Suggested fix
class LazyCommandGroup(TyperGroup):
def list_commands(self, ctx: click.Context) -> List[str]:
"""Return list of available commands without importing them."""
- commands = list(_LAZY_COMMANDS.keys())
- commands.extend(_SPECIAL_COMMANDS.keys())
+ commands = set(super().list_commands(ctx))
+ commands.update(_LAZY_COMMANDS.keys())
+ commands.update(_SPECIAL_COMMANDS.keys())
# Add special inline commands
commands.append("app")
# Add retrieval commands
- commands.extend(["index", "query", "delete"])
+ commands.update(["index", "query", "search"])
# Add standardise/standardize
- commands.extend(["standardise", "standardize"])
+ commands.update(["standardise", "standardize"])
return sorted(commands)
def get_command(self, ctx: click.Context, name: str) -> Optional[click.Command]:
"""Lazily import and return the command."""
+ existing = super().get_command(ctx, name)
+ if existing is not None:
+ return existing
# Check regular commands
if name in _LAZY_COMMANDS:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/praisonai/praisonai/cli/app.py` around lines 204 - 218, The list_commands
method in the LazyCommandGroup class hardcodes the wrong retrieval command name.
Replace "delete" with "search" in the commands list since retrieval.py actually
registers "search" rather than "delete". This hardcoded list should accurately
reflect the actual command names defined in retrieval.py to prevent lookup
failures when users try to access the correct commands.
| def _get_app_command(self) -> Optional[click.Command]: | ||
| """Get the app command.""" | ||
| @app.command(name="app", context_settings={"allow_interspersed_args": False}) | ||
| def app_cmd( | ||
| port: int = typer.Option(8000, "--port", "-p", help="Port to listen on"), | ||
| host: str = typer.Option("0.0.0.0", "--host", "-h", help="Host to bind to"), | ||
| config: str = typer.Option(None, "--config", "-c", help="Path to config file (YAML)"), | ||
| reload: bool = typer.Option(False, "--reload", "-r", help="Enable auto-reload for development"), | ||
| debug: bool = typer.Option(False, "--debug", "-d", help="Enable debug mode"), | ||
| name: str = typer.Option("PraisonAI App", "--name", "-n", help="Application name"), | ||
| ): | ||
| """ | ||
| Start an AgentOS server for production deployment. | ||
|
|
||
| AgentOS provides a FastAPI-based web service for deploying AI agents | ||
| with REST and WebSocket endpoints. | ||
| """ | ||
| from rich.console import Console | ||
| console = Console() | ||
|
|
||
| try: | ||
| from praisonai import AgentOS | ||
| from praisonaiagents import AgentOSConfig | ||
| except ImportError as e: | ||
| console.print(f"[red]Error importing AgentOS: {e}[/red]") | ||
| console.print("[yellow]Install with: pip install praisonai[api][/yellow]") | ||
| raise typer.Abort() | ||
|
|
||
| # Load agents from config file if provided | ||
| agents = [] | ||
| if config: | ||
| agents = self._load_agents_from_config_file(config, console) | ||
|
|
||
| # Create config | ||
| app_config = AgentOSConfig( | ||
| name=name, | ||
| host=host, | ||
| port=port, | ||
| reload=reload, | ||
| debug=debug, | ||
| ) | ||
|
|
||
| # Create and start app | ||
| console.print(f"\n[bold green]🚀 Starting {name}[/bold green]") | ||
| console.print(f"[dim]Host: {host}:{port}[/dim]") | ||
| if agents: | ||
| console.print(f"[dim]Agents: {len(agents)}[/dim]") | ||
| if reload: | ||
| console.print("[yellow]Auto-reload enabled (development mode)[/yellow]") | ||
| console.print() | ||
|
|
||
| try: | ||
| agent_app = AgentOS( | ||
| name=name, | ||
| agents=agents, | ||
| config=app_config, | ||
| ) | ||
| agent_app.serve() | ||
| except ImportError as e: | ||
| console.print(f"[red]Missing dependency: {e}[/red]") | ||
| console.print("[yellow]Install with: pip install praisonai[api][/yellow]") | ||
| raise typer.Abort() | ||
| except Exception as e: | ||
| console.print(f"[red]Error starting server: {e}[/red]") | ||
| raise typer.Abort() | ||
|
|
||
| # Bind the helper method | ||
| app_cmd._load_agents_from_config_file = self._load_agents_from_config_file | ||
| return app_cmd | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Typer 0.9.0, what does the @app.command()decorator return, and what type should a customTyperGroup.get_command() return?
💡 Result:
In Typer 0.9.0, the @app.command decorator acts as a function wrapper that returns the original, unmodified function [1][2]. When invoked, the decorator registers the command with the Typer application instance rather than replacing the function with a different object [2][3]. Regarding custom implementations of TyperGroup.get_command, the method should return a click.Command (or a subclass, such as TyperCommand) [4][5][3]. This is because TyperGroup inherits from click.Group, and its get_command method is expected by the underlying Click framework to return a valid Command object when it successfully resolves the command name [4][5]. If the command cannot be resolved, it should return None [6].
Citations:
- 1: https://typer.tiangolo.com/reference/typer/
- 2: https://github.com/tiangolo/typer/blob/master/typer/main.py
- 3: [BUG] Context .invoke() fails quietly and .forward() fails fastapi/typer#1756
- 4: https://github.com/tiangolo/typer/blob/master/typer/core.py
- 5: https://gist.github.com/gar1t/481cda3a0069bd61c5b08fda22d8b84c
- 6: https://gist.github.com/danielomiya/6a9c098c7094eefd1651eef2d967d3af
🏁 Script executed:
# Find the _get_app_command method and understand its context
rg -A 5 "def _get_app_command" src/praisonai/praisonai/cli/app.py
# Check what 'app' is at the beginning of this file
rg -B 5 "def _get_app_command" src/praisonai/praisonai/cli/app.py | head -20Repository: MervinPraison/PraisonAI
Length of output: 624
🏁 Script executed:
# Search for typer_get_command function
rg "typer_get_command" .
# Search for where _get_app_command is called/used
rg "_get_app_command" src/Repository: MervinPraison/PraisonAI
Length of output: 601
🏁 Script executed:
# Look at the overall structure of the app.py file to understand the class
head -100 src/praisonai/praisonai/cli/app.pyRepository: MervinPraison/PraisonAI
Length of output: 4041
🏁 Script executed:
# Check for imports and what 'app' is
rg "^app\s*=" src/praisonai/praisonai/cli/app.py | head -10
rg "from.*typer|import.*typer" src/praisonai/praisonai/cli/app.py | head -10Repository: MervinPraison/PraisonAI
Length of output: 189
🏁 Script executed:
# Get the complete _get_app_command implementation (lines 375-444)
sed -n '375,444p' src/praisonai/praisonai/cli/app.pyRepository: MervinPraison/PraisonAI
Length of output: 3129
🏁 Script executed:
# Search for how _get_app_command is used to understand the context
rg -B 3 "_get_app_command" src/praisonai/praisonai/cli/app.py | grep -v "def _get_app_command"Repository: MervinPraison/PraisonAI
Length of output: 232
Return type mismatch: _get_app_command() returns a function instead of click.Command and mutates global app on each call.
The method signature declares -> Optional[click.Command], but @app.command(...) returns the undecorated app_cmd function. Each invocation of this method also registers a duplicate command on the global app instance. Replace the global decorator with a local Typer instance and use typer_get_command() to convert it to a click.Command, consistent with other methods in this file.
Suggested fix
def _get_app_command(self) -> Optional[click.Command]:
"""Get the app command."""
- `@app.command`(name="app", context_settings={"allow_interspersed_args": False})
+ app_group = typer.Typer(add_completion=False)
+
+ `@app_group.command`(name="app", context_settings={"allow_interspersed_args": False})
def app_cmd(
@@
- # Bind the helper method
- app_cmd._load_agents_from_config_file = self._load_agents_from_config_file
- return app_cmd
+ return typer_get_command(app_group)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _get_app_command(self) -> Optional[click.Command]: | |
| """Get the app command.""" | |
| @app.command(name="app", context_settings={"allow_interspersed_args": False}) | |
| def app_cmd( | |
| port: int = typer.Option(8000, "--port", "-p", help="Port to listen on"), | |
| host: str = typer.Option("0.0.0.0", "--host", "-h", help="Host to bind to"), | |
| config: str = typer.Option(None, "--config", "-c", help="Path to config file (YAML)"), | |
| reload: bool = typer.Option(False, "--reload", "-r", help="Enable auto-reload for development"), | |
| debug: bool = typer.Option(False, "--debug", "-d", help="Enable debug mode"), | |
| name: str = typer.Option("PraisonAI App", "--name", "-n", help="Application name"), | |
| ): | |
| """ | |
| Start an AgentOS server for production deployment. | |
| AgentOS provides a FastAPI-based web service for deploying AI agents | |
| with REST and WebSocket endpoints. | |
| """ | |
| from rich.console import Console | |
| console = Console() | |
| try: | |
| from praisonai import AgentOS | |
| from praisonaiagents import AgentOSConfig | |
| except ImportError as e: | |
| console.print(f"[red]Error importing AgentOS: {e}[/red]") | |
| console.print("[yellow]Install with: pip install praisonai[api][/yellow]") | |
| raise typer.Abort() | |
| # Load agents from config file if provided | |
| agents = [] | |
| if config: | |
| agents = self._load_agents_from_config_file(config, console) | |
| # Create config | |
| app_config = AgentOSConfig( | |
| name=name, | |
| host=host, | |
| port=port, | |
| reload=reload, | |
| debug=debug, | |
| ) | |
| # Create and start app | |
| console.print(f"\n[bold green]🚀 Starting {name}[/bold green]") | |
| console.print(f"[dim]Host: {host}:{port}[/dim]") | |
| if agents: | |
| console.print(f"[dim]Agents: {len(agents)}[/dim]") | |
| if reload: | |
| console.print("[yellow]Auto-reload enabled (development mode)[/yellow]") | |
| console.print() | |
| try: | |
| agent_app = AgentOS( | |
| name=name, | |
| agents=agents, | |
| config=app_config, | |
| ) | |
| agent_app.serve() | |
| except ImportError as e: | |
| console.print(f"[red]Missing dependency: {e}[/red]") | |
| console.print("[yellow]Install with: pip install praisonai[api][/yellow]") | |
| raise typer.Abort() | |
| except Exception as e: | |
| console.print(f"[red]Error starting server: {e}[/red]") | |
| raise typer.Abort() | |
| # Bind the helper method | |
| app_cmd._load_agents_from_config_file = self._load_agents_from_config_file | |
| return app_cmd | |
| def _get_app_command(self) -> Optional[click.Command]: | |
| """Get the app command.""" | |
| app_group = typer.Typer(add_completion=False) | |
| `@app_group.command`(name="app", context_settings={"allow_interspersed_args": False}) | |
| def app_cmd( | |
| port: int = typer.Option(8000, "--port", "-p", help="Port to listen on"), | |
| host: str = typer.Option("0.0.0.0", "--host", "-h", help="Host to bind to"), | |
| config: str = typer.Option(None, "--config", "-c", help="Path to config file (YAML)"), | |
| reload: bool = typer.Option(False, "--reload", "-r", help="Enable auto-reload for development"), | |
| debug: bool = typer.Option(False, "--debug", "-d", help="Enable debug mode"), | |
| name: str = typer.Option("PraisonAI App", "--name", "-n", help="Application name"), | |
| ): | |
| """ | |
| Start an AgentOS server for production deployment. | |
| AgentOS provides a FastAPI-based web service for deploying AI agents | |
| with REST and WebSocket endpoints. | |
| """ | |
| from rich.console import Console | |
| console = Console() | |
| try: | |
| from praisonai import AgentOS | |
| from praisonaiagents import AgentOSConfig | |
| except ImportError as e: | |
| console.print(f"[red]Error importing AgentOS: {e}[/red]") | |
| console.print("[yellow]Install with: pip install praisonai[api][/yellow]") | |
| raise typer.Abort() | |
| # Load agents from config file if provided | |
| agents = [] | |
| if config: | |
| agents = self._load_agents_from_config_file(config, console) | |
| # Create config | |
| app_config = AgentOSConfig( | |
| name=name, | |
| host=host, | |
| port=port, | |
| reload=reload, | |
| debug=debug, | |
| ) | |
| # Create and start app | |
| console.print(f"\n[bold green]🚀 Starting {name}[/bold green]") | |
| console.print(f"[dim]Host: {host}:{port}[/dim]") | |
| if agents: | |
| console.print(f"[dim]Agents: {len(agents)}[/dim]") | |
| if reload: | |
| console.print("[yellow]Auto-reload enabled (development mode)[/yellow]") | |
| console.print() | |
| try: | |
| agent_app = AgentOS( | |
| name=name, | |
| agents=agents, | |
| config=app_config, | |
| ) | |
| agent_app.serve() | |
| except ImportError as e: | |
| console.print(f"[red]Missing dependency: {e}[/red]") | |
| console.print("[yellow]Install with: pip install praisonai[api][/yellow]") | |
| raise typer.Abort() | |
| except Exception as e: | |
| console.print(f"[red]Error starting server: {e}[/red]") | |
| raise typer.Abort() | |
| return typer_get_command(app_group) |
🧰 Tools
🪛 Ruff (0.15.17)
[error] 380-380: Possible binding to all interfaces
(S104)
[warning] 401-401: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 436-436: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
[warning] 437-437: Do not catch blind exception: Exception
(BLE001)
[warning] 439-439: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/praisonai/praisonai/cli/app.py` around lines 375 - 444, The method
_get_app_command() declares a return type of Optional[click.Command] but
currently returns the raw app_cmd function from the global app.command()
decorator, and also registers duplicate commands on the global app instance with
each invocation. Create a local Typer instance inside _get_app_command() instead
of using the global app decorator, apply the command decorator to app_cmd using
this local instance, then use typer_get_command() to convert the resulting
decorated command to a click.Command object before returning it, making it
consistent with other similar methods in the file.
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job ✅ Final Architecture Review - COMPLETEDReview Checklist:
📋 Phase 1: Architecture Review✅ Protocol-driven design: The lazy loading implementation follows good architectural patterns by deferring module imports until dispatch time. ✅ Performance: The lazy loading approach correctly reduces CLI startup latency by avoiding ~85 module imports on every invocation. ✅ Backward compatibility: All issues have been fixed - commands remain fully functional. 🔧 Phase 2: Issues FixedI've implemented fixes for both critical issues identified by Greptile and CodeRabbit: ✅ Fixed:
|
- Fix _get_app_command() to return click.Command instead of raw function - Creates local Typer instance to avoid mutating global app state - Uses typer_get_command() to properly convert to click.Command - Prevents duplicate command registration on repeated invocations - Fix retrieval commands availability - Check parent's registered commands first via super().get_command() - Replace hardcoded 'delete' with correct 'search' command in list - Use set operations to avoid duplicate command listings These fixes ensure all commands work properly with lazy loading while maintaining backward compatibility.
|
@claude i did a merge conflict fix, make sure its correct. |
|
Claude finished @MervinPraison's task —— View job ✅ Merge Conflict Resolution Review CompleteI've thoroughly reviewed the merged file Task List:
✅ All Critical Issues Properly Resolved:
✅ No Merge Conflict Artifacts Found:
Summary:The lazy loading implementation successfully:
The merge conflict resolution was handled correctly. The code is clean, functional, and ready for production use. |
| def _load_agents_from_config_file(self, config_path: str, console) -> list: | ||
| """Load agents from a YAML config file.""" | ||
| import yaml | ||
|
|
||
| try: | ||
| with open(config_path, 'r') as f: | ||
| config_data = yaml.safe_load(f) | ||
| except Exception as e: | ||
| console.print(f"[red]Error loading config: {e}[/red]") | ||
| return [] |
There was a problem hiding this comment.
import yaml crashes with a confusing traceback
import yaml at line 450 is outside any try/except. If PyYAML is not installed, a ModuleNotFoundError propagates out of _load_agents_from_config_file and back to the call site on line 407 inside app_cmd, which has no surrounding try block. Typer/Click's top-level error handler then prints a raw traceback rather than the friendly "install with pip" message the rest of the function uses. Wrapping the import and adding a user-facing message keeps the error consistent with the surrounding error-handling style.
1 participant
Fixes #1993
Summary
Implemented lazy loading for all ~85 CLI command modules to reduce startup latency. Commands are now imported only when actually invoked, not eagerly on every CLI invocation.
Changes
Impact
This change reduces CLI cold-start time by deferring imports until dispatch. The --version/--help fast paths already avoid this, but now any real subcommand also benefits from lazy loading.
Testing
Summary by CodeRabbit
Release Notes
New Features
stream-jsonoutput format option for command responsesRefactor