Feat: Installation Tutor Script (Fixes #140) #352
Conversation
📝 WalkthroughWalkthroughA new interactive installation assistant script is introduced in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
cortex/install_tutor.py (3)
3-3: Remove unused import.The
osmodule is imported but never used in this file.🔎 Proposed fix
import platform import sys -import os
66-74: Consider recommending dry-run verification.While the script only prints commands (doesn't execute them), you could enhance user safety by suggesting users first run with
--dry-runto verify what would be installed, especially since the coding guidelines emphasize "Dry-run by default for all installations in command execution."🔎 Suggested enhancement
print("\n3️⃣ Install Cortex Core:") + print(" 💡 TIP: Add --dry-run to preview changes first") install_cmd = "pip install cortex-ai"As per coding guidelines, dry-run defaults are recommended for installation operations.
88-89: Consider adding error handling for graceful exits.Adding basic exception handling for
KeyboardInterruptwould provide a cleaner user experience if the user wants to exit the assistant early.🔎 Suggested enhancement
if __name__ == "__main__": - generate_guide() + try: + generate_guide() + except KeyboardInterrupt: + print("\n\n⚠️ Installation assistant cancelled by user.") + sys.exit(0)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cortex/install_tutor.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
cortex/install_tutor.py
**/*install*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*install*.py: Dry-run by default for all installations in command execution
No silent sudo execution - require explicit user confirmation
Implement audit logging to ~/.cortex/history.db for all package operations
Files:
cortex/install_tutor.py
| def print_header(): | ||
| print("\n" + "="*50) | ||
| print(" 🧠 Cortex AI - Installation Assistant") | ||
| print("="*50 + "\n") | ||
| print("This tool will help you generate the correct installation commands") | ||
| print("based on your hardware and operating system.\n") |
There was a problem hiding this comment.
Add type hints and docstring.
Per coding guidelines, type hints and docstrings are required for all public APIs.
🔎 Proposed fix
-def print_header():
+def print_header() -> None:
+ """Print the branded header for the Cortex installation assistant."""
print("\n" + "="*50)
print(" 🧠 Cortex AI - Installation Assistant")
print("="*50 + "\n")As per coding guidelines, type hints and docstrings are required for Python code.
🤖 Prompt for AI Agents
In cortex/install_tutor.py around lines 5 to 10, the public function
print_header lacks a docstring and type hints; add a one- or two-line docstring
describing its purpose (prints the installation assistant header) and annotate
its signature with a return type of None (def print_header() -> None:). Ensure
the docstring follows PEP 257 (summary line and optional details) and includes
any relevant behavior; no parameter annotations are needed since it takes no
arguments.
| def get_user_choice(question, options): | ||
| print(f"❓ {question}") | ||
| for i, opt in enumerate(options, 1): | ||
| print(f" {i}. {opt}") | ||
|
|
||
| while True: | ||
| try: | ||
| choice = int(input("\n👉 Enter your choice (number): ")) | ||
| if 1 <= choice <= len(options): | ||
| return options[choice-1] | ||
| print("❌ Invalid choice. Please try again.") | ||
| except ValueError: | ||
| print("❌ Please enter a number.") |
There was a problem hiding this comment.
Add type hints and docstring.
Per coding guidelines, type hints and docstrings are required for all public APIs.
🔎 Proposed fix
-def get_user_choice(question, options):
+def get_user_choice(question: str, options: list[str]) -> str:
+ """
+ Prompt the user to select one option from a list.
+
+ Args:
+ question: The question to display to the user.
+ options: List of available options.
+
+ Returns:
+ The selected option string.
+ """
print(f"❓ {question}")
for i, opt in enumerate(options, 1):
print(f" {i}. {opt}")As per coding guidelines, type hints and docstrings are required for Python code.
🤖 Prompt for AI Agents
In cortex/install_tutor.py around lines 12 to 24, add a proper docstring and
type hints for the public function get_user_choice: annotate question as str,
options as a Sequence[str] (or list[str] for Python 3.9+), and the return type
as str; update the function signature accordingly and add a short triple-quoted
docstring describing parameters, return value and behavior (prompts user until a
valid numeric choice is made), and add any needed import from typing (e.g.,
Sequence) at the top of the file.
| except ValueError: | ||
| print("❌ Please enter a number.") | ||
|
|
||
| def generate_guide(): |
There was a problem hiding this comment.
Add type hints and docstring.
Per coding guidelines, type hints and docstrings are required for all public APIs.
🔎 Proposed fix
-def generate_guide():
+def generate_guide() -> None:
+ """
+ Generate an interactive installation guide for Cortex AI.
+
+ Detects the operating system, prompts for GPU/accelerator type and UI preference,
+ then prints tailored installation commands.
+ """
print_header()As per coding guidelines, type hints and docstrings are required for Python code.
📝 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 generate_guide(): | |
| def generate_guide() -> None: | |
| """ | |
| Generate an interactive installation guide for Cortex AI. | |
| Detects the operating system, prompts for GPU/accelerator type and UI preference, | |
| then prints tailored installation commands. | |
| """ | |
| print_header() |
🤖 Prompt for AI Agents
In cortex/install_tutor.py around line 26, add a proper docstring and function
type hints for the public generate_guide function: declare parameter and return
types (or use -> None if it returns nothing) and include a one-line summary plus
short description of parameters/return and behavior in the docstring following
PEP 257; ensure annotations use existing project types or typing imports if
needed.
| # Logic to generate commands | ||
| print("1️⃣ Prerequisites:") | ||
| if target_os == "Windows": | ||
| print(" - Ensure Python 3.10+ is installed (checking: python --version)") | ||
| print(" - Install Visual Studio C++ Build Tools") | ||
| elif target_os == "Linux": | ||
| print(" - sudo apt update && sudo apt install build-essential python3-dev") | ||
| elif target_os == "macOS": | ||
| print(" - Install Xcode Command Line Tools: xcode-select --install") |
There was a problem hiding this comment.
Add explicit warning before displaying sudo commands.
The script displays sudo commands without explicit warnings for user review. Per coding guidelines, "No silent sudo execution - require explicit user confirmation." Even though these commands are only printed (not executed), users may copy-paste them without careful review. Add a warning before showing privileged commands.
🔎 Proposed fix
print("1️⃣ Prerequisites:")
+ print(" ⚠️ NOTE: Review and understand all commands before executing, especially those requiring sudo.")
if target_os == "Windows":
print(" - Ensure Python 3.10+ is installed (checking: python --version)")
print(" - Install Visual Studio C++ Build Tools")As per coding guidelines, no silent sudo execution is allowed without explicit user confirmation.
📝 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.
| # Logic to generate commands | |
| print("1️⃣ Prerequisites:") | |
| if target_os == "Windows": | |
| print(" - Ensure Python 3.10+ is installed (checking: python --version)") | |
| print(" - Install Visual Studio C++ Build Tools") | |
| elif target_os == "Linux": | |
| print(" - sudo apt update && sudo apt install build-essential python3-dev") | |
| elif target_os == "macOS": | |
| print(" - Install Xcode Command Line Tools: xcode-select --install") | |
| # Logic to generate commands | |
| print("1️⃣ Prerequisites:") | |
| print(" ⚠️ NOTE: Review and understand all commands before executing, especially those requiring sudo.") | |
| if target_os == "Windows": | |
| print(" - Ensure Python 3.10+ is installed (checking: python --version)") | |
| print(" - Install Visual Studio C++ Build Tools") | |
| elif target_os == "Linux": | |
| print(" - sudo apt update && sudo apt install build-essential python3-dev") | |
| elif target_os == "macOS": | |
| print(" - Install Xcode Command Line Tools: xcode-select --install") |
🤖 Prompt for AI Agents
In cortex/install_tutor.py around lines 48 to 56, the script prints privileged
(sudo) commands for Linux without any explicit warning; add a clear warning line
immediately before printing any sudo or privileged commands (e.g., "Warning: the
following commands require sudo/root privileges — review before running") so
users are explicitly alerted before copy-pasting them; ensure the warning is
only printed for target_os == "Linux" (or any branch that prints sudo) and keep
it concise and prominent.
| print("\n4️⃣ Run Cortex:") | ||
| if target_ui == "Web UI": | ||
| print(" cortex-server --web") | ||
| elif target_ui == "API Server": | ||
| print(" cortex-server --host 0.0.0.0 --port 8000") | ||
| else: | ||
| print(" cortex run") |
There was a problem hiding this comment.
Fix string comparison bug that breaks Web UI option.
Line 77 compares target_ui == "Web UI", but the actual option string (defined on Line 41) is "Web UI (Browser-based)". This comparison will never be true, causing Web UI selection to fall through to the CLI command. This is a critical correctness bug.
🔎 Proposed fix
print("\n4️⃣ Run Cortex:")
- if target_ui == "Web UI":
+ if target_ui == "Web UI (Browser-based)":
print(" cortex-server --web")
elif target_ui == "API Server":
print(" cortex-server --host 0.0.0.0 --port 8000")📝 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.
| print("\n4️⃣ Run Cortex:") | |
| if target_ui == "Web UI": | |
| print(" cortex-server --web") | |
| elif target_ui == "API Server": | |
| print(" cortex-server --host 0.0.0.0 --port 8000") | |
| else: | |
| print(" cortex run") | |
| print("\n4️⃣ Run Cortex:") | |
| if target_ui == "Web UI (Browser-based)": | |
| print(" cortex-server --web") | |
| elif target_ui == "API Server": | |
| print(" cortex-server --host 0.0.0.0 --port 8000") | |
| else: | |
| print(" cortex run") |
🤖 Prompt for AI Agents
In cortex/install_tutor.py around lines 76 to 82, the if check uses target_ui ==
"Web UI" but the option value defined earlier is "Web UI (Browser-based)", so
the equality never matches and the Web UI branch falls through; fix by comparing
against the exact option string used when setting target_ui (e.g., target_ui ==
"Web UI (Browser-based)"), or use a stable test like target_ui.startswith("Web
UI") or "Web UI" in target_ui to detect the Web UI selection; update the
conditional accordingly so the correct cortex-server --web command is printed.
1 participant
I have implemented the interactive installation assistant script requested in Issue #140.
Features:
pip installcommands based on inputs./claim #140
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.