refactor: reorganize packages into capabilities directory

[Kamilbenkirane]

Summary

• Move all capability packages from packages/ to packages/capabilities/
• Update workspace configuration and test paths in pyproject.toml
• Update Makefile paths for typecheck, test, integration-test

This prepares for v0.3 architecture which separates capabilities from providers.

Test plan

• All pre-commit hooks pass (mypy, ruff, bandit)
• Unit tests pass with coverage
• Integration tests pass

[claude]

PR Review: Reorganize packages into capabilities directory

Summary

This PR successfully reorganizes the package structure by moving all capability packages from packages/ to packages/capabilities/. This is a well-executed refactoring that prepares the codebase for the v0.3 architecture separating capabilities from providers.

---

✅ Strengths

1. Clean file reorganization: All 157 files have been moved correctly without any code changes
2. Comprehensive path updates: The Makefile and pyproject.toml have been updated with the new paths
3. Test coverage: All pre-commit hooks, unit tests, and integration tests pass according to the PR description
4. Good README updates: LICENSE path references in package READMEs correctly updated from ../../LICENSE to ../../../LICENSE
5. Integration test enhancement: Added proper HTTP client cleanup fixture in speech-generation integration tests to prevent "Event loop is closed" errors with pytest-xdist

---

⚠️ Issues Found

Critical: GitHub Actions Workflows Not Updated

The CI/CD workflows still reference the old package paths and will fail:

File: .github/workflows/ci.yml

• Line 67: Type check still uses old paths

  uv run mypy packages/image-generation packages/text-generation packages/video-generation packages/speech-generation

  Should be:

  uv run mypy packages/capabilities/image-generation packages/capabilities/text-generation packages/capabilities/video-generation packages/capabilities/speech-generation

File: .github/workflows/publish.yml

• Lines 71-78: Path filters use old structure

  text-generation:
    - 'packages/text-generation/**'
  image-generation:
    - 'packages/image-generation/**'

  Should be:

  text-generation:
    - 'packages/capabilities/text-generation/**'
  image-generation:
    - 'packages/capabilities/image-generation/**'

• Line 139: Integration test path uses old structure

  run: uv run pytest packages/${{ matrix.package }}/tests/integration_tests/ -m integration -v

  Should be:

  run: uv run pytest packages/capabilities/${{ matrix.package }}/tests/integration_tests/ -m integration -v

---

📋 Recommendations

Before Merging:

1. Update .github/workflows/ci.yml line 67 with correct package paths
2. Update .github/workflows/publish.yml lines 71-78 and 139 with correct package paths
3. Run a local test to verify the Makefile commands work correctly:

   make typecheck
   make test

Code Quality:

The integration test fixture addition (packages/capabilities/speech-generation/tests/integration_tests/conftest.py:11-20) is excellent:

• Properly uses pytest_asyncio.fixture with autouse=True
• Correctly implements cleanup with AsyncGenerator
• Good docstring explaining the purpose
• Prevents race conditions with pytest-xdist

---

🔒 Security

No security concerns identified. This is a pure refactoring with no logic changes.

---

⚡ Performance

No performance impact. This is a structural change only.

---

🧪 Test Coverage

Test coverage appears adequate based on the PR description. The addition of the HTTP client cleanup fixture actually improves test reliability.

---

📝 Documentation

README files have been updated correctly with adjusted relative paths. Consider adding a migration note for developers who might have local paths referencing the old structure.

---

Overall Assessment

This is a well-planned refactoring that improves the codebase organization. However, the GitHub Actions workflows must be updated before merging to prevent CI/CD failures. Once those are fixed, this PR will be ready to merge.

Status: Approve pending workflow updates ✋

[claude]

Pull Request Review: Package Reorganization

Summary

This PR reorganizes the package structure by moving capability packages from packages/ to packages/capabilities/, preparing for the v0.3 architecture. This is a solid refactoring that improves code organization.

---

✅ Strengths

