feat: v1 architecture migration - modality-centric API

[Kamilbenkirane]

Summary

• Architecture shift: Migrate from capability-centric (v0.3.x) to modality-centric (v1) architecture
• Single package install: pip install celeste-ai (no extras required, removed multi-package structure)
• Namespace API: Domain-first entry points (celeste.text, celeste.images, celeste.audio, celeste.videos) with .sync and .stream execution modes
• New operations: images.edit, images.analyze, audio.analyze, videos.analyze
• All providers bundled: OpenAI, Anthropic, Google, Mistral, Cohere, xAI, ElevenLabs, BFL, Gradium, BytePlus, DeepSeek, Groq, Moonshot

Key Changes

Architecture

• Clients organized by output modality (text/images/audio/videos/embeddings)
• Operations become methods: generate, edit, analyze, embed, speak
• Domain = resource you work with; Modality = output type
• Cross-domain operations explicit: image/audio/video analysis → text modality

API

• create_client(Modality.TEXT, Operation.GENERATE) replaces capability-based client creation
• extra_body parameter for provider-specific options
• Artifact API: get_bytes(), get_base64() replace to_data_url()

Packaging

• Removed packages/ directory (old multi-package workspace)
• No entry-point discovery = faster startup
• Single version, single release pipeline

Files

• Added: src/celeste/modalities/, src/celeste/namespaces/, src/celeste/providers/, src/celeste/utils/
• Removed: packages/, src/celeste/registry.py, src/celeste/utils.py
• Updated: Core modules, tests, CI/CD workflows

Test Plan

• All pre-commit hooks pass (ruff, mypy, bandit)
• Unit tests pass
• Integration tests (require API keys in CI)
• Manual smoke test of namespace API

See CHANGELOG_V1.md for complete migration details.

[claude]

Pull Request Review: v1 Architecture Migration

Executive Summary

This is a major architectural refactor that transitions from a capability-centric to a modality-centric design. The scope is substantial (18,751 additions, 8,960 deletions across 300+ files), representing a complete restructuring of the SDK. Overall, the architecture is well-designed, but there are several areas that need attention before merging.

Recommendation: Approve with minor changes required.

---

🎯 Architecture Review

Strengths

1. Clean Separation of Concerns

   • The modality-centric approach (Modality.TEXT, Modality.IMAGES, etc.) is more intuitive than capability-based design
   • Clear distinction between domain (resource you work with) and modality (output type)
   • Namespace API (celeste.text.generate()) provides excellent DX with IDE autocomplete

2. Consistent API Design

   • Unified ModalityClient base class with generic types is elegant
   • The APIMixin pattern for provider implementations is clean and extensible
   • extra_body parameter provides escape hatch for provider-specific features

3. Type Safety

   • Extensive use of Pydantic for validation
   • Generic types on ModalityClient[In, Out, Params, Content] ensure type correctness
   • Proper use of TypedDict with Unpack for parameters

4. Single Package Install

   • Removing the multi-package structure significantly improves DX
   • No more confusion about which extras to install
   • Faster startup without entry-point discovery

Concerns

1. Breaking Changes Scale

   • This is effectively a v1 rewrite. Users will need significant migration effort
   • The deprecation path for capability parameter is good, but you might want to document migration more prominently
   • Consider providing a migration guide with code examples

2. Namespace API Client Reuse

   # In namespaces/domains.py, every call creates a new client
   async def generate(self, prompt: str, *, model: str, ...):
       client = create_client(...)  # New client every call!
       return await client.generate(prompt, ...)

   • This means no connection pooling benefits for namespace API users
   • HTTP clients are cached by modality/provider, but auth objects are recreated each time
   • Recommendation: Document this limitation or consider caching clients at the namespace level

3. Error Handling in _make_request

   # src/celeste/client.py:169
   response_data = await self._make_request(request_body, ...)

   • Provider implementations now return dict[str, Any] directly
   • The comment says "error handling is expected inside provider implementations"
   • Risk: Inconsistent error handling across providers
   • Recommendation: Establish clear guidelines or base error handling in provider template

---

🐛 Potential Bugs & Issues

1. HTTP Client Event Loop Recreation (src/celeste/http.py)

The CHANGELOG mentions:

"HTTP client now recreates the httpx.AsyncClient if the event loop changes"

Question: How is the event loop change detected? I didn't see the implementation in the diff. If you're caching clients by modality/provider globally, event loop changes in multi-loop scenarios could cause issues.

Recommendation: Verify this is properly implemented or document the limitation.

2. Deep Merge Behavior (src/celeste/client.py:55-77)

def _deep_merge(target: dict[str, Any], source: dict[str, Any]) -> dict[str, Any]:
    for key, value in source.items():
        if key in target and isinstance(target[key], dict) and isinstance(value, dict):
            APIMixin._deep_merge(target[key], value)
        else:
            target[key] = value
    return target

Issues:

• Mutates target in place (returns modified target, not a copy)
• No handling of list merging (overwrites lists entirely)
• Could lead to unexpected behavior if extra_body contains nested structures

Recommendation: Document the merge behavior clearly, or use a more defensive copy-based approach.

