feat(providers): add BFL Images API package

[Kamilbenkirane]

Summary

Add standalone celeste-bfl provider package with Images API mixin, following the established pattern from Anthropic and Google providers.

Changes

Provider Package (celeste-bfl)

• BFLImagesClient mixin with async polling workflow:
  • Phase 1: POST to /v1/{model_id} to submit job
  • Phase 2: Poll polling_url until Ready/Failed
  • Merges _submit_metadata into final response for usage tracking
• Usage parsing following API layer pattern:
  • map_usage_fields() static method with UsageField enum
  • _parse_usage() extracts and maps BFL fields (cost → BILLED_UNITS, input_mp → INPUT_MP, output_mp → OUTPUT_MP)
• Parameter mappers: Width, Height, Seed, Steps, Guidance, PromptUpsampling, SafetyTolerance, OutputFormat
• Config: API base URL, endpoints, polling interval (0.5s), timeout (120s)
• py.typed marker for typed package support

Capability Package (celeste-image-generation)

• Migrate BFL client to use API mixin pattern
• Simplify _parse_usage() to call super()._parse_usage() and wrap result
• Add celeste-bfl workspace dependency
• Remove redundant config.py (now in provider layer)

Test plan

• CI passes (lint, type check, tests)
• Integration test with BFL API

[claude]

Code Review: BFL Images API Provider Package

Summary

This PR successfully introduces a standalone celeste-bfl provider package following the established architectural patterns from Anthropic and Google providers. The implementation is clean and follows good separation of concerns between the provider layer and capability layer.

✅ Strengths

Architecture & Design

• Excellent pattern adherence: The mixin-based architecture mirrors the Anthropic and Google provider packages perfectly
• Clean separation of concerns: Provider-specific logic properly separated from capability logic
• Good code reuse: Capability layer now delegates to provider layer, eliminating 100+ lines of duplication
• Type safety: Includes py.typed marker for proper typing support

Code Quality

• Clear documentation: Well-documented docstrings explaining the async polling workflow
• Consistent naming: Follows established conventions
• Proper error handling: Appropriate use of ValidationError and informative error messages

🔍 Issues & Recommendations

1. CRITICAL: Missing Dependencies in pyproject.toml

File: packages/providers/bfl/pyproject.toml

The provider package is missing runtime dependencies. Compare with celeste-google which has dependencies = ["celeste-ai", "httpx"]. This needs to be added.

Impact: The package will not be installable standalone - users will get import errors.

2. BUG FIX: Inconsistent Return Type

Status: Fixed! The old code could return None but the new implementation correctly always returns a FinishReason object. Well done!

3. MISSING: No Unit Tests for Provider Package

Location: packages/providers/bfl/tests/ (does not exist)

Other providers have comprehensive unit tests. Add tests especially for usage mapping and parameter mappers.

4. MISSING: Entry Point Registration

Does BFL need an entry point registration like celeste-google has? This should be clarified.

5. POTENTIAL BUG: Polling Timeout Edge Case

If a request takes a long time near the timeout, the sleep could push past the timeout. Consider checking timeout before sleep.

6. PERFORMANCE: Polling Interval

0.5s polling means up to 240 requests over 2 minutes. Consider exponential backoff or document if this is the recommended interval.

7. MINOR: Unused Utility Function

The encode_image() function is defined but never used. Is this for future functionality?

8. TYPE SAFETY: Type Ignore Comments

Multiple type ignore comments. Consider using a Protocol for better typing (nice-to-have).

🛡️ Security

✅ No Security Concerns - Proper authentication, no hardcoded credentials, appropriate timeout handling

🧪 Test Coverage

• ✅ Integration test exists and updated
• ❌ No unit tests for provider package
• ❌ Test plan checkbox for Integration test with BFL API is unchecked

📊 Performance Considerations

• ✅ Non-blocking async/await usage
• ✅ Configurable timeout prevents hanging
• ⚠️ Fixed polling interval - consider exponential backoff
• ✅ Eliminated approximately 100 lines of duplication

🎯 Final Recommendations

Must Fix Before Merge

1. Add dependencies to packages/providers/bfl/pyproject.toml

Should Address

2. Add unit tests for the provider package
3. Clarify/add entry point registration if needed
4. Consider polling timeout edge case fix
5. Document or remove unused encode_image() function

Nice to Have

6. Add exponential backoff for polling
7. Improve type safety with Protocol
8. Add more comprehensive test coverage

✨ Overall Assessment

Score: 8/10 - This is solid work that follows established patterns and significantly improves code organization. The main blocker is the missing dependencies in pyproject.toml. Once that is fixed and tests are added, this will be ready to merge.

The architectural decision to extract the provider layer is excellent and sets up the codebase well for future BFL capabilities beyond image generation.

Great job on following the existing patterns! 🚀

[claude]

PR Review: BFL Images API Provider Package

Summary

This PR successfully implements a standalone celeste-bfl provider package following the established Celeste architecture patterns. The implementation is clean, well-structured, and aligns with existing patterns from celeste-anthropic and celeste-google providers.

✅ Strengths

Architecture & Design

• Excellent separation of concerns: Provider layer (celeste-bfl) is cleanly separated from capability layer (celeste-image-generation)
• Consistent with existing patterns: Follows the mixin pattern established by other providers
• Proper async polling implementation: The two-phase polling workflow (submit → poll) is well-implemented with timeout handling
• Good code reuse: Parameter mappers are properly shared between provider and capability layers through inheritance

