feat: Add multi-turn conversation support and OpenResponses/Ollama providers

[Kamilbenkirane]

Summary

This PR adds two major features to Celeste v1:

1. Multi-turn conversation support - First-class Message and Role types for building conversational workflows
2. OpenResponses + Ollama providers - Support for OpenAI Responses-compatible APIs and local model hosting

Changes

Messages (Multi-Turn Support)

• Adds Message and Role types (user, assistant, system, developer)
• All text entrypoints (generate, analyze, namespace APIs) now accept messages= parameter
• When messages are provided, they take precedence over prompt
• Provider normalization handles different API formats (Anthropic system messages, Google system_instruction, chat vs responses arrays)
• Foundation for agents and multi-step workflows

OpenResponses + Local Support

• New Provider.OPENRESPONSES implementing OpenAI Responses-compatible API (POST /v1/responses + SSE)
• New Provider.OLLAMA as local wrapper with default http://localhost:11434
• Added NoAuth for local providers
• Added base_url= parameter for custom API gateways
• Supports structured outputs and unregistered local models

Testing

• Unit tests added for analyze() with messages parameter
• All existing tests pass
• Template contract test updated to exclude wrapper providers

Documentation

• Updated CHANGELOG_V1.md with dedicated sections for both features

[claude]

PR Review: Multi-turn Conversation Support and OpenResponses/Ollama Providers

Summary

This PR adds two significant features to Celeste v1:

1. Multi-turn conversation support with first-class Message and Role types
2. OpenResponses + Ollama providers for OpenAI Responses-compatible APIs and local model hosting

The implementation is well-structured and follows the existing architecture patterns. Below is my detailed review.

---

✅ Strengths

1. Clean Type System Design

• The new Message, Role, and Content types in src/celeste/types.py are well-designed:
  • Role as StrEnum provides type safety while remaining serializable
  • Message uses Pydantic with extra="allow" for flexibility
  • Support for developer role alongside standard roles is forward-thinking

2. Consistent API Surface

• The messages= parameter is uniformly added across all text entrypoints:
  • generate(), analyze(), stream.generate(), stream.analyze()
  • Both sync and async variants
  • Namespace APIs (celeste.text.*, celeste.images.analyze(), etc.)
• Clear precedence: when messages provided, it takes priority over prompt

3. Provider-Specific Normalization

• Excellent handling of different API formats:
  • Anthropic: System messages extracted to system parameter (lines 126-152 in anthropic/client.py)
  • Google: System messages mapped to system_instruction with role mapping (assistant → model) (lines 128-161 in google/client.py)
  • OpenResponses/Ollama: Direct message array passing
• Each provider correctly handles the abstraction while respecting API constraints

4. NoAuth Implementation

• Clean addition of NoAuth class for local providers (src/celeste/auth.py:48-53)
• Properly exported and integrated into the auth system

5. Test Infrastructure

• Template contract test updated to exclude wrapper providers (line 98: wrapper_providers = {"ollama"})
• Maintains consistency checks across provider implementations

---

🔍 Issues & Concerns

CRITICAL: Missing Test Coverage for Multi-turn Conversations

The core feature of this PR—multi-turn conversation support—has no dedicated test coverage. I could not find:

• ❌ Tests for messages= parameter usage
• ❌ Tests validating conversation history handling
• ❌ Tests for different Role types (system, developer, assistant, user)
• ❌ Tests for provider-specific message normalization (Anthropic system blocks, Google system_instruction)
• ❌ Integration tests with actual multi-turn flows

Recommendation: Add comprehensive tests before merging:

# Example test structure needed:
async def test_generate_with_messages():
    messages = [
        Message(role=Role.SYSTEM, content="You are a helpful assistant"),
        Message(role=Role.USER, content="Hello"),
        Message(role=Role.ASSISTANT, content="Hi there!"),
        Message(role=Role.USER, content="How are you?")
    ]
    result = await celeste.text.generate(messages=messages, model="gpt-4o")
    assert result.content

async def test_anthropic_system_message_extraction():
    # Verify system messages go to system parameter
    
async def test_google_role_mapping():
    # Verify assistant -> model mapping

async def test_messages_precedence_over_prompt():
    # Verify messages takes priority when both provided

MEDIUM: Type Safety Concerns

1. Overly permissive Content type (src/celeste/types.py:20):

   type Content = str | JsonValue | dict[str, Any] | list[JsonValue | dict[str, Any]]

   • dict[str, Any] is very loose and could hide bugs
   • Consider tighter constraints or validation