1. Clean Structural Change: The refactoring is well-scoped and focused on directory reorganization
2. Configuration Updated: The pyproject.toml workspace configuration correctly updated to packages/capabilities/*
3. Build Tools Updated: Makefile paths properly updated for typecheck, test, and integration-test targets
4. Documentation Updated: README files in capability packages correctly updated their relative paths to LICENSE (from ../../LICENSE to ../../../LICENSE)
5. Test Coverage: According to the PR description, all pre-commit hooks, unit tests, and integration tests pass

---

⚠️ Issues Found

Critical: GitHub Workflows Not Updated

The GitHub Actions workflow files still reference the old paths and need to be updated:

.github/workflows/ci.yml:67

# Current (incorrect):
uv run mypy -p celeste && uv run mypy tests/ && uv run mypy packages/image-generation packages/text-generation packages/video-generation packages/speech-generation

# Should be:
uv run mypy -p celeste && uv run mypy tests/ && uv run mypy packages/capabilities/image-generation packages/capabilities/text-generation packages/capabilities/video-generation packages/capabilities/speech-generation

.github/workflows/publish.yml:72-78

# Current (incorrect):
filters: |
  text-generation:
    - 'packages/text-generation/**'
  image-generation:
    - 'packages/image-generation/**'
  video-generation:
    - 'packages/video-generation/**'
  speech-generation:
    - 'packages/speech-generation/**'

# Should be:
filters: |
  text-generation:
    - 'packages/capabilities/text-generation/**'
  image-generation:
    - 'packages/capabilities/image-generation/**'
  video-generation:
    - 'packages/capabilities/video-generation/**'
  speech-generation:
    - 'packages/capabilities/speech-generation/**'

.github/workflows/publish.yml:139

# Current (incorrect):
run: uv run pytest packages/${{ matrix.package }}/tests/integration_tests/ -m integration -v

# Should be:
run: uv run pytest packages/capabilities/${{ matrix.package }}/tests/integration_tests/ -m integration -v

These issues will cause CI failures:

• Type checking will fail because mypy won't find the packages at the old paths
• The publish workflow's change detection won't trigger for capability package changes
• Integration tests won't run from the correct directories

---

📋 Recommendations

Required Changes

1. Update .github/workflows/ci.yml: Change line 67 to reference packages/capabilities/*
2. Update .github/workflows/publish.yml:
   • Lines 72-78: Update path filters to packages/capabilities/*
   • Line 139: Update pytest path to include capabilities/ subdirectory

Optional Improvements

1. Consider CONTRIBUTING.md: The README files reference ../../CONTRIBUTING.md, but this file doesn't exist in the repository. Consider either:

   • Creating a CONTRIBUTING.md file, or
   • Removing references to it from the capability README files

2. Logo Path in READMEs: The video-generation and text-generation READMEs reference ../../logo.svg, which would now be at the wrong path level (should be ../../../logo.svg after the move). However, I notice the image-generation and speech-generation READMEs use absolute GitHub URLs for the logo, which is more robust. Consider standardizing this approach.

---

🎯 Conclusion

This is a well-intentioned refactoring that aligns with the v0.3 architecture goals. However, the GitHub Actions workflows must be updated before merging to prevent CI/CD failures. Once the workflow files are corrected, this PR will be ready to merge.

Recommendation: Request changes to update the workflow files, then approve once those are addressed.

[claude]

Pull Request Review - PR #72

Overall Assessment: ✅ Approved with Minor Recommendations

This is a well-executed refactoring that reorganizes the package structure to prepare for v0.3 architecture. The changes are primarily structural (moving files) with appropriate configuration updates.

---

✅ Strengths

1. Clean Structural Refactoring

   • All capability packages correctly moved from packages/ to packages/capabilities/
   • Consistent directory structure maintained across all capabilities
   • No code logic changes, minimizing risk of introducing bugs

2. Comprehensive Configuration Updates

   • ✅ pyproject.toml:62 - Workspace members correctly updated to packages/capabilities/*
   • ✅ pyproject.toml:82-88 - Test pythonpath correctly updated for all capabilities
   • ✅ Makefile:36,40,44 - All make targets (typecheck, test, integration-test) updated with new paths
   • ✅ .github/workflows/ci.yml:67 - CI type-check step updated with new paths

3. Documentation Updates

   • README files updated with corrected relative paths for LICENSE links (from ../../LICENSE to ../../../LICENSE)

4. Testing Verified

   • PR description indicates all pre-commit hooks pass (mypy, ruff, bandit)
   • Unit and integration tests passing
   • Good test coverage maintained

---

🔍 Issues Found

❗ Critical Issue: Inconsistent README Paths

Location: All capability package README files

Problem: The README files have inconsistent relative paths after the restructuring:

• logo.svg reference: Still uses ../../logo.svg (should be ../../../logo.svg)
• CONTRIBUTING.md reference: Still uses ../../CONTRIBUTING.md (should be ../../../CONTRIBUTING.md)
• LICENSE reference: ✅ Correctly updated to ../../../LICENSE

Files Affected:

• packages/capabilities/image-generation/README.md:3,61
• packages/capabilities/speech-generation/README.md:3,61
• packages/capabilities/text-generation/README.md:3,63
• packages/capabilities/video-generation/README.md:3,61

Note: logo.svg exists at the repository root. CONTRIBUTING.md doesn't currently exist but the links should still be consistent for future-proofing.

Recommended Fix:

# Before (incorrect):
<img src="../../logo.svg" ...>
See [CONTRIBUTING.md](../../CONTRIBUTING.md) for guidelines.

# After (correct):
<img src="../../../logo.svg" ...>
See [CONTRIBUTING.md](../../../CONTRIBUTING.md) for guidelines.

---

💡 Recommendations

1. Fix README Relative Paths (High Priority)

   • Update logo.svg and CONTRIBUTING.md links in all 4 capability READMEs to use ../../../ instead of ../../

2. Consider CI Path Generalization (Low Priority)

   • The CI workflow paths in .github/workflows/ci.yml are hardcoded for each capability
   • Consider using wildcards if possible: packages/capabilities/*/ to make adding new capabilities easier
   • Current approach is fine for now, but worth considering for future scalability