Code Quality

• Well-documented: Docstrings are clear and explain the async polling workflow
• Type hints: Proper type annotations throughout
• Error handling: Appropriate error messages with provider context
• Clean imports: Proper use of private imports (_WidthMapper) to distinguish provider vs capability mappers

Testing

• Comprehensive constraint tests: The Int constraint changes have good test coverage for string/float conversion
• Integration test update: Added missing model parameter to create_client() call

🔍 Issues Found

1. Missing name attribute on Provider Parameter Mappers (High Priority)

Location: packages/providers/bfl/src/celeste_bfl/images/parameters.py

All provider-level parameter mappers are missing the name attribute. While capability-level mappers correctly set this, the provider-level classes don't have it defined, which could cause issues if they're used directly.

Current:

class WidthMapper(ParameterMapper):
    """Map width to BFL width field."""
    
    def map(self, request, value, model):
        # No 'name' attribute!

Should be (following Anthropic/Google patterns):

from celeste_image_generation.parameters import ImageGenerationParameter

class WidthMapper(ParameterMapper):
    """Map width to BFL width field."""
    
    name = ImageGenerationParameter.WIDTH  # or appropriate enum value

Note: This may be intentional if these are meant to be "pure" provider mappers without capability-specific names, but it's inconsistent with other providers.

2. Boolean Handling in Int Constraint (Medium Priority)

Location: src/celeste/constraints.py:132-150

The new Int constraint accepts booleans and converts them to integers (True→1, False→0). This is technically correct Python behavior but could lead to unexpected bugs:

# This will now silently succeed:
Int()(True)   # Returns 1
Int()(False)  # Returns 0

Recommendation: Add explicit boolean check like the original:

def __call__(self, value: int | str | float) -> int:
    """Validate value is an integer or convert string/float to int."""
    # Reject booleans explicitly (even though isinstance(True, int) == True)
    if isinstance(value, bool):
        msg = f"Must be int, got bool"
        raise ConstraintViolationError(msg)
    
    if isinstance(value, str):
        if not value.lstrip("-").isdigit():
            msg = f"Must be int, got {value!r}"
            raise ConstraintViolationError(msg)
        return int(value)
    # ... rest of implementation

This prevents accidental boolean→int coercion which is rarely the intended behavior.

3. Unused encode_image Utility (Low Priority)

Location: packages/providers/bfl/src/celeste_bfl/images/utils.py

The encode_image() function is defined but never used in this PR. It's good to have for future features (like image-to-image), but should either:

• Be used now if needed, or
• Have a comment explaining it's for future use

4. Missing py.typed Content (Low Priority)

Location: packages/providers/bfl/src/celeste_bfl/py.typed

The file is empty, which is correct for PEP 561, but consider adding a comment for clarity:

# PEP 561 marker file for type checking support

🎯 Best Practices Observations

Excellent Patterns Used:

1. Metadata merging in async polling: The _submit_metadata pattern for preserving cost data is clever
2. Usage field mapping: Proper use of UsageField enum for standardization
3. Delegated mapping: AspectRatioMapper correctly delegates to WidthMapper/HeightMapper
4. Proper super() calls: Capability client correctly calls super()._parse_usage() for composition

Minor Suggestions:

1. Consider adding constants for status values: Instead of "Ready", "Error", "Failed" strings, consider:

   class BFLStatus(StrEnum):
       READY = "Ready"
       ERROR = "Error"
       FAILED = "Failed"

2. Error message consistency: Line 70 uses ValueError while line 50 (capability layer) uses ValidationError. Consider standardizing to ValidationError for user-facing errors.

🔒 Security Considerations

✅ No issues found:

• Proper timeout handling prevents infinite polling
• No credential leakage in error messages
• Safe file handling in encode_image() utility
• No injection vulnerabilities in request building

⚡ Performance Considerations

✅ Good performance characteristics:

• Appropriate polling interval (0.5s) balances responsiveness vs API load
• Timeout (120s) is reasonable for image generation
• No blocking operations outside of intended async polling
• Efficient response data handling

📊 Test Coverage

Covered:

• ✅ Constraint conversions (string→int, float→int)
• ✅ Integration test updated with model parameter

Missing (consider adding):

• Async polling timeout behavior
• Error status handling ("Error", "Failed")
• Usage field mapping edge cases (null values)
• AspectRatio rounding logic (multiple of 16)

🎓 Code Review Learning

This PR demonstrates excellent understanding of:

• Celeste's layered architecture (provider vs capability)
• Python mixin patterns and MRO
• Async polling patterns
• Type-safe enum usage

Final Recommendation

Approve with minor changes

The PR is well-implemented and ready to merge after addressing:

1. High Priority: Add name attributes to provider parameter mappers OR clarify if intentionally omitted
2. Medium Priority: Add boolean rejection to Int constraint to prevent unexpected coercion

The async polling implementation is solid, the architecture is clean, and the code follows established patterns well. Great work! 🎉

---

Lines of Code: +470, -228 (net +242)
Files Changed: 16
Risk Level: Low (well-tested pattern, isolated provider package)

🤖 Generated with Claude Sonnet 4.5