2. Message content flexibility (src/celeste/types.py:35):

   model_config = ConfigDict(extra="allow")

   • While flexible, this could allow invalid message structures to pass through
   • Consider adding validation for common cases

MEDIUM: Documentation Gaps

1. No docstrings for new types:

   • Message class has only "A message in a conversation" (line 33)
   • Should document:
     • Content structure expectations
     • Multi-modal content support (images, etc.)
     • Provider-specific behavior differences

2. Parameter documentation incomplete:

   • messages= parameter added to many methods but lacking detailed docs
   • Should explain:
     • Precedence over prompt
     • Expected message structure
     • Role handling differences per provider

LOW: Code Quality Observations

1. Duplicate logic in namespace APIs (src/celeste/namespaces/domains.py):

   • SyncStreamTextNamespace, StreamTextNamespace, SyncTextNamespace, and TextNamespace have near-identical generate() implementations
   • Consider extracting common logic to reduce duplication

2. Error handling for invalid message structures:

   • No explicit validation that messages list is non-empty when provided
   • No validation that message content types are compatible with chosen provider

3. OpenResponses client parameter constraints (openresponses/client.py:94-105):

   def model_post_init(self, __context: object) -> None:
       constraints = self.model.parameter_constraints
       if TextParameter.TEMPERATURE not in constraints:
           constraints[TextParameter.TEMPERATURE] = Float()

   • Directly mutating parameter_constraints dict could be problematic
   • Consider if this should be done at registration time instead

---

🛡️ Security Considerations

✅ No security concerns identified:

• No new authentication weaknesses introduced
• NoAuth is appropriately used only for local providers
• No sensitive data exposure in new types
• Message content properly handled without injection risks

---

🚀 Performance Considerations

✅ No significant performance concerns:

• Message normalization is lightweight
• No unnecessary copying or deep transformations
• Stream handling remains efficient

Minor optimization opportunity:

• Image/video/audio part building in Google client involves base64 encoding on every request
• Consider caching encoded artifacts if reused

---

📝 Recommendations Summary

Before Merge (Critical):

1. ✅ Add comprehensive test coverage for multi-turn conversations
   • Test messages= parameter across all providers
   • Test role handling and normalization
   • Test precedence rules

Consider for Follow-up:

2. Tighten type constraints on Content type
3. Add validation for message structures
4. Improve documentation for new parameters
5. Reduce duplication in namespace implementations
6. Document provider-specific behavior differences

---

🎯 Overall Assessment

Architecture: 9/10 - Clean design that fits well with existing patterns
Implementation Quality: 8/10 - Solid code, but needs more validation
Test Coverage: 4/10 - ⚠️ Major gap in testing the primary feature
Documentation: 6/10 - Basic coverage, needs more detail

Recommendation: Request test coverage for multi-turn conversations before approving for merge. The implementation is sound, but without tests, we can't confidently validate:

• Message handling works correctly across providers
• Edge cases are handled (empty messages, invalid roles, etc.)
• Provider-specific normalization doesn't lose information

The OpenResponses and Ollama provider additions look solid and follow the established patterns well.

---

📚 Testing Guideline Reference

Per tests/testing_guidelines.md:

• ✅ Follows src/ layout
• ⚠️ Missing parametrized tests for different providers with messages
• ⚠️ No fixtures for common message scenarios
• ⚠️ Branch coverage likely incomplete for new message code paths

Great work on the architecture! Looking forward to seeing the test coverage added.

[claude]

PR Review: Multi-turn conversation support and OpenResponses/Ollama providers

Summary

This PR adds significant new capabilities to Celeste v1, introducing multi-turn conversation support via first-class Message and Role types, plus support for OpenAI Responses-compatible APIs (OpenResponses) and local model hosting (Ollama). The changes are well-structured and follow the existing architectural patterns.

Code Quality and Best Practices ✅

Strengths:

1. Consistent Architecture: The new providers follow the established mixin pattern with proper separation of concerns
2. Type Safety: Excellent use of Pydantic models (Message, Role) with proper type annotations throughout
3. Code Organization: Clean separation between provider-agnostic code (types.py) and provider-specific implementations
4. Backwards Compatibility: The messages parameter gracefully coexists with the existing prompt parameter
5. Reusability: OpenResponses implementation is properly reused by Ollama, demonstrating good DRY principles

Areas for Improvement:

1. Message Validation (src/celeste/types.py:32-39)

