refactor: remove explicit None assignments for usage token fields

[Kamilbenkirane]

Summary

This PR removes unnecessary explicit None assignments for optional usage fields since they already default to None in their respective Usage classes.

Changes

Commit 1: Usage Cleanup

• Anthropic: Extract cached_tokens from cache_read_input_tokens instead of setting to None
• Google: Remove billed_tokens and cached_tokens explicit None
• OpenAI: Remove billed_tokens explicit None
• XAI: Remove billed_tokens explicit None
• ByteDance: Remove input_tokens and output_tokens explicit None

Commit 2: Mistral Reasoning Support

• Add support for Mistral Magistral reasoning models array format
• Handle content as array and extract text from blocks with type 'text'

Impact

• Code quality: Cleaner code by relying on default values
• No functional changes: Behavior remains identical
• Maintainability: Less redundant code to maintain

[claude]

Pull Request Review - PR #53

Summary

This PR makes two types of changes:

1. Refactoring: Removes explicit None assignments for optional usage token fields
2. Feature Addition: Adds support for Mistral Magistral reasoning models array format

✅ Positive Findings

Code Quality:

• Clean refactoring that reduces code redundancy
• Properly leverages Pydantic's default field values
• Consistent application of the pattern across all providers

Anthropic Enhancement:

• Good catch extracting cached_tokens from cache_read_input_tokens instead of setting it to None (anthropic/client.py:52, anthropic/streaming.py:108)
• This was likely a bug fix as the data was available but not being captured

Mistral Feature:

• Properly handles the new Magistral array format for reasoning models
• Consistent implementation in both client (mistral/client.py:67-75) and streaming (mistral/streaming.py:60-67)
• Good fallback behavior when no "text" block is found

🔍 Issues & Concerns

1. Potential Bug in Mistral Content Parsing (HIGH PRIORITY)

Location: packages/text-generation/src/celeste_text_generation/providers/mistral/client.py:67-75

The for-else construct doesn't handle the case where block might not be a dictionary. If the API returns malformed data (e.g., content = [None, {"type": "text", "text": "hello"}]), calling block.get("type") will raise an AttributeError.

Recommendation: Add type checking before calling .get()

Same issue exists in: packages/text-generation/src/celeste_text_generation/providers/mistral/streaming.py:61-67

2. Missing Test Coverage (MEDIUM PRIORITY)

The Mistral Magistral array format handling is a significant new feature, but there are no tests for it:

• No unit tests for parsing array content format
• No integration tests for Magistral models
• No tests verifying the fallback behavior when "text" block is missing

Recommendation: Add unit tests covering array format, multiple blocks, malformed blocks, and backward compatibility

3. Incomplete Documentation (LOW PRIORITY)

No inline comments explaining why this array format exists or reference to Mistral API documentation. Future maintainers might not understand this is model-specific behavior.

4. Code Duplication (LOW PRIORITY)

The streaming implementation duplicates the array parsing logic. Consider extracting to a shared helper method for DRY principles.

📊 Testing & Validation

Missing:

• No test execution results provided
• No manual validation of Magistral models shown
• No verification that existing tests pass

🔒 Security & Performance

No security concerns identified - Changes are purely data parsing/transformation

Performance: Negligible impact

📝 Overall Assessment

Refactoring portion (explicit None removal): ✅ Excellent - clean improvement

Mistral feature portion: ⚠️ Needs work

• The feature itself is good and needed
• Implementation has a potential bug with error handling
• Missing test coverage is concerning for production use

🎯 Recommendations

Before merging:

1. Fix the potential AttributeError in array parsing (both client and streaming)
2. Add test coverage for the Magistral array format
3. Verify existing tests pass with these changes

Nice to have:

• Add inline documentation explaining the array format
• Consider extracting shared parsing logic

Approval Status: ⏸️ Requesting changes

---

Review generated with Claude Code

[claude]

Code Review

I've reviewed PR #53 and have the following feedback:

✅ Strengths

1. Improved Code Quality: Removing explicit None assignments where defaults exist is a clean refactoring that reduces redundancy.

2. Correct Implementation: The changes properly rely on the default None values defined in the TextGenerationUsage and ImageGenerationUsage classes (text-generation/src/celeste_text_generation/io.py:27-32 and image-generation/src/celeste_image_generation/io.py:29-31).

3. Bug Fix: The Anthropic integration now correctly extracts cached_tokens from cache_read_input_tokens instead of setting it to None, which fixes a data loss issue.

4. Feature Addition: Mistral Magistral reasoning model support is a valuable addition that handles the array-based content format correctly.

⚠️ Areas for Improvement

1. Mistral Array Handling - Potential Edge Cases

In mistral/client.py:68-73 and mistral/streaming.py:61-66, the array content handling could be more robust. What happens if a block has type == "text" but the text field is None? Are there other block types that should be handled or logged? The streaming version has slightly different handling (checks if text_parts before joining) - consider making both implementations consistent.

2. Missing Test Coverage

The Mistral array content handling is a significant functional change, but there appear to be no unit tests for:

• Array-format content parsing in both client and streaming modes
• Edge cases (empty arrays, missing text field, unknown block types)
• Backward compatibility with string-format content

Recommendation: Add unit tests to verify the Mistral Magistral content parsing logic works correctly.

3. Documentation

The PR description mentions "Mistral Magistral reasoning models" but there's no code comment explaining why this array format exists or which specific models use it.

Suggestion: Add a comment explaining the array format and which models use it.

🔒 Security & Performance

• ✅ No security concerns identified
• ✅ No performance regressions - the changes are equivalent or better
• ✅ No risk of data leakage or injection vulnerabilities

📝 Summary

Approve with minor recommendations. The refactoring is sound and the Anthropic bug fix is important. The Mistral feature addition works but would benefit from:

1. Slightly more defensive coding for edge cases
2. Unit test coverage
3. Documentation comments

The changes are backward compatible and don't introduce breaking changes.

---

🤖 Generated with Claude Code