3. Model Resolution Logic (src/celeste/init.py:84-133)

if model is None:
    models = list_models(provider=provider, modality=modality, operation=operation)
    if not models:
        raise ModelNotFoundError(...)
    return models[0]  # Returns first model - deterministic?

Question: Is the order of models[0] deterministic? If not, auto-selection could be unpredictable.

Recommendation: Document the selection criteria (alphabetical? priority-based?) or make it explicit.

4. Synchronous Namespace Pattern

# namespaces/domains.py:139
return async_to_sync(self._client._predict)(inputs, **parameters)

Using asgiref.sync.async_to_sync is fine, but:

• Creates a new event loop if none exists
• Can cause issues in already-async contexts
• May have performance implications

Recommendation: Document that .sync methods should not be called from async contexts, or add runtime detection.

---

🔒 Security Review

Strengths

✅ No dangerous patterns found:

• No eval, exec, compile, or __import__ usage
• No unsafe deserialization (pickle, marshal, shelve)
• No command injection risks (subprocess, os.system)
• Bandit security scanning in CI (pyproject.toml:182-187)

✅ Proper secrets handling:

• All credentials use pydantic.SecretStr
• Secrets loaded from environment variables via pydantic-settings
• .env files supported via python-dotenv

Recommendations

1. API Key Validation

   # src/celeste/credentials.py:88-122
   def get_credentials(self, provider: Provider, override_key: str | SecretStr | None = None):
       if override_key:
           if isinstance(override_key, str):
               return SecretStr(override_key)  # No validation!

   • No validation of API key format before use
   • Risk: Silent failures if malformed keys are provided
   • Recommendation: Consider basic format validation (length, prefix checks) for common providers

2. HTTP Error Responses

   # src/celeste/client.py:290-303
   def _handle_error_response(self, response: httpx.Response) -> None:
       error_msg = error_data.get("error", {}).get("message", response.text)

   • Error messages from provider APIs are included in exceptions
   • Risk: Could leak sensitive info in logs if providers return unexpected data
   • Recommendation: Sanitize or limit error message content in production

3. Extra Body Parameter

   extra_body: dict[str, Any] | None = None

   • Allows arbitrary JSON to be merged into requests
   • Risk: Users could accidentally override critical fields
   • Recommendation: Document which fields are protected or consider field whitelisting

---

🧪 Test Coverage Assessment

Current State

• 48 test files (good coverage scope)
• Unit tests: Core functionality covered (artifacts, auth, credentials, parameters, streaming)
• Integration tests: Modality-specific tests for generate/edit/analyze operations
• Coverage target: 80% (pyproject.toml:81)
• Coverage exclusions: Provider and modality implementations omitted (intended for integration tests)

Concerns

1. PR Description:

   • All pre-commit hooks pass (ruff, mypy, bandit)
   • Unit tests pass
   • Integration tests (require API keys in CI)
   • Manual smoke test of namespace API

   Integration tests are not passing in CI, which is understandable but risky for a major refactor.

2. Provider Implementation Testing

   • Provider code is excluded from coverage (pyproject.toml:73-76)
   • Risk: Provider implementations could have untested edge cases
   • Recommendation: Run integration tests against at least 2-3 providers before stable v1.0

3. Namespace API Testing

   • The namespace API is the recommended entry point but manual testing is incomplete
   • Recommendation: Add smoke tests for namespace API even if they mock the underlying client

Test Quality (Spot Check)

From the structure, tests appear well-organized:

tests/
├── integration_tests/
│   ├── embeddings/
│   ├── images/
│   └── videos/
└── unit_tests/
    └── utils/

Recommendation: Consider adding:

• Tests for concurrent client usage (connection pooling)
• Tests for sync/async namespace interaction
• Tests for extra_body parameter edge cases

---

⚡ Performance Considerations

Positive

1. Connection Pooling: HTTP clients are cached by (provider, modality) - good for reuse
2. No Entry Point Discovery: Faster startup than v0.3.x
3. Lightweight Dependencies: No vendor SDKs, pure httpx

Concerns

1. Client Creation Overhead

   • Namespace API creates a new client on every call (as noted above)
   • Includes credential validation, auth object creation, and client instantiation
   • Impact: Minor for one-off calls, noticeable for high-frequency usage
   • Recommendation: Document performance characteristics or add client caching

2. Stream Metadata Building

   # src/celeste/streaming.py:140
   def _build_stream_metadata(self, events: list[dict[str, Any]]) -> dict[str, Any]:
       return {"raw_events": events}  # Accumulates all events in memory

   • Stores all stream events in metadata
   • Risk: Memory accumulation for long-running streams
   • Recommendation: Document this behavior or make metadata collection optional

3. Deep Merge Performance

   • Recursive dictionary merging on every request when extra_body is used
   • Impact: Negligible for typical use, but could matter for deeply nested payloads
   • Recommendation: Profile if this becomes a bottleneck

---

📦 Code Quality & Best Practices

Strengths

✅ Excellent tooling setup:

