feat: add custom system prompt management CLI #63
Conversation
WalkthroughA new prompt management CLI module was added and integrated into the main CLI, providing commands to show, set (interactive or from file), and clear custom system prompts, with environment-variable precedence and an embedded terminal editor. Changes
Sequence DiagramsequenceDiagram
participant User as User/Terminal
participant CLI as gac prompt CLI
participant FS as File System
participant Editor as prompt_toolkit Editor
User->>CLI: prompt set --edit
CLI->>FS: Resolve target path (GAC_SYSTEM_PROMPT_PATH or stored file)
FS-->>CLI: Path (and existing content if any)
CLI->>Editor: Launch editor with current content (VI mode)
Editor->>User: Display editor
User->>Editor: Edit and Save (Ctrl+S / Esc+Enter) or Cancel (Ctrl+C)
Editor-->>CLI: Return edited text or cancellation
alt saved
CLI->>FS: Ensure directories exist
CLI->>FS: Write prompt file
FS-->>CLI: Write success
CLI->>User: Confirmation message
else cancelled or empty
CLI->>User: Abort or no-op message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
cellwebb
force-pushed
the
main
branch
2 times, most recently
from
f35749f to
1a02629
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/gac/prompt_cli.py (1)
18-46: Consider logging OSError exceptions for debugging.The function correctly implements the precedence logic (env var → stored file → default). However, lines 34-35 and 42-43 silently catch
OSErrorexceptions. While this graceful degradation is appropriate for config file reading, adding logging would help debug file permission or corruption issues.Apply this diff to add debug logging:
try: content = env_file.read_text(encoding="utf-8") return content, f"GAC_SYSTEM_PROMPT_PATH={env_path}" - except OSError: + except OSError as e: + logger.debug(f"Failed to read env var prompt file {env_path}: {e}") pass # Check stored custom prompt file if CUSTOM_PROMPT_FILE.exists(): try: content = CUSTOM_PROMPT_FILE.read_text(encoding="utf-8") return content, str(CUSTOM_PROMPT_FILE) - except OSError: + except OSError as e: + logger.debug(f"Failed to read stored prompt file {CUSTOM_PROMPT_FILE}: {e}") pass
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/gac/cli.py(2 hunks)src/gac/prompt_cli.py(1 hunks)tests/test_prompt_cli.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use type annotations throughout the codebase
Use snake_case for modules and functions, CapWords for classes
Files:
src/gac/cli.pysrc/gac/prompt_cli.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Ruff formatter with 120-character line limit
Files:
src/gac/cli.pysrc/gac/prompt_cli.pytests/test_prompt_cli.py
🧬 Code graph analysis (2)
src/gac/cli.py (1)
src/gac/prompt_cli.py (1)
prompt(50-52)
tests/test_prompt_cli.py (1)
src/gac/prompt_cli.py (2)
get_active_custom_prompt(18-46)prompt(50-52)
🔇 Additional comments (13)
src/gac/cli.py (2)
26-26: LGTM!The import follows the established naming pattern for CLI command imports and maintains alphabetical ordering.
206-206: LGTM!Command registration follows the established pattern and maintains alphabetical ordering with other commands.
tests/test_prompt_cli.py (6)
12-26: Well-designed test fixtures.The
mock_pathsfixture properly isolates file system operations by overriding module-level constants, and therunnerfixture provides a clean interface for CLI testing.
29-83: Excellent test coverage for the show command.The tests comprehensively cover all prompt sources (none, env var, stored file) and correctly verify precedence rules. The output normalization on line 50 is a good practice for handling Rich's potential line wrapping.
85-119: Thorough coverage of file-based prompt setting.Tests validate successful copying, error handling for missing files, and automatic directory creation. The flexible error message assertion on line 104 is appropriate for different error formats.
121-190: Comprehensive interactive editing test coverage.The tests properly mock
_edit_text_interactiveto avoid interactive prompts and verify all key scenarios including new/update/abort flows. Lines 135, 150, and 186 correctly validate that the editor receives appropriate initial content, and line 189 ensures env-var-based editing updates the correct file.
192-213: Good coverage of clear command behavior.Tests validate both file deletion and idempotent behavior when no file exists. The assertion on line 212 correctly ensures graceful handling of the no-op case.
215-265: Complete coverage of the prompt resolution helper.Tests thoroughly validate
get_active_custom_promptacross all scenarios including precedence rules and graceful handling of missing files. Lines 262-265 correctly verify that nonexistent env var paths return(None, None)rather than raising errors.src/gac/prompt_cli.py (5)
1-16: Clean module setup.Module structure follows Python conventions with appropriate imports, docstring, and module-level constants. Logger and console instances are properly initialized.
55-73: Well-implemented show command.The local import on line 58 appropriately avoids potential circular dependencies. The logic correctly displays either the custom prompt or the default template, with appropriate title formatting based on the source.
76-179: Robust interactive editor implementation.The function implements a full-featured terminal editor with:
- Terminal size adaptation (lines 108-113) for better UX
- Clear key bindings (Ctrl+S, Esc+Enter for save; Ctrl+C for cancel)
- Comprehensive error handling (lines 174-179)
- VI editing mode for power users
The local imports on lines 81-89 appropriately defer dependency loading until needed.
182-205: Correctly determines edit target based on environment.The function properly respects
GAC_SYSTEM_PROMPT_PATHwhen set, falling back to the default stored file. Error handling is consistent withget_active_custom_prompt.
258-265: Clarify clear command behavior with environment variables.The
clearcommand only deletesCUSTOM_PROMPT_FILEand does not touch files referenced byGAC_SYSTEM_PROMPT_PATH. While this is probably the intended behavior (not modifying user-managed files), users might be confused if they have an env var set and expectclearto remove all custom prompts.Consider adding a note to the help text or output when
GAC_SYSTEM_PROMPT_PATHis set:@prompt.command() def clear() -> None: - """Clear custom system prompt (revert to default).""" + """Clear custom system prompt (revert to default). + + Note: This only clears the stored prompt file. If GAC_SYSTEM_PROMPT_PATH + is set, you'll need to unset it or delete that file separately. + """ + env_path = os.getenv("GAC_SYSTEM_PROMPT_PATH") + if env_path: + console.print(f"[yellow]Note: GAC_SYSTEM_PROMPT_PATH is set to {env_path}[/yellow]") + console.print("[yellow]You may need to unset this environment variable or delete that file.[/yellow]\n") + if CUSTOM_PROMPT_FILE.exists(): CUSTOM_PROMPT_FILE.unlink() console.print(f"Custom prompt deleted: {CUSTOM_PROMPT_FILE}") else: console.print("No custom prompt file to delete.")
| @prompt.command() | ||
| @click.option("--edit", "-e", is_flag=True, help="Edit prompt interactively in terminal") | ||
| @click.option("--file", "file_path", type=click.Path(exists=True), help="Copy prompt from file") | ||
| def set(edit: bool, file_path: str | None) -> None: | ||
| """Set custom system prompt via interactive editor or file.""" | ||
| # Require exactly one of --edit or --file | ||
| if edit and file_path: | ||
| console.print("[red]Error: --edit and --file are mutually exclusive[/red]") | ||
| raise click.Abort() | ||
|
|
||
| if not edit and not file_path: | ||
| console.print("[red]Error: either --edit or --file must be specified[/red]") | ||
| raise click.Abort() | ||
|
|
||
| if edit: | ||
| # Get the target file and its current content | ||
| target_file, initial_content = _get_prompt_file_to_edit() | ||
|
|
||
| # Create parent directory if needed | ||
| target_file.parent.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Open interactive editor | ||
| result = _edit_text_interactive(initial_content) | ||
|
|
||
| if result is None: | ||
| console.print("\n[yellow]Edit cancelled, no changes made.[/yellow]") | ||
| return | ||
|
|
||
| if not result: | ||
| console.print("\n[yellow]Empty prompt not saved.[/yellow]") | ||
| return | ||
|
|
||
| # Save result | ||
| target_file.write_text(result, encoding="utf-8") | ||
| console.print(f"\n[green]Custom prompt saved to {target_file}[/green]") | ||
|
|
||
| elif file_path: | ||
| # Copy file content | ||
| source_file = Path(file_path) | ||
| try: | ||
| content = source_file.read_text(encoding="utf-8") | ||
| # Create parent directory if needed | ||
| CUSTOM_PROMPT_FILE.parent.mkdir(parents=True, exist_ok=True) | ||
| CUSTOM_PROMPT_FILE.write_text(content, encoding="utf-8") | ||
| console.print(f"Custom prompt copied from {file_path} to {CUSTOM_PROMPT_FILE}") | ||
| except OSError as e: | ||
| console.print(f"[red]Error reading file {file_path}: {e}[/red]") | ||
| raise click.Abort() from e |
There was a problem hiding this comment.
Inconsistent behavior between --edit and --file options.
Lines 222-242 show that --edit respects GAC_SYSTEM_PROMPT_PATH (via _get_prompt_file_to_edit), but lines 244-255 show that --file always writes to CUSTOM_PROMPT_FILE, ignoring the environment variable.
This creates confusing behavior:
- User sets
GAC_SYSTEM_PROMPT_PATH=/custom/path - User runs
gac prompt set --file my.txt - File is copied to
~/.config/gac/custom_system_prompt.txt - User runs
gac prompt show - Still sees content from
/custom/pathbecause env var takes precedence - User's
--fileoperation appears to have no effect
Apply this diff to make --file consistent with --edit:
elif file_path:
# Copy file content
source_file = Path(file_path)
try:
content = source_file.read_text(encoding="utf-8")
+ # Determine target file (respects env var like --edit does)
+ env_path = os.getenv("GAC_SYSTEM_PROMPT_PATH")
+ target_file = Path(env_path) if env_path else CUSTOM_PROMPT_FILE
# Create parent directory if needed
- CUSTOM_PROMPT_FILE.parent.mkdir(parents=True, exist_ok=True)
- CUSTOM_PROMPT_FILE.write_text(content, encoding="utf-8")
- console.print(f"Custom prompt copied from {file_path} to {CUSTOM_PROMPT_FILE}")
+ target_file.parent.mkdir(parents=True, exist_ok=True)
+ target_file.write_text(content, encoding="utf-8")
+ console.print(f"Custom prompt copied from {file_path} to {target_file}")
except OSError as e:
console.print(f"[red]Error reading file {file_path}: {e}[/red]")
raise click.Abort() from e🤖 Prompt for AI Agents
In src/gac/prompt_cli.py around lines 208 to 255, the --file branch currently
always writes the copied prompt to CUSTOM_PROMPT_FILE which ignores
GAC_SYSTEM_PROMPT_PATH and causes inconsistent behavior versus --edit; change
the --file flow to obtain the target path via _get_prompt_file_to_edit() (same
as the --edit branch), create the parent dir for that target, then write the
source file content to that target (preserving the existing OSError handling),
and keep the console messages and error handling consistent with the --edit
path.
There was a problem hiding this comment.
If a user sets GAC_SYSTEM_PROMPT_PATH then that takes precedent. We accept that the user knows what they are doing. Should i maybe add a log for this specific case? Telling the user the env is set and therefore --file command will do nothing?
There was a problem hiding this comment.
@sogoiii, I understand your point about GAC_SYSTEM_PROMPT_PATH taking precedence, but there's an inconsistency issue here:
Current behavior:
gac prompt set --edit→ respects env var, writes to the path specified inGAC_SYSTEM_PROMPT_PATHif set (via_get_prompt_file_to_edit())gac prompt set --file→ ignores env var, always writes to~/.config/gac/custom_system_prompt.txt
This means:
export GAC_SYSTEM_PROMPT_PATH=/custom/prompt.txt
# This edits /custom/prompt.txt ✓
gac prompt set --edit
# This writes to ~/.config/gac/custom_system_prompt.txt, which won't be used ✗
gac prompt set --file source.txtFor consistency, I'd recommend:
Make both --edit and --file behave identically - both should respect GAC_SYSTEM_PROMPT_PATH. This way users get predictable behavior regardless of which option they use.
If you prefer keeping the current design:
Then yes, you should definitely add a warning or error (not just a log), something like:
env_path = os.getenv("GAC_SYSTEM_PROMPT_PATH")
if env_path:
console.print(f"[yellow]Warning: GAC_SYSTEM_PROMPT_PATH is set to {env_path}[/yellow]")
console.print("[yellow]The file will be copied to ~/.config/gac/custom_system_prompt.txt but won't be used.[/yellow]")
console.print("[yellow]Consider using --edit instead, or unset GAC_SYSTEM_PROMPT_PATH.[/yellow]")However, I'd still recommend option 1 (consistency) as the better UX. What do you think?
|
@sogoiii the linting failed because of mypy issues which I'm not worried about, but there are some failing tests. I did some reorganization and the default system prompt is now at src/gac/templates/system_prompt.txt which I think is perhaps why the tests are failing. |
|
@cellwebb ok weird. I ran |
Add comprehensive CLI commands for managing custom system prompts: - `gac prompt show`: Display active custom prompt or default - `gac prompt set --edit`: Edit prompt interactively in terminal - `gac prompt set --file <path>`: Copy prompt from file - `gac prompt clear`: Remove custom prompt and revert to default - `--show-system-prompt`: Global flag to display active prompt Custom prompts support precedence: 1. GAC_SYSTEM_PROMPT_PATH environment variable (highest) 2. ~/.config/gac/custom_system_prompt.txt (stored config) 3. Built-in default prompt (fallback) Interactive editor includes: - Multi-line text editing with prompt_toolkit - Scrollable interface adapting to terminal size - Visual hints for save/cancel shortcuts - Pre-populated with existing prompt content Includes comprehensive test coverage for all commands, precedence rules, edge cases, and error handling.
- Remove --show-system-prompt flag and its implementation logic - Clean up unused imports (Panel from rich.panel, get_active_custom_prompt) - Eliminate 21 lines of dead code including prompt display functionality - Simplify CLI interface by removing a maintenance burden feature
|
@cellwebb ok, my branch has drifted since i made the PR a week ago. I have to rebase against main. |
- Update prompt_cli.py to use _load_default_system_template() instead of DEFAULT_SYSTEM_TEMPLATE constant - Load default system template dynamically when displaying in 'show' command - Improve code modularity by centralizing template loading logic
sogoiii
force-pushed
the
feat/add-prompt-command
branch
from
505a87c to
6bc3006
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/gac/prompt_cli.py (1)
245-256: Inconsistent behavior between --edit and --file options remains unaddressed.This issue was flagged in a previous review but has not been resolved. The
--fileoption always writes toCUSTOM_PROMPT_FILE(line 252), ignoringGAC_SYSTEM_PROMPT_PATH, while--editrespects the environment variable via_get_prompt_file_to_edit()(line 225).This creates confusing UX:
- User sets
GAC_SYSTEM_PROMPT_PATH=/custom/path- User runs
gac prompt set --file my.txt- File is copied to
~/.config/gac/custom_system_prompt.txt- User runs
gac prompt show- Still sees content from
/custom/pathbecause env var takes precedence- User's
--fileoperation appears to have no effectApply this diff to make
--fileconsistent with--edit:elif file_path: # Copy file content source_file = Path(file_path) try: content = source_file.read_text(encoding="utf-8") + # Determine target file (respects env var like --edit does) + target_file, _ = _get_prompt_file_to_edit() # Create parent directory if needed - CUSTOM_PROMPT_FILE.parent.mkdir(parents=True, exist_ok=True) - CUSTOM_PROMPT_FILE.write_text(content, encoding="utf-8") - console.print(f"Custom prompt copied from {file_path} to {CUSTOM_PROMPT_FILE}") + target_file.parent.mkdir(parents=True, exist_ok=True) + target_file.write_text(content, encoding="utf-8") + console.print(f"Custom prompt copied from {file_path} to {target_file}") except OSError as e: console.print(f"[red]Error reading file {file_path}: {e}[/red]") raise click.Abort() from e
🧹 Nitpick comments (7)
src/gac/prompt_cli.py (3)
18-46: Consider logging OSError exceptions for better debuggability.The function silently catches and ignores
OSErrorexceptions when reading prompt files (lines 34-35, 42-43). While the fallback behavior is reasonable, logging these errors would help users diagnose issues like permission problems or I/O failures.Apply this diff to add logging:
try: content = env_file.read_text(encoding="utf-8") return content, f"GAC_SYSTEM_PROMPT_PATH={env_path}" - except OSError: - pass + except OSError as e: + logger.warning(f"Failed to read prompt from {env_path}: {e}") + pass # Check stored custom prompt file if CUSTOM_PROMPT_FILE.exists(): try: content = CUSTOM_PROMPT_FILE.read_text(encoding="utf-8") return content, str(CUSTOM_PROMPT_FILE) - except OSError: - pass + except OSError as e: + logger.warning(f"Failed to read prompt from {CUSTOM_PROMPT_FILE}: {e}") + pass
77-180: Consider extracting the hint text to a constant.The hint text appears twice (line 96 and line 126), with slightly different formatting. Extracting to a constant would reduce duplication and make future updates easier.
+ EDITOR_HINT = "Esc+Enter or Ctrl+S to save | Ctrl+C to cancel" + try: import shutil console.print("\n[bold]Edit your custom system prompt:[/bold]") - console.print("[dim]Esc+Enter or Ctrl+S to save | Ctrl+C to cancel[/dim]\n") + console.print(f"[dim]{EDITOR_HINT}[/dim]\n") # ... rest of code ... # Create hint window hint_window = Window( - content=FormattedTextControl(text=[("class:hint", " Esc+Enter or Ctrl+S to save | Ctrl+C to cancel ")]), + content=FormattedTextControl(text=[("class:hint", f" {EDITOR_HINT} ")]), height=1, dont_extend_height=True, )
183-206: Consider logging OSError exceptions for better debuggability.Similar to
get_active_custom_prompt(), this function silently ignoresOSErrorexceptions (lines 195-196, 204-205). Adding logging would help diagnose read failures.Apply this diff:
if target_file.exists(): try: content = target_file.read_text(encoding="utf-8") - except OSError: - pass + except OSError as e: + logger.warning(f"Failed to read existing prompt from {target_file}: {e}") + pass return target_file, content # Default to stored config file content = "" if CUSTOM_PROMPT_FILE.exists(): try: content = CUSTOM_PROMPT_FILE.read_text(encoding="utf-8") - except OSError: - pass + except OSError as e: + logger.warning(f"Failed to read existing prompt from {CUSTOM_PROMPT_FILE}: {e}") + pass return CUSTOM_PROMPT_FILE, contenttests/test_prompt_cli.py (4)
121-190: Consider adding a test for mutual exclusivity of --edit and --file.The
TestPromptSetEditclass tests the case where neither option is provided (lines 167-172), but doesn't test the case where both--editand--fileare provided together. This validation logic exists insrc/gac/prompt_cli.pylines 215-217.Add this test to the
TestPromptSetEditclass:def test_prompt_set_mutually_exclusive(self, runner, mock_paths, tmp_path, monkeypatch): """Error if both --edit and --file provided.""" monkeypatch.delenv("GAC_SYSTEM_PROMPT_PATH", raising=False) source_file = tmp_path / "source.txt" source_file.write_text("Test content", encoding="utf-8") result = runner.invoke(prompt, ["set", "--edit", "--file", str(source_file)]) assert result.exit_code != 0 assert "mutually exclusive" in result.output.lower()
121-190: Consider adding a test for empty prompt handling in --edit mode.The
setcommand handles the case where the interactive editor returns an empty string (lines 237-239 insrc/gac/prompt_cli.py), but this scenario isn't tested. Adding this test would improve coverage of edge cases.Add this test to the
TestPromptSetEditclass:def test_prompt_set_edit_empty_result(self, runner, mock_paths, monkeypatch): """Mock _edit_text_interactive() returning empty string, verify not saved.""" monkeypatch.delenv("GAC_SYSTEM_PROMPT_PATH", raising=False) with patch("gac.prompt_cli._edit_text_interactive") as mock_edit: mock_edit.return_value = "" # Empty string result = runner.invoke(prompt, ["set", "--edit"]) assert result.exit_code == 0 assert not mock_paths["custom_prompt"].exists() assert "empty" in result.output.lower()
85-119: Add a test for --file with GAC_SYSTEM_PROMPT_PATH set after fixing the consistency issue.Once the inconsistency between
--editand--file(flagged in the main code review) is fixed, add a test similar totest_prompt_set_edit_env_var_file(lines 174-189) to verify that--filealso respects the environment variable.After applying the fix to
src/gac/prompt_cli.py, add this test:def test_prompt_set_file_env_var_file(self, runner, mock_paths, tmp_path, monkeypatch): """When GAC_SYSTEM_PROMPT_PATH is set, --file copies to that file.""" env_prompt_file = tmp_path / "env_prompt.txt" monkeypatch.setenv("GAC_SYSTEM_PROMPT_PATH", str(env_prompt_file)) source_file = tmp_path / "source.txt" source_file.write_text("Source content", encoding="utf-8") result = runner.invoke(prompt, ["set", "--file", str(source_file)]) assert result.exit_code == 0 # Env file should be updated, not the default stored file assert env_prompt_file.read_text(encoding="utf-8") == "Source content" assert not mock_paths["custom_prompt"].exists()
192-213: Add a test documenting clear behavior when GAC_SYSTEM_PROMPT_PATH is set.To document the expected behavior flagged in the main code review, add a test that verifies what happens when
clearis called whileGAC_SYSTEM_PROMPT_PATHis set.Add this test to
TestPromptClear:def test_prompt_clear_with_env_var(self, runner, mock_paths, tmp_path, monkeypatch): """Clear only deletes stored file, not env var file.""" # Create both env var file and stored file env_prompt_file = tmp_path / "env_prompt.txt" env_prompt_file.write_text("Env content", encoding="utf-8") monkeypatch.setenv("GAC_SYSTEM_PROMPT_PATH", str(env_prompt_file)) mock_paths["config_dir"].mkdir(parents=True, exist_ok=True) mock_paths["custom_prompt"].write_text("Stored content", encoding="utf-8") result = runner.invoke(prompt, ["clear"]) assert result.exit_code == 0 # Stored file should be deleted assert not mock_paths["custom_prompt"].exists() # Env var file should remain assert env_prompt_file.exists() assert env_prompt_file.read_text(encoding="utf-8") == "Env content"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/gac/cli.py(2 hunks)src/gac/prompt_cli.py(1 hunks)tests/test_prompt_cli.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Ruff formatter with 120-character line limit
Files:
tests/test_prompt_cli.pysrc/gac/cli.pysrc/gac/prompt_cli.py
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use type annotations throughout the codebase
Use snake_case for modules and functions, CapWords for classes
Files:
src/gac/cli.pysrc/gac/prompt_cli.py
🧬 Code graph analysis (2)
src/gac/cli.py (1)
src/gac/prompt_cli.py (1)
prompt(50-52)
src/gac/prompt_cli.py (1)
src/gac/prompt.py (1)
_load_default_system_template(22-24)
🔇 Additional comments (2)
src/gac/cli.py (1)
27-27: LGTM! Clean integration of the prompt CLI.The import and command registration follow the existing patterns for other CLI subcommands. The alphabetical ordering and consistent structure maintain code organization.
Also applies to: 210-210
tests/test_prompt_cli.py (1)
1-265: Well-structured and comprehensive test suite.The tests are well-organized into logical test classes, use appropriate fixtures and mocking strategies, and cover the main functionality paths. The suggestions above are for enhancing coverage of edge cases rather than addressing critical gaps.
| @prompt.command() | ||
| def clear() -> None: | ||
| """Clear custom system prompt (revert to default).""" | ||
| if CUSTOM_PROMPT_FILE.exists(): | ||
| CUSTOM_PROMPT_FILE.unlink() | ||
| console.print(f"Custom prompt deleted: {CUSTOM_PROMPT_FILE}") | ||
| else: | ||
| console.print("No custom prompt file to delete.") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, explore the repository structure
git ls-files src/ | head -20Repository: cellwebb/gac
Length of output: 527
🏁 Script executed:
# Search for tests related to the clear command
fd -e py -path "*/test*" | xargs rg -l "clear" 2>/dev/null | head -10Repository: cellwebb/gac
Length of output: 461
🏁 Script executed:
# Check for documentation about GAC_SYSTEM_PROMPT_PATH
rg -n "GAC_SYSTEM_PROMPT_PATH" --type=md,txt --type=py -B2 -A2Repository: cellwebb/gac
Length of output: 86
🏁 Script executed:
# Look for the implementation of get_custom_prompt and related functions
rg -n "def get_custom_prompt|def show|GAC_SYSTEM_PROMPT_PATH" src/ -B1 -A3Repository: cellwebb/gac
Length of output: 2705
🏁 Script executed:
# Check the clear command implementation and surrounding code
sed -n '245,280p' src/gac/prompt_cli.pyRepository: cellwebb/gac
Length of output: 952
🏁 Script executed:
# View the full set command implementation
sed -n '175,210p' src/gac/prompt_cli.pyRepository: cellwebb/gac
Length of output: 1173
🏁 Script executed:
# Check test file for clear and set command tests
cat tests/test_prompt_cli.pyRepository: cellwebb/gac
Length of output: 12591
🏁 Script executed:
# Search for any documentation or comments about the clear command and env var interaction
rg -n "clear.*env|GAC_SYSTEM_PROMPT_PATH.*clear" src/ -B2 -A2Repository: cellwebb/gac
Length of output: 421
🏁 Script executed:
# Check if there's any documentation file that mentions this behavior
fd -type f \( -name "*.md" -o -name "*.rst" -o -name "*.txt" \) | xargs rg -l "GAC_SYSTEM_PROMPT_PATH\|clear.*prompt" 2>/dev/nullRepository: cellwebb/gac
Length of output: 228
Inconsistent behavior between set and clear commands with GAC_SYSTEM_PROMPT_PATH.
The set command (in --edit mode) respects GAC_SYSTEM_PROMPT_PATH and modifies the env-var-specified file if set, but the clear command ignores the environment variable and only deletes CUSTOM_PROMPT_FILE. This means running gac prompt clear followed by gac prompt show will still display the custom prompt from GAC_SYSTEM_PROMPT_PATH if it's set.
This inconsistency is not covered by tests. Consider either:
- Making
clearalso handle the env-var-specified file (delete or unset it) - Documenting this limitation in the command help text
- Adding a warning if
GAC_SYSTEM_PROMPT_PATHis set when runningclear
🤖 Prompt for AI Agents
In src/gac/prompt_cli.py around lines 259-266, the clear command currently only
unlinks CUSTOM_PROMPT_FILE and ignores GAC_SYSTEM_PROMPT_PATH; update it to
first check for an environment-specified path (GAC_SYSTEM_PROMPT_PATH) and use
that as the target file if present, falling back to CUSTOM_PROMPT_FILE
otherwise; attempt to unlink the chosen path, print a success message if deleted
or a "no custom prompt file" message if missing, and catch/log any filesystem
errors so behavior matches the set/edit command and avoids leaving an
env-controlled prompt behind.
There was a problem hiding this comment.
again the GAC_SYSTEM_PROMPT_PATH takes precedent. The env var is the original way of handling custom prompts. The new command is mean to handle new users who wont use the env var.
There was a problem hiding this comment.
@sogoiii, thanks for the clarification! I understand now – the env var is for power users who manage their own files, while the CLI commands are for casual users who don't use environment variables. That design rationale makes sense.
One small UX consideration: since set --edit does respect GAC_SYSTEM_PROMPT_PATH and will edit that file when set (via _get_prompt_file_to_edit()), it might be helpful to show users which file they're editing. For example:
[bold]Editing: /path/from/env/var[/bold]
Edit your custom system prompt:
[dim]Esc+Enter or Ctrl+S to save | Ctrl+C to cancel[/dim]
This would make it clear that they're editing the env-var file (which won't be affected by clear) vs the CLI-managed file. Just a thought for future improvement – not critical for this PR.
✏️ Learnings added
Learnt from: sogoiii
Repo: cellwebb/gac PR: 63
File: src/gac/prompt_cli.py:259-266
Timestamp: 2025-12-11T05:16:13.792Z
Learning: In gac CLI, GAC_SYSTEM_PROMPT_PATH environment variable is the original method for setting custom prompts and is intended for power users who manage their own prompt files. The new `gac prompt` CLI commands (set, clear, show) are designed for casual users who don't use environment variables. The clear command intentionally only deletes the CLI-managed file (~/.config/gac/custom_system_prompt.txt) and does not remove env-var-specified files, as those users are expected to manage their own files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2 participants
Purpose
Add comprehensive CLI commands for managing custom system prompts, enabling users to override the default prompt with their own custom instructions for commit message generation.
Summary
gac promptsubcommand suite with show/set/clear operations--editflag for terminal-based editing--fileoptionGAC_SYSTEM_PROMPT_PATHChanges
CLI Commands
gac prompt show- Display the currently active system prompt (custom or default)gac prompt set --edit- Open interactive terminal editor with:gac prompt set --file <path>- Import custom prompt from filegac prompt clear- Remove custom prompt and revert to defaultPrompt Precedence
GAC_SYSTEM_PROMPT_PATHenvironment variable (highest priority)~/.config/gac/custom_system_prompt.txt(stored configuration)Code Structure
src/gac/prompt_cli.py(265 lines) - Main prompt management logicsrc/gac/cli.py- Integrated new subcommandtests/test_prompt_cli.py(265 lines) - Comprehensive test suiteImpact Analysis
Validation
Summary by CodeRabbit
New Features
promptcommand group for managing system promptsprompt show: displays the active prompt and its sourceprompt set: create or update prompts via interactive editor or external fileprompt clear: remove custom prompt settingsTests
✏️ Tip: You can customize this high-level summary in your review settings.