feat: Introduce intelligent package manager wrapper #195
Conversation
WalkthroughThis PR introduces an intelligent package manager wrapper that auto-detects the system's package manager (apt/yum/dnf), maps natural language requests to packages across multiple domains (Python, data science, web, databases, dev tools, etc.), and generates concrete package manager commands. It includes comprehensive test coverage with 20+ common requests. Changes
Sequence DiagramsequenceDiagram
participant User
participant PM as PackageManager
participant PkgMap as Package Mappings
participant SubProcess as subprocess
User->>PM: parse("install python with data science")
activate PM
PM->>PM: _normalize_text()
Note over PM: "install python with data science"
PM->>PM: _extract_action()
Note over PM: Action = "install"
PM->>PM: _find_matching_packages()
PM->>PkgMap: lookup "python" + "data science"
PkgMap-->>PM: ["python3", "python3-pip", "python3-numpy", "python3-pandas"]
PM->>PM: Build command template<br/>(apt/yum/dnf)
PM->>SubProcess: apt install -y python3 python3-pip...
SubProcess-->>PM: Success
deactivate PM
PM-->>User: ["apt install -y python3 python3-pip python3-numpy python3-pandas"]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (6)
cortex/__init__.py (1)
2-6: Public API export looks good; optionally sort__all__for Ruff.Re‑exporting
PackageManagerandPackageManagerTypefrom the package root is a good call and matches the tests’ usage. To satisfy Ruff (RUF022) and avoid lint noise, you can sort__all__alphabetically:-__all__ = ["main", "PackageManager", "PackageManagerType"] +__all__ = ["PackageManager", "PackageManagerType", "main"]test/test_packages.py (2)
1-4: Drop the shebang from this test module.The shebang isn’t needed for a test file that’s run via
python -m unittest/pytest and also triggers Ruff’s EXE001 (file not executable). Simplify the header like this:-#!/usr/bin/env python3 -""" -Unit tests for the intelligent package manager wrapper. -""" +""" +Unit tests for the intelligent package manager wrapper. +"""
20-25: SimplifysetUpby removing the unusedsubprocess.runpatch.Because you pass
pm_type=PackageManagerType.APT,_detect_package_manageris never called here, so patchingcortex.packages.subprocess.runinsetUphas no effect. You can simplify:def setUp(self): """Set up test fixtures.""" - # Mock package manager detection to use apt for consistent testing - with patch('cortex.packages.subprocess.run') as mock_run: - mock_run.return_value = MagicMock(returncode=0) - self.pm = PackageManager(pm_type=PackageManagerType.APT) + # Use a fixed package manager type to keep tests deterministic + self.pm = PackageManager(pm_type=PackageManagerType.APT)This keeps tests deterministic and removes an unnecessary mock.
cortex/packages.py (3)
1-7: Remove shebang and update docstring to mention DNF.This is a library module, so the shebang isn’t needed and triggers EXE001 if the file isn’t executable. Also, the docstring omits DNF even though it’s supported. You can tidy both:
-#!/usr/bin/env python3 -""" -Intelligent Package Manager Wrapper for Cortex Linux - -Translates natural language requests into apt/yum package manager commands. -Supports common software installations, development tools, and libraries. -""" +""" +Intelligent Package Manager Wrapper for Cortex Linux. + +Translates natural language requests into apt/yum/dnf package manager commands. +Supports common software installations, development tools, and libraries. +"""
9-20: Consider usingshutil.whichinstead of spawningwhichsubprocesses.
_detect_package_manageronly needs to know whetherapt,dnf, oryumexist; usingshutil.whichavoids extra subprocesses, timeouts, and S607 warnings. Suggested refactor:-import re -import subprocess -import platform -from typing import List, Dict, Optional, Tuple, Set -from enum import Enum +import re +import shutil +import subprocess +import platform +from typing import List, Dict, Optional, Tuple, Set +from enum import Enumdef _detect_package_manager(self) -> PackageManagerType: - """Detect the package manager based on the system.""" - try: - # Check for apt - result = subprocess.run( - ["which", "apt"], - capture_output=True, - text=True, - timeout=2 - ) - if result.returncode == 0: - return PackageManagerType.APT - - # Check for dnf (preferred over yum on newer systems) - result = subprocess.run( - ["which", "dnf"], - capture_output=True, - text=True, - timeout=2 - ) - if result.returncode == 0: - return PackageManagerType.DNF - - # Check for yum - result = subprocess.run( - ["which", "yum"], - capture_output=True, - text=True, - timeout=2 - ) - if result.returncode == 0: - return PackageManagerType.YUM - except (subprocess.TimeoutExpired, FileNotFoundError): - pass - - # Default to apt (most common) - return PackageManagerType.APT + """Detect the package manager based on the system.""" + # Prefer APT, then DNF, then YUM + if shutil.which("apt"): + return PackageManagerType.APT + if shutil.which("dnf"): + return PackageManagerType.DNF + if shutil.which("yum"): + return PackageManagerType.YUM + # Default to apt (most common) + return PackageManagerType.APT
362-393: Fix the unnecessary f-string for"apt update".The
updatebranch for APT uses an f‑string without placeholders (f"apt update"), which is what Ruff flags (F541). You can drop thefwithout changing behavior:- elif action == "update": - return [f"apt update", f"apt upgrade -y {' '.join(packages)}"] + elif action == "update": + return ["apt update", f"apt upgrade -y {' '.join(packages)}"]The rest of
parse(validation, error handling, and per‑PM command construction) looks solid and is well covered by the tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cortex/__init__.py(1 hunks)cortex/packages.py(1 hunks)test/test_packages.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/test_packages.py (1)
cortex/packages.py (6)
PackageManager(23-452)PackageManagerType(16-20)parse(362-406)_normalize_text(261-270)_extract_action(272-282)get_package_info(408-452)
cortex/__init__.py (1)
cortex/packages.py (2)
PackageManager(23-452)PackageManagerType(16-20)
🪛 Ruff (0.14.5)
test/test_packages.py
1-1: Shebang is present but file is not executable
(EXE001)
cortex/packages.py
1-1: Shebang is present but file is not executable
(EXE001)
49-49: Starting a process with a partial executable path
(S607)
59-59: Starting a process with a partial executable path
(S607)
69-69: Starting a process with a partial executable path
(S607)
376-376: Avoid specifying long messages outside the exception class
(TRY003)
382-382: Avoid specifying long messages outside the exception class
(TRY003)
391-391: f-string without any placeholders
Remove extraneous f prefix
(F541)
420-420: subprocess call: check for execution of untrusted input
(S603)
421-421: Starting a process with a partial executable path
(S607)
436-436: subprocess call: check for execution of untrusted input
(S603)
cortex/__init__.py
6-6: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🔇 Additional comments (4)
test/test_packages.py (1)
27-361: Test coverage forPackageManageris strong and well-aligned with behavior.The suite thoroughly exercises Python variants, web/dev/database/system bundles, multiple actions (install/remove/update/search), case‑insensitivity, multi‑category requests, and
get_package_infofor both APT and YUM/DNF with mocks. This gives good safety net around the NL‑to‑command translation behavior.cortex/packages.py (3)
111-259: Package mappings look consistent and comprehensive.The curated mappings for Python variants, web/dev tools, databases, system/network utilities, media, security, editors, and VCS align with the tests and acceptance criteria (20+ common requests). The separation into
"apt"vs"yum"keys and reuse of"yum"for DNF keep the implementation clear and maintainable.
261-360: NL matching logic matches requirements and test expectations.The normalization + action extraction +
_find_matching_packagespipeline correctly prioritizes Python special cases, multi‑word categories (e.g., “web development”, “system monitoring”), and single‑word software (docker/nginx/git/etc.), and aggregates them via a set before sorting. This matches the behavior asserted in the tests (multi‑category requests, name variations, YUM vs APT differences).
408-452:get_package_infosubprocess usage is reasonable and guarded.
get_package_infouses fixed executables (apt-cache,yum/dnf) withshell=False, timeouts, and graceful handling ofTimeoutExpired/FileNotFoundError, returningNoneon failure. Given that, the S603/S607 hints here are effectively false positives and I wouldn’t change this logic unless you want to further constrainpackage_nameor silence the lints with# noqa.
|
thanks @mikejmorgan-ai |
2 participants
A lightweight Python helper that converts natural language requests into apt/yum/dnf commands.
Basic Installation
Development Tools
Data Science & Machine Learning
Databases
System Tools
Different Actions
Case Insensitive & Variations
Multi-Distribution Examples
Error Handling
Package Manager Detection
Package Information Lookup
Key Features
Screencast.from.18-11-25.12.06.16.PM.IST.webm
/closes #7