Skip to content

refactor: standardize capability naming to kebab-case#24

Merged
Kamilbenkirane merged 2 commits intomainfrom
refactor/capability-naming-kebab-case
Nov 9, 2025
Merged

refactor: standardize capability naming to kebab-case#24
Kamilbenkirane merged 2 commits intomainfrom
refactor/capability-naming-kebab-case

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

Summary

This PR standardizes capability naming conventions across the codebase by changing from snake_case to kebab-case, improving consistency and alignment with package naming conventions.

Changes

Capability Naming Standardization

  • Updated all capability enum values from snake_case to kebab-case:
    • text_generationtext-generation
    • image_generationimage-generation
    • image_intelligenceimage-intelligence
    • video_generationvideo-generation
    • video_intelligencevideo-intelligence
    • audio_intelligenceaudio-intelligence

Package Entry Points

  • Updated entry point name in pyproject.toml from text_generation to text-generation to match the new naming convention

Test Organization

  • Moved integration tests from tests/integration_tests/ to package-specific locations (packages/*/tests/integration_tests/)
  • Updated Makefile to reflect new test paths

Code Quality Improvements

  • Updated error messages to use capability enums directly instead of .value for better type safety
  • Removed deprecated claude-sonnet-3-7 model from Anthropic provider

Impact

  • Breaking Change: This changes the string representation of capabilities, which may affect any code that relies on the string value
  • Consistency: Aligns capability naming with package naming conventions (kebab-case)
  • Type Safety: Error messages now use enum values directly, improving type checking

Testing

  • ✅ All unit tests updated and passing
  • ✅ All integration tests updated and passing
  • ✅ Pre-commit hooks passed (linting, formatting, type checking, security checks)
  • ✅ Test coverage maintained

- Update version in root pyproject.toml
- Update version in text-generation package
- This version includes optional dependencies configuration
- Change capability enum values from snake_case to kebab-case
  (e.g., text_generation → text-generation)
- Update entry point name to match kebab-case convention
- Move integration tests to package-specific directories
- Update Makefile to reflect new test paths
- Remove deprecated claude-sonnet-3-7 model
- Update error messages to use capability enums directly
@claude
Copy link
Copy Markdown

claude Bot commented Nov 9, 2025

Code Review Summary

This PR introduces a solid refactoring to standardize capability naming from snake_case to kebab-case. The changes are well-implemented and comprehensive.


✅ Strengths

Code Quality

  • Excellent consistency: The naming convention change is applied systematically across the entire codebase
  • Type safety improvements: Changed error messages to use enum values directly instead of .value, which improves type checking
  • Comprehensive test updates: All tests properly updated to reflect the new naming convention
  • Clean deprecation: Removed outdated claude-sonnet-3-7 model appropriately

Best Practices

  • Good test coverage: Test coverage maintained at high standards with both unit and integration tests updated
  • Proper versioning: Both main package and text-generation package versions bumped from 0.1.0 to 0.1.1
  • Clear documentation: PR description clearly identifies this as a breaking change with impact assessment

🔍 Areas for Improvement

1. Breaking Change Documentation (High Priority)

Issue: This is a breaking change that will affect users, but there's no CHANGELOG or migration guide.

Recommendation:

  • Add a CHANGELOG.md entry documenting this breaking change
  • Consider adding a migration guide showing the old vs new values
  • Document in CHANGELOG: BREAKING: Capability enum values changed from snake_case to kebab-case

2. Test Path Organization (Medium Priority)

Location: Makefile:44

The glob pattern packages/*/tests/integration_tests/ might cause issues if some packages don't have integration tests yet.

Recommendation:

  • Ensure all packages follow the same test directory structure
  • Verify pytest doesn't fail when some packages lack integration tests

3. Error Message Improvement (Low Priority)

Locations: src/celeste/client.py:33, src/celeste/client.py:243

Consider whether error messages would be clearer showing both the enum value and name to help users understand both the string value and enum name.

4. Model Removal Documentation (Low Priority)

Location: packages/text-generation/src/celeste_text_generation/providers/anthropic/models.py

Document why claude-sonnet-3-7 model was removed (deprecated by Anthropic, placeholder, etc.)


🔒 Security Considerations

No security concerns identified. The changes are purely refactoring with no impact on authentication, input validation, or data handling.


⚡ Performance Considerations

No performance impact. The changes are compile-time (enum values) with no runtime performance implications.


🧪 Test Coverage

Excellent: All affected tests updated comprehensively with good use of parametrized tests and proper regex escaping.


📋 Recommendations Summary

Must Have (Before Merge):

  1. Add CHANGELOG.md entry documenting the breaking change with before/after examples

Should Have:

  1. Add migration guide for users
  2. Verify packages/*/tests/integration_tests/ glob pattern works correctly

Nice to Have:

  1. Document the reason for claude-sonnet-3-7 removal
  2. Consider enhancing error messages to show both enum name and value

🎯 Overall Assessment

Quality Score: 8.5/10

This is a well-executed refactoring that improves consistency across the codebase. The main gap is documentation around the breaking change. Once the CHANGELOG is added and the migration path is clear for users, this is ready to merge.

Approval Status: ✅ Approved with minor recommendations

Great work on maintaining test coverage and consistency throughout this refactoring!

@Kamilbenkirane Kamilbenkirane merged commit 08634f7 into main Nov 9, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant