Version/v2 #34
Version/v2 #34
Conversation
- Bug Fix
- Clean Up Test Directory
- Add dual support for legacy 'is_live' flags and new 'current_version' tracking - Enhanced prompt loader with automatic version header removal and metadata integration - New VersionManager CLI tool for manual version operations (list, create, switch, get) - New HookManager for git pre-commit hook installation and management - Enhanced CLI with 'version' and 'hooks' command groups - Comprehensive test suite covering all version management functionality - Updated .gitignore to exclude v2/ and updated-v2/ directories - Added documentation: TESTING_VERSIONING.md and VERSIONING_GUIDE.md Features: - Automatic version creation on current.md changes via pre-commit hook - Manual version management through CLI tools - Version switching with config.yaml current_version tracking - Rich console output with beautiful formatting - Backward compatibility with existing prompt systems - Safe hook installation with backup/restore functionality - Comprehensive error handling and edge case coverage This implementation provides a complete version control system for prompt development, enabling teams to track prompt evolution, switch between versions, and maintain prompt quality over time.
- Corrects relative import path to properly locate test_helpers module - Enables proper execution of pre-commit hook test suite
- Removed test bypass functionality from test_file.txt. - Updated config.yaml to streamline tool selection based on programming language. - Enhanced version management in v004.md and v005.md for better clarity and structure. - Improved error handling and path validation in VersionManager to prevent directory traversal attacks. - Implemented folder-based prompt management with automatic migration from legacy YAML files, ensuring data integrity and logging migration activities. - Updated tests to reflect changes in prompt loading and versioning, ensuring robust coverage for new features.
- Improved error handling for commit hash retrieval in the pre-commit hook. - Updated comments to clarify that current_version should not be set automatically. - Refactored test cases to ensure proper handling of prompts and versions. - Adjusted output capturing in tests to use stdout instead of stderr for better consistency.
|
Warning Rate limit exceeded@Nisarg38 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
WalkthroughIntroduces a folder-based prompts workspace with agents, adds a Python pre-commit hook for auto-versioning, replaces the CLI with Click/Rich commands, adds HookManager/VersionManager/Workspace utilities, refactors core Promptix and PromptixBuilder to dependency-injection components, migrates storage to per-agent folders, adds migration tooling and many tests/fixtures, and updates docs and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Git as Git
participant Hook as pre-commit (Promptix)
participant FS as Filesystem
participant YAML as config.yaml
Dev->>Git: git commit (changes in prompts/*)
Git->>Hook: Invoke pre-commit
alt SKIP_PROMPTIX_HOOK set
Hook-->>Git: Exit 0 (bypassed)
else Process staged changes
Hook->>FS: Detect staged files (current.md, config.yaml)
opt current.md changed
Hook->>FS: Read current.md
Hook->>FS: Atomic create versions/vNNN.md (+ header)
Hook->>YAML: Update versions map, last_modified
Hook->>Git: git add versions/vNNN.md config.yaml
end
opt config.yaml changed (current_version set)
Hook->>YAML: Read current_version
Hook->>FS: Copy versions/current_version -> current.md (strip header)
Hook->>Git: git add current.md
end
Hook-->>Git: Exit 0
end
sequenceDiagram
autonumber
actor User
participant CLI as promptix CLI (Click)
participant VM as VersionManager
participant HM as HookManager
participant WS as Workspace (prompts/)
User->>CLI: version create --agent A --name v003 --notes "..."
CLI->>VM: create_version(A, v003)
VM->>WS: Read current.md, write versions/v003.md (+header)
VM->>WS: Update config.yaml (versions map, current_version)
VM-->>CLI: Done
User->>CLI: hooks install
CLI->>HM: install_hook()
HM->>WS: Place pre-commit, backup/enable as needed
HM-->>CLI: Status
sequenceDiagram
autonumber
actor App
participant PX as Promptix (DI)
participant PL as PromptLoader
participant VM as VersionManager (core)
participant VV as VariableValidator
participant TR as TemplateRenderer
App->>PX: get_prompt("Agent", version?, vars)
PX->>PL: get_prompt_data("Agent")
PL-->>PX: agent data (versions, schema, config)
PX->>VM: resolve version data (live or specified)
PX->>VM: get system_instruction/template
PX->>VV: validate variables against schema
PX->>TR: render(template, vars)
TR-->>PX: system message
PX-->>App: rendered prompt
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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: 37
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
tests/quality/test_edge_cases.py (1)
498-523:many_varsdictionary is created but never used in the test.The test creates a dictionary with 1000 variables (line 501) but neither the builder assignment at lines 504-508 nor the alternative approach at lines 511-523 actually uses
many_vars. The comment at line 510 suggests adding variables one by one, but the loop at lines 515-517 iterates without applying the variables to the builder.Based on the relevant code snippets, the builder supports
.with_var(dict)to set multiple variables at once. Consider actually testing the maximum variable count scenario:Apply this diff to test with the many variables:
try: builder = ( Promptix.builder("SimpleChat") .with_user_name("test") .with_assistant_name("test") + .with_var(many_vars) ) - - # Alternative approach - add variables one by one - builder = Promptix.builder("SimpleChat") - builder.with_user_name("test") - builder.with_assistant_name("test") - - for key, value in list(many_vars.items())[:100]: # Limit to reasonable number - # We can't directly test this without knowing builder internals - pass - config = builder.build()prompts/TemplateDemo/versions/v003.md (1)
1-17: Duplicate content detected.This template is identical to
prompts/TemplateDemo/versions/v1.mdandprompts/TemplateDemo/versions/v007.md. Please refer to the verification request in the v1.md review for details on checking this duplication across all TemplateDemo version files.prompts/CodeReviewer/versions/v008.md (1)
1-6: Add missing placeholders and structured feedback guidance in prompts/CodeReviewer/versions/v008.md
- Include
{{severity}}placeholder for specifying scrutiny level.- Add
{{active_tools}}placeholder to surface enabled tools.- Provide output structure guidance with sections: “Summary”, “Critical Issues”, “Improvements”, “Positives”.
prompts/CodeReviewer/versions/v009.md (1)
1-6: Summary incorrect: v009.md lacks structured output sections
The template only provides basic review instructions and does not define 'Summary', 'Critical Issues', 'Improvements', or 'Positives' sections as stated.prompts/ComplexCodeReviewer/versions/v010.md (1)
1-8: Consolidate duplicate version files in prompts/ComplexCodeReviewer/versions
Versions v002.md through v010.md and v1.md all share the same content (MD5 27f3239c91a3f17b71ea8c727a248279); remove or differentiate these files to avoid redundant, confusing versions.
♻️ Duplicate comments (5)
prompts/SimpleChat/versions/v004.md (1)
1-1: Duplicate content noted.This template is identical to v1 and v006. See the comment on
prompts/SimpleChat/versions/v006.mdregarding verification of version content.prompts/simple_chat/versions/v004.md (1)
1-11: Duplicate content across SimpleChat versions.This version is identical to v003.md and v005.md. See the comment on v005.md for details.
prompts/simple_chat/versions/v003.md (1)
1-11: Duplicate content across SimpleChat versions.This version is identical to v004.md and v005.md. See the comment on v005.md for details.
prompts/ComplexCodeReviewer/versions/v007.md (1)
1-7: Duplicate content flagged.This file is identical to
prompts/ComplexCodeReviewer/versions/v008.mdandprompts/ComplexCodeReviewer/current.md. See comments on those files for detailed analysis and verification scripts.prompts/SimpleChat/versions/v005.md (1)
1-1: Template content is correct.The prompt template syntax is valid and the placeholders {{assistant_name}} and {{user_name}} align with the test usage shown in test_01_basic.py. The duplication concern across versions has been flagged in the v011.md review.
🧹 Nitpick comments (69)
src/promptix/tools/hook_manager.py (5)
64-74: Simplify exception handling.The bare
except Exceptionwith fallback return value is acceptable here since this is a simple predicate check, but the logic can be streamlined.Consider this cleaner approach:
def is_promptix_hook(self) -> bool: """Check if existing hook is a Promptix hook""" if not self.pre_commit_hook.exists(): return False try: - with open(self.pre_commit_hook, 'r') as f: - content = f.read() - return 'Promptix pre-commit hook' in content + content = self.pre_commit_hook.read_text() + return 'Promptix pre-commit hook' in content except Exception: return False
76-91: Add error context to backup failures.The bare
except Exceptionat line 89 swallows error details that would help diagnose backup failures (e.g., permission errors, disk full). Consider logging the exception type or using a more specific exception.Apply this diff:
try: shutil.copy2(self.pre_commit_hook, self.backup_hook) self.print_status(f"Backed up existing hook to {self.backup_hook.name}", "info") return True - except Exception as e: + except (IOError, OSError) as e: self.print_status(f"Failed to backup existing hook: {e}", "error") return False
93-105: Add error context to restore failures.Same issue as
backup_existing_hook— the bareexcept Exceptionat line 103 obscures the root cause of restoration failures.Apply this diff:
try: shutil.copy2(self.backup_hook, self.pre_commit_hook) self.backup_hook.unlink() self.print_status("Restored original pre-commit hook", "info") return True - except Exception as e: + except (IOError, OSError) as e: self.print_status(f"Failed to restore backup: {e}", "error") return False
145-146: Improve exception specificity for installation.The bare
except Exceptionat line 145 catches too broadly. Installation failures are typically IO-related.Apply this diff:
- except Exception as e: + except (IOError, OSError) as e: self.print_status(f"Failed to install hook: {e}", "error")
277-335: Unused argparse variables are benign.Lines 296, 299, 300, 303, 306 assign subparser results to variables that are never used. This is a common pattern with argparse subparsers — the variables aren't needed because
args.commandcarries the command name. Static analysis correctly flags these but they don't impact functionality.Remove the unused assignments to clean up the linter warnings:
# Install command - install_cmd = subparsers.add_parser('install', help='Install pre-commit hook') - install_cmd.add_argument('--force', action='store_true', - help='Overwrite existing hook') + subparsers.add_parser('install', help='Install pre-commit hook').add_argument( + '--force', action='store_true', help='Overwrite existing hook') # Uninstall command - uninstall_cmd = subparsers.add_parser('uninstall', help='Uninstall pre-commit hook') + subparsers.add_parser('uninstall', help='Uninstall pre-commit hook') # Enable/disable commands - enable_cmd = subparsers.add_parser('enable', help='Enable disabled hook') - disable_cmd = subparsers.add_parser('disable', help='Disable hook temporarily') + subparsers.add_parser('enable', help='Enable disabled hook') + subparsers.add_parser('disable', help='Disable hook temporarily') # Status command - status_cmd = subparsers.add_parser('status', help='Show hook status') + subparsers.add_parser('status', help='Show hook status') # Test command - test_cmd = subparsers.add_parser('test', help='Test hook without committing') + subparsers.add_parser('test', help='Test hook without committing')tests/fixtures/test_prompts/CodeReviewer/current.md (1)
1-6: Consider adding output structure guidance for consistency.The current template lacks the structured output sections present in v2 (Summary, Critical Issues, Improvements, Positives). While this may be intentional for flexibility, adding output guidance would ensure more consistent and parseable responses across versions.
Consider adding a line similar to v2:
```{{programming_language}} {{code_snippet}}
+Provide feedback in sections: 'Summary', 'Critical Issues', 'Improvements', 'Positives'.
</blockquote></details> <details> <summary>prompts/TemplateDemo/versions/v010.md (1)</summary><blockquote> `1-16`: **Verify intentional duplication with v006.md.** This template file is byte-for-byte identical to `prompts/TemplateDemo/versions/v006.md`. If v006 and v010 are meant to represent distinct snapshots in the version history, this duplication is expected. However, if the versions should differ or if this is accidental, consider whether intermediate versions (v007, v008, v009) evolved the template and whether v010 should reflect those changes. Run the following script to compare all TemplateDemo versions and identify duplicates: ```shell #!/bin/bash # Description: Compare all TemplateDemo version files to identify duplicates. # Find all version files and compute checksums cd prompts/TemplateDemo/versions md5sum v*.md | sort -k1,1 # Show which files have identical content md5sum v*.md | awk '{print $1}' | uniq -d | while read hash; do echo "=== Duplicate content (hash: $hash) ===" md5sum v*.md | grep "^$hash" | awk '{print $2}' doneprompts/CodeReviewer/versions/v007.md (1)
1-5: Consider adding output structure guidance.Unlike v2.md (line 7), this version doesn't specify the expected feedback format. Without explicit sections (Summary, Critical Issues, Improvements, Positives), responses may be inconsistently structured across different invocations or versions.
Consider appending output format guidance:
```{{programming_language}} {{code_snippet}}
+Provide your feedback organized into sections: 'Summary', 'Critical Issues', 'Improvements', and 'Positives'.
</blockquote></details> <details> <summary>prompts/CodeReviewer/versions/v011.md (1)</summary><blockquote> `1-6`: **v011 is identical to v007 - consider consolidation or differentiation.** The content of this file is byte-for-byte identical to `prompts/CodeReviewer/versions/v007.md`. Maintaining duplicate version files creates unnecessary maintenance burden and potential confusion about version differences. Consider one of the following: 1. If versions should differ, update v011 with distinct improvements (e.g., add output format guidance similar to v2.md line 7) 2. If they're intentionally identical for compatibility, document the rationale 3. If this is unintentional duplication, remove the redundant version Run this script to confirm the duplication: ```shell #!/bin/bash # Description: Compare v007 and v011 content echo "=== Content comparison ===" diff -u prompts/CodeReviewer/versions/v007.md prompts/CodeReviewer/versions/v011.md echo -e "\n=== All CodeReviewer versions ===" fd -e md . prompts/CodeReviewer/versions --exec wc -l {} \; --exec echo {} \;prompts/ComplexCodeReviewer/current.md (1)
1-7: Reconsider versioning strategy for prompts/ComplexCodeReviewer/current.md.current.md is byte-for-byte identical to every version in prompts/ComplexCodeReviewer/versions (v002.md–v010.md), which duplicates maintenance and risks drift. Consolidate by either:
- Symlinking current.md to the active version
- Dynamically loading the active version via config
- Introducing meaningful, version-specific differences
prompts/simple_chat/versions/v009.md (1)
1-11: Standardize prompt directory naming or confirm loader support
Thev009.mdtemplate inprompts/simple_chat/correctly uses{{personality}}and{{domain}}as defined in its ownconfig.yaml. However, co-existingprompts/SimpleChat/andprompts/simple_chat/directories (differing only by case and underscore) may cause lookup failures on case-sensitive or Windows filesystems. Consolidate under a single naming convention or update the prompt loader to recognize both.tests/fixtures/test_prompts/SimpleChat/config.yaml (1)
33-39: Document and cover model mismatch between test fixtures and production
- Add a comment in tests/fixtures/test_prompts/*/config.yaml explaining why fixtures use legacy models (
gpt-3.5-turbo/gpt-4) while production prompts usegpt-4o- Add a unit test asserting that
version_managermaps those legacy model names togpt-4osrc/promptix/core/storage/utils.py (2)
75-75: Prefer yaml.safe_dump for writing configuration.While
yaml.dump()works here,yaml.safe_dump()is the recommended best practice for writing YAML files containing only standard Python types, which is the case with your config_data structure.Apply this diff:
- yaml.dump(config_data, f, sort_keys=False, allow_unicode=True) + yaml.safe_dump(config_data, f, sort_keys=False, allow_unicode=True)Based on learnings
69-69: Use ASCII apostrophe in comment.The comment contains a typographic right single quotation mark (') instead of a standard ASCII apostrophe ('). While minor, using ASCII characters in code is a best practice for compatibility.
Apply this diff:
- # Write config.yaml if it doesn't already exist + # Write config.yaml if it doesn't already existsrc/promptix/tools/version_manager.py (4)
1-1: Make the script executable or remove the shebang.The file has a shebang (
#!/usr/bin/env python3) but is not marked as executable. Either make it executable or remove the shebang if it's only meant to be imported as a module.If this file should be executable, run:
chmod +x src/promptix/tools/version_manager.pyOtherwise, remove line 1.
34-44: Consider deduplicating the print_status method.The
print_statusmethod with identical icons appears in at least three places across the codebase (HookManager, PrecommitHelper, and now VersionManager). This duplication makes maintenance harder.Consider extracting this to a shared utility module:
# src/promptix/tools/utils.py def print_status(message: str, status: str = "info"): """Print colored status messages""" icons = { "info": "📝", "success": "✅", "warning": "⚠️", "error": "❌", "version": "🔄", "list": "📋", "install": "🔧", "uninstall": "🗑️" } print(f"{icons.get(status, '📝')} {message}")Then import and use it across all three locations.
Based on learnings (code duplication identified in relevant snippets).
280-280: Remove unnecessary f-string prefix.Line 280 uses an f-string without any placeholders, which is flagged by static analysis.
Apply this diff:
- self.print_status(f"Updated current.md and config.yaml", "info") + self.print_status("Updated current.md and config.yaml", "info")
390-390: Remove unused variable.The
list_cmdvariable is assigned but never used. This is dead code.Apply this diff:
# List agents command - list_cmd = subparsers.add_parser('list', help='List all agents') + subparsers.add_parser('list', help='List all agents')src/promptix/tools/studio/data.py (1)
23-27: Clarify automatic migration behavior in comments.The comment states that FolderBasedPromptManager "automatically handles migration" and that "existing prompts.yaml will be backed up and migrated safely." However, users might not understand when this migration occurs or what triggers it. The comment should clarify that migration happens on first initialization when both legacy YAML exists and no migration marker is present.
Apply this diff to improve the comment:
# Use folder-based prompt management - # Note: FolderBasedPromptManager automatically handles migration - # from existing YAML files to folder structure on first initialization. - # Your existing prompts.yaml will be backed up and migrated safely. + # Note: FolderBasedPromptManager automatically migrates existing prompts.yaml + # to folder structure on first use (when prompts.yaml exists but prompts/ does not). + # The original prompts.yaml will be backed up as prompts.yaml.backup. self._folder_manager = FolderBasedPromptManager()src/promptix/core/config.py (2)
68-81: Document the purpose of the .promptix directory.The method creates a
.promptixdirectory (lines 78-80) but doesn't document what it's used for or what files might be stored there. This could confuse future maintainers.Apply this diff to improve documentation:
def create_default_workspace(self) -> Path: """ Create the default prompts workspace directory structure. + + Creates: + - prompts/ directory for agent configurations + - .promptix/ directory for workspace metadata and configuration Returns: Path to created prompts/ directory """ prompts_dir = self.get_prompts_workspace_path() prompts_dir.mkdir(parents=True, exist_ok=True) - # Create .promptix directory for workspace configuration + # Create .promptix directory for workspace metadata (migration markers, etc.) promptix_config_dir = self.working_directory / ".promptix" promptix_config_dir.mkdir(parents=True, exist_ok=True) return prompts_dir
137-159: Consider deprecation warning for legacy file path method.The
get_prompt_file_pathmethod supports the legacy single-file approach for backward compatibility. However, there's no indication to users that this is deprecated or that they should migrate to the workspace approach. Consider logging a deprecation warning when this method is called and a legacy file is found.Apply this diff to add a deprecation warning:
def get_prompt_file_path(self) -> Optional[Path]: """ Get path to legacy prompts file (backward compatibility). This method supports the legacy single-file approach for backward compatibility. In the new system, we prefer the workspace approach using prompts/ directory. + + .. deprecated:: + Use get_prompts_workspace_path() instead. This method will be removed in v3.0. Returns: Path to prompts.yaml file if it exists, None otherwise """ base_dir = self.working_directory # Check for existing YAML files in the preferred order for ext in self.get_supported_extensions(): file_path = base_dir / f"prompts{ext}" if file_path.exists(): + import logging + logging.warning( + f"Found legacy prompts file at {file_path}. " + "Consider migrating to the new workspace structure using prompts/ directory. " + "The single-file approach will be deprecated in v3.0." + ) return file_path return Nonesrc/promptix/core/storage/manager.py (3)
6-6: Remove unused importcreate_default_prompts_file.The function
create_default_prompts_fileis imported but never used in this file. Onlycreate_default_prompts_folderis invoked at line 57.Apply this diff to remove the unused import:
-from .utils import create_default_prompts_file, create_default_prompts_folder +from .utils import create_default_prompts_folder
82-83: Improve exception handling for better debugging.The current exception handling loses the original exception context and catches
Exceptionbroadly.Apply this diff to preserve exception context:
except Exception as e: - raise ValueError(f"Failed to load prompts: {str(e)}") + raise ValueError(f"Failed to load prompts: {str(e)}") from eAlternatively, consider catching more specific exceptions (e.g.,
OSError,yaml.YAMLError) if possible to avoid masking unexpected errors.
85-100: Avoid mutating global config; pass workspace_path to PromptLoader instead
Search across the codebase found no threading or async use ofconfigin production modules, so the thread-safety risk is theoretical. RefactorPromptLoader.load_promptsto accept aworkspace_pathparameter and invoke it withself._prompts_directory.parentto remove the global-state side effect and improve testability.src/promptix/core/workspace_manager.py (4)
5-5: Remove unused importos.The
osmodule is imported but never used in this file.Apply this diff to remove the unused import:
-import os import yaml
38-40: Consider simplifying agent name validation.The current validation logic removes hyphens and underscores before checking if the remaining string is alphanumeric. While correct, this approach is less readable.
Consider using a regex for clearer validation:
+import re + # Validate agent name -if not agent_name or not agent_name.replace('_', '').replace('-', '').isalnum(): +if not agent_name or not re.match(r'^[a-zA-Z0-9_-]+$', agent_name): raise ValueError("Agent name must contain only letters, numbers, hyphens, and underscores")This makes the validation intent more explicit and easier to understand.
74-74: Remove unnecessary f-string prefix.The f-string has no placeholders, so the
fprefix is unnecessary.Apply this diff:
- print(f"📁 Created workspace directory: prompts/") + print("📁 Created workspace directory: prompts/")
234-234: Remove unnecessary f-string prefix.The f-string has no placeholders.
Apply this diff:
- print(f"🔄 Created pre-commit hook at .git/hooks/pre-commit") + print("🔄 Created pre-commit hook at .git/hooks/pre-commit")tests/integration/test_versioning_integration.py (5)
210-211: Remove unused variable assignment.The
promptsvariable is assigned but never used.Apply this diff:
# Force reload loader = PromptLoader() - prompts = loader.load_prompts(force_reload=True) + loader.load_prompts(force_reload=True)
246-246: Remove unused variable assignment.The
version2variable is assigned but never used in this test.If you need to preserve the variable for documentation purposes, prefix it with an underscore:
- version2 = tester.create_version_snapshot("prompts/test_agent/current.md") - assert version2 is not None + _version2 = tester.create_version_snapshot("prompts/test_agent/current.md") + assert _version2 is not NoneOr remove it entirely if not needed:
with open(current_md, "w") as f: f.write("Updated assistant content with improvements") - version2 = tester.create_version_snapshot("prompts/test_agent/current.md") - assert version2 is not None + result = tester.create_version_snapshot("prompts/test_agent/current.md") + assert result is not None
278-279: Remove unused variable assignment.The
promptsvariable is assigned but never used.Apply this diff:
loader = PromptLoader() - prompts = loader.load_prompts(force_reload=True) + loader.load_prompts(force_reload=True)
325-325: Prefix unused variable with underscore.The
messagesvariable is unpacked but never used.Apply this diff:
- success, processed_count, messages = tester.main_hook_logic(staged_files) + success, processed_count, _messages = tester.main_hook_logic(staged_files)
383-389: Improve error handling test to verify expected behavior.The current test has a try-except-pass block that silently swallows exceptions. This makes it unclear whether the test is passing because the API handles errors gracefully or because the test isn't checking anything.
Consider one of these approaches:
- Assert that a specific exception is raised:
# Should handle corrupted config gracefully try: loader = PromptLoader() - prompts = loader.load_prompts() + loader.load_prompts() # Might raise StorageError or skip the corrupted agent - except Exception: - # Error handling depends on implementation - pass + except Exception as e: + # Verify it's the expected error type + assert isinstance(e, (StorageError, ValueError))
- Or check that loading succeeds but skips the corrupted agent:
- try: - loader = PromptLoader() - prompts = loader.load_prompts() - # Might raise StorageError or skip the corrupted agent - except Exception: - # Error handling depends on implementation - pass + loader = PromptLoader() + prompts = loader.load_prompts() + # Corrupted agent should be skipped + assert 'test_agent' not in prompts or prompts['test_agent'] == {}tests/conftest.py (2)
330-334: Inconsistent initialization of prompts_data.In
MockPromptLoader.__init__,prompts_datais initialized as an empty dict (line 333), but in lines 423-427 ofmock_prompt_loader_with_edge_cases, it's directly assignedEDGE_CASE_DATA. This inconsistency could lead to confusion about whenload_prompts()needs to be called.Consider documenting this pattern more clearly or adding a parameter to control whether to auto-load:
def __init__(self, prompts_dir=None, auto_load=False): self.prompts_dir = Path(prompts_dir) if prompts_dir else TEST_PROMPTS_DIR self._loaded = False self.prompts_data = {} if auto_load: self.load_prompts()
340-395: Missing error handling for file I/O operations.Multiple file operations (lines 353-354, 360-361, 369-370) lack explicit error handling. While there's a try-except in the outer loop (lines 342-351), individual file read failures could produce unclear error messages.
Wrap file operations in try-except blocks with specific error messages:
try: with open(config_file, 'r') as f: config = yaml.safe_load(f) except yaml.YAMLError as e: raise ValueError(f"Invalid YAML in {config_file}: {e}") except IOError as e: raise ValueError(f"Cannot read {config_file}: {e}")src/promptix/core/components/prompt_loader.py (3)
62-65: Exception handling pattern could be improved.The broad
except Exceptionwithraise ... from eat line 65 is flagged by static analysis. While the exception chaining is correct, the error message embeds dynamic content which makes exception classes harder to reuse (TRY003).Consider creating a specific exception type or moving the message to StorageError's details:
- except Exception as e: - raise StorageError("Failed to load prompts from workspace", {"cause": str(e)}) from e + except Exception as e: + raise StorageError( + "Failed to load prompts from workspace", + {"workspace_path": str(workspace_path), "cause": str(e)} + ) from eThis separates the static message from dynamic context, addressing the TRY003 warning.
313-313: Consider using next(iter(...)) for better readability.Static analysis (RUF015) suggests using
next(iter(versions.keys()))instead oflist(versions.keys())[0]for better performance and readability.Apply this diff:
- live_key = 'current' if 'current' in versions else list(versions.keys())[0] + live_key = 'current' if 'current' in versions else next(iter(versions.keys()))
348-351: Version header regex pattern could be more robust.The regex at line 350 removes version headers, but it uses
.*?which might not handle multi-line headers correctly if someone adds newlines within the comment.Consider making the regex more explicit:
- content = re.sub(r'^<!--\s*Version\s+.*?-->\s*\n?', '', content, flags=re.MULTILINE) + content = re.sub(r'^<!--\s*Version\s+[^>]+?-->\s*\n?', '', content, flags=re.MULTILINE)Using
[^>]+?instead of.*?ensures we don't accidentally match across multiple HTML comments.hooks/pre-commit (1)
277-282: Missing return value check for git add.The
stage_files()function catchesCalledProcessError(line 281) but only logs a warning. The caller doesn't know if staging succeeded or failed, which could lead to inconsistent git state.Consider returning a boolean or re-raising the exception:
def stage_files(files: List[str]): - """Stage files for git commit""" + """Stage files for git commit + + Returns: + bool: True if staging succeeded, False otherwise + """ try: subprocess.run(['git', 'add'] + files, check=True) + return True except subprocess.CalledProcessError as e: print_status(f"Failed to stage files: {e}", "warning") + return FalseThen update callers to check the return value.
src/promptix/tools/cli.py (8)
10-10: Remove unused import.
Pathis imported but never used in this file.Apply this diff:
-from pathlib import Path
15-15: Remove unused import.
Textfromrich.textis imported but never used.Apply this diff:
-from rich.text import Text
19-19: Remove unused import.
rich_printis imported but never used in this file.Apply this diff:
-from rich import print as rich_print
199-199: Resolve function name conflict.The
listcommand at line 199 redefines thelistcommand defined at line 167 within the same module scope (both are in different Click groups, but static analysis flags the redefinition).Consider renaming one of these commands for clarity, or acknowledge that Click's grouping keeps them separate. If this is intentional, you can safely ignore this hint.
284-284: Resolve function name conflict.The
createcommand at line 284 redefines thecreatecommand defined at line 126. Both are in different Click groups (versionvsagent), so this is safe, but the linter flags it.If intentional (which appears to be the case given Click's command grouping), you can safely ignore this hint. Otherwise, consider unique names.
320-328: Remove unnecessary f-string prefixes.Lines 320-328 use f-strings without any placeholders. Remove the
fprefix for clarity.Apply this diff:
install_panel = Panel( - f"[bold green]✅ Promptix pre-commit hook installed![/bold green]\n\n" - f"[blue]What happens now:[/blue]\n" - f"• Every time you edit current.md and commit, a new version is created\n" - f"• When you change current_version in config.yaml, that version is deployed\n" - f"• Use 'SKIP_PROMPTIX_HOOK=1 git commit' to bypass when needed\n\n" - f"[blue]Try it:[/blue]\n" - f"• Edit any prompts/*/current.md file\n" - f"• Run: git add . && git commit -m 'Test versioning'\n" - f"• Check the new version in prompts/*/versions/", + "[bold green]✅ Promptix pre-commit hook installed![/bold green]\n\n" + "[blue]What happens now:[/blue]\n" + "• Every time you edit current.md and commit, a new version is created\n" + "• When you change current_version in config.yaml, that version is deployed\n" + "• Use 'SKIP_PROMPTIX_HOOK=1 git commit' to bypass when needed\n\n" + "[blue]Try it:[/blue]\n" + "• Edit any prompts/*/current.md file\n" + "• Run: git add . && git commit -m 'Test versioning'\n" + "• Check the new version in prompts/*/versions/", title="Hook Installation Complete", border_style="green" )
425-425: Prefer unpacking over concatenation.Use unpacking for cleaner list construction.
Apply this diff:
- original_args = ['openai'] + ctx.args + original_args = ['openai', *ctx.args]
159-164: Narrow exception handling.Catching bare
Exceptionat lines 162-164 is too broad. Consider catching specific exceptions (e.g.,OSError,RuntimeError) or let unexpected exceptions propagate.Apply this pattern throughout the file where bare
Exceptionis caught:- except Exception as e: - error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {e}") + except (OSError, RuntimeError) as e: + error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {e}")Note: This applies to lines 162, 189, 207, 224, 243, 276, 298, 336, 352, 365, 378, 391, 404, 429, and 450. Consider whether each should catch a narrower exception type.
src/promptix/core/builder.py (8)
8-8: Remove unused import.
Optionalfromtypingis imported but not used in this file (the file usesOptionalfrom line 8 in type hints, but this appears to be flagged incorrectly—verify if this is a false positive).After verification, if truly unused:
-from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, UnionHowever,
Optionalis used at lines 116, 171, 283 in type hints, so this is likely a false positive. You can ignore this hint.
22-23: Remove unused imports.
ToolNotFoundErrorandToolProcessingErrorare imported but never raised or used in this file.Apply this diff:
UnsupportedClientError, - ToolNotFoundError, - ToolProcessingError, ValidationError,
118-121: Use a custom exception class for validation errors.Instead of constructing a long message inline, define a custom exception or use the existing
ValidationErrormore consistently.Apply this diff:
if field not in self.properties: if not self.allow_additional: - raise ValidationError( - f"Field '{field}' is not defined in the schema and additional properties are not allowed.", - details={"field": field, "value": value} - ) + raise ValidationError( + f"Field '{field}' not in schema", + details={"field": field, "value": value, "allow_additional": False} + )
449-460: Improve exception handling for tools template rendering.The code catches
TemplateRenderErrorbut allows other exceptions to bubble up. Consider logging unexpected exceptions or wrapping them consistently.Apply this diff:
) if template_result: self._process_template_result(template_result, available_tools, selected_tools) except TemplateRenderError as e: if self._logger: self._logger.warning(f"Error processing tools template: {e!s}") - # Let unexpected exceptions bubble up + except Exception as e: + if self._logger: + self._logger.error(f"Unexpected error processing tools template: {e!s}") + # Re-raise to maintain existing behavior for unexpected errors + raiseNote: This ensures unexpected errors are logged before propagating.
470-473: Narrow exception handling for tools processing.Catching bare
Exceptionat line 470 is too broad. Consider catching specific exceptions related to adapter processing.Apply this diff:
try: # Convert to the format expected by the adapter adapter = self._adapters[self._client] return adapter.process_tools(selected_tools) - except Exception as e: + except (KeyError, ValueError, TypeError) as e: if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") + self._logger.warning(f"Error processing tools: {e!s}") return []
527-534: Narrow exception handling for system message rendering.Line 530 catches multiple specific exceptions plus bare
ImportErrorandRuntimeError. Consider whetherImportErrorandRuntimeErrorare expected here.If
ImportErrorandRuntimeErrorare not expected during normal operation, narrow the exception handling:- except (ValueError, ImportError, RuntimeError, RequiredVariableError, VariableValidationError) as e: + except (RequiredVariableError, VariableValidationError, TemplateRenderError) as e:Otherwise, document why these exceptions are caught.
565-568: Narrow exception handling for tools processing.Catching bare
Exceptionat line 565 is too broad. Use specific exception types.Apply this diff:
# Process tools configuration try: tools = self._process_tools_template() if tools: model_config["tools"] = tools - except Exception as e: + except (TemplateRenderError, ValueError, KeyError) as e: if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") + self._logger.warning(f"Error processing tools: {e!s}")
572-576: Narrow exception handling for adapter configuration.Catching bare
Exceptionat line 573 is too broad.Apply this diff:
adapter = self._adapters[self._client] try: model_config = adapter.adapt_config(model_config, self.version_data) - except Exception as e: + except (ValueError, KeyError, TypeError) as e: if self._logger: - self._logger.warning(f"Error adapting configuration for client {self._client}: {str(e)}") + self._logger.warning(f"Error adapting configuration for client {self._client}: {e!s}")src/promptix/tools/studio/folder_manager.py (10)
8-8: Remove unused import.The
osmodule is imported but never used in this file.Apply this diff:
-import os import yaml
91-91: Remove unnecessary f-string prefix.The string on line 91 has no placeholders.
Apply this diff:
- self._logger.info(f"YAML migration already completed (marker found)") + self._logger.info("YAML migration already completed (marker found)")
117-118: Uselogging.exceptionfor better error context.When logging exceptions, prefer
logger.exception()overlogger.error()to automatically include the traceback.Apply this diff:
except Exception as e: - self._logger.error(f"Failed to migrate prompt {prompt_id}: {str(e)}") + self._logger.exception(f"Failed to migrate prompt {prompt_id}: {e}")
127-127: Remove unnecessary f-string prefix.The string on line 127 has no placeholders.
Apply this diff:
- self._logger.info(f"Migration completed successfully!") + self._logger.info("Migration completed successfully!")
132-135: Uselogging.exceptionfor better error context.When logging exceptions in the outer exception handler, use
logger.exception()to include the traceback automatically.Apply this diff:
except Exception as e: - self._logger.error(f"Failed to migrate YAML prompts: {str(e)}") - self._logger.error("Your existing YAML file is unchanged. Please report this issue.") + self._logger.exception(f"Failed to migrate YAML prompts: {e}") + self._logger.error("Your existing YAML file is unchanged. Please report this issue.") raise
232-253: Use logger instead of print statements.The exception handling uses
print()instead of the initializedself._logger, which is inconsistent with the rest of the class.Apply this diff:
except Exception as e: - print(f"Warning: Error loading prompts: {e}") + self._logger.warning(f"Error loading prompts: {e}") return {"schema": 1.0}
255-318: Use logger instead of print statements.The exception handling uses
print()instead of the initializedself._logger, which is inconsistent with the rest of the class.Apply this diff:
except Exception as e: - print(f"Warning: Error loading prompt from {prompt_dir}: {e}") + self._logger.warning(f"Error loading prompt from {prompt_dir}: {e}") return None
327-392: Use logger instead of print statements.The exception handling uses
print()instead of the initializedself._logger, which is inconsistent with the rest of the class.Apply this diff:
except Exception as e: - print(f"Error in save_prompt: {str(e)}") - print(traceback.format_exc()) + self._logger.exception(f"Error in save_prompt: {e}") raise
394-405: Remove duplicate import and use logger.Two issues:
shutilis imported locally but already imported at the module level (line 18)- Exception handling uses
print()instead ofself._loggerApply this diff:
def delete_prompt(self, prompt_id: str) -> bool: """Delete a prompt by ID.""" try: prompt_dir = self.prompts_dir / prompt_id if prompt_dir.exists(): - import shutil shutil.rmtree(prompt_dir) return True return False except Exception as e: - print(f"Error deleting prompt {prompt_id}: {e}") + self._logger.error(f"Error deleting prompt {prompt_id}: {e}") return False
470-525: Use logger instead of print statements.The exception handling uses
print()instead of the initializedself._logger, which is inconsistent with the rest of the class.Apply this diff:
except Exception as e: - print(f"Error in add_version: {str(e)}") - print(traceback.format_exc()) + self._logger.exception(f"Error in add_version: {e}") raisetests/functional/test_versioning_edge_cases.py (3)
8-22: Remove unused imports.The
stat(line 13) andMagicMock(line 15) modules are imported but never used in the test file.Apply this diff:
import pytest import tempfile import shutil import yaml import os -import stat from pathlib import Path -from unittest.mock import patch, MagicMock +from unittest.mock import patch import sys
384-406: Consider adding explicit assertion or documenting test intent.The
version_namevariable is assigned but never checked. While the comment indicates the test is meant to verify "it shouldn't crash," consider making this explicit.You could either:
- Add an explicit assertion:
assert version_name is not None or version_name is None # Should not crash- Remove the variable if not needed:
_ = tester.create_version_snapshot(...)- Add a more explicit comment about the expected behavior
434-467: Consider adding explicit assertion or documenting test intent.The
version_namevariable is assigned but never checked. While the comment indicates "should work or fail gracefully," consider making this explicit.You could either:
- Add an explicit assertion:
assert version_name is not None or version_name is None # Should not crash- Remove the variable if not needed:
_ = tester.create_version_snapshot(...)- Add a more explicit comment about the expected behavior and what "gracefully" means
prompts/SimpleChat/versions/v011.md (1)
1-1: Consolidate or document identical SimpleChat version files
v011.md is identical to v1.md and v003.md–v010.md; only v2.md differs. If these are intended as stable snapshots, document your versioning strategy (e.g. in a README) or remove redundant files.
…andling - Updated TESTING_VERSIONING.md to streamline test execution commands and improve clarity. - Removed outdated demo instructions from VERSIONING_GUIDE.md. - Enhanced error handling in VersionManager for path validation and file operations. - Refactored prompt loading logic in tests for consistency and backward compatibility.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (5)
src/promptix/core/workspace_manager.py (1)
169-177: Excellent portable file comparison implementation!The hook now properly detects available comparison tools (
cmpordiff) and provides a clear error message if neither is available. This addresses portability concerns across different shell environments.tests/conftest.py (2)
94-104: Nice DRY extraction of shared loader logic.Good move consolidating folder parsing into a single helper used by both the fixture and MockPromptLoader.
178-186: Clearer compatibility fixture.The rename + docstring make it explicit this returns a directory path. Good cleanup.
src/promptix/tools/hook_manager.py (1)
1-1: Ensure the script is executable.Shebang is present; set the executable bit so it can be invoked directly.
#!/bin/bash # Verify and fix test -x src/promptix/tools/hook_manager.py || chmod +x src/promptix/tools/hook_manager.py ls -l src/promptix/tools/hook_manager.pysrc/promptix/tools/cli.py (1)
63-76: Streamlit/app path validation looks good and addresses prior concern.Using
shutil.which("streamlit"), convertingapp_pathto an absolute path, and invoking withoutshell=Trueresolves the earlier safety issues around PATH resolution and user input.Also applies to: 122-124
🧹 Nitpick comments (31)
prompts/CodeReviewer/versions/v014.md (2)
1-5: Consider adding structured output format instructions.The template asks the assistant to "provide feedback" but doesn't specify the expected structure. According to the AI summary, CodeReviewer templates typically include standard sections (Summary, Critical Issues, Improvements, Positives). Consider adding explicit instructions for the output format to ensure consistency with other CodeReviewer versions.
For example, you could extend the template like this:
You are a code review assistant specialized in {{programming_language}}. Please review the following code snippet and provide feedback on {{review_focus}}: ```{{programming_language}} {{code_snippet}}
+Please structure your review with the following sections:
+- Summary: Brief overview of the code quality
+- Critical Issues: Bugs, security vulnerabilities, or breaking problems
+- Improvements: Suggestions for better practices, performance, or maintainability
+- Positives: What the code does well--- `6-6`: **Remove trailing blank line.** Line 6 appears to be an unmarked trailing line that may cause formatting inconsistencies. Consider removing it or ensuring it's intentionally part of the template structure. </blockquote></details> <details> <summary>prompts/TemplateDemo/versions/v012.md (1)</summary><blockquote> `1-16`: **Safer defaults and simpler conditions for robustness** Use defaults for possibly missing vars, normalize difficulty once, and rely on truthiness for elements. Trim each element to avoid stray whitespace. ```diff -You are creating a {{content_type}} about {{theme}}. +You are creating a {{ content_type | default('content') }} about {{ theme | default('your topic') }}. -{% if difficulty == 'beginner' %} +{% set level = difficulty | default('advanced') %} +{% if level == 'beginner' %} Keep it simple and accessible for beginners. -{% elif difficulty == 'intermediate' %} +{% elif level == 'intermediate' %} Include some advanced concepts but explain them clearly. {% else %} Don't hold back on technical details and advanced concepts. {% endif %} -{% if elements|length > 0 %} +{% if elements %} Be sure to include the following elements: {% for element in elements %} -- {{element}} +- {{ element | trim }} {% endfor %} {% endif %}prompts/TemplateDemo/versions/v011.md (1)
1-16: Apply the same robustness tweaks as v012Mirror the defaulting/normalization and list trim to avoid undefineds and extra whitespace.
-You are creating a {{content_type}} about {{theme}}. +You are creating a {{ content_type | default('content') }} about {{ theme | default('your topic') }}. -{% if difficulty == 'beginner' %} +{% set level = difficulty | default('advanced') %} +{% if level == 'beginner' %} ... -{% elif difficulty == 'intermediate' %} +{% elif level == 'intermediate' %} ... -{% if elements|length > 0 %} +{% if elements %} ... -- {{element}} +- {{ element | trim }} ...prompts/TemplateDemo/versions/v014.md (2)
1-16: Harden template vars and element renderingSame rationale: defaults, normalized difficulty, truthy check, trim element.
-You are creating a {{content_type}} about {{theme}}. +You are creating a {{ content_type | default('content') }} about {{ theme | default('your topic') }}. -{% if difficulty == 'beginner' %} +{% set level = difficulty | default('advanced') %} +{% if level == 'beginner' %} ... -{% elif difficulty == 'intermediate' %} +{% elif level == 'intermediate' %} ... -{% if elements|length > 0 %} +{% if elements %} ... -- {{element}} +- {{ element | trim }} ...
1-16: Reduce duplication across v011–v014All four versions are identical. Consider an include/partial or generation step (or symlink to current.md) to prevent drift and ease updates.
prompts/TemplateDemo/versions/v013.md (1)
1-16: Consistency + intent check for v013Template matches v011/v012/v014 verbatim. If this version was meant to differ, confirm content; otherwise apply robustness tweaks as above.
-You are creating a {{content_type}} about {{theme}}. +You are creating a {{ content_type | default('content') }} about {{ theme | default('your topic') }}. -{% if difficulty == 'beginner' %} +{% set level = difficulty | default('advanced') %} +{% if level == 'beginner' %} ... -{% elif difficulty == 'intermediate' %} +{% elif level == 'intermediate' %} ... -{% if elements|length > 0 %} +{% if elements %} ... -- {{element}} +- {{ element | trim }} ...prompts/ComplexCodeReviewer/versions/v011.md (1)
1-7: v011 and v012 are identical
No differences found between prompts/ComplexCodeReviewer/versions/v011.md and v012.md; remove the redundant version or update it to reflect the intended changes.TESTING_VERSIONING.md (1)
17-31: Add language specifier to fenced code block.The test structure diagram should specify a language for proper rendering and syntax highlighting.
Apply this diff:
-``` +```text tests/ ├── unit/ # Unit tests for individual components │ ├── test_precommit_hook.py # Pre-commit hook functionalityVERSIONING_GUIDE.md (1)
41-56: Add language specifier to fenced code block.The file structure diagram should specify a language for proper rendering.
Apply this diff:
-``` +```text my-project/ ├── prompts/ # Git-friendly prompt workspacesrc/promptix/core/workspace_manager.py (2)
74-74: Remove unnecessary f-string prefix.The string on line 74 doesn't contain any placeholders, so the
fprefix is unnecessary.Apply this diff:
- print(f"📁 Created workspace directory: prompts/") + print("📁 Created workspace directory: prompts/")
244-244: Remove unnecessary f-string prefix.The string on line 244 doesn't contain any placeholders, so the
fprefix is unnecessary.Apply this diff:
- print(f"🔄 Created pre-commit hook at .git/hooks/pre-commit") + print("🔄 Created pre-commit hook at .git/hooks/pre-commit")tests/architecture/test_components.py (1)
501-521: Exception handling needs specificity.Similar to the previous test, this catches
(FileNotFoundError, LookupError)but should catch the actual Promptix exceptions (PromptNotFoundError,NoLiveVersionError). The indentation issue from past comments appears to be fixed (lines 512-514 are properly aligned).Apply this diff to use the correct exception types that are already imported:
- except (FileNotFoundError, LookupError): + except (PromptNotFoundError, NoLiveVersionError): # If no workspace prompts available, just test that the builder method exists assert callable(getattr(Promptix, 'builder', None))tests/conftest.py (2)
111-120: Specify utf‑8 when reading test fixtures (portability).Open calls default to locale encodings; make reads deterministic across OSes.
- with open(config_file, 'r') as f: + with open(config_file, 'r', encoding='utf-8') as f: config = yaml.safe_load(f) ... - with open(current_file, 'r') as f: + with open(current_file, 'r', encoding='utf-8') as f: current_template = f.read() ... - with open(version_file, 'r') as f: + with open(version_file, 'r', encoding='utf-8') as f: template = f.read()Optionally also sort for determinism:
- for prompt_dir in prompts_dir.iterdir(): + for prompt_dir in sorted(prompts_dir.iterdir()): ... - for version_file in versions_dir.glob("*.md"): + for version_file in sorted(versions_dir.glob("*.md")):Also applies to: 127-129
131-138: Live-version detection only matches “v1”; extend to “v001” style.Fixtures that use zero‑padded names won’t mark any entry live. Normalize with a regex/int parse.
- versions[version_name] = { - "is_live": version_name == "v1", # Assume v1 is live for testing + import re + m = re.match(r'^v0*1$', version_name) + versions[version_name] = { + "is_live": bool(m), # Treat v1, v01, v001 as live in testsAlso applies to: 140-151
src/promptix/tools/hook_manager.py (2)
255-276: Symlink‑safe hook execution looks good; add a timeout to avoid hangs.Path resolution and parent check mitigate symlink escapes. Add a timeout to prevent a wedged hook from stalling CI.
- result = subprocess.run([str(resolved_hook)], - capture_output=True, text=True) + result = subprocess.run([str(resolved_hook)], + capture_output=True, text=True, + timeout=30)
76-92: Narrow broad exception catches in helpers.Catching bare Exception makes diagnosing errors harder. Prefer OSError/IOError for FS ops and subprocess.CalledProcessError for commands.
Examples:
- backup_existing_hook/restore_backup: catch OSError as e
- is_promptix_hook: catch (OSError, UnicodeDecodeError) as e
- enable/disable: catch OSError as e
Keep the user‑friendly print_status, but narrow the except types.
Also applies to: 93-106, 186-188, 205-207
hooks/pre-commit (4)
84-102: Add utf‑8 encoding when reading/writing YAML config.For portability, specify encoding explicitly. Also preserve unicode on dump.
- with open(config_path, 'r') as f: - return yaml.safe_load(f) + with open(config_path, 'r', encoding='utf-8') as f: + return yaml.safe_load(f) ... - with open(config_path, 'w') as f: - yaml.dump(config, f, default_flow_style=False, sort_keys=False) + with open(config_path, 'w', encoding='utf-8') as f: + yaml.dump(config, f, default_flow_style=False, sort_keys=False, allow_unicode=True)
235-259: Prefer Path.read_text/write_text with encoding for prompt files.Simplifies I/O and ensures consistent encoding.
- with open(current_path, 'r') as f: - current_content = f.read() + current_content = Path(current_path).read_text(encoding='utf-8') ... - with open(version_file, 'r') as f: - content = f.read() + content = Path(version_file).read_text(encoding='utf-8') ... - with open(version_file, 'w') as f: - f.write(version_header) - f.write(content) + Path(version_file).write_text(version_header + content, encoding='utf-8') ... - if current_md.exists(): - with open(current_md, 'r') as f: - current_content = f.read() - with open(version_file, 'r') as f: - version_content = f.read() + if current_md.exists(): + current_content = Path(current_md).read_text(encoding='utf-8') + version_content = Path(version_file).read_text(encoding='utf-8') ... - with open(current_md, 'w') as f: - f.write(content) + Path(current_md).write_text(content, encoding='utf-8')Also applies to: 329-350
264-276: Improve author detection beyond $USER.Use common git/env fallbacks for better attribution.
- config['versions'][version_name] = { + author = (os.getenv('GIT_AUTHOR_NAME') + or os.getenv('GIT_COMMITTER_NAME') + or os.getenv('USER') + or os.getenv('USERNAME') + or 'unknown') + config['versions'][version_name] = { 'created_at': created_at, - 'author': os.getenv('USER', 'unknown'), + 'author': author, 'commit': commit_hash, 'notes': 'Auto-versioned on commit' }
71-80: Staged-file detection shouldn’t rely on working-tree existence.A staged file can be renamed/added while path.exists() briefly disagrees. Consider dropping the path.exists() checks and trust the staged list.
- if path.name == 'current.md' and path.exists(): + if path.name == 'current.md': changes['current_md'].append(file_path) - elif path.name == 'config.yaml' and path.exists(): + elif path.name == 'config.yaml': changes['config_yaml'].append(file_path)src/promptix/core/components/prompt_loader.py (2)
263-267: Preserve original traceback when wrapping exception.Chain the original exception for current.md read errors.
- except Exception as e: - raise StorageError( + except Exception as e: + raise StorageError( f"Failed to load current prompt for agent {agent_dir.name}", {"current_path": str(current_path), "error": str(e)} - ) + ) from e
315-321: Use next(iter(...)) for fallback live-version selection.Minor readability/performance improvement.
- live_key = 'current' if 'current' in versions else list(versions.keys())[0] + live_key = 'current' if 'current' in versions else next(iter(versions.keys()))src/promptix/tools/cli.py (8)
144-151: Avoid redefining command functions (create,list) across groups.Defining
createandlisttwice (foragentandversion) triggers F811 and hurts readability. Keep CLI names vianame=..., but give unique Python function names.@agent.command() -@click.argument('name') -def create(name: str): +@click.argument('name') +def create_agent_cmd(name: str): @@ -@agent.command() -def list(): +@agent.command(name="list") +def list_agents_cmd(): @@ -@version.command() -def list(): +@version.command(name="list") +def list_versions_cmd(): @@ -@version.command() +@version.command(name="create") @click.argument('agent') @click.option('--name', help='Version name (auto-generated if not provided)') @click.option('--notes', default='Manually created', help='Version notes') -def create(agent: str, name: str, notes: str): +def create_version_cmd(agent: str, name: str, notes: str):Also applies to: 186-197, 218-230, 300-306
93-101: Remove unusedtaskvariable in progress block.Assigned but never used (F841). Keep the spinner without capturing the task.
- task = progress.add_task("Searching for available port...", total=None) + progress.add_task("Searching for available port...", total=None)
340-348: Drop unnecessary f-strings (F541).These lines contain no placeholders.
- f"[bold green]✅ Promptix pre-commit hook installed![/bold green]\n\n" - f"[blue]What happens now:[/blue]\n" - f"• Every time you edit current.md and commit, a new version is created\n" - f"• When you change current_version in config.yaml, that version is deployed\n" - f"• Use 'SKIP_PROMPTIX_HOOK=1 git commit' to bypass when needed\n\n" - f"[blue]Try it:[/blue]\n" - f"• Edit any prompts/*/current.md file\n" - f"• Run: git add . && git commit -m 'Test versioning'\n" - f"• Check the new version in prompts/*/versions/", + "[bold green]✅ Promptix pre-commit hook installed![/bold green]\n\n" + "[blue]What happens now:[/blue]\n" + "• Every time you edit current.md and commit, a new version is created\n" + "• When you change current_version in config.yaml, that version is deployed\n" + "• Use 'SKIP_PROMPTIX_HOOK=1 git commit' to bypass when needed\n\n" + "[blue]Try it:[/blue]\n" + "• Edit any prompts/*/current.md file\n" + "• Run: git add . && git commit -m 'Test versioning'\n" + "• Check the new version in prompts/*/versions/",
133-134: Prefer explicit conversion flag in f-strings (RUF010).Use
{e!s}instead of wrappingstr(e).- error_console.print(f"[bold red]❌ Error launching Promptix Studio:[/bold red] {str(e)}") + error_console.print(f"[bold red]❌ Error launching Promptix Studio:[/bold red] {e!s}") @@ - error_console.print(f"[bold red]❌ Error:[/bold red] {str(e)}") + error_console.print(f"[bold red]❌ Error:[/bold red] {e!s}") @@ - error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {str(e)}") + error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {e!s}")Also applies to: 449-451, 471-472
182-185: Avoid broadexcept Exception+sys.exit(1)across commands.Catching all exceptions obscures actionable errors and hampers testing with Click’s runner. Prefer narrow exceptions and raise
click.ClickExceptionto let Click handle exit codes and formatting.Example pattern:
- except ValueError as e: - error_console.print(f"[bold red]❌ Error:[/bold red] {e!s}") - sys.exit(1) - except Exception as e: - error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {e!s}") - sys.exit(1) + except ValueError as e: + raise click.ClickException(str(e)) + except OSError as e: + raise click.ClickException(str(e))Repeat similarly for other commands.
Also applies to: 209-212, 227-230, 244-247, 263-266, 296-299, 318-321, 353-359, 369-375, 383-388, 395-401, 409-414, 421-427, 449-452, 470-473
11-21: Remove unused imports (Flake8 F401).
Path,Text, andrich_printare imported but unused.-from pathlib import Path -from rich.text import Text -from rich import print as rich_print
55-61: Let Click validate the port range; drop redundant runtime check.Use
click.IntRange(1, 65535)so invalid ports are rejected before entering the function. The explicitisinstance/bounds check can then be removed.-@click.option( - '--port', '-p', - default=8501, - type=int, - help='Port to run the studio on' -) +@click.option( + '--port', '-p', + default=8501, + type=click.IntRange(1, 65535), + help='Port to run the studio on' +) @@ - # Validate and normalize port - if not isinstance(port, int) or port < 1 or port > 65535: - error_console.print("[bold red]❌ Error:[/bold red] Port must be between 1 and 65535") - sys.exit(1) + # Port already validated by Click's IntRangeAlso applies to: 84-88
444-447: Minor: build argv using sequence unpacking (RUF005).- original_args = ['openai'] + ctx.args + original_args = ['openai', *ctx.args]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
TESTING_VERSIONING.md(1 hunks)VERSIONING_GUIDE.md(1 hunks)check_duplicate_versions.py(1 hunks)hooks/pre-commit(1 hunks)prompts/CodeReviewer/config.yaml(1 hunks)prompts/CodeReviewer/versions/v004.md(1 hunks)prompts/CodeReviewer/versions/v012.md(1 hunks)prompts/CodeReviewer/versions/v013.md(1 hunks)prompts/CodeReviewer/versions/v014.md(1 hunks)prompts/CodeReviewer/versions/v015.md(1 hunks)prompts/ComplexCodeReviewer/versions/v011.md(1 hunks)prompts/ComplexCodeReviewer/versions/v012.md(1 hunks)prompts/ComplexCodeReviewer/versions/v013.md(1 hunks)prompts/ComplexCodeReviewer/versions/v014.md(1 hunks)prompts/SimpleChat/versions/v012.md(1 hunks)prompts/SimpleChat/versions/v013.md(1 hunks)prompts/SimpleChat/versions/v014.md(1 hunks)prompts/SimpleChat/versions/v015.md(1 hunks)prompts/TemplateDemo/versions/v011.md(1 hunks)prompts/TemplateDemo/versions/v012.md(1 hunks)prompts/TemplateDemo/versions/v013.md(1 hunks)prompts/TemplateDemo/versions/v014.md(1 hunks)prompts/simple_chat/versions/v010.md(1 hunks)prompts/simple_chat/versions/v011.md(1 hunks)prompts/simple_chat/versions/v012.md(1 hunks)prompts/simple_chat/versions/v013.md(1 hunks)src/promptix/core/components/prompt_loader.py(5 hunks)src/promptix/core/storage/utils.py(1 hunks)src/promptix/core/workspace_manager.py(1 hunks)src/promptix/tools/cli.py(1 hunks)src/promptix/tools/hook_manager.py(1 hunks)src/promptix/tools/version_manager.py(1 hunks)tests/architecture/test_components.py(3 hunks)tests/conftest.py(6 hunks)
✅ Files skipped from review due to trivial changes (5)
- prompts/ComplexCodeReviewer/versions/v014.md
- prompts/ComplexCodeReviewer/versions/v013.md
- prompts/CodeReviewer/versions/v015.md
- prompts/SimpleChat/versions/v014.md
- prompts/SimpleChat/versions/v012.md
🚧 Files skipped from review as they are similar to previous changes (3)
- prompts/CodeReviewer/config.yaml
- prompts/CodeReviewer/versions/v004.md
- src/promptix/core/storage/utils.py
🧰 Additional context used
🧬 Code graph analysis (7)
tests/architecture/test_components.py (9)
src/promptix/core/components/prompt_loader.py (3)
PromptLoader(15-389)load_prompts(28-65)is_loaded(110-116)src/promptix/core/components/variable_validator.py (1)
VariableValidator(20-75)src/promptix/core/components/template_renderer.py (1)
TemplateRenderer(13-146)src/promptix/core/components/model_config_builder.py (1)
ModelConfigBuilder(12-217)src/promptix/core/exceptions.py (10)
PromptixError(11-22)PromptNotFoundError(27-36)VersionNotFoundError(39-49)NoLiveVersionError(52-61)MultipleLiveVersionsError(64-73)TemplateRenderError(76-86)VariableValidationError(96-109)RequiredVariableError(112-123)ConfigurationError(140-148)InvalidMemoryFormatError(231-240)src/promptix/core/container.py (2)
Container(25-196)reset_container(279-282)src/promptix/core/base.py (2)
Promptix(19-257)builder(195-206)tests/conftest.py (4)
mock_config(254-260)test_prompts_dir(90-92)load_prompts(346-353)is_loaded(355-357)src/promptix/core/config.py (3)
get_prompts_workspace_path(49-56)has_prompts_workspace(58-66)create_default_workspace(68-81)
src/promptix/core/components/prompt_loader.py (2)
src/promptix/core/exceptions.py (2)
StorageError(182-184)PromptNotFoundError(27-36)src/promptix/core/config.py (4)
get_prompts_workspace_path(49-56)has_prompts_workspace(58-66)create_default_workspace(68-81)get(107-125)
tests/conftest.py (3)
src/promptix/core/config.py (2)
get(107-125)get_prompt_file_path(137-155)src/promptix/core/components/prompt_loader.py (1)
load_prompts(28-65)src/promptix/tools/studio/folder_manager.py (1)
load_prompts(232-253)
src/promptix/tools/version_manager.py (4)
src/promptix/tools/hook_manager.py (3)
print_status(44-54)status(208-243)main(293-351)tests/test_helpers/precommit_helper.py (3)
print_status(31-40)load_config(82-89)save_config(91-99)src/promptix/tools/cli.py (5)
status(403-413)get(251-265)main(453-472)agent(140-142)version(214-216)src/promptix/core/components/prompt_loader.py (1)
list_agents(95-108)
src/promptix/tools/cli.py (3)
src/promptix/tools/hook_manager.py (8)
HookManager(18-290)has_existing_hook(60-62)install_hook(107-146)uninstall_hook(148-168)enable_hook(189-206)disable_hook(170-187)status(208-243)test_hook(245-290)src/promptix/tools/version_manager.py (6)
VersionManager(23-436)list_agents(161-185)list_versions(187-240)get_version(241-278)switch_version(280-342)create_version(344-436)src/promptix/core/workspace_manager.py (3)
WorkspaceManager(12-273)create_agent(25-68)list_agents(247-261)
src/promptix/tools/hook_manager.py (3)
src/promptix/tools/version_manager.py (1)
print_status(34-44)tests/test_helpers/precommit_helper.py (1)
print_status(31-40)src/promptix/tools/cli.py (2)
status(403-413)get(251-265)
src/promptix/core/workspace_manager.py (2)
src/promptix/core/config.py (3)
set_working_directory(44-47)get_prompts_workspace_path(49-56)create_default_workspace(68-81)src/promptix/core/components/prompt_loader.py (1)
list_agents(95-108)
🪛 Flake8 (7.2.0)
src/promptix/tools/version_manager.py
[error] 98-98: local variable 'e' is assigned to but never used
(F841)
[error] 335-335: f-string is missing placeholders
(F541)
[error] 453-453: local variable 'list_cmd' is assigned to but never used
(F841)
src/promptix/tools/cli.py
[error] 11-11: 'pathlib.Path' imported but unused
(F401)
[error] 16-16: 'rich.text.Text' imported but unused
(F401)
[error] 20-20: 'rich.print as rich_print' imported but unused
(F401)
[error] 99-99: local variable 'task' is assigned to but never used
(F841)
[error] 219-219: redefinition of unused 'list' from line 187
(F811)
[error] 304-304: redefinition of unused 'create' from line 146
(F811)
[error] 340-340: f-string is missing placeholders
(F541)
src/promptix/tools/hook_manager.py
[error] 312-312: local variable 'uninstall_cmd' is assigned to but never used
(F841)
[error] 315-315: local variable 'enable_cmd' is assigned to but never used
(F841)
[error] 316-316: local variable 'disable_cmd' is assigned to but never used
(F841)
[error] 319-319: local variable 'status_cmd' is assigned to but never used
(F841)
[error] 322-322: local variable 'test_cmd' is assigned to but never used
(F841)
src/promptix/core/workspace_manager.py
[error] 5-5: 'os' imported but unused
(F401)
[error] 74-74: f-string is missing placeholders
(F541)
[error] 244-244: f-string is missing placeholders
(F541)
🪛 markdownlint-cli2 (0.18.1)
VERSIONING_GUIDE.md
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
TESTING_VERSIONING.md
17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.13.1)
check_duplicate_versions.py
15-15: Probable use of insecure hash functions in hashlib: md5
(S324)
src/promptix/core/components/prompt_loader.py
62-62: Consider moving this statement to an else block
(TRY300)
65-65: Avoid specifying long messages outside the exception class
(TRY003)
252-255: Avoid specifying long messages outside the exception class
(TRY003)
263-263: Do not catch blind exception: Exception
(BLE001)
264-267: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
264-267: Avoid specifying long messages outside the exception class
(TRY003)
318-318: Prefer next(iter(versions.keys())) over single element slice
Replace with next(iter(versions.keys()))
(RUF015)
384-384: Do not catch blind exception: Exception
(BLE001)
src/promptix/tools/version_manager.py
1-1: Shebang is present but file is not executable
(EXE001)
32-32: Avoid specifying long messages outside the exception class
(TRY003)
97-97: Consider moving this statement to an else block
(TRY300)
98-98: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
120-120: Do not catch blind exception: Exception
(BLE001)
153-153: Consider moving this statement to an else block
(TRY300)
335-335: f-string without any placeholders
Remove extraneous f prefix
(F541)
453-453: Local variable list_cmd is assigned to but never used
Remove assignment to unused variable list_cmd
(F841)
498-498: Do not catch blind exception: Exception
(BLE001)
src/promptix/tools/cli.py
99-99: Local variable task is assigned to but never used
Remove assignment to unused variable task
(F841)
122-122: subprocess call: check for execution of untrusted input
(S603)
133-133: Use explicit conversion flag
Replace with conversion flag
(RUF010)
182-182: Do not catch blind exception: Exception
(BLE001)
209-209: Do not catch blind exception: Exception
(BLE001)
219-219: Redefinition of unused list from line 187
(F811)
227-227: Do not catch blind exception: Exception
(BLE001)
244-244: Do not catch blind exception: Exception
(BLE001)
263-263: Do not catch blind exception: Exception
(BLE001)
296-296: Do not catch blind exception: Exception
(BLE001)
304-304: Redefinition of unused create from line 146
(F811)
305-305: Docstring contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF002)
310-310: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
318-318: Do not catch blind exception: Exception
(BLE001)
340-340: f-string without any placeholders
Remove extraneous f prefix
(F541)
341-341: f-string without any placeholders
Remove extraneous f prefix
(F541)
342-342: f-string without any placeholders
Remove extraneous f prefix
(F541)
343-343: f-string without any placeholders
Remove extraneous f prefix
(F541)
344-344: f-string without any placeholders
Remove extraneous f prefix
(F541)
345-345: f-string without any placeholders
Remove extraneous f prefix
(F541)
346-346: f-string without any placeholders
Remove extraneous f prefix
(F541)
347-347: f-string without any placeholders
Remove extraneous f prefix
(F541)
348-348: f-string without any placeholders
Remove extraneous f prefix
(F541)
356-356: Do not catch blind exception: Exception
(BLE001)
372-372: Do not catch blind exception: Exception
(BLE001)
385-385: Do not catch blind exception: Exception
(BLE001)
398-398: Do not catch blind exception: Exception
(BLE001)
411-411: Do not catch blind exception: Exception
(BLE001)
424-424: Do not catch blind exception: Exception
(BLE001)
445-445: Consider ['openai', *ctx.args] instead of concatenation
Replace with ['openai', *ctx.args]
(RUF005)
449-449: Do not catch blind exception: Exception
(BLE001)
450-450: Use explicit conversion flag
Replace with conversion flag
(RUF010)
470-470: Do not catch blind exception: Exception
(BLE001)
471-471: Use explicit conversion flag
Replace with conversion flag
(RUF010)
src/promptix/tools/hook_manager.py
32-32: Avoid specifying long messages outside the exception class
(TRY003)
43-43: Avoid specifying long messages outside the exception class
(TRY003)
72-72: Consider moving this statement to an else block
(TRY300)
73-73: Do not catch blind exception: Exception
(BLE001)
88-88: Consider moving this statement to an else block
(TRY300)
89-89: Do not catch blind exception: Exception
(BLE001)
102-102: Consider moving this statement to an else block
(TRY300)
103-103: Do not catch blind exception: Exception
(BLE001)
140-140: os.chmod setting a permissive mask 0o755 on file or directory
(S103)
145-145: Do not catch blind exception: Exception
(BLE001)
167-167: Do not catch blind exception: Exception
(BLE001)
186-186: Do not catch blind exception: Exception
(BLE001)
205-205: Do not catch blind exception: Exception
(BLE001)
275-275: subprocess call: check for execution of untrusted input
(S603)
289-289: Do not catch blind exception: Exception
(BLE001)
312-312: Local variable uninstall_cmd is assigned to but never used
Remove assignment to unused variable uninstall_cmd
(F841)
315-315: Local variable enable_cmd is assigned to but never used
Remove assignment to unused variable enable_cmd
(F841)
316-316: Local variable disable_cmd is assigned to but never used
Remove assignment to unused variable disable_cmd
(F841)
319-319: Local variable status_cmd is assigned to but never used
Remove assignment to unused variable status_cmd
(F841)
322-322: Local variable test_cmd is assigned to but never used
Remove assignment to unused variable test_cmd
(F841)
349-349: Do not catch blind exception: Exception
(BLE001)
src/promptix/core/workspace_manager.py
40-40: Avoid specifying long messages outside the exception class
(TRY003)
48-48: Avoid specifying long messages outside the exception class
(TRY003)
57-57: Avoid specifying long messages outside the exception class
(TRY003)
74-74: f-string without any placeholders
Remove extraneous f prefix
(F541)
244-244: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.11)
🔇 Additional comments (25)
check_duplicate_versions.py (3)
13-19: MD5 is appropriate for duplicate detection.The static analysis tool flags MD5 as potentially insecure, but for this use case (detecting duplicate file content), MD5 is perfectly suitable. This is not a cryptographic application where collision resistance matters; it's a content-deduplication tool where MD5's speed and low collision probability for legitimate files make it ideal.
56-78: LGTM!The report formatting is clear and user-friendly, with good visual hierarchy and a helpful recommendation message.
98-99: LGTM!Standard and correct entry point pattern with proper exit code propagation.
prompts/SimpleChat/versions/v013.md (1)
1-1: LGTM! Template follows established SimpleChat pattern.The single-line prompt template correctly uses
{{assistant_name}}and{{user_name}}placeholders and aligns with the existing SimpleChat versioning approach.prompts/ComplexCodeReviewer/versions/v012.md (1)
1-7: LGTM! Well-structured code review template.The template correctly:
- Integrates multiple contextual placeholders (
{{active_tools}},{{programming_language}},{{severity}},{{review_focus}},{{code_snippet}})- Uses language-tagged code blocks for proper syntax highlighting
- Defines a clear four-section feedback structure
prompts/simple_chat/versions/v012.md (2)
1-11: Template structure is well-designed (assuming path is correct).If this is indeed a separate agent, the template structure is good:
- Clear persona definition with
{{personality}}and{{domain}}placeholders- Comprehensive guidelines section
- Professional tone and helpful structure
- Appropriate use of multi-line formatting
1-11: Ignore path/placeholder inconsistency – separate simple_chat agent
Theprompts/simple_chat/directory defines its own agent (withpersonality/domainschema in its config) alongside theprompts/SimpleChat/agent (withuser_name/assistant_name). Both contain their ownversions/v012.mdandconfig.yaml, so no path or placeholder changes are needed.Likely an incorrect or invalid review comment.
prompts/SimpleChat/versions/v015.md (1)
1-1: Confirm duplication in v015.md
v015.md is identical to v013.md (and v014.md). Clarify whether this is an intentional rollback/reversion or an unintended duplicate.prompts/simple_chat/versions/v013.md (1)
1-11: LGTM! Well-structured prompt template.The template follows best practices with clear role definition, guidelines, and placeholder variables for customization. The structure is clean and the instructions are concise.
VERSIONING_GUIDE.md (1)
304-319: Manual testing section is clear and actionable.The step-by-step manual testing workflow provides practical commands that users can follow to verify the versioning system.
src/promptix/core/workspace_manager.py (1)
38-40: Agent name validation is robust.The validation correctly allows alphanumeric characters, hyphens, and underscores while rejecting invalid names. The logic is clear and appropriate.
TESTING_VERSIONING.md (1)
36-49: requirements-test.txt verified. File exists and lists the expected test dependencies, so the quick-start instructions are accurate.prompts/simple_chat/versions/v010.md (1)
1-11: LGTM! Well-structured prompt template.The versioned prompt template is clear and follows a logical structure with appropriate placeholders for customization. The guidelines are concise and the tone is professional.
tests/architecture/test_components.py (2)
86-119: Excellent improvement to workspace-loading test!The test now uses real fixture data (via
tmp_pathandshutil.copytree) and validates:
- The returned structure is a dict
- At least 3 agents are loaded
- Expected agent names are present
- Agent structure includes
versionsdictThis addresses the previous feedback about strengthening the test assertions.
121-134: Test rename improves clarity.The test is now properly named
test_load_prompts_uses_workspaceand accurately reflects that it verifies workspace-based loading behavior. The docstring is also aligned with the test intent.src/promptix/tools/version_manager.py (8)
59-124: Excellent path validation improvements!The updated
_validate_pathmethod now addresses all previous security concerns:
- Checks that both base and candidate paths exist before resolving
- Uses
resolve(strict=True)to get canonical paths- Explicitly handles symlinks and validates their targets
- Catches platform-specific issues (different drives on Windows)
- Uses specific exception types (
OSError,RuntimeError,ValueError) instead of bareExceptionfor most cases- Only uses generic
Exceptionfor truly unexpected errors with clear loggingThe layered validation approach (exist check → resolve → symlink check → containment check) is robust.
133-147: Excellent exception handling in load_config!The method now catches specific exception types (
FileNotFoundError,PermissionError,yaml.YAMLError) instead of bareException, making error diagnosis much clearer. This addresses previous feedback about blind exception catching.
148-160: Good specific exception handling in save_config.Similar to
load_config, this method catches specific exceptions (PermissionError,OSError) which improves diagnostics.
187-241: Robust version listing with path validation.The
list_versionsmethod properly validates the agent path using_validate_pathbefore proceeding, preventing directory traversal attacks. Error handling is appropriate.
241-279: Secure version content retrieval.The
get_versionmethod validates both the agent path and version file path, and catches specific file-related exceptions (FileNotFoundError,PermissionError,OSError) instead of generic exceptions.
280-343: Comprehensive version switching with proper error handling.The
switch_versionmethod:
- Validates paths to prevent traversal attacks
- Catches specific exceptions for better diagnostics
- Properly updates both config.yaml and current.md
- Removes version headers from current.md
344-437: Well-implemented version creation with auto-numbering.The
create_versionmethod includes:
- Path validation throughout
- Auto-generation of version numbers
- Version metadata tracking
- Proper error handling with specific exception types
The implementation is secure and handles edge cases well.
481-501: Main CLI error handling is appropriate.The main function catches
ValueErrorfor expected errors (like missing workspace) and uses a genericExceptioncatch for unexpected errors, which is acceptable at this top-level CLI entry point since it logs the error and exits gracefully. This is different from internal methods where specific exceptions are preferred.tests/conftest.py (1)
253-260: Type mismatch risk in mock_config path getters.Production config.get_prompt_file_path() returns Optional[Path]; the mock returns str. If tests pass this into code paths that do Path operations (e.g., Path(...) / 'x'), this may break. Verify consumers only treat these as strings, or return Path objects here.
- mock.get_prompts_dir.return_value = str(test_prompts_dir) - mock.get_prompt_file_path.return_value = str(test_prompts_dir) + mock.get_prompts_dir.return_value = test_prompts_dir + mock.get_prompt_file_path.return_value = test_prompts_dirsrc/promptix/core/components/prompt_loader.py (1)
195-226: Good resilience: skip broken agents but report count.Catching StorageError only and tallying skipped agents strikes a nice balance between robustness and debuggability.
- Updated README.md for improved clarity and structure, including a new quick start section and enhanced feature descriptions. - Modified VersionManager's path validation method to include an optional parameter for existence checks, improving error handling and security. - Added backward compatibility fixture in tests for legacy support.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/promptix/tools/version_manager.py (4)
104-104: Remove unused exception variable.The exception variable
eis captured but never used.Apply this diff:
- except ValueError as e: + except ValueError: # Check if it's a different drive issue (Windows)
126-129: Replace broad exception handler with specific exception types.The broad
Exceptioncatch can mask programming errors and makes debugging difficult.Apply this diff:
- except Exception as e: - # Log unexpected errors before returning False - self.print_status(f"Unexpected path validation error for {path_type}: {e}", "error") - return FalseThe existing
OSErrorandRuntimeErrorhandlers should catch all legitimate path resolution errors. Remove the blanket exception handler to allow unexpected errors to propagate.
341-341: Remove unnecessary f-string prefix.The string has no placeholders, so the
fprefix is unnecessary.Apply this diff:
- self.print_status(f"Updated current.md and config.yaml", "info") + self.print_status("Updated current.md and config.yaml", "info")
460-460: Remove unused subparser variable.The
list_cmdsubparser is assigned but never used.Apply this diff:
# List agents command - list_cmd = subparsers.add_parser('list', help='List all agents') + subparsers.add_parser('list', help='List all agents')
🧹 Nitpick comments (7)
README.md (2)
5-5: Fix heading hierarchy to silence MD001Right after the H1 title we jump straight to an H3, which triggers the
heading-incrementlint (MD001). Please promote this tagline to an H2 so the hierarchy stays sequential.-### Local-First Prompt Management for Production LLM Applications +## Local-First Prompt Management for Production LLM Applications
398-401: Convert emphasized “heading” into a real headingThe closing banner is currently bold text, so markdownlint (MD036) flags it as “emphasis instead of a heading.” Converting it to an actual heading keeps the styling while satisfying the linter.
-**Made with ❤️ by developers, for developers** +### Made with ❤️ by developers, for developerssrc/promptix/tools/version_manager.py (2)
34-44: Extract shared print_status to a common utility module.This method duplicates the implementation in
src/promptix/tools/hook_manager.py(lines 43-53). Both use identical icon mappings and formatting logic.Consider creating a shared utility module:
# src/promptix/tools/utils.py def print_status(message: str, status: str = "info"): """Print colored status messages""" icons = { "info": "📝", "success": "✅", "warning": "⚠️", "error": "❌", "version": "🔄", "list": "📋", "install": "🔧", "uninstall": "🗑️" } print(f"{icons.get(status, '📝')} {message}")Then import and use it in both
version_manager.pyandhook_manager.py.
502-507: Consider more specific exception handling in main().While catching broad
Exceptionat the top level prevents crashes, you could improve error reporting by catchingValueErrorseparately (already done at line 502-504) and being more specific about other expected errors.The current implementation is acceptable for a CLI entry point, but for better diagnostics you could add handlers for common errors like
KeyboardInterrupt,FileNotFoundError, orPermissionErrorbefore the broad catch.tests/conftest.py (3)
20-21: Remove unused TEST_PROMPT_NAMES constant
TheTEST_PROMPT_NAMESdefined at tests/conftest.py:21 isn’t referenced anywhere else—remove it to reduce maintenance. If you do need a list of prompt directories, generate it dynamically fromTEST_PROMPTS_DIRinstead.
178-194: Remove the unusedtemp_prompts_dir_compatfixture.The
temp_prompts_dir_compatfixture isn’t referenced by any tests (only exists intests/conftest.py), while all backwards-compatibility users rely ontemp_prompts_file. Deletetemp_prompts_dir_compat, consolidate its doc intotemp_prompts_file, and update that fixture’s docstring to note the legacy alias.
94-154: Document and optionally log skipped files; retainv1as live
- Test fixtures under
tests/fixtures/test_promptsdon’t include anyis_liveflags inconfig.yaml, so defaulting toversion_name == "v1"is correct.- (Optional) Add comments or logging around skipped directories/malformed YAML to improve test-debug visibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(1 hunks)prompts/CodeReviewer/versions/v016.md(1 hunks)prompts/ComplexCodeReviewer/versions/v015.md(1 hunks)prompts/SimpleChat/versions/v016.md(1 hunks)prompts/TemplateDemo/versions/v015.md(1 hunks)prompts/simple_chat/versions/v014.md(1 hunks)src/promptix/tools/version_manager.py(1 hunks)tests/conftest.py(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- prompts/simple_chat/versions/v014.md
🧰 Additional context used
🧬 Code graph analysis (2)
src/promptix/tools/version_manager.py (3)
src/promptix/tools/hook_manager.py (2)
print_status(44-54)status(208-243)src/promptix/tools/cli.py (4)
status(403-413)get(251-265)agent(140-142)version(214-216)src/promptix/core/components/prompt_loader.py (1)
list_agents(95-108)
tests/conftest.py (4)
src/promptix/core/config.py (2)
get(107-125)get_prompt_file_path(137-155)src/promptix/core/components/prompt_loader.py (1)
load_prompts(28-65)src/promptix/tools/studio/data.py (1)
load_prompts(29-31)src/promptix/tools/studio/folder_manager.py (1)
load_prompts(232-253)
🪛 Flake8 (7.2.0)
src/promptix/tools/version_manager.py
[error] 104-104: local variable 'e' is assigned to but never used
(F841)
[error] 341-341: f-string is missing placeholders
(F541)
[error] 460-460: local variable 'list_cmd' is assigned to but never used
(F841)
🪛 markdownlint-cli2 (0.18.1)
README.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
398-398: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.13.1)
src/promptix/tools/version_manager.py
1-1: Shebang is present but file is not executable
(EXE001)
32-32: Avoid specifying long messages outside the exception class
(TRY003)
103-103: Consider moving this statement to an else block
(TRY300)
104-104: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
126-126: Do not catch blind exception: Exception
(BLE001)
159-159: Consider moving this statement to an else block
(TRY300)
341-341: f-string without any placeholders
Remove extraneous f prefix
(F541)
460-460: Local variable list_cmd is assigned to but never used
Remove assignment to unused variable list_cmd
(F841)
505-505: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.9)
- GitHub Check: Tests (Python 3.10)
🔇 Additional comments (17)
prompts/CodeReviewer/versions/v016.md (1)
1-6: Verify that the simplified template structure is intentional.This template is significantly more basic than the pattern described in the AI summary for preceding versions (v012-v015), which mention "structured feedback sections (e.g., Summary, Critical Issues, Improvements, Positives)." The current v016 template:
- Requests a review but provides no guidance on output format or structure
- Lacks explicit instructions for organizing feedback by severity or category
- May produce inconsistent review outputs compared to more structured predecessor versions
If structured output is desired for consistency with the CodeReviewer prompt family, consider adding explicit formatting instructions. For example:
You are a code review assistant specialized in {{programming_language}}. Please review the following code snippet and provide feedback on {{review_focus}}: ```{{programming_language}} {{code_snippet}}
+Please structure your feedback as follows:
+
+## Summary
+[Brief overview of the code quality]
+
+## Critical Issues
+[Issues that must be addressed]
+
+## Improvements
+[Suggested enhancements]
+
+## Positives
+[Things done well]Otherwise, if this simplified structure is intentional for v016, please confirm the design decision. </blockquote></details> <details> <summary>prompts/ComplexCodeReviewer/versions/v015.md (1)</summary><blockquote> `1-7`: **LGTM! Template structure is clean and consistent.** The prompt template follows a clear structure with well-named placeholders, proper markdown code fence syntax, and defines a structured feedback format that aligns with the ComplexCodeReviewer versioning pattern mentioned in the PR context. </blockquote></details> <details> <summary>prompts/TemplateDemo/versions/v015.md (3)</summary><blockquote> `1-2`: **LGTM! Clear and concise opening statement.** The template placeholders are properly formatted using Jinja2 syntax, and the opening statement effectively sets up the content generation context. --- `3-9`: **Verify the catch-all else clause behavior is intentional.** The conditional structure correctly handles `beginner` and `intermediate` difficulty levels, but the `else` clause will match both `advanced` and any unexpected values (typos, invalid inputs). If this catch-all behavior is intentional to default to advanced mode, the implementation is fine. However, if strict validation is preferred, consider explicitly checking for `'advanced'` and handling invalid values separately. --- `11-16`: **LGTM! Proper conditional iteration with clean Markdown formatting.** The template correctly guards against empty collections using the length filter and properly iterates over elements to render a bulleted list. The Jinja2 syntax and Markdown formatting are both correct. </blockquote></details> <details> <summary>src/promptix/tools/version_manager.py (10)</summary><blockquote> `26-32`: **LGTM!** The initialization correctly validates the prompts directory and establishes the workspace path. The inline error message is contextually appropriate. --- `131-137`: **LGTM!** The agent discovery logic is straightforward and correct. --- `139-152`: **LGTM!** Exception handling has been improved to catch specific exception types (`FileNotFoundError`, `PermissionError`, `yaml.YAMLError`) rather than a broad `Exception`, addressing previous review feedback. --- `154-165`: **LGTM!** Config saving uses appropriate exception handling and YAML best practices (sort_keys=False to preserve order). --- `167-191`: **LGTM!** The agent listing logic is clear and provides comprehensive information about each agent. --- `193-246`: **LGTM!** Version listing properly validates paths, handles missing directories gracefully, and displays comprehensive version metadata with clear current version indication. --- `247-284`: **LGTM!** Version retrieval includes thorough path validation at each level and specific exception handling for file operations. --- `286-348`: **LGTM on version switching logic!** The version switching implementation is robust with proper path validation, config updates, file deployment, and specific exception handling. --- `350-443`: **LGTM!** Version creation logic is comprehensive and handles all edge cases correctly: - Auto-generates sequential version names when not provided - Properly validates non-existent paths using `must_exist=False` - Checks version existence before validation to provide clear error messages - Updates all necessary metadata and config entries --- `446-511`: **LGTM on CLI structure!** The argparse-based CLI is well-organized with clear subcommands that map directly to VersionManager methods. The command routing logic is straightforward and correct. </blockquote></details> <details> <summary>tests/conftest.py (2)</summary><blockquote> `347-393`: **LGTM! Clean refactoring to use shared helper.** The `MockPromptLoader` class now correctly uses the `_load_prompts_from_directory` helper function, eliminating code duplication. The separation between filesystem-based test data and hardcoded edge case data is appropriate and well-documented. --- `112-112`: **Good use of yaml.safe_load for security.** Using `yaml.safe_load` instead of `yaml.load` is the correct choice for loading untrusted or test data, as it avoids executing arbitrary Python code embedded in YAML files. Based on learnings </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
prompts/SimpleChat/versions/v017.md (1)
1-1: LGTM! Consider enriching the prompt for production use.The prompt template is well-formed with proper placeholder syntax. However, it's notably minimal compared to typical production prompts. While acceptable for a versioned template, consider adding guidance on tone, limitations, or behavior boundaries in future iterations.
Optionally, verify consistency with other SimpleChat versions:
#!/bin/bash # Description: Compare v017 with other SimpleChat version files to ensure consistency # Check the structure and length of other version files echo "=== Checking other SimpleChat versions ===" fd -e md . prompts/SimpleChat/versions/ --exec wc -l {} \; echo -e "\n=== Sample of other version content ===" fd -e md . prompts/SimpleChat/versions/ -x head -n 3 {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)prompts/CodeReviewer/versions/v017.md(1 hunks)prompts/ComplexCodeReviewer/versions/v016.md(1 hunks)prompts/SimpleChat/versions/v017.md(1 hunks)prompts/TemplateDemo/versions/v016.md(1 hunks)prompts/simple_chat/versions/v015.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.9)
- GitHub Check: Tests (Python 3.10)
🔇 Additional comments (6)
prompts/CodeReviewer/versions/v017.md (1)
1-5: Template aligns with existing CodeReviewer pattern.Intro sentence and language-tagged fenced block match prior versions, so this slots cleanly into the versioned prompt sequence with no issues spotted.
prompts/TemplateDemo/versions/v016.md (1)
1-16: Template update looks solid.Branches cover all allowed difficulty values and the optional elements block matches the existing TemplateDemo pattern. Nicely aligned with earlier versions.
prompts/ComplexCodeReviewer/versions/v016.md (3)
1-1: LGTM!The instruction line is well-structured with clear placeholder positions and logical flow from context to review parameters.
3-5: LGTM!The code block structure correctly uses the
{{programming_language}}placeholder for dynamic syntax highlighting. This assumes the templating system will resolve placeholders before rendering.
7-7: LGTM!The output format specification clearly defines the four expected sections, maintaining consistency with the established ComplexCodeReviewer version pattern.
prompts/simple_chat/versions/v015.md (1)
3-11: LGTM: Well-structured prompt template.The role description, guidelines, and opening prompt are clear, professional, and follow good prompt engineering practices. The structure is logical and provides actionable guidance for the assistant's behavior.
Bug Fix Clean Up
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
README.md (4)
5-5: Optional: Fix heading level increment.The heading jumps from h1 (line 3) to h3, which violates markdown best practices. Consider using h2 instead.
-### Local-First Prompt Management for Production LLM Applications +## Local-First Prompt Management for Production LLM Applications
92-106: Optional: Add language identifier to fenced code block.For better syntax highlighting and clarity, specify the language for this code block (appears to be a directory tree structure).
-``` +```text prompts/ ├── CustomerSupport/
385-394: Optional: Add language identifier to fenced code block.Specify the language for this directory structure code block.
-``` +```text prompts/ └── CustomerSupport/
536-536: Optional: Use heading instead of emphasis.This appears to be a heading-level statement. Consider using a proper markdown heading instead of bold text.
-**Made with ❤️ by developers, for developers** +### Made with ❤️ by developers, for developerstests/test_helpers/precommit_helper.py (2)
131-206: Consider using atomic file creation for consistency.The production hook at
hooks/pre-commitusesatomic_create_version_file(lines 136-219) withO_CREAT|O_EXCLto prevent race conditions, but this test helper uses a simple file write (lines 160-168).While race conditions are unlikely in tests, using the same atomic creation logic would:
- Ensure tests exercise the actual production code path
- Catch bugs in the atomic implementation during testing
- Maintain consistency between test and production behavior
Consider either:
- Extracting
atomic_create_version_fileto a shared module and using it here- Adding a note explaining why the simpler approach is acceptable for tests
280-301: Nice improvement: robust path normalization.This
stage_filesimplementation has better path handling than the production hook athooks/pre-commit(lines 386-391). The explicit path resolution, relative path computation, and validation that files are within the repo are all excellent safety measures.This improved implementation could be ported back to the production hook in
hooks/pre-commit. Would you like me to open an issue to track that improvement?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)hooks/pre-commit(1 hunks)tests/test_helpers/precommit_helper.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_helpers/precommit_helper.py (2)
src/promptix/tools/hook_manager.py (2)
print_status(44-54)status(208-243)src/promptix/tools/version_manager.py (3)
print_status(34-44)load_config(139-152)save_config(154-165)
🪛 Flake8 (7.2.0)
tests/test_helpers/precommit_helper.py
[error] 9-9: 'sys' imported but unused
(F401)
[error] 10-10: 'shutil' imported but unused
(F401)
🪛 markdownlint-cli2 (0.18.1)
README.md
5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
385-385: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
536-536: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.13.1)
tests/test_helpers/precommit_helper.py
63-63: Starting a process with a partial executable path
(S607)
100-100: Do not catch blind exception: Exception
(BLE001)
109-109: Consider moving this statement to an else block
(TRY300)
110-110: Do not catch blind exception: Exception
(BLE001)
177-177: Do not catch blind exception: Exception
(BLE001)
202-202: Do not catch blind exception: Exception
(BLE001)
260-260: Consider moving this statement to an else block
(TRY300)
262-262: Do not catch blind exception: Exception
(BLE001)
270-270: Starting a process with a partial executable path
(S607)
299-299: subprocess call: check for execution of untrusted input
(S603)
299-299: Consider ['git', 'add', '--', *normalized] instead of concatenation
Replace with ['git', 'add', '--', *normalized]
(RUF005)
340-340: Do not catch blind exception: Exception
(BLE001)
348-348: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.10)
🔇 Additional comments (7)
hooks/pre-commit (6)
1-15: LGTM! Clear module documentation.The docstring effectively describes the hook's features and user flow, making it easy for maintainers to understand the module's purpose.
30-53: LGTM! Good Windows compatibility handling.The graceful fallback for Windows encoding issues is well-implemented and prevents the hook from failing on systems with limited encoding support.
136-219: Excellent atomic file creation implementation.The race condition fix using
O_CREAT|O_EXCLwith exponential backoff is correct and robust. The implementation properly handles:
- Atomic file creation
- Collision detection and retry
- File cleanup on write failure
- Clear error reporting
246-272: Verify atomic_create_version_file call doesn't include header in content.The comment at line 251 says "We'll add the header in the atomic creation function", but
atomic_create_version_filedoesn't add headers—it just writes the content parameter as-is. The header is actually added afterward at lines 264-271.This works correctly, but the comment is misleading since you're not passing the header to the atomic function.
Consider updating the comment for clarity:
- # Prepare version content with header (will be added with actual version name) - # We'll add the header in the atomic creation function + # Prepare version content (header will be added after atomic creation)
290-292: Good design decision: explicit version switching only.The comment correctly highlights that
current_versionshould not be auto-set during version creation—only during explicit version switches. This prevents unintended version changes.
442-443: LGTM! Non-blocking exit ensures commit workflow integrity.Always exiting with 0 ensures the hook never blocks commits, which aligns with the safe automation design mentioned in the docstring.
tests/test_helpers/precommit_helper.py (1)
95-112: Broad exception catching is acceptable for test infrastructure.Static analysis flags the broad
Exceptioncatches, but for a test helper that needs to gracefully handle various error conditions and report them viaprint_status, this is reasonable.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (9)
prompts/TemplateDemo/versions/v004.md (1)
11-16: Guard against undefinedelements.If the caller omits
elements,elements|lengthwill raise anUndefinedError, breaking the render. Add a defined check (or default) before calling|length.-{% if elements|length > 0 %} +{% if elements is defined and elements|length > 0 %}prompts/CodeReviewer/versions/v004.md (1)
1-5: Restore the missing placeholders and structured guidanceThis version is still the barebones template and remains identical to earlier versions—there’s no
{{active_tools}},{{severity}}, or section guidance despite the version summary promising them. That duplication keeps the agent from emitting the richer, severity-aware review we need. Please rework v004 so it’s distinct and includes the promised placeholders/sections before merging.check_duplicate_versions.py (1)
93-101: Make prompts_dir resolution location‑agnostic.This still binds
prompts_dirto the script’s directory. When the script lives underscripts/, it resolves toscripts/prompts, so the lookup fails—same issue noted previously. Walk upward until apromptsfolder is found (or allow an override) so the tool works from its actual install location.- repo_root = Path(__file__).parent - prompts_dir = repo_root / "prompts" + repo_root = Path(__file__).resolve().parent + while repo_root != repo_root.parent and not (repo_root / "prompts").exists(): + repo_root = repo_root.parent + prompts_dir = repo_root / "prompts"tests/unit/test_enhanced_prompt_loader.py (1)
22-52: Code duplication: Windows helper functions.The
remove_readonlyandsafe_rmtreefunctions are duplicated across multiple test files. See the earlier comment intest_hook_manager.pyfor a recommendation to extract these to a shared test utilities module.tests/unit/test_precommit_hook.py (1)
28-58: Code duplication: Windows helper functions.The
remove_readonlyandsafe_rmtreefunctions are duplicated across multiple test files. See the earlier comment intest_hook_manager.pyfor a recommendation to extract these to a shared test utilities module.src/promptix/tools/version_manager.py (4)
1-1: Shebang present — ensure file is executable.#!/bin/bash # Make executable if needed test -x src/promptix/tools/version_manager.py || chmod +x src/promptix/tools/version_manager.py
341-341: Remove f-prefix from literal string.- self.print_status(f"Updated current.md and config.yaml", "info") + self.print_status("Updated current.md and config.yaml", "info")
460-460: Drop unused parser variable.- list_cmd = subparsers.add_parser('list', help='List all agents') + subparsers.add_parser('list', help='List all agents')
104-104: Drop unused exception variable.Avoid capturing
eyou don’t use.- except ValueError as e: + except ValueError: # Check if it's a different drive issue (Windows)
🧹 Nitpick comments (24)
tests/unit/test_hook_manager.py (4)
10-10: Remove unused import.The
subprocessmodule is imported but never used in this test file.import pytest import tempfile import shutil -import subprocess import io import sys
16-16: Remove unused imports.
MagicMockandcallare imported but never used in this test file.-from unittest.mock import patch, MagicMock, call +from unittest.mock import patch
21-52: Consider extracting Windows helpers to shared test utilities.The
remove_readonlyandsafe_rmtreefunctions appear duplicated across multiple test files (test_hook_manager.py,test_enhanced_prompt_loader.py,test_precommit_hook.py).Consider extracting these to a shared test utilities module (e.g.,
tests/test_helpers/filesystem_utils.py) to reduce duplication and ensure consistent behavior across all tests.# tests/test_helpers/filesystem_utils.py import os import sys import stat import shutil def remove_readonly(func, path, excinfo): """Error handler for Windows read-only file removal.""" os.chmod(path, stat.S_IWRITE) func(path) def safe_rmtree(path): """Safely remove a directory tree, handling Windows permission issues.""" try: if sys.platform == 'win32': shutil.rmtree(path, onerror=remove_readonly) else: shutil.rmtree(path) except Exception: try: shutil.rmtree(path, ignore_errors=True) except: passThen import in test files:
from test_helpers.filesystem_utils import safe_rmtree
491-508: Enhance test to verify actual permission error behavior.The test mocks
shutil.copy2to raisePermissionError, but the actual installation process involves multiple file operations. Consider testing the full error path or clarifying that this tests only the copy operation failure.Alternatively, document that this test specifically validates the error message when the copy operation fails, which is acceptable for a unit test.
tests/unit/test_enhanced_prompt_loader.py (2)
16-16: Remove unused import.
MagicMockis imported but never used in this test file.-from unittest.mock import patch, MagicMock +from unittest.mock import patch
438-453: Consider testing with actual PermissionError instead of broad patching.The test patches
builtins.openglobally, which may mask other file operations. Consider using a more targeted mock or creating an actual read-only file to test this scenario more realistically.# Alternative approach: Make the version file read-only version_file.touch() version_file.chmod(0o000) # Remove all permissions try: loader = PromptLoader() prompts = loader.load_prompts() # ... assertions finally: version_file.chmod(0o644) # Restore for cleanuptests/unit/test_precommit_hook.py (2)
15-15: Remove unused imports.
MagicMockandcallare imported but never used in this test file.-from unittest.mock import patch, MagicMock, call +from unittest.mock import patch
292-295: Prefer subprocess.run over os.system for git initialization.Using
os.systemis flagged by security linters and doesn't provide error handling. Prefersubprocess.runfor better control and security.- os.chdir(temp_dir) - os.system("git init") - os.system("git config user.name 'Test User'") - os.system("git config user.email '[email protected]'") + os.chdir(temp_dir) + subprocess.run(["git", "init"], check=True, capture_output=True) + subprocess.run(["git", "config", "user.name", "Test User"], check=True, capture_output=True) + subprocess.run(["git", "config", "user.email", "[email protected]"], check=True, capture_output=True)Note: You'll need to ensure
subprocessis imported at the top of the file (it's currently imported but flagged as unused, so this would resolve that issue as well).src/promptix/tools/version_manager.py (4)
126-129: Avoid blanketexcept Exceptionin path validation.Catching all exceptions here can hide programming errors; prefer specific exceptions and let unknowns surface.
- except Exception as e: - # Log unexpected errors before returning False - self.print_status(f"Unexpected path validation error for {path_type}: {e}", "error") - return False + except (OSError, RuntimeError, ValueError) as e: + self.print_status(f"Path validation error for {path_type}: {e}", "error") + return False
139-166: Usesafe_dumpand explicit UTF‑8 encoding for YAML I/O.Safer and consistent with
safe_load; avoids environment‑dependent encodings. Based on learnings.- with open(config_path, 'r') as f: + with open(config_path, 'r', encoding='utf-8') as f: return yaml.safe_load(f) ... - with open(config_path, 'w') as f: - yaml.dump(config, f, default_flow_style=False, sort_keys=False) + with open(config_path, 'w', encoding='utf-8') as f: + yaml.safe_dump(config, f, default_flow_style=False, sort_keys=False)
267-285: Specify UTF‑8 when reading/writing prompt files.Prevents cross‑platform surprises (Windows defaults). Tests already use UTF‑8.
- with open(version_file, 'r') as f: + with open(version_file, 'r', encoding='utf-8') as f: content = f.read() ... - with open(current_md, 'r') as f: + with open(current_md, 'r', encoding='utf-8') as f: content = f.read() ... - with open(current_md, 'w') as f: + with open(current_md, 'w', encoding='utf-8') as f: f.write(content) ... - with open(version_file, 'r') as f: + with open(version_file, 'r', encoding='utf-8') as f: content = f.read() - with open(version_file, 'w') as f: + with open(version_file, 'w', encoding='utf-8') as f: f.write(version_header) f.write(content)Also applies to: 333-339, 408-415
411-424: Use a cross‑platform way to get the username.
$USERisn’t set on Windows; prefergetpass.getuser()with a fallback.+import getpass ... - 'author': os.getenv('USER', 'unknown'), + 'author': (getpass.getuser() if getpass.getuser() else os.getenv('USER', 'unknown')) or 'unknown',src/promptix/tools/cli.py (6)
14-21: Remove unused imports.-from pathlib import Path -from typing import Optional +from typing import Optional ... -from rich.text import Text ... -from rich import print as rich_print
93-101: Unusedtaskvariable in progress block.Prefix with
_to satisfy linters.- task = progress.add_task("Searching for available port...", total=None) + _task = progress.add_task("Searching for available port...", total=None)
340-348: Literal strings in f-strings.Drop the
fprefix on static lines to fix F541 without changing output.- f"[bold green]✅ Promptix pre-commit hook installed![/bold green]\n\n" - f"[blue]What happens now:[/blue]\n" - f"• Every time you edit current.md and commit, a new version is created\n" - f"• When you change current_version in config.yaml, that version is deployed\n" - f"• Use 'SKIP_PROMPTIX_HOOK=1 git commit' to bypass when needed\n\n" - f"[blue]Try it:[/blue]\n" - f"• Edit any prompts/*/current.md file\n" - f"• Run: git add . && git commit -m 'Test versioning'\n" - f"• Check the new version in prompts/*/versions/", + "[bold green]✅ Promptix pre-commit hook installed![/bold green]\n\n" + "[blue]What happens now:[/blue]\n" + "• Every time you edit current.md and commit, a new version is created\n" + "• When you change current_version in config.yaml, that version is deployed\n" + "• Use 'SKIP_PROMPTIX_HOOK=1 git commit' to bypass when needed\n\n" + "[blue]Try it:[/blue]\n" + "• Edit any prompts/*/current.md file\n" + "• Run: git add . && git commit -m 'Test versioning'\n" + "• Check the new version in prompts/*/versions/",
133-134: Prefer explicit conversion flags in f-strings.Replace
str(e)with{e!s}.- error_console.print(f"[bold red]❌ Error launching Promptix Studio:[/bold red] {str(e)}") + error_console.print(f"[bold red]❌ Error launching Promptix Studio:[/bold red] {e!s}") ... - error_console.print(f"[bold red]❌ Error:[/bold red] {str(e)}") + error_console.print(f"[bold red]❌ Error:[/bold red] {e!s}") ... - error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {str(e)}") + error_console.print(f"[bold red]❌ Unexpected error:[/bold red] {e!s}")Also applies to: 450-451, 478-479
445-448: Small style: use star expansion when reconstructing argv.- original_args = ['openai'] + ctx.args + original_args = ['openai', *ctx.args] sys.argv = original_args
179-185: Narrow broad exception handlers in src/promptix/tools/cli.pyReplace each generic
except Exception as e: …with catches for the specific errors you expect (for example,
ValueError,OSError,subprocess.CalledProcessError), and allow other exceptions to bubble up.
Occurrences at lines: 182, 209, 227, 244, 263, 296, 318, 356, 372, 385, 398, 411, 424, 449, 477.tests/integration/test_versioning_integration.py (3)
54-60: Tighten exception handling in test cleanup.Avoid bare
except; either log the error or useignore_errors=True.- except Exception as e: + except Exception: # If all else fails, try one more time with ignore_errors - try: - shutil.rmtree(path, ignore_errors=True) - except: - # Last resort: just pass and let the OS clean up temp files - pass + shutil.rmtree(path, ignore_errors=True)
171-195: Drop unusedpromptsassignments.Call
load_prompts()without capturing the result to avoid F841.- prompts = loader.load_prompts(force_reload=True) + loader.load_prompts(force_reload=True) ... - prompts = loader.load_prompts() + loader.load_prompts()Also applies to: 301-317
107-114: Use explicit UTF‑8 encoding in file I/O within tests.Improves cross‑platform determinism.
- with open(agent_dir / "config.yaml", "w") as f: + with open(agent_dir / "config.yaml", "w", encoding="utf-8") as f: yaml.dump(config_content, f, default_flow_style=False) ... - with open(current_md, "w") as f: + with open(current_md, "w", encoding="utf-8") as f: f.write("You are a helpful assistant. Help {{user_name}} with {{task_type}} tasks efficiently.") ... - with open(git_workspace / "prompts" / "test_agent" / "config.yaml", "r") as f: + with open(git_workspace / "prompts" / "test_agent" / "config.yaml", "r", encoding="utf-8") as f: config = yaml.safe_load(f) ... - with open(config_path, "r") as f: + with open(config_path, "r", encoding="utf-8") as f: config = yaml.safe_load(f) ... - with open(config_path, "w") as f: + with open(config_path, "w", encoding="utf-8") as f: yaml.dump(config, f, default_flow_style=False)Also applies to: 151-154, 172-175, 283-286, 302-305
tests/functional/test_versioning_edge_cases.py (3)
13-16: Remove unused imports.-import stat -from unittest.mock import patch, MagicMock +from unittest.mock import patch
37-40: Use robust cleanup on Windows.Prefer
ignore_errors=Trueor thesafe_rmtreehelper used elsewhere to avoid failures with read‑only git objects.- shutil.rmtree(temp_dir) + shutil.rmtree(temp_dir, ignore_errors=True)Also applies to: 312-314
418-422: Avoid assigning unusedversion_namevariables.Prefix with
_to satisfy linters.- version_name = tester.create_version_snapshot("prompts/encoding_agent/current.md") + _ = tester.create_version_snapshot("prompts/encoding_agent/current.md") ... - version_name = tester.create_version_snapshot("prompts/symlink_agent/current.md") + _ = tester.create_version_snapshot("prompts/symlink_agent/current.md")Also applies to: 479-483
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
check_duplicate_versions.py(1 hunks)prompts/CodeReviewer/versions/v004.md(1 hunks)prompts/CodeReviewer/versions/v005.md(1 hunks)prompts/CodeReviewer/versions/v006.md(1 hunks)prompts/ComplexCodeReviewer/versions/v003.md(1 hunks)prompts/ComplexCodeReviewer/versions/v004.md(1 hunks)prompts/ComplexCodeReviewer/versions/v005.md(1 hunks)prompts/SimpleChat/versions/v004.md(1 hunks)prompts/SimpleChat/versions/v005.md(1 hunks)prompts/SimpleChat/versions/v006.md(1 hunks)prompts/TemplateDemo/versions/v003.md(1 hunks)prompts/TemplateDemo/versions/v004.md(1 hunks)prompts/TemplateDemo/versions/v005.md(1 hunks)prompts/simple_chat/versions/v003.md(1 hunks)prompts/simple_chat/versions/v004.md(1 hunks)prompts/simple_chat/versions/v005.md(1 hunks)src/promptix/tools/cli.py(1 hunks)src/promptix/tools/hook_manager.py(1 hunks)src/promptix/tools/version_manager.py(1 hunks)tests/conftest.py(6 hunks)tests/functional/test_versioning_edge_cases.py(1 hunks)tests/integration/test_versioning_integration.py(1 hunks)tests/unit/test_enhanced_prompt_loader.py(1 hunks)tests/unit/test_hook_manager.py(1 hunks)tests/unit/test_precommit_hook.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- prompts/SimpleChat/versions/v005.md
- prompts/SimpleChat/versions/v006.md
- prompts/CodeReviewer/versions/v006.md
🧰 Additional context used
🧬 Code graph analysis (10)
tests/unit/test_enhanced_prompt_loader.py (2)
src/promptix/core/components/prompt_loader.py (2)
PromptLoader(15-389)load_prompts(28-65)src/promptix/core/exceptions.py (1)
StorageError(182-184)
tests/integration/test_versioning_integration.py (6)
src/promptix/core/base.py (2)
Promptix(19-257)builder(195-206)src/promptix/core/components/prompt_loader.py (1)
load_prompts(28-65)src/promptix/tools/version_manager.py (2)
switch_version(286-348)main(446-507)src/promptix/tools/hook_manager.py (3)
HookManager(18-290)install_hook(107-146)main(293-351)tests/test_helpers/precommit_helper.py (4)
PreCommitHookTester(19-355)create_version_snapshot(131-206)handle_version_switch(208-264)main_hook_logic(303-355)tests/conftest.py (3)
remove_readonly(28-36)safe_rmtree(39-58)load_prompts(388-395)
check_duplicate_versions.py (1)
src/promptix/tools/version_manager.py (1)
main(446-507)
tests/functional/test_versioning_edge_cases.py (3)
tests/test_helpers/precommit_helper.py (3)
PreCommitHookTester(19-355)create_version_snapshot(131-206)handle_version_switch(208-264)tests/unit/test_precommit_hook.py (1)
test_empty_current_md_file(461-481)tests/unit/test_enhanced_prompt_loader.py (1)
mock_open(439-442)
src/promptix/tools/hook_manager.py (3)
src/promptix/tools/version_manager.py (2)
print_status(34-44)main(446-507)tests/test_helpers/precommit_helper.py (1)
print_status(31-53)src/promptix/tools/cli.py (3)
status(403-413)get(251-265)main(453-479)
tests/unit/test_hook_manager.py (1)
src/promptix/tools/hook_manager.py (11)
HookManager(18-290)is_git_repo(56-58)has_existing_hook(60-62)is_promptix_hook(64-74)backup_existing_hook(76-91)install_hook(107-146)uninstall_hook(148-168)disable_hook(170-187)enable_hook(189-206)status(208-243)test_hook(245-290)
tests/unit/test_precommit_hook.py (1)
tests/test_helpers/precommit_helper.py (6)
PreCommitHookTester(19-355)find_promptix_changes(73-93)get_next_version_number(114-129)create_version_snapshot(131-206)handle_version_switch(208-264)is_hook_bypassed(55-57)
src/promptix/tools/cli.py (4)
src/promptix/tools/hook_manager.py (8)
HookManager(18-290)has_existing_hook(60-62)install_hook(107-146)uninstall_hook(148-168)enable_hook(189-206)disable_hook(170-187)status(208-243)test_hook(245-290)src/promptix/tools/version_manager.py (6)
VersionManager(23-443)list_agents(167-191)list_versions(193-246)get_version(247-284)switch_version(286-348)create_version(350-443)src/promptix/core/config.py (2)
Config(190-196)get(107-125)src/promptix/core/workspace_manager.py (3)
WorkspaceManager(12-273)create_agent(25-68)list_agents(247-261)
src/promptix/tools/version_manager.py (4)
src/promptix/tools/hook_manager.py (3)
print_status(44-54)status(208-243)main(293-351)tests/test_helpers/precommit_helper.py (3)
print_status(31-53)load_config(95-102)save_config(104-112)src/promptix/tools/cli.py (5)
status(403-413)get(251-265)main(453-479)agent(140-142)version(214-216)src/promptix/core/components/prompt_loader.py (1)
list_agents(95-108)
tests/conftest.py (4)
tests/integration/test_versioning_integration.py (2)
remove_readonly(30-38)safe_rmtree(41-60)src/promptix/core/config.py (2)
get(107-125)get_prompt_file_path(137-155)src/promptix/core/components/prompt_loader.py (1)
load_prompts(28-65)src/promptix/tools/studio/folder_manager.py (1)
load_prompts(232-253)
🪛 Flake8 (7.2.0)
tests/unit/test_enhanced_prompt_loader.py
[error] 16-16: 'unittest.mock.MagicMock' imported but unused
(F401)
[error] 19-19: 'promptix.core.exceptions.StorageError' imported but unused
(F401)
[error] 50-50: do not use bare 'except'
(E722)
tests/integration/test_versioning_integration.py
[error] 54-54: local variable 'e' is assigned to but never used
(F841)
[error] 58-58: do not use bare 'except'
(E722)
[error] 140-140: continuation line under-indented for visual indent
(E128)
[error] 162-162: continuation line under-indented for visual indent
(E128)
[error] 245-245: local variable 'prompts' is assigned to but never used
(F841)
[error] 259-259: continuation line under-indented for visual indent
(E128)
[error] 280-280: local variable 'version2' is assigned to but never used
(F841)
[error] 313-313: local variable 'prompts' is assigned to but never used
(F841)
[error] 419-419: local variable 'prompts' is assigned to but never used
(F841)
tests/functional/test_versioning_edge_cases.py
[error] 13-13: 'stat' imported but unused
(F401)
[error] 15-15: 'unittest.mock.MagicMock' imported but unused
(F401)
[error] 418-418: local variable 'version_name' is assigned to but never used
(F841)
[error] 479-479: local variable 'version_name' is assigned to but never used
(F841)
tests/unit/test_hook_manager.py
[error] 10-10: 'subprocess' imported but unused
(F401)
[error] 16-16: 'unittest.mock.MagicMock' imported but unused
(F401)
[error] 16-16: 'unittest.mock.call' imported but unused
(F401)
[error] 49-49: do not use bare 'except'
(E722)
tests/unit/test_precommit_hook.py
[error] 15-15: 'unittest.mock.MagicMock' imported but unused
(F401)
[error] 15-15: 'unittest.mock.call' imported but unused
(F401)
[error] 56-56: do not use bare 'except'
(E722)
src/promptix/tools/cli.py
[error] 11-11: 'pathlib.Path' imported but unused
(F401)
[error] 16-16: 'rich.text.Text' imported but unused
(F401)
[error] 20-20: 'rich.print as rich_print' imported but unused
(F401)
[error] 99-99: local variable 'task' is assigned to but never used
(F841)
[error] 219-219: redefinition of unused 'list' from line 187
(F811)
[error] 304-304: redefinition of unused 'create' from line 146
(F811)
[error] 340-340: f-string is missing placeholders
(F541)
src/promptix/tools/version_manager.py
[error] 104-104: local variable 'e' is assigned to but never used
(F841)
[error] 341-341: f-string is missing placeholders
(F541)
[error] 460-460: local variable 'list_cmd' is assigned to but never used
(F841)
tests/conftest.py
[error] 16-16: 'typing.Optional' imported but unused
(F401)
[error] 56-56: do not use bare 'except'
(E722)
🪛 Ruff (0.13.1)
tests/unit/test_enhanced_prompt_loader.py
22-22: Unused function argument: excinfo
(ARG001)
46-46: Do not catch blind exception: Exception
(BLE001)
50-50: Do not use bare except
(E722)
50-52: try-except-pass detected, consider logging the exception
(S110)
441-441: Avoid specifying long messages outside the exception class
(TRY003)
tests/integration/test_versioning_integration.py
30-30: Unused function argument: excinfo
(ARG001)
54-54: Do not catch blind exception: Exception
(BLE001)
54-54: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
58-58: Do not use bare except
(E722)
58-60: try-except-pass detected, consider logging the exception
(S110)
74-74: Starting a process with a partial executable path
(S607)
75-75: Starting a process with a partial executable path
(S607)
76-76: Starting a process with a partial executable path
(S607)
125-125: os.chmod setting a permissive mask 0o755 on file or directory
(S103)
138-138: Starting a process with a partial executable path
(S607)
139-139: Starting a process with a partial executable path
(S607)
155-155: Starting a process with a partial executable path
(S607)
161-161: Starting a process with a partial executable path
(S607)
245-245: Local variable prompts is assigned to but never used
Remove assignment to unused variable prompts
(F841)
280-280: Local variable version2 is assigned to but never used
Remove assignment to unused variable version2
(F841)
313-313: Local variable prompts is assigned to but never used
Remove assignment to unused variable prompts
(F841)
359-359: Unpacked variable messages is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
419-419: Local variable prompts is assigned to but never used
Remove assignment to unused variable prompts
(F841)
421-423: try-except-pass detected, consider logging the exception
(S110)
421-421: Do not catch blind exception: Exception
(BLE001)
check_duplicate_versions.py
20-20: Probable use of insecure hash functions in hashlib: md5
(S324)
tests/functional/test_versioning_edge_cases.py
337-337: Avoid specifying long messages outside the exception class
(TRY003)
369-369: Avoid specifying long messages outside the exception class
(TRY003)
418-418: Local variable version_name is assigned to but never used
Remove assignment to unused variable version_name
(F841)
479-479: Local variable version_name is assigned to but never used
Remove assignment to unused variable version_name
(F841)
src/promptix/tools/hook_manager.py
32-32: Avoid specifying long messages outside the exception class
(TRY003)
43-43: Avoid specifying long messages outside the exception class
(TRY003)
72-72: Consider moving this statement to an else block
(TRY300)
73-73: Do not catch blind exception: Exception
(BLE001)
88-88: Consider moving this statement to an else block
(TRY300)
89-89: Do not catch blind exception: Exception
(BLE001)
102-102: Consider moving this statement to an else block
(TRY300)
103-103: Do not catch blind exception: Exception
(BLE001)
140-140: os.chmod setting a permissive mask 0o755 on file or directory
(S103)
145-145: Do not catch blind exception: Exception
(BLE001)
167-167: Do not catch blind exception: Exception
(BLE001)
186-186: Do not catch blind exception: Exception
(BLE001)
205-205: Do not catch blind exception: Exception
(BLE001)
275-275: subprocess call: check for execution of untrusted input
(S603)
289-289: Do not catch blind exception: Exception
(BLE001)
349-349: Do not catch blind exception: Exception
(BLE001)
tests/unit/test_hook_manager.py
21-21: Unused function argument: excinfo
(ARG001)
45-45: Do not catch blind exception: Exception
(BLE001)
49-49: Do not use bare except
(E722)
49-51: try-except-pass detected, consider logging the exception
(S110)
tests/unit/test_precommit_hook.py
28-28: Unused function argument: excinfo
(ARG001)
52-52: Do not catch blind exception: Exception
(BLE001)
56-56: Do not use bare except
(E722)
56-58: try-except-pass detected, consider logging the exception
(S110)
293-293: Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without shell
(S605)
293-293: Starting a process with a partial executable path
(S607)
294-294: Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without shell
(S605)
294-294: Starting a process with a partial executable path
(S607)
295-295: Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without shell
(S605)
295-295: Starting a process with a partial executable path
(S607)
src/promptix/tools/cli.py
99-99: Local variable task is assigned to but never used
Remove assignment to unused variable task
(F841)
122-122: subprocess call: check for execution of untrusted input
(S603)
133-133: Use explicit conversion flag
Replace with conversion flag
(RUF010)
182-182: Do not catch blind exception: Exception
(BLE001)
209-209: Do not catch blind exception: Exception
(BLE001)
219-219: Redefinition of unused list from line 187
(F811)
227-227: Do not catch blind exception: Exception
(BLE001)
244-244: Do not catch blind exception: Exception
(BLE001)
263-263: Do not catch blind exception: Exception
(BLE001)
296-296: Do not catch blind exception: Exception
(BLE001)
304-304: Redefinition of unused create from line 146
(F811)
305-305: Docstring contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF002)
310-310: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
318-318: Do not catch blind exception: Exception
(BLE001)
340-340: f-string without any placeholders
Remove extraneous f prefix
(F541)
341-341: f-string without any placeholders
Remove extraneous f prefix
(F541)
342-342: f-string without any placeholders
Remove extraneous f prefix
(F541)
343-343: f-string without any placeholders
Remove extraneous f prefix
(F541)
344-344: f-string without any placeholders
Remove extraneous f prefix
(F541)
345-345: f-string without any placeholders
Remove extraneous f prefix
(F541)
346-346: f-string without any placeholders
Remove extraneous f prefix
(F541)
347-347: f-string without any placeholders
Remove extraneous f prefix
(F541)
348-348: f-string without any placeholders
Remove extraneous f prefix
(F541)
356-356: Do not catch blind exception: Exception
(BLE001)
372-372: Do not catch blind exception: Exception
(BLE001)
385-385: Do not catch blind exception: Exception
(BLE001)
398-398: Do not catch blind exception: Exception
(BLE001)
411-411: Do not catch blind exception: Exception
(BLE001)
424-424: Do not catch blind exception: Exception
(BLE001)
445-445: Consider ['openai', *ctx.args] instead of concatenation
Replace with ['openai', *ctx.args]
(RUF005)
449-449: Do not catch blind exception: Exception
(BLE001)
450-450: Use explicit conversion flag
Replace with conversion flag
(RUF010)
477-477: Do not catch blind exception: Exception
(BLE001)
478-478: Use explicit conversion flag
Replace with conversion flag
(RUF010)
src/promptix/tools/version_manager.py
32-32: Avoid specifying long messages outside the exception class
(TRY003)
103-103: Consider moving this statement to an else block
(TRY300)
104-104: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
126-126: Do not catch blind exception: Exception
(BLE001)
159-159: Consider moving this statement to an else block
(TRY300)
341-341: f-string without any placeholders
Remove extraneous f prefix
(F541)
460-460: Local variable list_cmd is assigned to but never used
Remove assignment to unused variable list_cmd
(F841)
505-505: Do not catch blind exception: Exception
(BLE001)
tests/conftest.py
28-28: Unused function argument: excinfo
(ARG001)
52-52: Do not catch blind exception: Exception
(BLE001)
56-56: Do not use bare except
(E722)
56-58: try-except-pass detected, consider logging the exception
(S110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.12)
- GitHub Check: Tests (Python 3.10)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.11)
- GitHub Check: Tests (Python 3.9)
- GitHub Check: Tests (Python 3.10)
🔇 Additional comments (7)
prompts/SimpleChat/versions/v004.md (1)
1-1: LGTM!The prompt template is well-structured with clear placeholder syntax ({{assistant_name}} and {{user_name}}) and defines a straightforward assistant persona. The content is grammatically correct and consistent with the SimpleChat versioning pattern.
prompts/TemplateDemo/versions/v005.md (2)
1-1: LGTM!The opening line clearly establishes the prompt context using appropriate Jinja2 variable syntax.
11-16: LGTM!The conditional elements section correctly checks for non-empty collections before iterating and uses proper Jinja2 syntax for the loop.
prompts/simple_chat/versions/v004.md (1)
1-11: LGTM with caveat on duplication.The prompt template structure is appropriate. However, note the potential duplication with
v003.mdflagged in the previous file review.tests/unit/test_enhanced_prompt_loader.py (1)
170-191: LGTM: Excellent test coverage for current_version tracking.This test properly validates that the
current_versionfield inconfig.yamlcorrectly controls which version is marked as live. The assertions check both the version structure and the live version selection.tests/unit/test_precommit_hook.py (2)
326-362: LGTM: Excellent multi-agent test coverage.This test properly validates that the pre-commit hook can handle multiple agents being modified in the same commit, which is an important real-world scenario.
406-419: LGTM: Proper graceful failure handling.The test correctly verifies that missing config files don't crash the hook and that
Noneis returned to indicate failure. This is appropriate error handling for a pre-commit hook that should never block commits.
| import hashlib | ||
| from pathlib import Path | ||
| from collections import defaultdict | ||
| from typing import Dict, List | ||
|
|
||
|
|
||
| def get_file_hash(file_path: Path) -> str: | ||
| """Calculate MD5 hash of a file. | ||
| Returns: | ||
| Hash string on success, None on failure (logs error) | ||
| """ | ||
| try: | ||
| md5_hash = hashlib.md5() | ||
| with open(file_path, 'rb') as f: | ||
| for chunk in iter(lambda: f.read(4096), b''): | ||
| md5_hash.update(chunk) | ||
| return md5_hash.hexdigest() | ||
| except (OSError, IOError) as e: | ||
| print(f"⚠️ Error reading file {file_path}: {e}") | ||
| return None |
There was a problem hiding this comment.
Correct the return type annotation.
get_file_hash can return None, but the signature still advertises -> str, which will trip static typing and mislead callers. Update the annotation (and imports) to reflect the optional return.
-from typing import Dict, List
+from typing import Dict, List, Optional
@@
-def get_file_hash(file_path: Path) -> str:
+def get_file_hash(file_path: Path) -> Optional[str]:🧰 Tools
🪛 Ruff (0.13.1)
20-20: Probable use of insecure hash functions in hashlib: md5
(S324)
🤖 Prompt for AI Agents
In check_duplicate_versions.py around lines 7 to 27, the function signature
declares get_file_hash as returning str but the function returns None on
failure; update the typing to Optional[str] by importing Optional from typing
and change the signature to -> Optional[str], and adjust the docstring return
description to state it returns Optional[str] (hash string or None) so static
type checkers and callers reflect the possible None return.
| You are a code review assistant specialized in {{programming_language}}. Please review the following code snippet and provide feedback on {{review_focus}}: | ||
|
|
||
| ```{{programming_language}} | ||
| {{code_snippet}} | ||
| ``` |
There was a problem hiding this comment.
Bring this template up to the documented spec
Right now v005 is the same minimal snippet as v004, so the agent never learns about {{active_tools}}, {{severity}}, or the required Summary/Critical/Improvements/Positives sections. That’s a regression for workflows expecting the richer format. Please expand the template accordingly. One option:
-You are a code review assistant specialized in {{programming_language}}. Please review the following code snippet and provide feedback on {{review_focus}}:
-
-```{{programming_language}}
-{{code_snippet}}
-```
+You are a code review assistant specialized in {{programming_language}}. Available tools: {{active_tools}}.
+Your objective is to review the following code while focusing on {{review_focus}}. Calibrate the tone and depth of feedback according to {{severity}}.
+
+```{{programming_language}}
+{{code_snippet}}
+```
+
+Structure your response with the headings below, even if a section is empty:
+1. Summary – concise recap of the overall findings.
+2. Critical Issues – blocking bugs or violations; cite lines and classify with {{severity}} terminology.
+3. Improvements – actionable but non-blocking suggestions.
+4. Positives – highlight what’s working well.
+
+Keep comments specific, reference exact lines, and suggest concrete fixes when flagging issues.📝 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.
| You are a code review assistant specialized in {{programming_language}}. Please review the following code snippet and provide feedback on {{review_focus}}: | |
| ```{{programming_language}} | |
| {{code_snippet}} | |
| ``` | |
| You are a code review assistant specialized in {{programming_language}}. Available tools: {{active_tools}}. | |
| Your objective is to review the following code while focusing on {{review_focus}}. Calibrate the tone and depth of feedback according to {{severity}}. | |
🤖 Prompt for AI Agents
In prompts/CodeReviewer/versions/v005.md around lines 1 to 5, the template was
truncated to the minimal v004 form and is missing the required variables and
structured sections ({{active_tools}}, {{severity}} and the
Summary/Critical/Improvements/Positives headings); update the prompt to
reintroduce an "Available tools: {{active_tools}}" line, an objective sentence
that mentions {{review_focus}} and calibrates by {{severity}}, wrap
{{code_snippet}} in the language-specific code block, and append explicit
required sections: 1. Summary, 2. Critical Issues (cite lines and classify with
{{severity}} terminology), 3. Improvements, and 4. Positives, plus a final
sentence requiring comments to reference exact lines and suggest concrete fixes.
| You are a {{personality}} assistant specialized in {{domain}}. | ||
|
|
||
| Your role is to provide helpful, accurate, and engaging responses to user questions and requests. Always maintain a professional and friendly tone while adapting to the user's needs. | ||
|
|
||
| Key guidelines: | ||
| - Be concise but thorough in your explanations | ||
| - Ask clarifying questions when needed | ||
| - Provide examples when helpful | ||
| - Stay focused on the {{domain}} domain when specified | ||
|
|
||
| How can I help you today? No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
❓ Verification inconclusive
Verify: v003.md and v004.md appear to have identical content.
Based on the AI summaries and annotated code, both v003.md and v004.md contain the exact same prompt template. This could cause confusion in the version history, as users would expect different versions to have distinct content.
Please verify whether this duplication is intentional. If not, consider updating one of the versions to differentiate them, or removing the duplicate.
🏁 Script executed:
#!/bin/bash
# Compare v003.md and v004.md to confirm they are identical
diff prompts/simple_chat/versions/v003.md prompts/simple_chat/versions/v004.mdLength of output: 0
Differentiate or remove duplicate prompt versions
The files prompts/simple_chat/versions/v003.md and v004.md are identical; update one to reflect its intended changes or remove the redundant version.
🤖 Prompt for AI Agents
In prompts/simple_chat/versions/v003.md around lines 1 to 11 the file is
identical to v004.md; either update v003.md to contain the intended changes for
that version or remove the redundant file and ensure only the correct version
remains. If updating, edit the content to reflect the intended diffs (change
summary, behavior or examples), increment the version string or metadata if
applicable, and run a quick diff against v004.md to confirm they now differ; if
deleting, remove v003.md and update any references (docs, changelog, CI/version
lists) to point to v004.md so no broken links or duplicate entries remain.
| {% if difficulty == 'beginner' %} | ||
| Keep it simple and accessible for beginners. | ||
| {% elif difficulty == 'intermediate' %} | ||
| Include some advanced concepts but explain them clearly. | ||
| {% else %} | ||
| Don't hold back on technical details and advanced concepts. | ||
| {% endif %} |
There was a problem hiding this comment.
Consider adding validation for the difficulty parameter.
The else clause treats any non-beginner/non-intermediate value as advanced, which could lead to inappropriate guidance if difficulty is misspelled, null, or an unexpected value.
Consider one of these approaches:
Option 1: Make the else clause explicit
{% if difficulty == 'beginner' %}
Keep it simple and accessible for beginners.
{% elif difficulty == 'intermediate' %}
Include some advanced concepts but explain them clearly.
-{% else %}
+{% elif difficulty == 'advanced' %}
Don't hold back on technical details and advanced concepts.
+{% else %}
+{# Invalid difficulty level: {{difficulty}} #}
{% endif %}Option 2: Document expected values
Add a comment at the top of the file specifying that difficulty should be one of: 'beginner', 'intermediate', 'advanced'.
📝 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.
| {% if difficulty == 'beginner' %} | |
| Keep it simple and accessible for beginners. | |
| {% elif difficulty == 'intermediate' %} | |
| Include some advanced concepts but explain them clearly. | |
| {% else %} | |
| Don't hold back on technical details and advanced concepts. | |
| {% endif %} | |
| {% if difficulty == 'beginner' %} | |
| Keep it simple and accessible for beginners. | |
| {% elif difficulty == 'intermediate' %} | |
| Include some advanced concepts but explain them clearly. | |
| {% elif difficulty == 'advanced' %} | |
| Don't hold back on technical details and advanced concepts. | |
| {% else %} | |
| {# Invalid difficulty level: {{ difficulty }} #} | |
| {% endif %} |
🤖 Prompt for AI Agents
In prompts/TemplateDemo/versions/v005.md around lines 3 to 9, the Jinja
conditional treats any value other than 'beginner' or 'intermediate' as
advanced, which can misclassify misspelled or missing values; add explicit
validation or a default check before the conditional (e.g., normalize and assert
difficulty is one of ['beginner','intermediate','advanced'] or set
difficulty='beginner'/'intermediate'/'advanced' when missing/invalid) and update
the template to handle an explicit 'advanced' branch and a clear
fallback/validation branch; alternatively add a top-of-file comment documenting
the allowed values and expected behavior so callers pass valid difficulty
values.
| @agent.command() | ||
| @click.argument('name') | ||
| def create(name: str): | ||
| """Create a new agent | ||
| NAME: Name for the new agent (e.g., 'code-reviewer') | ||
| """ | ||
| try: |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Function name collisions (list, create) at module scope.
Two commands named list and create are redefined; rename functions and set the Click command name explicitly to keep CLI UX the same.
-@agent.command()
-def create(name: str):
+@agent.command(name="create")
+def create_agent(name: str):
...
-@agent.command()
-def list():
+@agent.command(name="list")
+def list_agents_cmd():
...
-@version.command()
-def list():
+@version.command(name="list")
+def list_versions_all_cmd():
...
-@version.command()
-def get(agent: str, version_name: str):
+@version.command(name="get")
+def get_version_cmd(agent: str, version_name: str):
...
-@version.command()
-def switch(agent: str, version_name: str):
+@version.command(name="switch")
+def switch_version_cmd(agent: str, version_name: str):
...
-@version.command()
-def create(agent: str, name: str, notes: str):
+@version.command(name="create")
+def create_version_cmd(agent: str, name: str, notes: str):Also applies to: 186-193, 218-230, 231-247, 267-299, 300-321
🤖 Prompt for AI Agents
In src/promptix/tools/cli.py around lines 144-151 (and also update the similar
collisions at 186-193, 218-230, 231-247, 267-299, 300-321), the module defines
multiple functions with generic names like create and list which collide at
module scope; rename each Python function to a unique internal name (e.g.,
create_agent_cmd, list_agents_cmd) and set the Click command name explicitly
with @agent.command(name='create') or @agent.command(name='list') so the CLI
remains unchanged while avoiding function name collisions; update any internal
references to the renamed functions accordingly.
| if candidate_path.exists() and candidate_path.is_symlink(): | ||
| # Resolve the symlink target | ||
| resolved_candidate = candidate_path.resolve(strict=True) | ||
|
|
||
| # Check if symlink target is outside the base directory | ||
| try: | ||
| resolved_candidate.relative_to(resolved_base) | ||
| except ValueError: | ||
| self.print_status( | ||
| f"Invalid {path_type}: symlink target {resolved_candidate} is outside base directory", | ||
| "error" | ||
| ) | ||
| return False | ||
| elif candidate_path.exists(): | ||
| # Resolve existing non-symlink paths normally | ||
| resolved_candidate = candidate_path.resolve(strict=True) | ||
| else: | ||
| # For non-existent paths, resolve without strict mode | ||
| # This validates the path structure without requiring existence | ||
| resolved_candidate = candidate_path.resolve(strict=False) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Handle broken symlinks too (don’t require .exists() to check is_symlink).
A broken symlink won’t hit the symlink branch; check is_symlink() regardless and reject targets outside base (or log appropriately).
- # Handle symlinks explicitly (only if path exists)
- if candidate_path.exists() and candidate_path.is_symlink():
+ # Handle symlinks explicitly (including broken symlinks)
+ if candidate_path.is_symlink():
# Resolve the symlink target
- resolved_candidate = candidate_path.resolve(strict=True)
+ try:
+ resolved_candidate = candidate_path.resolve(strict=True)
+ except (OSError, RuntimeError):
+ self.print_status(
+ f"Invalid {path_type}: broken symlink {candidate_path}",
+ "error"
+ )
+ return FalseCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/promptix/tools/version_manager.py around lines 79 to 99, change the logic
so you call candidate_path.is_symlink() regardless of candidate_path.exists();
if it's a symlink, resolve the symlink target using
candidate_path.resolve(strict=False) (or os.readlink if you prefer) and then
check that resolved target is within resolved_base via relative_to,
logging/rejecting if outside; keep the existing branches for non-symlink
existing paths (resolve(strict=True)) and non-existent non-symlinks
(resolve(strict=False)), and ensure any exceptions from resolve are handled the
same way you currently log errors.
| import stat | ||
| import yaml | ||
| import shutil | ||
| from typing import Dict, List, Any, Optional |
There was a problem hiding this comment.
Remove unused Optional import to keep lint happy.
Optional is no longer referenced, and Flake8/Ruff already raise F401 here. Please drop it so the fixture module stays lint-clean.
-from typing import Dict, List, Any, Optional
+from typing import Dict, List, Any📝 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.
| from typing import Dict, List, Any, Optional | |
| from typing import Dict, List, Any |
🧰 Tools
🪛 Flake8 (7.2.0)
[error] 16-16: 'typing.Optional' imported but unused
(F401)
🤖 Prompt for AI Agents
In tests/conftest.py around line 16, the import line includes Optional which is
no longer used; remove Optional from the typing import list so the line reads
only the actually used names (e.g., from typing import Dict, List, Any) to
satisfy linters and eliminate the F401 unused import.
| versions[version_name] = { | ||
| "is_live": version_name == "v1", # Assume v1 is live for testing | ||
| "config": { | ||
| "system_instruction": template, | ||
| "model": config.get("config", {}).get("model", "gpt-3.5-turbo"), | ||
| "temperature": config.get("config", {}).get("temperature", 0.7) | ||
| }, | ||
| "schema": config.get("schema", {}) | ||
| } | ||
|
|
||
| # Add current as live version if no versions found | ||
| if not versions: | ||
| versions["v1"] = { | ||
| "is_live": True, | ||
| "config": { | ||
| "system_instruction": current_template, | ||
| "model": config.get("config", {}).get("model", "gpt-3.5-turbo"), | ||
| "temperature": config.get("config", {}).get("temperature", 0.7) | ||
| }, | ||
| "schema": config.get("schema", {}) | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix live-version detection for generated prompt data.
Hard-coding is_live to version_name == "v1" fails as soon as the fixture directory contains versions like v001 or v005 (see the SimpleChat fixture you just added). The loop then flags every version as inactive, breaking consumers that rely on at least one live variant. Please read the is_live flag from config.yaml (e.g., config["versions"][version_name]["is_live"]) or, at minimum, fall back to whichever version the config marks as live and ensure there is always a single live entry.
- if versions_dir.exists():
+ config_versions = config.get("versions", {})
+ live_version = config.get("config", {}).get("live_version")
+ if versions_dir.exists():
for version_file in versions_dir.glob("*.md"):
version_name = version_file.stem
with open(version_file, 'r') as f:
template = f.read()
-
- versions[version_name] = {
- "is_live": version_name == "v1", # Assume v1 is live for testing
+ is_live = config_versions.get(version_name, {}).get("is_live")
+ if is_live is None and live_version:
+ is_live = version_name == live_version
+
+ versions[version_name] = {
+ "is_live": bool(is_live),
"config": {
"system_instruction": template,
"model": config.get("config", {}).get("model", "gpt-3.5-turbo"),
"temperature": config.get("config", {}).get("temperature", 0.7)
},
"schema": config.get("schema", {})
}
-
- # Add current as live version if no versions found
+ # Add current as live version if no versions found
if not versions:
versions["v1"] = {
"is_live": True,
"config": {
"system_instruction": current_template,
"model": config.get("config", {}).get("model", "gpt-3.5-turbo"),
"temperature": config.get("config", {}).get("temperature", 0.7)
},
"schema": config.get("schema", {})
}
-
- prompts_data[prompt_name] = {"versions": versions}
+ if versions and not any(v["is_live"] for v in versions.values()):
+ first_version = next(iter(versions))
+ versions[first_version]["is_live"] = True
+
+ prompts_data[prompt_name] = {"versions": versions}📝 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.
| versions[version_name] = { | |
| "is_live": version_name == "v1", # Assume v1 is live for testing | |
| "config": { | |
| "system_instruction": template, | |
| "model": config.get("config", {}).get("model", "gpt-3.5-turbo"), | |
| "temperature": config.get("config", {}).get("temperature", 0.7) | |
| }, | |
| "schema": config.get("schema", {}) | |
| } | |
| # Add current as live version if no versions found | |
| if not versions: | |
| versions["v1"] = { | |
| "is_live": True, | |
| "config": { | |
| "system_instruction": current_template, | |
| "model": config.get("config", {}).get("model", "gpt-3.5-turbo"), | |
| "temperature": config.get("config", {}).get("temperature", 0.7) | |
| }, | |
| "schema": config.get("schema", {}) | |
| } | |
| # Pull any explicit version metadata | |
| config_versions = config.get("versions", {}) | |
| live_version = config.get("config", {}).get("live_version") | |
| if versions_dir.exists(): | |
| for version_file in versions_dir.glob("*.md"): | |
| version_name = version_file.stem | |
| with open(version_file, 'r') as f: | |
| template = f.read() | |
| # Determine which version is marked live | |
| is_live = config_versions.get(version_name, {}).get("is_live") | |
| if is_live is None and live_version: | |
| is_live = version_name == live_version | |
| versions[version_name] = { | |
| "is_live": bool(is_live), | |
| "config": { | |
| "system_instruction": template, | |
| "model": config.get("config", {}).get("model", "gpt-3.5-turbo"), | |
| "temperature": config.get("config", {}).get("temperature", 0.7), | |
| }, | |
| "schema": config.get("schema", {}), | |
| } | |
| # Add current as live version if no versions found | |
| if not versions: | |
| versions["v1"] = { | |
| "is_live": True, | |
| "config": { | |
| "system_instruction": current_template, | |
| "model": config.get("config", {}).get("model", "gpt-3.5-turbo"), | |
| "temperature": config.get("config", {}).get("temperature", 0.7), | |
| }, | |
| "schema": config.get("schema", {}), | |
| } | |
| # Ensure at least one live version | |
| if versions and not any(v["is_live"] for v in versions.values()): | |
| first_version = next(iter(versions)) | |
| versions[first_version]["is_live"] = True | |
| prompts_data[prompt_name] = {"versions": versions} |
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
Chores