3. Version Bump Consideration

   • This is a structural change that doesn't affect public APIs
   • Version numbers appear unchanged (e.g., celeste-image-generation@0.2.10)
   • Confirm whether this warrants a version bump based on your versioning strategy

---

🔒 Security Assessment

✅ No security concerns identified

• No code logic changes
• No new dependencies
• No sensitive data exposure
• Build and security scanning (bandit) tools still properly configured

---

🎯 Performance Considerations

✅ No performance impact

• File moves don't affect runtime performance
• Import paths remain unchanged (package names are the same)
• Test execution paths properly updated

---

🧪 Test Coverage

✅ Test coverage maintained

• All test files moved correctly with their respective packages
• Test paths updated in pyproject.toml
• Integration and unit tests verified passing per PR description
• Coverage threshold (80%) still enforced

---

📋 Code Quality

✅ Excellent code quality practices observed

• Type checking with mypy maintained
• Linting with ruff configured
• Pre-commit hooks enforced
• Consistent formatting

---

🎬 Recommendation

Action Required: Fix the README relative paths for logo.svg and CONTRIBUTING.md before merging.

Once that's addressed, this PR is ready to merge. The refactoring is clean, well-tested, and sets up a good foundation for the v0.3 architecture separating capabilities from providers.

---

Reviewed by: Claude Sonnet 4.5
Review Date: 2025-12-14

[claude]

PR Review: Reorganize packages into capabilities directory

Summary

This PR successfully reorganizes the package structure by moving all capability packages from packages/ to packages/capabilities/. This is a pure refactoring change with no code modifications - only directory restructuring and path updates.

---

✅ Strengths

1. Thorough Path Updates

• All GitHub workflows (.github/workflows/ci.yml, .github/workflows/publish.yml) correctly updated
• Makefile paths updated for typecheck, test, and integration-test targets
• pyproject.toml workspace configuration updated ([tool.uv.workspace] members)
• Test paths in pyproject.toml updated (pythonpath settings)
• README files updated with corrected LICENSE links

