Feat/270 custom ai roles #307
Conversation
WalkthroughAdds a role/persona system: new RoleManager module, five built-in role YAMLs, CLI role subcommands and wiring, and CommandInterpreter now accepts and applies a role's prompt additions during initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CortexCLI
participant CI as CommandInterpreter
participant RM as RoleManager
participant Builtin as Built-in Roles (YAML)
participant Custom as Custom Roles (YAML)
User->>CLI: run `cortex --role security install ...`
CLI->>CLI: set self.role = "security"
CLI->>CI: instantiate CommandInterpreter(..., role="security")
CI->>RM: RM.get_role("security")
RM->>Custom: check ~/.cortex/roles/security.yaml
alt custom exists
Custom-->>RM: return Role (custom)
else custom missing
RM->>Builtin: read cortex/roles/security.yaml
Builtin-->>RM: return Role (builtin)
end
RM-->>CI: Role object (cached)
CI->>CI: build system prompt including role.prompt_additions/priorities
CI-->>CLI: interpreter ready (role-aware)
CLI->>User: execute command with role-enhanced prompt
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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: 2
🧹 Nitpick comments (5)
tests/test_role_manager.py (1)
406-413: Weak assertion in default role test.Line 411 asserts
len(role.priorities) >= 0, which is always True for any list. Consider either checking for specific expected priorities or verifying the list is non-empty if that's the requirement.🔎 Suggested improvement
def test_default_role_priorities(self): """Test that default role has expected priorities""" manager = RoleManager() try: role = manager.get_role("default") - assert "reliability" in role.priorities or len(role.priorities) >= 0 + # Default role should have meaningful priorities defined + assert isinstance(role.priorities, list) + assert len(role.priorities) > 0 except RoleNotFoundError: pass # Skip if not in correct directorycortex/cli.py (2)
555-581: Consider adding return type hint to the docstring.The method implementation is clean and provides a well-formatted table output. Per coding guidelines requiring type hints, consider adding the return type annotation:
- def role_list(self): + def role_list(self) -> int:
659-663: Handle editor subprocess errors for better user feedback.
subprocess.runwithoutcheck=Truesilently ignores editor failures. If the editor fails to launch or exits with an error, the user won't know. Consider adding error handling:🔎 Proposed fix
try: - subprocess.run([editor, str(role_path)]) + result = subprocess.run([editor, str(role_path)]) + if result.returncode != 0: + cx_print(f"Editor exited with code {result.returncode}", "warning") except FileNotFoundError: cx_print(f"Could not open editor. Edit the file manually: {role_path}", "warning")cortex/role_manager.py (1)
123-145: Consider validatingprioritiestype.The validation doesn't verify that
prioritiesis a list. If a YAML file haspriorities: "single-value"instead of a list,Role.from_dict()will assign the string directly, which could cause issues downstream.🔎 Proposed fix
if not role.prompt_additions: raise RoleValidationError("Role must have prompt_additions") + + if not isinstance(role.priorities, list): + raise RoleValidationError("Role priorities must be a list")cortex/llm/interpreter.py (1)
97-98: Consider using the logging framework.The codebase consistently uses Python's logging module across 15+ files. Printing directly to stderr at line 98 is inconsistent with this established pattern. Replace with:
import logging logger = logging.getLogger(__name__) # ... logger.warning(f"Could not load role '{role_name}': {e}")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cortex/cli.py(9 hunks)cortex/llm/interpreter.py(6 hunks)cortex/role_manager.py(1 hunks)cortex/roles/__init__.py(1 hunks)cortex/roles/datascience.yaml(1 hunks)cortex/roles/default.yaml(1 hunks)cortex/roles/devops.yaml(1 hunks)cortex/roles/security.yaml(1 hunks)cortex/roles/sysadmin.yaml(1 hunks)tests/test_role_manager.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/roles/__init__.pycortex/llm/interpreter.pycortex/cli.pytests/test_role_manager.pycortex/role_manager.py
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
cortex/roles/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_role_manager.py
🧬 Code graph analysis (3)
cortex/llm/interpreter.py (1)
cortex/role_manager.py (3)
RoleManager(67-374)RoleNotFoundError(24-27)get_role(147-180)
cortex/cli.py (1)
cortex/role_manager.py (9)
RoleManager(67-374)RoleError(18-21)RoleNotFoundError(24-27)list_roles(182-231)get_role(147-180)role_exists(358-370)create_role(233-285)get_role_template(339-356)delete_role(309-337)
tests/test_role_manager.py (1)
cortex/role_manager.py (14)
Role(37-64)RoleError(18-21)RoleManager(67-374)RoleNotFoundError(24-27)RoleValidationError(30-33)_validate_role(123-145)_load_role_from_file(93-121)get_role(147-180)list_roles(182-231)create_role(233-285)delete_role(309-337)role_exists(358-370)get_role_template(339-356)clear_cache(372-374)
🔇 Additional comments (35)
cortex/roles/__init__.py (1)
1-3: LGTM!The package initializer is minimal and well-documented. The comments clearly explain the role loading behavior.
cortex/roles/default.yaml (1)
1-14: LGTM!The default role definition is well-structured with clear, general-purpose guidelines. The priorities (reliability, simplicity, compatibility) align well with a default configuration.
cortex/roles/security.yaml (1)
1-22: LGTM!The security role provides comprehensive security-focused guidelines covering hardening, auditing, and secure configuration. The priorities appropriately emphasize security and compliance concerns.
cortex/roles/devops.yaml (1)
1-23: LGTM!The devops role provides excellent guidelines for modern DevOps practices, covering containerization, orchestration, and observability. The priorities appropriately emphasize automation and scalability.
cortex/roles/datascience.yaml (1)
1-23: LGTM!The datascience role provides comprehensive guidelines for ML/AI and scientific computing, with appropriate focus on GPU optimization, reproducibility, and performance. The priorities align well with data science requirements.
cortex/roles/sysadmin.yaml (1)
1-26: LGTM!The sysadmin role provides comprehensive system administration guidelines covering monitoring, maintenance, and disaster recovery. The priorities appropriately emphasize stability and backup.
tests/test_role_manager.py (5)
1-20: LGTM!Imports are well-organized (standard library, third-party, local) and the module docstring is present.
21-74: LGTM!The Role dataclass tests are comprehensive, covering creation, serialization, deserialization, and default handling. All tests have clear docstrings.
105-345: LGTM!The RoleManager tests provide excellent coverage of validation, loading, precedence rules, CRUD operations, and utility methods. The test structure is clear with well-named methods and comprehensive docstrings.
347-387: LGTM!The caching tests properly verify that roles are cached after first load and that
clear_cache()invalidates the cache correctly by checking object identity.
389-424: LGTM!The built-in role tests appropriately handle missing roles (when running from different directories) and verify role content matches expectations.
cortex/cli.py (9)
21-23: LGTM!Clean import of
RoleManagerand related exceptions. The exception hierarchy supports proper error handling in the CLI methods.
37-43: LGTM!The role parameter is properly typed and stored. The default value
"default"aligns with the built-in role names inRoleManager.BUILTIN_ROLES.
200-211: LGTM!Good integration of the role parameter into the install workflow. The debug message at line 200 aids troubleshooting, and passing
role=self.roletoCommandInterpreterensures role-specific prompts are applied.
583-602: LGTM!Good exception handling for
RoleNotFoundErrorwith a user-friendly error message. The role details are displayed clearly using Rich formatting.
666-676: LGTM!Clean delegation to
RoleManager.delete_role()with proper error handling. TheRoleErrorcatch covers both built-in role protection and not-found scenarios.
708-714: LGTM!Clear examples demonstrating both the
--roleflag for installation and therolesubcommand for management. This helps users discover the feature.
747-752: LGTM!Good choice making
--rolea global flag available to all commands. The help text clearly lists available built-in roles.
818-838: LGTM!Well-structured subparser for role commands. The
--fromargument usingdest="from_template"correctly avoids the Python keyword conflict.
875-891: LGTM!Clean dispatch logic with appropriate fallback to
role_list()when no action is specified. The use ofgetattr(args, "from_template", None)safely handles the optional argument.cortex/role_manager.py (11)
1-15: LGTM!Good module-level docstring explaining the purpose of the role management system. The imports are appropriate for YAML parsing and dataclass usage.
18-33: LGTM!Clean exception hierarchy with specific exceptions for different error scenarios. This enables targeted error handling in the CLI layer.
36-64: LGTM!Good use of
dataclasswith proper type hints. The use offield(default_factory=list)correctly avoids the mutable default argument pitfall. Theto_dict()method appropriately excludesis_builtinsince it's metadata not meant for persistence.
81-91: LGTM!The directory initialization with
mkdir(parents=True, exist_ok=True)is idempotent and safe. The cache type hintdict[str, Role]provides good documentation.
93-121: LGTM!Proper use of
yaml.safe_load()for security. The error handling distinguishes between YAML parsing errors and file access errors, providing appropriate exception types for each.
147-180: LGTM!Clean implementation with cache-first lookup. The custom role precedence over built-in roles is well-documented and correctly implemented.
182-230: LGTM!The precedence handling with
seen_namescorrectly ensures custom roles override built-in roles. Silently skipping invalid role files is appropriate for a listing operation to avoid breaking the entire list.
233-285: LGTM!Good implementation with template copying support. The defensive
priorities.copy()at line 265 prevents shared state issues. Note that the method allows overwriting existing roles by design—the CLI layer handles the existence check before calling this method.
287-307: LGTM!Excellent atomic write pattern using a temp file and
Path.replace(). The cleanup of the temp file in the exception handler prevents orphaned files.
309-337: LGTM!Good protection against deleting built-in roles. The logic correctly distinguishes between: (1) built-in with no custom override (error), (2) custom override exists (delete allowed), and (3) pure custom role (delete allowed).
339-374: LGTM!The template provides clear placeholders for users creating custom roles. The
role_exists()method correctly checks both custom and built-in directories.cortex/llm/interpreter.py (4)
21-22: LGTM!Documentation clearly describes the new role-based customization feature.
32-32: LGTM!The role parameter is properly added with a sensible default, well-documented, and stored appropriately. The default value ensures backward compatibility.
Also applies to: 42-42, 47-47
49-51: Good initialization order.Loading the role before cache initialization is correct since the role affects the system prompt, which is used in cache keys (line 278). The initial
Noneassignment provides a safe fallback state.
120-158: LGTM!The system prompt construction properly integrates role-specific additions. The defensive
getattrcheck on line 145 is good practice even though_roleis always initialized. The string formatting clearly separates the base prompt from role-specific content.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/test_role_manager.py (1)
98-104: Fix tautological and incorrect assertions.Two issues with this test:
- Line 101's assertion is tautological (always True), as flagged in a previous review.
- Line 103 checks
manager.custom_roles_dir.parent.exists()instead ofmanager.custom_roles_dir.exists(). SinceRoleManager.__init__callsmkdir(parents=True, exist_ok=True)oncustom_roles_dir(per cortex/role_manager.py lines 85-86), the test should verify that the directory itself was created.🔎 Proposed fix
def test_manager_initialization(self): """Test RoleManager initialization""" manager = RoleManager() - assert manager.builtin_roles_dir.exists() or not manager.builtin_roles_dir.exists() + assert isinstance(manager.builtin_roles_dir, Path) # Custom roles dir should be created - assert manager.custom_roles_dir.parent.exists() + assert manager.custom_roles_dir.exists()
🧹 Nitpick comments (4)
tests/test_role_manager.py (4)
7-7: Unused import:mock.The
mockmodule is imported but never used in this test file. Consider removing it to keep imports clean.🔎 Proposed fix
import tempfile from pathlib import Path -from unittest import mock import pytest
144-180: Consider adding test for empty YAML file.The file loading tests cover valid YAML and invalid syntax. However,
RoleManager._load_role_from_file(lines 111-112 in role_manager.py) also handles empty YAML files with a specific error. Consider adding a test for this case:def test_load_role_empty_file(self, temp_dirs): """Test loading a role from an empty YAML file""" custom_dir, builtin_dir = temp_dirs manager = RoleManager() manager.custom_roles_dir = custom_dir manager.builtin_roles_dir = builtin_dir role_file = builtin_dir / "empty.yaml" role_file.touch() # Create empty file with pytest.raises(RoleValidationError, match="Empty role file"): manager._load_role_from_file(role_file)
215-243: Consider testing override scenario in list_roles.The current test verifies basic listing functionality. Consider adding a test case where a custom role overrides a built-in role to verify that:
- The role appears only once in the list
- The
is_custom_overrideflag is set correctly- The
is_builtinflag is False for the overrideThis would provide more comprehensive coverage of
list_rolesbehavior (lines 181-230 in role_manager.py).
338-346: Consider validating template YAML syntax.The test verifies that the template contains expected keys by string matching. For more robust testing, consider parsing the template as YAML to ensure it's syntactically valid:
def test_get_role_template(self, manager_with_temp_dirs): """Test getting a role template""" manager = manager_with_temp_dirs template = manager.get_role_template() # Verify it's valid YAML data = yaml.safe_load(template) assert "name" in data assert "description" in data assert "prompt_additions" in data assert "priorities" in data
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_role_manager.py(1 hunks)
🧰 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:
tests/test_role_manager.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/test_role_manager.py
🧬 Code graph analysis (1)
tests/test_role_manager.py (1)
cortex/role_manager.py (14)
Role(37-64)RoleError(18-21)RoleManager(67-374)RoleNotFoundError(24-27)RoleValidationError(30-33)_validate_role(123-145)_load_role_from_file(93-121)get_role(147-180)list_roles(182-231)create_role(233-285)delete_role(309-337)role_exists(358-370)get_role_template(339-356)clear_cache(372-374)
🔇 Additional comments (8)
tests/test_role_manager.py (8)
21-74: LGTM: Comprehensive Role dataclass tests.The Role dataclass tests cover creation, serialization (
to_dict), deserialization (from_dict), and default handling. The verification thatis_builtinis excluded from serialization (line 52) correctly aligns with the implementation.
79-96: LGTM: Clean fixture setup for test isolation.The fixtures properly use temporary directories to isolate tests. Note that
manager_with_temp_dirspatches directory paths after initialization, which bypasses themkdir(parents=True, exist_ok=True)call inRoleManager.__init__(line 85 in role_manager.py). This is acceptable for testing since the fixture manually creates these directories.
105-143: LGTM: Thorough validation test coverage.The validation tests comprehensively cover all validation rules from
RoleManager._validate_role: missing name, description, prompt_additions, and invalid name format. Testing the private_validate_rolemethod directly is appropriate for unit testing.
181-214: LGTM: Correct precedence and error handling tests.The tests properly verify that custom roles override built-in roles (line 206 checks custom description is returned) and that missing roles raise
RoleNotFoundError. This aligns with theget_roleimplementation's precedence logic.
244-282: LGTM: Create role tests cover basic and template scenarios.Both tests correctly verify role creation. The template test (lines 256-282) properly validates that empty
descriptionandprompt_additionsare inherited from the template, matching the implementation logic (lines 248-253 in role_manager.py).
283-318: LGTM: Delete tests properly verify deletion and built-in protection.The tests correctly verify that custom roles can be deleted and built-in roles are protected from deletion. The error message check on line 316 matches the implementation (line 325 in role_manager.py).
319-337: LGTM: Straightforward existence check test.The test correctly verifies both positive and negative cases for
role_exists, matching the implementation that checks both custom and built-in paths.
347-387: LGTM: Cache tests properly use identity checks.The caching tests correctly use identity checks (
is/is not) to verify that cached roles return the same object instance (line 366) and that cache clearing forces reloading (line 386). This properly validates the caching mechanism.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cortex/llm/interpreter.py(6 hunks)tests/test_role_manager.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_role_manager.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/llm/interpreter.py
🧬 Code graph analysis (1)
cortex/llm/interpreter.py (1)
cortex/role_manager.py (3)
RoleManager(67-374)RoleNotFoundError(24-27)get_role(147-180)
🔇 Additional comments (2)
cortex/llm/interpreter.py (2)
76-104: Docstring accurately reflects exception handling behavior.The docstring has been properly updated to reflect that
ValueErroris raised only when the role is not found, while other exceptions result in a warning with graceful fallback. This addresses the previous review concern.
125-164: LGTM: Clean role-aware prompt construction.The implementation correctly incorporates role-specific additions into the system prompt with proper defensive checks and clear formatting.
| self.role_name = role | ||
|
|
||
| # Load role configuration | ||
| self._role = None |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type annotation for the _role attribute.
The _role attribute should have a type annotation per coding guidelines. Import Role under the TYPE_CHECKING block to avoid circular imports, then annotate as Optional[Role].
🔎 Suggested changes
At the top of the file, add the Role import to the TYPE_CHECKING block:
if TYPE_CHECKING:
from cortex.semantic_cache import SemanticCache
+ from cortex.role_manager import RoleThen annotate the attribute:
- self._role = None
+ self._role: Optional[Role] = NoneAs per coding guidelines: "Type hints required in 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.
| self._role = None | |
| self._role: Optional[Role] = None |
| self._role = None | |
| if TYPE_CHECKING: | |
| from cortex.semantic_cache import SemanticCache | |
| from cortex.role_manager import Role |
🤖 Prompt for AI Agents
In cortex/llm/interpreter.py around line 50, annotate the `_role` attribute as
Optional[Role] and add the necessary TYPE_CHECKING import: import Role under an
existing TYPE_CHECKING block (from cortex.llm.types or the correct module) and
ensure Optional is imported from typing; then change the assignment to include
the type annotation (self._role: Optional[Role] = None) to satisfy the
type-hinting guideline and avoid circular imports by using TYPE_CHECKING.
|
Hi @Zafeeruddin - please only work on issues that are assigned to you, and make sure to comment when you start working on an issue so others know. Issue cortexlinux/cortex-pro#1 is assigned to @pavanimanchala53 and is already in progress, which caused duplicate work. Take a look at https://github.com/cortexlinux/cortex/blob/main/Contributing.md |
|
|
@mikejmorgan-ai Please have a look |
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cortex/cli.py(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/cli.py
🔇 Additional comments (6)
cortex/cli.py (6)
20-22: LGTM! Clean import of role management components.The import statement is well-organized with a descriptive comment.
37-37: LGTM! Role parameter integration is clean.The role parameter is properly typed and stored with a sensible default.
Also applies to: 43-43
319-319: LGTM! Role integration into installation flow is correct.Debug logging and parameter passing are implemented consistently with the existing pattern.
Also applies to: 330-330
820-820: LGTM! Help text additions are clear and helpful.The role command documentation and examples follow the existing pattern and provide good user guidance.
Also applies to: 829-834
867-872: LGTM! Argument parser setup is well-structured.The global
--roleflag and role subcommands are properly configured with appropriate defaults and help text. Good use ofdest="from_template"to avoid keyword conflicts.Also applies to: 949-968
976-976: LGTM! Command dispatch logic is robust.The role parameter is correctly threaded through, and the command dispatch handles all role subcommands with safe defaults. Good defensive use of
getattrfor optional arguments.Also applies to: 1008-1024
| def role_list(self): | ||
| """List all available roles""" | ||
| from rich.table import Table | ||
|
|
||
| manager = RoleManager() | ||
| roles = manager.list_roles() | ||
|
|
||
| cx_header("Available Roles") | ||
|
|
||
| table = Table(show_header=True, header_style="bold cyan", box=None) | ||
| table.add_column("Role", style="green") | ||
| table.add_column("Description") | ||
| table.add_column("Type", style="dim") | ||
|
|
||
| for role in roles: | ||
| role_type = "built-in" | ||
| if role["is_custom_override"]: | ||
| role_type = "custom (override)" | ||
| elif not role["is_builtin"]: | ||
| role_type = "custom" | ||
|
|
||
| table.add_row(role["name"], role["description"], role_type) | ||
|
|
||
| console.print(table) | ||
| console.print() | ||
| cx_print("Use [bold]cortex --role <name> install <pkg>[/bold] to use a role", "info") | ||
| return 0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add return type hint and enhance docstring.
The method is missing a return type hint, and the docstring could be more descriptive.
🔎 Proposed enhancement
- def role_list(self):
- """List all available roles"""
+ def role_list(self) -> int:
+ """List all available roles (built-in and custom).
+
+ Returns:
+ int: Exit code (0 for success)
+ """
from rich.table import TableAs per coding guidelines, type hints are required in 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 role_list(self): | |
| """List all available roles""" | |
| from rich.table import Table | |
| manager = RoleManager() | |
| roles = manager.list_roles() | |
| cx_header("Available Roles") | |
| table = Table(show_header=True, header_style="bold cyan", box=None) | |
| table.add_column("Role", style="green") | |
| table.add_column("Description") | |
| table.add_column("Type", style="dim") | |
| for role in roles: | |
| role_type = "built-in" | |
| if role["is_custom_override"]: | |
| role_type = "custom (override)" | |
| elif not role["is_builtin"]: | |
| role_type = "custom" | |
| table.add_row(role["name"], role["description"], role_type) | |
| console.print(table) | |
| console.print() | |
| cx_print("Use [bold]cortex --role <name> install <pkg>[/bold] to use a role", "info") | |
| return 0 | |
| def role_list(self) -> int: | |
| """List all available roles (built-in and custom). | |
| Returns: | |
| int: Exit code (0 for success) | |
| """ | |
| from rich.table import Table | |
| manager = RoleManager() | |
| roles = manager.list_roles() | |
| cx_header("Available Roles") | |
| table = Table(show_header=True, header_style="bold cyan", box=None) | |
| table.add_column("Role", style="green") | |
| table.add_column("Description") | |
| table.add_column("Type", style="dim") | |
| for role in roles: | |
| role_type = "built-in" | |
| if role["is_custom_override"]: | |
| role_type = "custom (override)" | |
| elif not role["is_builtin"]: | |
| role_type = "custom" | |
| table.add_row(role["name"], role["description"], role_type) | |
| console.print(table) | |
| console.print() | |
| cx_print("Use [bold]cortex --role <name> install <pkg>[/bold] to use a role", "info") | |
| return 0 |
🤖 Prompt for AI Agents
In cortex/cli.py around lines 674 to 700, the role_list method lacks a return
type hint and has a minimal docstring; update the docstring to briefly describe
what the method does and its return value, and add an explicit return type hint
(-> int) to the method signature. Ensure the docstring follows project style
(one-line summary plus optional short detail) and mentions that it prints roles
to console and returns 0 on success.
| def role_show(self, name: str): | ||
| """Show details of a specific role""" | ||
| try: | ||
| manager = RoleManager() | ||
| role = manager.get_role(name) | ||
|
|
||
| cx_header(f"Role: {role.name}") | ||
| console.print(f"[bold]Description:[/bold] {role.description}") | ||
| console.print(f"[bold]Type:[/bold] {'Built-in' if role.is_builtin else 'Custom'}") | ||
|
|
||
| if role.priorities: | ||
| console.print(f"[bold]Priorities:[/bold] {', '.join(role.priorities)}") | ||
|
|
||
| console.print("\n[bold]Prompt Additions:[/bold]") | ||
| console.print(f"[dim]{role.prompt_additions}[/dim]") | ||
|
|
||
| return 0 | ||
| except RoleNotFoundError: | ||
| self._print_error(f"Role '{name}' not found") | ||
| return 1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add return type hint and enhance docstring.
The method lacks a return type hint, and the docstring should document parameters and return values.
🔎 Proposed enhancement
- def role_show(self, name: str):
- """Show details of a specific role"""
+ def role_show(self, name: str) -> int:
+ """Show details of a specific role.
+
+ Args:
+ name: The role name to display
+
+ Returns:
+ int: Exit code (0 for success, 1 if role not found)
+ """
try:As per coding guidelines, type hints are required in Python code.
🤖 Prompt for AI Agents
In cortex/cli.py around lines 702 to 721, the role_show method is missing a
return type hint and has an underspecified docstring; update the signature to
include a return type hint (-> int) and expand the docstring to document the
parameters and return value (describe the 'name' parameter and that the method
returns 0 on success and 1 on not found), keeping existing behavior and
exception handling unchanged.
| def role_create(self, name: str, description: str = "", from_template: str | None = None): | ||
| """Create a new custom role""" | ||
| import subprocess | ||
| import tempfile | ||
|
|
||
| manager = RoleManager() | ||
|
|
||
| # Check if role already exists | ||
| if manager.role_exists(name): | ||
| self._print_error(f"Role '{name}' already exists") | ||
| return 1 | ||
|
|
||
| # If from_template, copy the template | ||
| if from_template: | ||
| try: | ||
| template_role = manager.get_role(from_template) | ||
| manager.create_role( | ||
| name=name, | ||
| description=description or f"Custom role based on {from_template}", | ||
| prompt_additions=template_role.prompt_additions, | ||
| priorities=template_role.priorities.copy(), | ||
| ) | ||
| self._print_success(f"Created role '{name}' from template '{from_template}'") | ||
| cx_print(f"Edit at: ~/.cortex/roles/{name}.yaml", "info") | ||
| return 0 | ||
| except RoleNotFoundError: | ||
| self._print_error(f"Template role '{from_template}' not found") | ||
| return 1 | ||
| except RoleError as e: | ||
| self._print_error(f"Failed to create role: {e}") | ||
| return 1 | ||
|
|
||
| # Create from scratch - use template and open in editor | ||
| template = manager.get_role_template().replace("my-custom-role", name) | ||
|
|
||
| if description: | ||
| template = template.replace( | ||
| '"Brief description of what this role does"', | ||
| f'"{description}"' | ||
| ) | ||
|
|
||
| # Save template to the roles directory | ||
| role_path = manager.custom_roles_dir / f"{name}.yaml" | ||
| role_path.parent.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| with open(role_path, "w") as f: | ||
| f.write(template) | ||
|
|
||
| self._print_success(f"Created role template at: {role_path}") | ||
| cx_print("Edit the file to customize your role's behavior", "info") | ||
|
|
||
| # Try to open in editor | ||
| editor = os.environ.get("EDITOR", "nano") | ||
| cx_print(f"Opening in {editor}...", "info") | ||
|
|
||
| try: | ||
| subprocess.run([editor, str(role_path)]) | ||
| except FileNotFoundError: | ||
| cx_print(f"Could not open editor. Edit the file manually: {role_path}", "warning") | ||
|
|
||
| return 0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add return type hint, enhance docstring, and remove unused import.
Several improvements needed for code quality and documentation.
🔎 Proposed enhancements
- def role_create(self, name: str, description: str = "", from_template: str | None = None):
- """Create a new custom role"""
+ def role_create(self, name: str, description: str = "", from_template: str | None = None) -> int:
+ """Create a new custom role, optionally from a template.
+
+ Args:
+ name: Name for the new role
+ description: Optional description for the role
+ from_template: Optional existing role name to copy from
+
+ Returns:
+ int: Exit code (0 for success, 1 on error)
+ """
import subprocess
- import tempfileNotes:
tempfileis imported at line 726 but never used- As per coding guidelines, type hints and comprehensive docstrings are required
🤖 Prompt for AI Agents
In cortex/cli.py around lines 723 to 783, remove the unused "tempfile" import,
add an explicit return type hint "-> int" to the role_create signature, and
replace the short docstring with a full one-line summary plus parameter and
return descriptions (name: role name, description: optional text, from_template:
optional template to copy; return: 0 on success, 1 on error). Ensure imports
still include subprocess and os as needed and update any linter comments if
required.
| def role_delete(self, name: str): | ||
| """Delete a custom role""" | ||
| manager = RoleManager() | ||
|
|
||
| try: | ||
| manager.delete_role(name) | ||
| self._print_success(f"Deleted role '{name}'") | ||
| return 0 | ||
| except RoleError as e: | ||
| self._print_error(str(e)) | ||
| return 1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add return type hint and enhance docstring.
The method needs a return type hint and better documentation.
🔎 Proposed enhancement
- def role_delete(self, name: str):
- """Delete a custom role"""
+ def role_delete(self, name: str) -> int:
+ """Delete a custom role.
+
+ Args:
+ name: Name of the role to delete
+
+ Returns:
+ int: Exit code (0 for success, 1 on error)
+ """
manager = RoleManager()As per coding guidelines, type hints are required in Python code.
🤖 Prompt for AI Agents
In cortex/cli.py around lines 785 to 795, the role_delete method is missing a
return type hint and has a minimal docstring; update the signature to include a
return type of int and expand the docstring to describe the purpose, parameters
(name: str), return value (0 on success, 1 on failure), and which exceptions are
handled (RoleError) so callers and static type checkers have clear expectations.
Thanks for letting me know |
|
@Zafeeruddin Thanks for working on Custom AI Roles! We merged #310 from @pavanimanchala53 which implemented the same feature. Please check our other open issues for more contribution opportunities - we appreciate your effort! |
3 participants
Related Issue
Closes cortexlinux/cortex-pro#1
Summary
This PR implements Role-Based Prompts for Cortex, allowing specialized AI personas for different use cases like security auditing, DevOps, data science, and system administration.
What's New
Built-in Roles (5 roles included):
defaultsecuritydevopsdatasciencesysadminCLI Integration:
Use roles during installation
cortex --role security install nginx
cortex -r devops install docker
Manage roles
cortex role list # List all roles
cortex role show security # View role details
cortex role create my-role --from security # Create custom role
cortex role delete my-role # Delete custom roleCustom Roles: Users can create their own roles stored in
~/.cortex/roles/*.yamlFiles Changed
cortex/role_manager.pycortex/roles/*.yamlcortex/cli.py--roleflag androlesubcommandcortex/llm/interpreter.pytests/test_role_manager.pyTesting
Checklist
pytest tests/test_role_manager.py)[#270] Feat: Custom AI RolesAI Disclosure
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.