-
Notifications
You must be signed in to change notification settings - Fork 0
Release v0.1.13: Refactor Promptix architecture #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tion and focused components - Updated version to 0.1.13 in pyproject.toml and __init__.py - Refactored core components including Promptix, PromptixBuilder, and added a dependency injection container - Introduced new exception handling for better error management - Enhanced template rendering and variable validation mechanisms - Added comprehensive tests for the new architecture and components This release improves modularity and testability of the Promptix library.
WalkthroughRefactors Promptix into a dependency‑injected architecture: adds a DI container, modular components (loader, validator, renderer, version manager, model config builder), a refactored Promptix core and builder, a comprehensive exception hierarchy, test updates, and bumps project version to 0.1.13. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Promptix as Promptix (refactored)
participant Container as DI Container
participant Loader as PromptLoader
participant VerMgr as VersionManager
participant Validator as VariableValidator
participant Renderer as TemplateRenderer
participant Cfg as ModelConfigBuilder
User->>Promptix: get_prompt(template, version?, **vars)
Note over Promptix,Container: Resolve dependencies via container
Promptix->>Loader: get_prompt_data(template)
alt prompt not found
Loader-->>Promptix: raises PromptNotFoundError
Promptix-->>User: error
else prompt found
Promptix->>VerMgr: get_version_data(versions, version)
Promptix->>VerMgr: get_system_instruction(version_data)
Promptix->>Validator: validate_variables(schema, vars)
Promptix->>Renderer: render_template(system_instruction, vars)
Renderer-->>Promptix: system_message
Promptix-->>User: rendered_prompt
end
sequenceDiagram
autonumber
actor User
participant Builder as PromptixBuilder
participant Container as DI Container
participant Loader as PromptLoader
participant VerMgr as VersionManager
participant Renderer as TemplateRenderer
participant Cfg as ModelConfigBuilder
participant Adapter as Client Adapter
User->>Builder: with_data()/with_memory()/with_version()/for_client()
Note over Builder,Container: Resolve dependencies (loader, validator, renderer, version, cfg, adapters)
Builder->>Loader: get_prompt_data(template)
Builder->>VerMgr: get_version_data(...)/validate
Builder->>Renderer: render_template(system_instruction, vars)
alt system_only
Builder-->>User: system_message
else full config
Builder->>Cfg: build_model_config(system_message, memory, version_data)
Builder->>Adapter: adapt(final_config) (if adapter selected)
Adapter-->>Builder: adapted_config (or fallback)
Builder-->>User: model_config
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
🧹 Nitpick comments (31)
src/promptix/core/exceptions.py (1)
19-22
: Guard against leaking sensitive data in str output.
__str__
concatenates fulldetails
into the message, risking PII/secret leakage in logs.Consider limiting
__str__
to the message and exposing details via an explicit accessor, or redact common secret keys.- def __str__(self) -> str: - if self.details: - return f"{self.message}. Details: {self.details}" - return self.message + def __str__(self) -> str: + return self.messagesrc/promptix/core/components/prompt_loader.py (2)
10-10
: Minor correctness and cleanup: supported formats and imports.
- Include both yaml extensions in
supported_formats
.- Remove the unused import of
FileNotFoundError
.Apply:
-from ..exceptions import StorageError, FileNotFoundError, UnsupportedFormatError +from ..exceptions import StorageError, UnsupportedFormatError @@ - supported_formats=["yaml"] + supported_formats=["yaml", "yml"]Also applies to: 49-54
68-74
: Tiny flow nit: simplify returns after successful load.Keeps one return path and satisfies TRY300.
Apply:
- self._prompts = loader.load(prompt_file) - if self._logger: - self._logger.info(f"Successfully loaded prompts from {prompt_file}") - self._loaded = True - return self._prompts + self._prompts = loader.load(prompt_file) + if self._logger: + self._logger.info(f"Successfully loaded prompts from {prompt_file}") + self._loaded = True + return self._promptsAlternatively, move the final
return self._prompts
to a single place at method end if you prefer a single exit.src/promptix/core/components/model_config_builder.py (3)
101-106
: Broaden allowed message roles (tools/functions) in memory.Real chats often include 'tool' (and sometimes legacy 'function') messages. Current validation rejects them and may break tool-enabled flows.
Apply:
- if msg["role"] not in ["user", "assistant", "system"]: + if msg["role"] not in ["user", "assistant", "system", "tool", "function"]: raise InvalidMemoryFormatError( f"Message role at index {i} must be 'user', 'assistant', or 'system'", invalid_message=msg )
165-206
: Anthropic path: filter/guard against 'system' messages in memory.Anthropic expects system as a separate field; 'system' messages inside messages may cause API errors. Either filter them or warn.
Example:
def prepare_anthropic_config( self, system_message: str, memory: List[Dict[str, str]], version_data: Dict[str, Any], prompt_name: str ) -> Dict[str, Any]: @@ - # Anthropic format uses separate system parameter + # Anthropic format uses separate system parameter; exclude any 'system' entries from memory + filtered_memory = [m for m in memory if m.get("role") != "system"] model_config = { "model": config["model"], "system": system_message, - "messages": memory + "messages": filtered_memory }Also consider logging a warning if any system-role messages were filtered.
Also applies to: 101-106
152-164
: Simplify tools extraction and type-check.Use dict.get and early return; keeps it clean and satisfies RUF019.
- if "tools" in config and config["tools"]: - tools = config["tools"] + tools = config.get("tools") + if tools: if not isinstance(tools, list): if self._logger: self._logger.warning("Tools configuration must be a list") return - - model_config["tools"] = tools + model_config["tools"] = tools # If tools are present, also set tool_choice if specified - if "tool_choice" in config: - model_config["tool_choice"] = config["tool_choice"] + if "tool_choice" in config: + model_config["tool_choice"] = config["tool_choice"]src/promptix/core/components/__init__.py (1)
14-20
: Optional: sort all for consistency (RUF022).Keeps exports tidy; no runtime impact.
__all__ = [ - "PromptLoader", - "VariableValidator", - "TemplateRenderer", - "VersionManager", - "ModelConfigBuilder" + "ModelConfigBuilder", + "PromptLoader", + "TemplateRenderer", + "VariableValidator", + "VersionManager", ]tests/test_07_architecture_refactor.py (2)
437-441
: Tighten exception expectation.Assert specific MissingDependencyError to make the test more precise.
- with pytest.raises(Exception) as exc_info: + from promptix.core.exceptions import MissingDependencyError + with pytest.raises(MissingDependencyError) as exc_info: container.get("nonexistent_service")
525-534
: Add an Anthropic builder integration test.Covers the anthropic path and adapter invocation.
Example:
config = (Promptix.builder("TestPrompt") .for_client("anthropic") .with_role("a helpful assistant") .with_memory([{"role": "user", "content": "Hello"}]) .build()) assert "system" in config and config["system"] assert "messages" in config and config["messages"][0]["role"] == "user"src/promptix/core/components/template_renderer.py (2)
22-27
: Consider safe autoescape defaults for HTML/XML.Use select_autoescape so HTML/XML templates are escaped while others remain unchanged.
-from jinja2 import BaseLoader, Environment, TemplateError +from jinja2 import BaseLoader, Environment, TemplateError, select_autoescape @@ - self._jinja_env = Environment( + self._jinja_env = Environment( loader=BaseLoader(), trim_blocks=True, - lstrip_blocks=True + lstrip_blocks=True, + autoescape=select_autoescape(["html", "xml"]) )Note: from_string without a filename will not auto-escape by default; this mainly protects named HTML/XML templates.
57-69
: Preserve exception context and use explicit conversion flags.Chain original exceptions (B904) and prefer {!s} for clarity (RUF010).
- except TemplateError as e: - raise TemplateRenderError( + except TemplateError as e: + raise TemplateRenderError( prompt_name=prompt_name, - template_error=str(e), + template_error=f"{e!s}", variables=variables - ) - except Exception as e: + ) from e + except Exception as e: # Catch any other rendering errors - raise TemplateRenderError( + raise TemplateRenderError( prompt_name=prompt_name, - template_error=f"Unexpected error: {str(e)}", + template_error=f"Unexpected error: {e!s}", variables=variables - ) + ) from e @@ - except json.JSONDecodeError as json_error: - raise TemplateRenderError( + except json.JSONDecodeError as json_error: + raise TemplateRenderError( prompt_name=prompt_name, - template_error=f"Tools template rendered invalid JSON: {str(json_error)}", + template_error=f"Tools template rendered invalid JSON: {json_error!s}", variables=template_vars - ) + ) from json_error @@ - except TemplateError as e: - raise TemplateRenderError( + except TemplateError as e: + raise TemplateRenderError( prompt_name=prompt_name, - template_error=f"Tools template rendering failed: {str(e)}", + template_error=f"Tools template rendering failed: {e!s}", variables=variables - ) - except Exception as e: - raise TemplateRenderError( + ) from e + except Exception as e: + raise TemplateRenderError( prompt_name=prompt_name, - template_error=f"Unexpected error in tools template: {str(e)}", + template_error=f"Unexpected error in tools template: {e!s}", variables=variables - ) + ) from eAlso applies to: 111-116, 118-129
src/promptix/core/components/variable_validator.py (2)
50-53
: Remove unused variable to satisfy Ruff F841.The local
optional
is unused. Either remove it or wire the unknown-vars check. Minimal fix below.- required = schema.get("required", []) - optional = schema.get("optional", []) - types_dict = schema.get("types", {}) + required = schema.get("required", []) + types_dict = schema.get("types", {})Also update the commented hint to avoid future NameError when enabled:
- # allowed_vars = set(required + optional) + # allowed_vars = set(required + schema.get("optional", []))Also applies to: 66-66
27-33
: Optionally add a strict mode for unknown variables.If you want to make unknown-variable rejection opt-in without changing current behavior, add a flag.
- def validate_variables( + def validate_variables( self, schema: Dict[str, Any], user_vars: Dict[str, Any], - prompt_name: str + prompt_name: str, + allow_unknown: bool = True, ) -> None:- # --- 2) Check for unknown variables (optional check) --- - # Currently disabled to allow flexibility, but can be enabled by uncommenting: - # allowed_vars = set(required + optional) - # unknown_vars = [k for k in user_vars if k not in allowed_vars] - # if unknown_vars: - # raise VariableValidationError( - # prompt_name=prompt_name, - # variable_name=','.join(unknown_vars), - # error_message=f"unknown variables not allowed", - # provided_value=unknown_vars - # ) + # --- 2) Check for unknown variables (optional) --- + if not allow_unknown: + allowed_vars = set(schema.get("required", [])) | set(schema.get("optional", [])) + unknown_vars = [k for k in user_vars if k not in allowed_vars] + if unknown_vars: + raise VariableValidationError( + prompt_name=prompt_name, + variable_name=",".join(unknown_vars), + error_message="unknown variables not allowed", + provided_value=unknown_vars, + expected_type=None, + )Also applies to: 64-75
src/promptix/core/components/version_manager.py (2)
80-90
: Guard on explicit None rather than truthiness for version.Version labels like "0" or "" would be treated as falsy. Use an explicit None check.
- if version: + if version is not None: # Use explicitly requested version if version not in versions: available_versions = list(versions.keys()) raise VersionNotFoundError( version=version, prompt_name=prompt_name, available_versions=available_versions )
108-113
: Prefer domain exceptions over ValueError for missing fields.For consistency with the rest of the module (custom exceptions), consider raising a dedicated validation exception (e.g., ValidationError) with structured details instead of ValueError.
Would you like me to replace these with a concrete exception type from
core/exceptions.py
and update tests accordingly?Also applies to: 158-162
src/promptix/core/container.py (3)
116-124
: Factory semantics: clarify intent or add lazy-singleton variant.
get()
instantiates a new object on each factory access. If components like loaders/renderers are heavy and intended to be reused, consider aregister_lazy_singleton
that caches the first factory result; otherwise, add a brief docstring noting factories are per-call.Do you want me to add a
register_lazy_singleton
helper and switch core components to it?
8-8
: Drop unused import.
Generic
is imported but not used.-from typing import Any, Dict, Optional, Type, TypeVar, Generic, Callable +from typing import Any, Dict, Optional, Type, TypeVar, Callable
257-267
: Global container is not thread-safe (FYI).
get_container/set_container/reset_container
are race-prone under concurrent initialization. If multithreading is expected, guard with a lock.Also applies to: 279-283
src/promptix/core/base_refactored.py (1)
105-131
: Docstrings: list version-related exceptions thrown by VersionManagerCalls can raise VersionNotFoundError, NoLiveVersionError, MultipleLiveVersionsError. Document them.
src/promptix/core/builder_refactored.py (12)
100-119
: Consider moving long error text into exception class or constantsReduces duplication and meets TRY003.
151-154
: Use schema.required list rather than per-property 'required'JSON Schema encodes required fields at the schema level; property-level required flags are nonstandard and may under-warn.
- missing_fields = [] - for field, props in self.properties.items(): - if props.get("required", False) and field not in self._data: - missing_fields.append(field) + schema_required = self.version_data.get("schema", {}).get("required", []) or [] + missing_fields = [f for f in schema_required if f not in self._data]
299-317
: Remove unused kwargs from with_tool signatureEliminate ARG002 and clarify API.
- def with_tool(self, tool_name: str, *args, **kwargs) -> "PromptixBuilder": + def with_tool(self, tool_name: str, *args) -> "PromptixBuilder":
432-437
: Use dict.get to simplify activation checksAddresses RUF019 and reads clearer.
- if (tool_name in self._data and self._data[tool_name]) or \ - (prefixed_name in self._data and self._data[prefixed_name]): + if self._data.get(tool_name) or self._data.get(prefixed_name):
461-469
: Catch adapter.process_tools-specific failures and log with explicit conversionUse ToolProcessingError if adapters raise it; keep unexpected exceptions bubbling.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") + except ToolProcessingError as e: + if self._logger: + self._logger.warning(f"Error processing tools: {e!s}") return []
557-564
: Narrow catch for tools processing in build() and use explicit conversionLet unexpected errors propagate; they indicate programming faults.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") + except ToolProcessingError as e: + if self._logger: + self._logger.warning(f"Error processing tools: {e!s}")
566-572
: Narrow catch for adapter.adapt_config and log explicitlyUse adapter-specific error type if available; otherwise keep generic but avoid swallowing all.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error adapting configuration for client {self._client}: {str(e)}") + except ToolProcessingError as e: + if self._logger: + self._logger.warning(f"Error adapting configuration for client {self._client}: {e!s}")
215-240
: Optional: accept common client aliases (e.g., 'claude' => 'anthropic')Improves ergonomics while preserving strict adapter keys.
- if client not in self._adapters: + aliases = {"claude": "anthropic", "openai": "openai"} + client = aliases.get(client.lower(), client.lower()) + if client not in self._adapters: available_clients = list(self._adapters.keys()) raise UnsupportedClientError( client_name=client, available_clients=available_clients )
1-6
: Module/docstring alignment with new behaviorConsider noting exception remapping (StorageError -> PromptNotFoundError) and adapter-driven flows to aid users.
513-520
: Required fields warning: include which are missing in one lineHelpful single-line log with join.
- self._logger.warning(f"Required field '{field}' is missing from prompt parameters") + self._logger.warning(f"Missing required fields: {', '.join(missing_fields)}")
247-262
: Compatibility check: also consider version_data.config.providerYou already compute effective_provider; include it in the warning to reduce confusion.
504-574
: Security: ensure extra params cannot overwrite required fields silentlyWhen merging with model_config, guard against overwriting mandatory keys like 'model' or 'messages'.
- model_config.update(self._model_params) + protected = {"model", "messages", "system"} + model_config.update({k: v for k, v in self._model_params.items() if k not in protected})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
pyproject.toml
(1 hunks)src/promptix/__init__.py
(1 hunks)src/promptix/core/base_refactored.py
(1 hunks)src/promptix/core/builder_refactored.py
(1 hunks)src/promptix/core/components/__init__.py
(1 hunks)src/promptix/core/components/model_config_builder.py
(1 hunks)src/promptix/core/components/prompt_loader.py
(1 hunks)src/promptix/core/components/template_renderer.py
(1 hunks)src/promptix/core/components/variable_validator.py
(1 hunks)src/promptix/core/components/version_manager.py
(1 hunks)src/promptix/core/container.py
(1 hunks)src/promptix/core/exceptions.py
(1 hunks)tests/test_01_basic.py
(1 hunks)tests/test_02_builder.py
(1 hunks)tests/test_03_template_features.py
(1 hunks)tests/test_07_architecture_refactor.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
src/promptix/core/components/model_config_builder.py (2)
src/promptix/core/exceptions.py (2)
InvalidMemoryFormatError
(231-240)ConfigurationError
(140-148)src/promptix/core/base_refactored.py (1)
build_model_config
(135-182)
src/promptix/core/components/__init__.py (5)
src/promptix/core/components/prompt_loader.py (1)
PromptLoader
(16-129)src/promptix/core/components/variable_validator.py (1)
VariableValidator
(16-210)src/promptix/core/components/template_renderer.py (1)
TemplateRenderer
(13-146)src/promptix/core/components/version_manager.py (1)
VersionManager
(16-185)src/promptix/core/components/model_config_builder.py (1)
ModelConfigBuilder
(12-217)
tests/test_03_template_features.py (1)
src/promptix/core/exceptions.py (1)
PromptNotFoundError
(27-36)
src/promptix/core/base_refactored.py (8)
src/promptix/core/container.py (5)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)src/promptix/core/components/prompt_loader.py (4)
PromptLoader
(16-129)get_prompt_data
(88-110)get_prompts
(78-86)reload_prompts
(120-129)src/promptix/core/components/variable_validator.py (2)
VariableValidator
(16-210)validate_variables
(27-101)src/promptix/core/components/template_renderer.py (3)
TemplateRenderer
(13-146)render_template
(29-69)validate_template
(131-146)src/promptix/core/components/version_manager.py (4)
VersionManager
(16-185)get_version_data
(59-93)get_system_instruction
(95-113)list_versions
(115-136)src/promptix/core/components/model_config_builder.py (2)
ModelConfigBuilder
(12-217)build_model_config
(23-74)src/promptix/core/exceptions.py (2)
PromptNotFoundError
(27-36)ConfigurationError
(140-148)src/promptix/core/builder_refactored.py (1)
PromptixBuilder
(28-608)
tests/test_01_basic.py (1)
src/promptix/core/exceptions.py (1)
PromptNotFoundError
(27-36)
src/promptix/core/components/template_renderer.py (2)
src/promptix/core/exceptions.py (1)
TemplateRenderError
(76-86)src/promptix/core/base_refactored.py (1)
validate_template
(230-239)
src/promptix/core/components/prompt_loader.py (5)
src/promptix/core/exceptions.py (3)
StorageError
(182-184)FileNotFoundError
(187-196)UnsupportedFormatError
(199-209)src/promptix/core/storage/loaders.py (2)
PromptLoaderFactory
(120-132)get_loader
(122-132)src/promptix/core/storage/utils.py (1)
create_default_prompts_file
(12-87)src/promptix/core/config.py (3)
check_for_unsupported_files
(135-146)get_prompt_file_path
(49-62)get_default_prompt_file_path
(64-67)src/promptix/core/base_refactored.py (1)
reload_prompts
(241-245)
tests/test_02_builder.py (2)
src/promptix/core/exceptions.py (2)
PromptNotFoundError
(27-36)UnsupportedClientError
(156-165)src/promptix/core/builder_refactored.py (1)
build
(504-573)
tests/test_07_architecture_refactor.py (8)
src/promptix/core/components/prompt_loader.py (4)
PromptLoader
(16-129)is_loaded
(112-118)load_prompts
(29-76)get_prompt_data
(88-110)src/promptix/core/components/variable_validator.py (2)
VariableValidator
(16-210)validate_variables
(27-101)src/promptix/core/components/template_renderer.py (4)
TemplateRenderer
(13-146)render_template
(29-69)render_tools_template
(71-129)validate_template
(131-146)src/promptix/core/components/version_manager.py (4)
VersionManager
(16-185)find_live_version
(27-57)get_version_data
(59-93)get_system_instruction
(95-113)src/promptix/core/components/model_config_builder.py (4)
ModelConfigBuilder
(12-217)validate_memory_format
(76-117)build_model_config
(23-74)prepare_anthropic_config
(165-206)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 (10)
Container
(25-196)get_container
(257-266)reset_container
(279-282)get
(100-128)get
(220-231)register_singleton
(71-78)register_factory
(80-87)create_scope
(190-196)override
(153-160)override
(211-218)src/promptix/core/base_refactored.py (5)
Promptix
(20-245)validate_template
(230-239)builder
(185-196)build_model_config
(135-182)get_prompt
(40-59)
src/promptix/__init__.py (2)
src/promptix/core/base_refactored.py (1)
Promptix
(20-245)src/promptix/core/base.py (1)
Promptix
(12-327)
src/promptix/core/components/version_manager.py (2)
src/promptix/core/exceptions.py (3)
NoLiveVersionError
(52-61)MultipleLiveVersionsError
(64-73)VersionNotFoundError
(39-49)src/promptix/core/base_refactored.py (1)
list_versions
(206-228)
src/promptix/core/components/variable_validator.py (1)
src/promptix/core/exceptions.py (3)
VariableValidationError
(96-109)RequiredVariableError
(112-123)create_validation_error
(307-333)
src/promptix/core/builder_refactored.py (8)
src/promptix/core/container.py (6)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)register_adapter
(169-188)src/promptix/core/components/prompt_loader.py (3)
PromptLoader
(16-129)get_prompt_data
(88-110)get_prompts
(78-86)src/promptix/core/components/variable_validator.py (1)
validate_builder_type
(145-210)src/promptix/core/components/template_renderer.py (2)
TemplateRenderer
(13-146)render_tools_template
(71-129)src/promptix/core/components/version_manager.py (1)
find_live_version
(27-57)src/promptix/core/components/model_config_builder.py (3)
validate_memory_format
(76-117)prepare_anthropic_config
(165-206)build_model_config
(23-74)src/promptix/core/adapters/_base.py (1)
ModelAdapter
(4-20)src/promptix/core/exceptions.py (4)
PromptNotFoundError
(27-36)VersionNotFoundError
(39-49)UnsupportedClientError
(156-165)ValidationError
(91-93)
src/promptix/core/container.py (4)
src/promptix/enhancements/logging.py (1)
setup_logging
(24-51)src/promptix/core/adapters/_base.py (1)
ModelAdapter
(4-20)src/promptix/core/exceptions.py (2)
MissingDependencyError
(281-290)InvalidDependencyError
(293-303)src/promptix/core/builder_refactored.py (1)
register_adapter
(86-98)
🪛 Ruff (0.12.2)
src/promptix/core/components/model_config_builder.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
63-65: Avoid specifying long messages outside the exception class
(TRY003)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
90-93: Avoid specifying long messages outside the exception class
(TRY003)
96-99: Avoid specifying long messages outside the exception class
(TRY003)
102-105: Avoid specifying long messages outside the exception class
(TRY003)
108-111: Avoid specifying long messages outside the exception class
(TRY003)
114-117: Avoid specifying long messages outside the exception class
(TRY003)
152-152: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
187-187: Avoid specifying long messages outside the exception class
(TRY003)
192-194: Avoid specifying long messages outside the exception class
(TRY003)
src/promptix/core/components/__init__.py
14-20: __all__
is not sorted
Apply an isort-style sorting to __all__
(RUF022)
src/promptix/core/base_refactored.py
81-81: Do not catch blind exception: Exception
(BLE001)
81-81: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
83-86: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
166-166: Do not catch blind exception: Exception
(BLE001)
166-166: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
168-171: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
220-220: Do not catch blind exception: Exception
(BLE001)
220-220: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
222-225: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
src/promptix/core/components/template_renderer.py
22-22: By default, jinja2 sets autoescape
to False
. Consider using autoescape=True
or the select_autoescape
function to mitigate XSS vulnerabilities.
(S701)
55-55: Consider moving this statement to an else
block
(TRY300)
58-62: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
63-63: Do not catch blind exception: Exception
(BLE001)
65-69: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
67-67: Use explicit conversion flag
Replace with conversion flag
(RUF010)
112-116: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
114-114: Use explicit conversion flag
Replace with conversion flag
(RUF010)
119-123: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
121-121: Use explicit conversion flag
Replace with conversion flag
(RUF010)
124-124: Do not catch blind exception: Exception
(BLE001)
125-129: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
127-127: Use explicit conversion flag
Replace with conversion flag
(RUF010)
142-142: Consider moving this statement to an else
block
(TRY300)
145-145: Do not catch blind exception: Exception
(BLE001)
src/promptix/core/components/prompt_loader.py
50-54: Abstract raise
to an inner function
(TRY301)
73-73: Consider moving this statement to an else
block
(TRY300)
75-75: Do not catch blind exception: Exception
(BLE001)
76-76: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
76-76: Avoid specifying long messages outside the exception class
(TRY003)
76-76: Use explicit conversion flag
Replace with conversion flag
(RUF010)
103-109: Avoid specifying long messages outside the exception class
(TRY003)
src/promptix/core/components/version_manager.py
110-112: Avoid specifying long messages outside the exception class
(TRY003)
160-162: Avoid specifying long messages outside the exception class
(TRY003)
src/promptix/core/components/variable_validator.py
51-51: Local variable optional
is assigned to but never used
Remove assignment to unused variable optional
(F841)
src/promptix/core/builder_refactored.py
69-69: Do not catch blind exception: Exception
(BLE001)
69-69: Local variable e
is assigned to but never used
Remove assignment to unused variable e
(F841)
71-74: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
112-115: Avoid specifying long messages outside the exception class
(TRY003)
140-140: Avoid specifying long messages outside the exception class
(TRY003)
299-299: Unused method argument: kwargs
(ARG002)
435-435: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
436-436: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
453-453: Do not catch blind exception: Exception
(BLE001)
455-455: Use explicit conversion flag
Replace with conversion flag
(RUF010)
466-466: Do not catch blind exception: Exception
(BLE001)
468-468: Use explicit conversion flag
Replace with conversion flag
(RUF010)
526-526: Do not catch blind exception: Exception
(BLE001)
528-528: Use explicit conversion flag
Replace with conversion flag
(RUF010)
561-561: Do not catch blind exception: Exception
(BLE001)
563-563: Use explicit conversion flag
Replace with conversion flag
(RUF010)
569-569: Do not catch blind exception: Exception
(BLE001)
571-571: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (9)
pyproject.toml (1)
7-7
: Version bump verified:pyproject.toml
(line 7) andsrc/promptix/__init__.py
(line 26) both use0.1.13
.tests/test_03_template_features.py (1)
114-116
: Exception alignment looks good.Switch to PromptNotFoundError matches the new exception surface.
tests/test_01_basic.py (1)
31-33
: Good move to explicit PromptNotFoundError.Improves clarity and keeps tests aligned with the refactor.
tests/test_02_builder.py (1)
87-98
: Updated negative-path assertions look correct.Asserting PromptNotFoundError and UnsupportedClientError matches the new API.
src/promptix/__init__.py (1)
24-27
: Public import switch and version bump LGTM.Exports remain stable; version matches PR intent (0.1.13).
src/promptix/core/base_refactored.py (1)
241-246
: LGTM: explicit reload with info logReload flow is simple and side-effect is logged.
src/promptix/core/builder_refactored.py (3)
85-99
: LGTM: adapter registration via containerType-checked registration and central registry usage look good.
120-141
: LGTM: dynamic with_ setters*Clean chainable API with per-field validation.
583-609
: LGTM: concise tooling debug snapshotHandy for inspecting builder state without side effects.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/promptix/core/builder_refactored.py (1)
547-557
: Avoid blind except when generating system message.Repeat earlier guidance: catch (PromptNotFoundError, ValidationError, VersionNotFoundError) and log {e!s}; let other errors bubble.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error generating system message: {str(e)}") + except (PromptNotFoundError, ValidationError, VersionNotFoundError) as e: + if self._logger: + self._logger.warning(f"Error generating system message: {e!s}")
🧹 Nitpick comments (3)
src/promptix/core/components/prompt_loader.py (2)
114-116
: Docstring: update Raises to match implementation.This method now raises PromptNotFoundError, not StorageError.
- Raises: - StorageError: If prompt is not found. + Raises: + PromptNotFoundError: If the prompt is not found.
8-11
: Remove unused imports.Path and FileNotFoundError aren’t used here.
-from pathlib import Path from typing import Any, Dict, Optional -from ..exceptions import StorageError, FileNotFoundError, UnsupportedFormatError +from ..exceptions import StorageError, UnsupportedFormatErrorsrc/promptix/core/builder_refactored.py (1)
487-495
: Narrow catch on adapter.process_tools to avoid masking bugs.Catch ToolProcessingError; let unexpected exceptions surface.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") - return [] + except ToolProcessingError as e: + if self._logger: + self._logger.warning(f"Error processing tools: {e!s}") + return []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/promptix/core/base_refactored.py
(1 hunks)src/promptix/core/builder_refactored.py
(1 hunks)src/promptix/core/components/prompt_loader.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/promptix/core/components/prompt_loader.py (5)
src/promptix/core/exceptions.py (4)
StorageError
(182-184)FileNotFoundError
(187-196)UnsupportedFormatError
(199-209)PromptNotFoundError
(27-36)src/promptix/core/storage/loaders.py (2)
PromptLoaderFactory
(120-132)get_loader
(122-132)src/promptix/core/storage/utils.py (1)
create_default_prompts_file
(12-87)src/promptix/core/config.py (3)
check_for_unsupported_files
(135-146)get_prompt_file_path
(49-62)get_default_prompt_file_path
(64-67)src/promptix/core/base_refactored.py (1)
reload_prompts
(250-254)
src/promptix/core/base_refactored.py (8)
src/promptix/core/container.py (5)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)src/promptix/core/components/prompt_loader.py (4)
PromptLoader
(16-142)get_prompt_data
(105-123)get_prompts
(95-103)reload_prompts
(133-142)src/promptix/core/components/variable_validator.py (2)
VariableValidator
(16-210)validate_variables
(27-101)src/promptix/core/components/template_renderer.py (3)
TemplateRenderer
(13-146)render_template
(29-69)validate_template
(131-146)src/promptix/core/components/version_manager.py (4)
VersionManager
(16-185)get_version_data
(59-93)get_system_instruction
(95-113)list_versions
(115-136)src/promptix/core/components/model_config_builder.py (2)
ModelConfigBuilder
(12-217)build_model_config
(23-74)src/promptix/core/exceptions.py (3)
PromptNotFoundError
(27-36)ConfigurationError
(140-148)StorageError
(182-184)src/promptix/core/builder_refactored.py (1)
PromptixBuilder
(28-59)
src/promptix/core/builder_refactored.py (8)
src/promptix/core/container.py (5)
get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)register_adapter
(169-188)src/promptix/core/components/prompt_loader.py (3)
PromptLoader
(16-142)get_prompt_data
(105-123)get_prompts
(95-103)src/promptix/core/components/variable_validator.py (2)
VariableValidator
(16-210)validate_builder_type
(145-210)src/promptix/core/components/template_renderer.py (2)
TemplateRenderer
(13-146)render_tools_template
(71-129)src/promptix/core/components/model_config_builder.py (4)
ModelConfigBuilder
(12-217)validate_memory_format
(76-117)prepare_anthropic_config
(165-206)build_model_config
(23-74)src/promptix/core/adapters/_base.py (1)
ModelAdapter
(4-20)src/promptix/core/exceptions.py (8)
PromptNotFoundError
(27-36)VersionNotFoundError
(39-49)UnsupportedClientError
(156-165)ToolNotFoundError
(250-259)ToolProcessingError
(262-271)ValidationError
(91-93)StorageError
(182-184)TemplateRenderError
(76-86)src/promptix/core/base_refactored.py (3)
Promptix
(19-254)render_prompt
(60-110)build_model_config
(142-189)
🪛 Ruff (0.12.2)
src/promptix/core/components/prompt_loader.py
119-119: SyntaxError: Expected an indented block after if
statement
src/promptix/core/base_refactored.py
147-147: Unused method argument: variables
(ARG002)
185-185: Undefined name system_message
(F821)
src/promptix/core/builder_refactored.py
62-62: SyntaxError: Expected an expression
62-63: SyntaxError: Expected an expression
63-63: SyntaxError: Expected class, function definition or async function definition after decorator
63-63: SyntaxError: Expected an identifier, but found a keyword 'from' that cannot be used here
63-63: SyntaxError: Simple statements must be separated by newlines or semicolons
63-63: SyntaxError: Expected one or more symbol names after import
63-64: SyntaxError: missing closing parenthesis )
80-80: SyntaxError: Unexpected indentation
86-86: SyntaxError: Expected a statement
86-86: SyntaxError: Invalid annotated assignment target
86-86: SyntaxError: Expected an identifier, but found a keyword 'try' that cannot be used here
86-87: SyntaxError: Expected an expression
87-87: SyntaxError: Invalid assignment target
88-88: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
88-88: SyntaxError: Simple statements must be separated by newlines or semicolons
88-88: SyntaxError: Expected a statement
88-89: SyntaxError: Expected an expression
89-89: SyntaxError: Invalid assignment target
90-90: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
90-90: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Expected a parameter name
92-92: SyntaxError: Expected a parameter name
92-92: SyntaxError: Duplicate keyword argument ""
92-93: SyntaxError: Expected ')', found newline
93-93: SyntaxError: Unexpected indentation
113-113: SyntaxError: unindent does not match any outer indentation level
114-114: SyntaxError: Expected class, function definition or async function definition after decorator
🪛 GitHub Actions: CI
src/promptix/core/components/prompt_loader.py
[error] 118-119: IndentationError: expected an indented block after 'if' statement on line 118. Command: pytest --cov=promptix --cov-report=xml.
🔇 Additional comments (4)
src/promptix/core/components/prompt_loader.py (1)
45-93
: Exception handling looks solid; good re-raise and chaining.Explicitly letting UnsupportedFormatError/StorageError bubble and normalizing ValueError to UnsupportedFormatError is correct and aligns with the contract.
src/promptix/core/base_refactored.py (1)
78-89
: Nice narrowing to StorageError with cause chaining.This avoids blind excepts and preserves root causes as requested in earlier reviews.
Also applies to: 167-178, 225-235
src/promptix/core/builder_refactored.py (2)
479-483
: Import is added; keep the narrow catch to TemplateRenderError.This is the right granularity and logging uses {e!s}. Looks good.
148-167
: Dynamic with_ setter + schema validation: LGTM.*Good pattern: funnels through _validate_type and supports chaining.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/promptix/core/builder_refactored.py (1)
547-557
: Narrow broad except and log with explicit conversionCatching Exception masks unrelated bugs; limit to expected errors from rendering path and improve logging.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error generating system message: {str(e)}") + except (PromptNotFoundError, ValidationError, VersionNotFoundError, TemplateRenderError, ConfigurationError) as e: + if self._logger: + self._logger.warning(f"Error generating system message: {e!s}")
🧹 Nitpick comments (3)
src/promptix/core/components/prompt_loader.py (2)
114-116
: Docstring mismatch: update raised exception typeThis method raises PromptNotFoundError, not StorageError.
- Raises: - StorageError: If prompt is not found. + Raises: + PromptNotFoundError: If the prompt is not found.
49-55
: Consistency: include both YAML extensions in supported_formatsElsewhere you advertise “yaml” and “yml”. Make this consistent here.
- supported_formats=["yaml"] + supported_formats=["yaml", "yml"]src/promptix/core/builder_refactored.py (1)
492-496
: Narrow catch to ToolProcessingError in adapter processingAvoid swallowing unexpected adapter bugs.
- except Exception as e: - if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") - return [] + except ToolProcessingError as e: + if self._logger: + self._logger.warning(f"Error processing tools: {e!s}") + return []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/promptix/core/base_refactored.py
(1 hunks)src/promptix/core/builder_refactored.py
(1 hunks)src/promptix/core/components/prompt_loader.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/promptix/core/components/prompt_loader.py (5)
src/promptix/core/exceptions.py (4)
StorageError
(182-184)FileNotFoundError
(187-196)UnsupportedFormatError
(199-209)PromptNotFoundError
(27-36)src/promptix/core/storage/loaders.py (2)
PromptLoaderFactory
(120-132)get_loader
(122-132)src/promptix/core/storage/utils.py (1)
create_default_prompts_file
(12-87)src/promptix/core/config.py (3)
check_for_unsupported_files
(135-146)get_prompt_file_path
(49-62)get_default_prompt_file_path
(64-67)src/promptix/core/base_refactored.py (1)
reload_prompts
(250-254)
src/promptix/core/base_refactored.py (8)
src/promptix/core/container.py (5)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)src/promptix/core/components/prompt_loader.py (4)
PromptLoader
(16-142)get_prompt_data
(105-123)get_prompts
(95-103)reload_prompts
(133-142)src/promptix/core/components/variable_validator.py (2)
VariableValidator
(16-210)validate_variables
(27-101)src/promptix/core/components/template_renderer.py (3)
TemplateRenderer
(13-146)render_template
(29-69)validate_template
(131-146)src/promptix/core/components/version_manager.py (4)
VersionManager
(16-185)get_version_data
(59-93)get_system_instruction
(95-113)list_versions
(115-136)src/promptix/core/components/model_config_builder.py (2)
ModelConfigBuilder
(12-217)build_model_config
(23-74)src/promptix/core/exceptions.py (3)
PromptNotFoundError
(27-36)ConfigurationError
(140-148)StorageError
(182-184)src/promptix/core/builder_refactored.py (1)
PromptixBuilder
(28-59)
src/promptix/core/builder_refactored.py (8)
src/promptix/core/container.py (6)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)register_adapter
(169-188)src/promptix/core/components/prompt_loader.py (3)
PromptLoader
(16-142)get_prompt_data
(105-123)get_prompts
(95-103)src/promptix/core/components/variable_validator.py (1)
validate_builder_type
(145-210)src/promptix/core/components/template_renderer.py (1)
render_tools_template
(71-129)src/promptix/core/components/version_manager.py (1)
find_live_version
(27-57)src/promptix/core/components/model_config_builder.py (4)
ModelConfigBuilder
(12-217)validate_memory_format
(76-117)prepare_anthropic_config
(165-206)build_model_config
(23-74)src/promptix/core/adapters/_base.py (1)
ModelAdapter
(4-20)src/promptix/core/exceptions.py (5)
PromptNotFoundError
(27-36)UnsupportedClientError
(156-165)ValidationError
(91-93)StorageError
(182-184)TemplateRenderError
(76-86)
🪛 Ruff (0.12.2)
src/promptix/core/components/prompt_loader.py
119-119: SyntaxError: Expected an indented block after if
statement
src/promptix/core/base_refactored.py
147-147: Unused method argument: variables
(ARG002)
185-185: Undefined name system_message
(F821)
src/promptix/core/builder_refactored.py
62-62: SyntaxError: Expected an expression
62-63: SyntaxError: Expected an expression
63-63: SyntaxError: Expected class, function definition or async function definition after decorator
63-63: SyntaxError: Expected an identifier, but found a keyword 'from' that cannot be used here
63-63: SyntaxError: Simple statements must be separated by newlines or semicolons
63-63: SyntaxError: Expected one or more symbol names after import
63-64: SyntaxError: missing closing parenthesis )
80-80: SyntaxError: Unexpected indentation
86-86: SyntaxError: Expected a statement
86-86: SyntaxError: Invalid annotated assignment target
86-86: SyntaxError: Expected an identifier, but found a keyword 'try' that cannot be used here
86-87: SyntaxError: Expected an expression
87-87: SyntaxError: Invalid assignment target
88-88: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
88-88: SyntaxError: Simple statements must be separated by newlines or semicolons
88-88: SyntaxError: Expected a statement
88-89: SyntaxError: Expected an expression
89-89: SyntaxError: Invalid assignment target
90-90: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
90-90: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Expected a parameter name
92-92: SyntaxError: Expected a parameter name
92-92: SyntaxError: Duplicate keyword argument ""
92-93: SyntaxError: Expected ')', found newline
93-93: SyntaxError: Unexpected indentation
113-113: SyntaxError: unindent does not match any outer indentation level
114-114: SyntaxError: Expected class, function definition or async function definition after decorator
🪛 GitHub Actions: CI
src/promptix/core/components/prompt_loader.py
[error] 118-119: IndentationError: expected an indented block after 'if' statement on line 118 in prompt_loader.py. Likely a missing block after 'if prompt_template not in prompts'. Command: pytest --cov=promptix --cov-report=xml
🔇 Additional comments (2)
src/promptix/core/components/prompt_loader.py (1)
75-93
: Good exception normalization and chainingNice job re-raising Promptix errors and mapping factory ValueErrors to UnsupportedFormatError with proper chaining.
src/promptix/core/builder_refactored.py (1)
94-107
: Good: StorageError narrowing and cause preservationSwitching from blind except to catching StorageError and chaining to PromptNotFoundError is correct and aligns with the new exception hierarchy.
- Added new exceptions: StorageError, RequiredVariableError, and VariableValidationError for better error management. - Updated the _initialize_prompt_data method to handle StorageError specifically. - Enhanced the variable validation logic to prevent boolean values from being treated as integers. - Improved system message rendering in the Promptix class to ensure proper message generation. These changes enhance the robustness and clarity of the Promptix library's error handling and prompt rendering capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/promptix/core/exceptions.py (2)
187-197
: Renamed to avoid clobbering built-in — resolvedUsing
StorageFileNotFoundError
avoids shadowingFileNotFoundError
. Good resolution of prior feedback.
226-241
: Memory error hierarchy rename — resolved
PromptixMemoryError
andInvalidMemoryFormatError
no longer shadow built-ins. Approved.src/promptix/core/components/variable_validator.py (1)
121-128
: Excluding bools from number/integer and accepting tuples for arrays — LGTMThis addresses the classic bool-as-int pitfall and broadens array acceptance appropriately.
src/promptix/core/builder_refactored.py (1)
18-28
: Import missing exceptions to avoid NameError and enable narrower catches
TemplateRenderError
is used but not imported;ConfigurationError
is also referenced in components and is useful for narrowed catches below.from .exceptions import ( PromptNotFoundError, VersionNotFoundError, UnsupportedClientError, ToolNotFoundError, ToolProcessingError, ValidationError, StorageError, RequiredVariableError, - VariableValidationError + VariableValidationError, + TemplateRenderError, + ConfigurationError, )
🧹 Nitpick comments (27)
src/promptix/core/exceptions.py (3)
8-8
: Remove unused import
Union
isn’t used.-from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional
156-166
: Polish message formatting for readabilityJoin
available_clients
for a cleaner message.- message = f"Unsupported client: {client_name}. Available clients: {available_clients}" + message = f"Unsupported client: {client_name}. Available clients: {', '.join(available_clients)}"
306-334
: Align return annotation or expand behavior
create_validation_error
only producesVariableValidationError
. Either:
- Narrow the return type to
VariableValidationError
, or- Expand logic to sometimes return other
ValidationError
subtypes (if that’s the intent).-def create_validation_error(prompt_name: str, field: str, value: Any, - expected_type: Optional[str] = None, - enum_values: Optional[List[Any]] = None) -> ValidationError: +def create_validation_error(prompt_name: str, field: str, value: Any, + expected_type: Optional[str] = None, + enum_values: Optional[List[Any]] = None) -> VariableValidationError:src/promptix/tools/studio/data.py (6)
92-101
: Avoid shadowing importedconfig
Local variable
config = version_data['config']
shadows the imported module. Rename to reduce confusion.- config = version_data['config'] + version_cfg = version_data['config'] - if 'model' not in config or config['model'] is None: - config['model'] = "gpt-4o" - if 'provider' not in config or config['provider'] is None: - config['provider'] = "openai" + if 'model' not in version_cfg or version_cfg['model'] is None: + version_cfg['model'] = "gpt-4o" + if 'provider' not in version_cfg or version_cfg['provider'] is None: + version_cfg['provider'] = "openai"
221-233
: Same shadowing in add_versionRename local
config
for clarity.- config = content['config'] - if 'model' not in config or config['model'] is None: - config['model'] = "gpt-4o" - if 'provider' not in config or config['provider'] is None: - config['provider'] = "openai" - if 'system_instruction' not in config: - config['system_instruction'] = "You are a helpful AI assistant." - if 'temperature' not in config: - config['temperature'] = 0.7 - if 'max_tokens' not in config: - config['max_tokens'] = 1024 - if 'top_p' not in config: - config['top_p'] = 1.0 + cfg = content['config'] + if 'model' not in cfg or cfg['model'] is None: + cfg['model'] = "gpt-4o" + if 'provider' not in cfg or cfg['provider'] is None: + cfg['provider'] = "openai" + if 'system_instruction' not in cfg: + cfg['system_instruction'] = "You are a helpful AI assistant." + if 'temperature' not in cfg: + cfg['temperature'] = 0.7 + if 'max_tokens' not in cfg: + cfg['max_tokens'] = 1024 + if 'top_p' not in cfg: + cfg['top_p'] = 1.0
46-51
: Prefer logging over print statementsSwitch to
logging
for structured, configurable logs (Studio can still default to WARNING/INFO).+import logging +logger = logging.getLogger(__name__) @@ - print(f"Warning: Schema validation error: {e}") + logger.warning("Schema validation error: %s", e) @@ - print(f"Warning: Error loading prompts: {e}") + logger.warning("Error loading prompts: %s", e) @@ - print(f"Warning: No config found in saved version {version_id}") + logger.warning("No config found in saved version %s", version_id) @@ - print(f"Error in save_prompt: {str(e)}") - print(traceback.format_exc()) + logger.error("Error in save_prompt: %s", e, exc_info=True) @@ - print(f"Adding version {version} to prompt {prompt_id}") + logger.info("Adding version %s to prompt %s", version, prompt_id) @@ - print("No config provided in content") + logger.info("No config provided in content") @@ - print(f"Final config: model={content['config'].get('model')}, provider={content['config'].get('provider')}") + logger.debug("Final config: model=%s, provider=%s", + content['config'].get('model'), + content['config'].get('provider')) @@ - print(f"Error in add_version: {str(e)}") - print(traceback.format_exc()) + logger.error("Error in add_version: %s", e, exc_info=True)Also applies to: 61-66, 112-115, 117-118, 202-208, 260-261, 279-280
64-66
: Specify file encoding on save fallbackEnsure consistent UTF-8 writes like the loader’s save path.
- with open(self.storage_path, 'w') as f: + with open(self.storage_path, 'w', encoding='utf-8') as f: yaml.dump(prompts, f, sort_keys=False, allow_unicode=True)
192-194
: Use domain exception for consistencyRaise
PromptNotFoundError
instead of genericValueError
.- if not prompt: - raise ValueError(f"Prompt with ID {prompt_id} not found") + if not prompt: + from promptix.core.exceptions import PromptNotFoundError + raise PromptNotFoundError(prompt_id)
146-148
: Avoid ID collisions on delete/recreateCounting keys can reuse IDs after deletions. Consider UUIDs.
- prompt_count = sum(1 for k in prompts.keys() if k != "schema") - prompt_id = f"prompt_{prompt_count + 1}" + import uuid + prompt_id = f"prompt_{uuid.uuid4().hex[:8]}"src/promptix/core/storage/loaders.py (3)
124-125
: Standardize unsupported formats handlingUse
UnsupportedFormatError
for unknown extensions too, to keep error surface consistent.- else: - raise ValueError(f"Unsupported file format: {file_path.suffix}. Only YAML (.yaml, .yml) files are supported.") + else: + ext = file_path.suffix.lstrip('.').lower() or file_path.suffix + raise UnsupportedFormatError( + str(file_path), + ext, + ["yaml", "yml"] + )
98-104
: Handle empty YAML files gracefully
yaml.safe_load
can returnNone
. Default to a minimal valid structure before validation.def load(self, file_path: Path) -> Dict[str, Any]: with open(file_path, 'r', encoding='utf-8') as f: - data = yaml.safe_load(f) + data = yaml.safe_load(f) + if data is None: + data = {"schema": 1.0} self.validate_loaded(data) return data
1-3
: Nit: file header pathHeader says
# src/promptix/core/loaders.py
but file lives undercore/storage/loaders.py
.-# src/promptix/core/loaders.py +# src/promptix/core/storage/loaders.pysrc/promptix/core/components/variable_validator.py (2)
165-202
: DRY the builder validation using the same type checks (adds array/object support).Reuse
_validate_type_constraint
here to cover all supported types uniformly and reduce duplication.@@ - # Type validation - if expected_type == "string": - if not isinstance(value, str): - raise VariableValidationError( - prompt_name="builder", - variable_name=field, - error_message=f"must be a string, got {type(value).__name__}", - provided_value=value, - expected_type="string" - ) - elif expected_type == "number": - if not (isinstance(value, (int, float)) and not isinstance(value, bool)): - raise VariableValidationError( - prompt_name="builder", - variable_name=field, - error_message=f"must be a number, got {type(value).__name__}", - provided_value=value, - expected_type="number" - ) - elif expected_type == "integer": - if not (isinstance(value, int) and not isinstance(value, bool)): - raise VariableValidationError( - prompt_name="builder", - variable_name=field, - error_message=f"must be an integer, got {type(value).__name__}", - provided_value=value, - expected_type="integer" - ) - elif expected_type == "boolean": - if not isinstance(value, bool): - raise VariableValidationError( - prompt_name="builder", - variable_name=field, - error_message=f"must be a boolean, got {type(value).__name__}", - provided_value=value, - expected_type="boolean" - ) + # Type validation (reuse common checks, includes array/object) + if expected_type: + self._validate_type_constraint( + var_name=field, + var_value=value, + expected_type=expected_type, + prompt_name="builder", + )
51-51
: Ruff F841:optional
is unused unless you enforce unknown-variable checks.Adopting the
additionalProperties
check above will use it and resolve F841. Otherwise, drop the assignment.- optional = schema.get("optional", []) + optional = schema.get("optional", []) # used by unknown-variable checkssrc/promptix/core/components/prompt_loader.py (4)
10-10
: Remove unused import.
StorageFileNotFoundError
is not referenced in this module.-from ..exceptions import StorageError, StorageFileNotFoundError, UnsupportedFormatError +from ..exceptions import StorageError, UnsupportedFormatError
50-54
: List all supported YAML extensions for consistency.Factory accepts both
.yaml
and.yml
; mirror that here.- supported_formats=["yaml"] + supported_formats=["yaml", "yml"]
83-84
: Drop redundantlocals()
guard.
prompt_file
is always bound by this point; the substring check is sufficient.- if 'Unsupported file format' in str(e) and 'prompt_file' in locals(): + if 'Unsupported file format' in str(e):
114-121
: Docstring mismatch: raise type.This path raises
PromptNotFoundError
, notStorageError
.- Raises: - StorageError: If prompt is not found. + Raises: + PromptNotFoundError: If the prompt is not found.src/promptix/core/base_refactored.py (1)
183-189
: Optional: avoid redundant prompt fetch in build_model_config.You already have
version_data
; deriving the system message directly here can skip an extraget_prompt_data
call.- system_message = self.render_prompt(prompt_template, version, **variables) + # Inline render to avoid re-fetch; same logic as render_prompt + try: + template_text = self._version_manager.get_system_instruction(version_data, prompt_template) + except ValueError as err: + raise ConfigurationError( + config_issue="Missing 'config.system_instruction'", + config_path=f"{prompt_template}.versions" + ) from err + schema = version_data.get("schema", {}) + self._variable_validator.validate_variables(schema, variables, prompt_template) + system_message = self._template_renderer.render_template(template_text, variables, prompt_template)src/promptix/core/builder_refactored.py (8)
147-160
: Tidy kwargs typingAnnotate
**kwargs
asAny
(pep-484 style) rather thanDict[...]
.- def with_data(self, **kwargs: Dict[str, Any]): + def with_data(self, **kwargs: Any):
304-321
: Remove unused**kwargs
kwargs
is not used; drop it to satisfy ARG002.- def with_tool(self, tool_name: str, *args, **kwargs) -> "PromptixBuilder": + def with_tool(self, tool_name: str, *args) -> "PromptixBuilder":
437-443
: Use dict.get for simpler truthy checksRemoves redundant key checks (RUF019).
- if (tool_name in self._data and self._data[tool_name]) or \ - (prefixed_name in self._data and self._data[prefixed_name]): + if self._data.get(tool_name) or self._data.get(prefixed_name): selected_tools[tool_name] = available_tools[tool_name]
465-468
: Guard adapter lookup and raise domain error instead of KeyErrorEnsures consistent error surface when default client isn’t registered.
- adapter = self._adapters[self._client] + adapter = self._adapters.get(self._client) + if adapter is None: + raise UnsupportedClientError( + client_name=self._client, + available_clients=list(self._adapters.keys()) + ) return adapter.process_tools(selected_tools)
469-472
: Narrow catch to ToolProcessingError; use explicit conversion flagAvoid BLE001 and adopt
{e!s}
(RUF010).- except Exception as e: - if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") + except ToolProcessingError as e: + if self._logger: + self._logger.warning(f"Error processing tools: {e!s}") return []
502-506
: Validate dict-form template results against available toolsPrevents unknown tools from slipping in while keeping back-compat.
- elif isinstance(template_result, dict): - for tool_name, tool_config in template_result.items(): - if tool_name not in selected_tools: - selected_tools[tool_name] = tool_config + elif isinstance(template_result, dict): + for tool_name, tool_config in template_result.items(): + if tool_name not in selected_tools: + if tool_name in available_tools: + selected_tools[tool_name] = available_tools[tool_name] + elif self._logger: + self._logger.warning(f"Ignoring unknown tool '{tool_name}' from template result")
560-567
: Remove redundant broad catch around_process_tools_template
The callee already handles template/tool errors and returns []; avoid BLE001 and duplicate logging.
- try: - tools = self._process_tools_template() - if tools: - model_config["tools"] = tools - except Exception as e: - if self._logger: - self._logger.warning(f"Error processing tools: {str(e)}") + tools = self._process_tools_template() + if tools: + model_config["tools"] = tools
569-575
: Harden adapter retrieval and narrow adaptation error handlingAvoid KeyError and BLE001; use
{e!s}
for logging.- adapter = self._adapters[self._client] - try: - model_config = adapter.adapt_config(model_config, self.version_data) - except Exception as e: - if self._logger: - self._logger.warning(f"Error adapting configuration for client {self._client}: {str(e)}") + adapter = self._adapters.get(self._client) + if adapter is None: + raise UnsupportedClientError( + client_name=self._client, + available_clients=list(self._adapters.keys()) + ) + try: + model_config = adapter.adapt_config(model_config, self.version_data) + except (ConfigurationError, ValueError, TypeError) as e: + if self._logger: + self._logger.warning(f"Error adapting configuration for client {self._client}: {e!s}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/promptix/core/base_refactored.py
(1 hunks)src/promptix/core/builder_refactored.py
(1 hunks)src/promptix/core/components/prompt_loader.py
(1 hunks)src/promptix/core/components/variable_validator.py
(1 hunks)src/promptix/core/exceptions.py
(1 hunks)src/promptix/core/storage/loaders.py
(2 hunks)src/promptix/tools/studio/data.py
(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/promptix/core/components/prompt_loader.py (5)
src/promptix/core/exceptions.py (4)
StorageError
(182-184)StorageFileNotFoundError
(187-196)UnsupportedFormatError
(199-209)PromptNotFoundError
(27-36)src/promptix/core/storage/loaders.py (5)
PromptLoaderFactory
(113-125)PromptLoader
(84-96)get_loader
(115-125)load
(86-87)load
(99-103)src/promptix/core/storage/utils.py (1)
create_default_prompts_file
(12-87)src/promptix/core/config.py (3)
check_for_unsupported_files
(135-146)get_prompt_file_path
(49-62)get_default_prompt_file_path
(64-67)src/promptix/core/base_refactored.py (1)
reload_prompts
(253-257)
src/promptix/core/storage/loaders.py (1)
src/promptix/core/exceptions.py (1)
UnsupportedFormatError
(199-209)
src/promptix/tools/studio/data.py (2)
src/promptix/core/storage/loaders.py (2)
PromptLoaderFactory
(113-125)InvalidPromptSchemaError
(9-13)src/promptix/core/exceptions.py (1)
UnsupportedFormatError
(199-209)
src/promptix/core/builder_refactored.py (7)
src/promptix/core/container.py (6)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)register_adapter
(169-188)src/promptix/core/components/prompt_loader.py (2)
get_prompt_data
(105-122)get_prompts
(95-103)src/promptix/core/components/variable_validator.py (1)
validate_builder_type
(145-210)src/promptix/core/components/template_renderer.py (1)
render_tools_template
(71-129)src/promptix/core/components/version_manager.py (1)
find_live_version
(27-57)src/promptix/core/components/model_config_builder.py (2)
validate_memory_format
(76-117)build_model_config
(23-74)src/promptix/core/exceptions.py (8)
PromptNotFoundError
(27-36)VersionNotFoundError
(39-49)UnsupportedClientError
(156-165)ValidationError
(91-93)StorageError
(182-184)RequiredVariableError
(112-123)VariableValidationError
(96-109)TemplateRenderError
(76-86)
src/promptix/core/base_refactored.py (7)
src/promptix/core/container.py (5)
get_container
(257-266)get_typed
(130-151)get_typed
(233-250)get
(100-128)get
(220-231)src/promptix/core/components/prompt_loader.py (4)
PromptLoader
(16-141)get_prompt_data
(105-122)get_prompts
(95-103)reload_prompts
(132-141)src/promptix/core/components/variable_validator.py (2)
VariableValidator
(16-210)validate_variables
(27-101)src/promptix/core/components/template_renderer.py (3)
TemplateRenderer
(13-146)render_template
(29-69)validate_template
(131-146)src/promptix/core/components/version_manager.py (4)
VersionManager
(16-185)get_version_data
(59-93)get_system_instruction
(95-113)list_versions
(115-136)src/promptix/core/components/model_config_builder.py (2)
ModelConfigBuilder
(12-217)build_model_config
(23-74)src/promptix/core/exceptions.py (3)
PromptNotFoundError
(27-36)ConfigurationError
(140-148)StorageError
(182-184)
src/promptix/core/components/variable_validator.py (1)
src/promptix/core/exceptions.py (3)
VariableValidationError
(96-109)RequiredVariableError
(112-123)create_validation_error
(307-333)
🪛 Ruff (0.12.2)
src/promptix/core/components/prompt_loader.py
50-54: Abstract raise
to an inner function
(TRY301)
73-73: Consider moving this statement to an else
block
(TRY300)
90-90: Avoid specifying long messages outside the exception class
(TRY003)
93-93: Avoid specifying long messages outside the exception class
(TRY003)
src/promptix/core/builder_refactored.py
117-120: Avoid specifying long messages outside the exception class
(TRY003)
145-145: Avoid specifying long messages outside the exception class
(TRY003)
304-304: Unused method argument: kwargs
(ARG002)
440-440: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
441-441: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
456-456: Undefined name TemplateRenderError
(F821)
469-469: Do not catch blind exception: Exception
(BLE001)
471-471: Use explicit conversion flag
Replace with conversion flag
(RUF010)
564-564: Do not catch blind exception: Exception
(BLE001)
566-566: Use explicit conversion flag
Replace with conversion flag
(RUF010)
572-572: Do not catch blind exception: Exception
(BLE001)
574-574: Use explicit conversion flag
Replace with conversion flag
(RUF010)
src/promptix/core/components/variable_validator.py
51-51: Local variable optional
is assigned to but never used
Remove assignment to unused variable optional
(F841)
🔇 Additional comments (12)
src/promptix/core/exceptions.py (2)
11-23
: Solid base exception designClear message+details pattern and helpful str override. Looks good.
199-210
: Centralized UnsupportedFormatError is consistentConstructor and details payload are clear and now standardized across modules.
src/promptix/tools/studio/data.py (1)
19-21
: Correct 3-arg UnsupportedFormatError usageSignature matches the centralized definition. Good.
src/promptix/core/storage/loaders.py (2)
119-123
: Correct 3-arg UnsupportedFormatError usageMatches centralized signature and standardizes details. Approved.
9-14
: LGTM: schema error typeLocal
InvalidPromptSchemaError
keeps concerns scoped; message is clear.src/promptix/core/components/variable_validator.py (1)
50-53
: Support JSON-schemaproperties
&additionalProperties
in validate_variables
Validator only readsschema["types"]
, so it ignoresschema["properties"]
(includingenum
) and doesn’t enforceadditionalProperties: False
. Extendvalidate_variables
in src/promptix/core/components/variable_validator.py to:
- Read
properties = schema.get("properties", {})
, handling each property’stype
andenum
- Respect
additionalProperties = schema.get("additionalProperties", True)
when checking unknown variables
Verify your prompt schemas define types/enums underproperties
(not justtypes
) and useadditionalProperties
as intended.src/promptix/core/base_refactored.py (3)
77-89
: Exception narrowing and chaining — well done.Catching
StorageError
only and re-raisingPromptNotFoundError
withfrom err
preserves cause and avoids masking bugs.
183-192
: Reuse ofrender_prompt
for building system message is acceptable.Ensures one canonical validation/rendering path; minor duplicate access is cached by the loader.
228-239
: Consistent error mapping in list_versions — LGTM.Same narrowed catch and chaining pattern applied here.
src/promptix/core/builder_refactored.py (3)
64-89
: Nice: clean, typed init flow with cause-chained re-raiseScoped
StorageError
catch +from err
is solid; schema extraction looks correct.
446-460
: Good: scoped catch for template rendering errorsCatching only
TemplateRenderError
and logging with{e!s}
is the right trade-off.
516-523
: Raise on missing required fields instead of warn-only
The builder (src/promptix/core/builder_refactored.py 516–523) currently only logs absent required fields and falls back to a generic prompt. To avoid masking errors, consider raising RequiredVariableError instead of merely warning.
This release improves modularity and testability of the Promptix library.
Summary by CodeRabbit
New Features
Refactor
Tests
Chores