2. Consistent Implementation

• All 4 capability packages moved uniformly: text-generation, image-generation, video-generation, speech-generation
• Package internal structure preserved correctly
• No code changes, purely structural - minimizes risk

3. CI/CD Alignment

• Workflow filters updated to match new paths
• Integration test paths updated correctly
• Type checking paths updated in all locations

---

⚠️ Issues Found

CRITICAL: Missing __init__.py Files

The refactor appears to have lost several __init__.py files in test directories:

Missing files:

• packages/capabilities/image-generation/tests/__init__.py
• packages/capabilities/image-generation/tests/unit_tests/__init__.py
• packages/capabilities/speech-generation/tests/__init__.py
• packages/capabilities/text-generation/tests/__init__.py
• packages/capabilities/video-generation/tests/__init__.py

Evidence:

# Current state shows NO __init__.py in these locations:
packages/capabilities/image-generation/tests/
├── integration_tests/
│   └── test_image_generation/__init__.py  ✅
└── unit_tests/
    └── providers/google/__init__.py  ✅
# Missing: tests/__init__.py and unit_tests/__init__.py ❌

Impact:
While Python 3.3+ supports namespace packages without __init__.py, this inconsistency could cause:

• Import issues in some test runners
• Inconsistent behavior across environments
• pytest discovery problems in certain configurations

Recommendation:
Verify if these files existed before the move. If they did, they need to be restored. If they didn't, consider adding them for consistency with the integration_tests directories that do have them.

---

📋 Code Quality Assessment

Best Practices: ✅ Excellent

• Clean git history (file moves, not copies)
• Atomic change - single responsibility
• No premature optimization or over-engineering

Architecture: ✅ Good

• Logical separation of capabilities into subdirectory
• Prepares for v0.3 architecture (capabilities vs providers)
• Maintains existing package structure

Testing: ⚠️ Needs Verification

• Test paths updated correctly in config
• Pre-commit hooks reportedly passing
• Unit and integration tests reportedly passing
• BUT: Missing __init__.py files need verification

---

🔒 Security Considerations

No security concerns identified:

• No code changes, only directory moves
• No new dependencies
• No changes to authentication, validation, or data handling
• Bandit security scans still configured correctly

---

⚡ Performance Considerations

No performance impact:

• Pure structural change
• Import paths unchanged (packages use absolute imports via pyproject.toml configuration)
• No runtime behavior changes

---

🧪 Test Coverage

Configuration: ✅ Updated correctly

• pytest paths: packages/capabilities/*/tests/unit_tests ✅
• Coverage paths: maintained ✅
• Integration test paths: packages/capabilities/*/tests/integration_tests/ ✅

Verification needed:

• Confirm all tests still pass locally
• Verify test discovery works correctly despite missing __init__.py files

---

📝 Action Items

Before merging:

1. CRITICAL: Verify __init__.py files in test directories

   # Check if these should exist:
   find packages/capabilities/*/tests -type d | while read dir; do
     if [ ! -f "$dir/__init__.py" ]; then
       echo "Missing: $dir/__init__.py"
     fi
   done

2. Recommended: Run full test suite locally to confirm:

   make test
   make integration-test
   make typecheck

3. Nice to have: Consider adding a test to CI that validates package structure integrity

---

🎯 Verdict

Overall: Strong refactoring with one critical issue to address

This is a well-executed structural refactor that successfully reorganizes the codebase for the v0.3 architecture. The path updates are comprehensive and consistent across all configuration files.

However, the missing __init__.py files in test directories need to be addressed before merging to ensure test discovery works consistently across all environments.

Recommendation: Request changes to verify/fix the missing __init__.py files, then approve.

---

💡 Suggestions for Future

1. Consider documenting the new structure in a top-level ARCHITECTURE.md
2. Add validation in CI to catch missing __init__.py files in package directories
3. Document the capabilities vs providers separation for contributors

---

Great work on this refactor! The execution is clean and the preparation for v0.3 is well-planned. Just need to verify those test directory __init__.py files and you're good to go! 🚀