The Message model uses extra="allow" which permits arbitrary fields:

class Message(BaseModel):
    model_config = ConfigDict(extra="allow")  # Is this needed?
    role: Role
    content: Content

Recommendation: Document why extra="allow" is needed, or consider using extra="forbid" for stricter validation unless extensibility is required for provider-specific fields.

2. Input Precedence Logic (src/celeste/namespaces/domains.py:190-192)

The precedence of messages over prompt is implemented but not explicitly validated. Consider adding a warning or error if both are provided.

3. Error Handling in OpenResponses (client.py:143-178)

The content parsing has a fallback to empty string but no logging. Add logging when falling back to empty content to aid debugging.

Potential Bugs or Issues 🔍

Critical:

None identified - The code appears solid.

Medium Priority:

1. Missing None Check in Message Handling (openresponses/client.py:145-146)

If inputs.messages is an empty list [], this will create an empty input array. Consider if this is valid for the API.

2. Video Support Not Implemented

The analyze() method accepts video parameter but OpenResponses/Ollama may not actually support video content. Verify video support or remove the parameter.

Low Priority:

3. Test Template Exclusion (test_provider_api_templates.py:98)

Ollama is excluded as a "wrapper provider" - consider adding a comment explaining the exclusion criteria.

Performance Considerations ⚡

Strengths:

1. Streaming Support: Properly implemented for both OpenResponses and Ollama
2. Efficient SSE Parsing: The streaming implementation filters out content-only events
3. Connection Reuse: Uses existing HTTP client infrastructure

Considerations:

1. Default Base URL Repetition (ollama/client.py)

The default base URL is repeated in three methods. Consider consolidating:

base_url = base_url or DEFAULT_BASE_URL  # Repeated 3 times

Security Concerns 🔒

Strengths:

1. NoAuth Class: Properly implemented for local providers without exposing credentials
2. SecretStr Usage: Continues to use SecretStr for API keys
3. No Command Injection: No shell execution or unsafe string interpolation

Considerations:

1. Local Provider Security

Using localhost is generally safe, but document that users should ensure their local Ollama instance is secured. Consider adding a warning if used with non-localhost URLs (SSRF risk if user input).

Assessment: Overall security posture is good with proper credential handling.

Test Coverage 📊

Current State:

• ✅ Unit tests added for analyze() with messages parameter
• ✅ Template contract test updated to exclude wrapper providers
• ✅ All existing tests pass

Gaps:

1. Message Role Validation

No tests for:

• Empty messages list
• Mixed prompt + messages (precedence behavior)
• Invalid role strings
• Content type variations

2. OpenResponses/Ollama Integration Tests

Missing tests for:

• Streaming with messages
• Error responses from OpenResponses API
• Connection failures to Ollama
• Empty response handling

3. Provider-Specific Message Normalization

The PR mentions "Provider normalization handles different API formats" but no tests verify Anthropic/Google system message handling.

Recommendations:

# Add tests like:
def test_message_precedence_over_prompt():
    """Verify messages take precedence when both provided."""

def test_empty_messages_list():
    """Test behavior with empty messages list."""

def test_openresponses_streaming_with_messages():
    """Test streaming with multi-turn conversation."""

Documentation and Type Hints 📝

Strengths:

1. Comprehensive Docstrings: All new classes and methods are well-documented
2. Type Annotations: Excellent coverage with proper use of Unpack, etc.
3. Changelog Updated: Changes properly documented

Minor Improvements:

1. Consider adding usage examples in docstrings showing multi-turn conversations
2. Document the DEVELOPER role - what's its intended use?

Overall Assessment 🎯

Verdict: APPROVE with Minor Suggestions ✅

This is a high-quality PR that adds significant value while maintaining code quality and architectural consistency. The implementation follows best practices and integrates well with the existing codebase.

Priority Action Items:

1. High: Add tests for message functionality (empty lists, precedence, validation)
2. Medium: Document or validate the extra="allow" in Message model
3. Medium: Add integration tests for OpenResponses/Ollama error cases
4. Low: Add usage examples in docstrings
5. Low: Consider consolidating base_url defaulting in Ollama client

Breaking Changes:

None - the changes are additive and maintain backwards compatibility.

Merge Recommendation:

Ready to merge after addressing the high-priority testing gaps. The code quality is excellent, and the architectural decisions are sound. The minor suggestions can be addressed in follow-up PRs if needed.

---

Great work on this feature! The multi-turn conversation support and local model integration are valuable additions to Celeste. 🚀