fix(speech-generation): return correct finish reason type in Google provider

[Kamilbenkirane]

Override _parse_finish_reason to return SpeechGenerationFinishReason instead of base FinishReason class, fixing Pydantic validation error in integration tests.

[claude]

PR Review: Fix Google Speech Generation Finish Reason Type

Summary

This PR correctly fixes a Pydantic validation error by overriding _parse_finish_reason to return SpeechGenerationFinishReason instead of the base FinishReason class. The fix follows established patterns in the codebase.

---

✅ Code Quality & Best Practices

Excellent:

• Consistent Pattern: The implementation follows the exact same pattern used in other capabilities (see packages/capabilities/image-generation/src/celeste_image_generation/providers/google/gemini.py:74-86)
• Proper Import: Correctly imports SpeechGenerationFinishReason at the top of the file
• Clean Override: Uses super()._parse_finish_reason() to delegate to parent class, then wraps result in the capability-specific type
• Good Documentation: Clear docstring explaining the method's purpose

Code Style:

• Type hints are correct and consistent with the codebase
• Formatting follows project conventions
• Method placement is logical (between _parse_usage and _parse_content)

---

✅ Bug Fix Validation

Root Cause Analysis:
The bug occurred because:

1. The base Client.generate() method (src/celeste/client.py:126) calls self._parse_finish_reason(response_data)
2. GoogleCloudTTSClient._parse_finish_reason returns base FinishReason (packages/providers/google/src/celeste_google/cloud_tts/client.py:69-71)
3. SpeechGenerationOutput.finish_reason expects SpeechGenerationFinishReason | None (packages/capabilities/speech-generation/src/celeste_speech_generation/io.py:30)
4. Pydantic validation fails because FinishReason is not a subclass of SpeechGenerationFinishReason

Fix Correctness:

• ✅ The override correctly converts FinishReason → SpeechGenerationFinishReason
• ✅ Preserves the reason field from parent class
• ✅ Returns correct type expected by SpeechGenerationOutput

---

🔍 Potential Issues & Suggestions

Minor Consideration:
The current implementation assumes finish_reason.reason is always accessible. While this is safe given the parent class always returns a FinishReason object, you could add a null check for extra robustness:

def _parse_finish_reason(
    self, response_data: dict[str, Any]
) -> SpeechGenerationFinishReason:
    """Parse finish reason from response."""
    finish_reason = super()._parse_finish_reason(response_data)
    return SpeechGenerationFinishReason(reason=finish_reason.reason if finish_reason else None)

However, this is not required since:

1. GoogleCloudTTSClient._parse_finish_reason always returns a FinishReason object (never None)
2. The current implementation is consistent with image-generation (which also doesn't null-check)

---

⚡ Performance

No concerns:

• Minimal overhead (just object construction)
• No unnecessary operations
• Consistent with other provider implementations

---

🔒 Security

No concerns:

• No user input processing
• No external calls
• Simple data transformation

---

🧪 Test Coverage

Current State:

• Integration tests exist at packages/capabilities/speech-generation/tests/integration_tests/test_speech_generation/test_generate.py:27
• Google provider is tested with parametrized test cases (line 13-14)
• The integration test validates SpeechGenerationUsage type (line 70-72) but doesn't explicitly check finish_reason

Recommendation:
Consider adding a unit test similar to packages/capabilities/image-generation/tests/unit_tests/providers/google/test_finish_reason.py to:

1. Explicitly test the type returned by _parse_finish_reason
2. Verify the reason field is correctly propagated
3. Document this behavior for future maintainers

Example test structure:

def test_parse_finish_reason_returns_correct_type(client):
    response_data = {}
    result = client._parse_finish_reason(response_data)
    assert isinstance(result, SpeechGenerationFinishReason)
    assert result.reason is None  # GoogleCloudTTS returns None

However, since the integration test already exercises this code path and the fix is minimal, this is optional and can be addressed in a future PR.

---

✅ Overall Assessment

Score: 9.5/10

This is a well-crafted, minimal fix that:

• ✅ Solves the exact problem described
• ✅ Follows established patterns in the codebase
• ✅ Maintains consistency with other providers
• ✅ Has no security or performance concerns
• ✅ Is covered by existing integration tests

Recommendation: Approve and merge 🚀

The only minor improvement would be adding a dedicated unit test, but this is not blocking given the existing integration test coverage.