• Ruff for linting and formatting
• MyPy for type checking with strict settings
• Bandit for security scanning
• Pre-commit hooks configured
• CI pipeline with multi-platform testing (Linux, macOS, Windows)

✅ Code style:

• Consistent naming conventions
• Good use of type hints throughout
• Clear docstrings on public APIs
• Proper use of __all__ for public exports

✅ Architecture patterns:

• Good use of ABC for base classes
• Proper separation of concerns
• Registry pattern for providers/models/auth
• Generic types for type safety

Areas for Improvement

1. Documentation Comments

   # src/celeste/client.py:330
   _ = streaming  # Passed through to provider mixins

   • Some implementation details rely on comments
   • Recommendation: Consider more explicit parameter forwarding or better docstrings

2. Type Ignore Comments

   # Multiple locations
   return super()._build_request(...)  # type: ignore[misc,no-any-return]

   • Several type: ignore comments in the mixin pattern
   • Note: This is acceptable for cooperative multiple inheritance, but verify MyPy configuration excludes these paths appropriately (✅ confirmed in pyproject.toml:162-180)

3. Magic Method Implementation

   # src/celeste/streaming.py (mentioned in CHANGELOG)
   # Added sync iteration support via anyio blocking portals

   • Sync context managers and iteration added for Stream class
   • Risk: Blocking portals can be tricky with nested event loops
   • Recommendation: Add documentation warnings about sync usage in async contexts

---

🔄 Migration & Backwards Compatibility

Good Practices

✅ Deprecation warning for capability parameter:

warnings.warn(
    "capability parameter is deprecated, use modality/operation instead",
    DeprecationWarning,
    stacklevel=2,
)

✅ Translation layer maintains compatibility:

_CAPABILITY_TO_MODALITY_OPERATION = {
    Capability.TEXT_GENERATION: (Modality.TEXT, Operation.GENERATE),
    # ...
}

Recommendations

1. Version Communication

   • Version is set to 0.9.0 (beta) - appropriate for v1 preview
   • README has a clear v1 beta callout banner - good
   • Recommendation: Add migration guide to documentation (beyond CHANGELOG_V1.md)

2. Breaking Changes Documentation

   • README includes "Behavior changes since v0.3.9" section - excellent
   • CHANGELOG_V1.md is comprehensive (154 lines)
   • Recommendation: Consider a dedicated migration tool or compatibility shim package for large projects

3. API Surface Stability

   • The namespace API is marked as "recommended" entry point
   • create_client is marked as "advanced"
   • Question: Will the namespace API signatures remain stable through v1.0?
   • Recommendation: Document API stability guarantees for the beta period

---

📋 Specific File Comments

src/celeste/init.py

• Clean public API exports
• Good deprecation handling
• ✅ Circular import avoided by importing namespaces last (line 288)

src/celeste/client.py

• Well-structured base classes
• APIMixin and ModalityClient are clean abstractions
• ⚠️ Deep merge mutates in place (mentioned above)

src/celeste/namespaces/domains.py

• 890 lines of repetitive code (4 domains × sync/async/stream namespaces)
• Consideration: Could use metaclasses or code generation to reduce boilerplate
• Note: Repetition ensures type safety and IDE autocomplete, so this is acceptable

src/celeste/credentials.py

• Good use of pydantic-settings
• ✅ Proper handling of auth classes vs API keys
• ⚠️ No validation on override keys (mentioned above)

pyproject.toml

• Excellent configuration
• Strict MyPy settings with appropriate exclusions
• Good coverage configuration
• ✅ Security scanning enabled

---

🎯 Final Recommendations

Must Fix (Before Merge)

1. Verify HTTP client event loop handling - Confirm implementation or document limitations
2. Document deep merge behavior - Clarify mutation and list handling
3. Add namespace API smoke tests - Even with mocked clients

Should Fix (Before v1.0 Stable)

1. Run integration tests - At least 2-3 providers before stable release
2. Add API key format validation - Prevent silent failures
3. Document performance characteristics - Especially for namespace API vs client reuse
4. Create migration guide - Beyond CHANGELOG, with code examples
5. Verify sync/async boundary behavior - Add runtime warnings if needed

Nice to Have (Post v1.0)

1. Provider response caching - Optional caching layer for repeated calls
2. Telemetry/observability hooks - For production monitoring
3. Client pooling for namespace API - Improve performance for high-frequency usage
4. Migration tooling - Automated migration from v0.3.x to v1.x

---

✅ Conclusion

This is a well-architected refactor that significantly improves the SDK's design and developer experience. The modality-centric approach is intuitive, the type safety is excellent, and the namespace API is a major DX win.

The main concerns are around:

1. Testing completeness (integration tests not run)
2. Documentation gaps (namespace API client reuse, sync/async boundaries)
3. Minor implementation details (deep merge, error handling consistency)

None of these are blockers for a beta release (v0.9.0), but they should be addressed before v1.0 stable.

Overall Assessment: 🟢 Approve with recommendations

Great work on this major refactor! The architecture is solid and the migration path is well-thought-out. Address the testing and documentation gaps, and this will be a strong v1 release.

---

Reviewed by: Claude Code (Sonnet 4.5)
Review Date: 2026-01-15