fix(minimax): preserve usage-only streaming chunks - #1249
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughMinimax streaming now preserves usage-only chunks and filters unrelated objects. Unit tests verify content retention, empty-choice terminal chunks, and prompt, completion, and total token counts. ChangesMinimax streaming usage
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/any_llm/providers/minimax/minimax.py`:
- Around line 47-48: Flatten the nested conditions in the chunk-processing logic
by combining the OpenAIChatCompletionChunk type check with its
choices/delta-or-usage predicate into a single if statement, preserving the
existing filter behavior. Run the repository pre-commit checks and confirm Ruff
lint and formatting pass.
In `@tests/unit/providers/test_minimax_provider.py`:
- Around line 139-145: Update the test assertions for the first content chunk in
the Minimax provider conversion to verify that result[0].usage is None, while
preserving the existing usage-only chunk assertions for result[1].
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2915796c-fe6e-46e7-8d43-6eead6294b00
📒 Files selected for processing (2)
src/any_llm/providers/minimax/minimax.pytests/unit/providers/test_minimax_provider.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
The usage guard let Minimax's terminal chunk through whole. That chunk carries a `message` instead of a `delta`, so the OpenAI SDK parses `delta` as None and chunk validation raises, which is the failure mozilla-ai#657 introduced this filter to avoid. Yield the usage while dropping the unusable choices, and cover the real wire shape with a regression test built through the SDK deserializer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
... and 22 files with indirect coverage changes 🚀 New features to boost your workflow:
|
njbrake
left a comment
There was a problem hiding this comment.
LGTM. Thank you for the contribution!
…unk (#1392) ## Description Native Anthropic streaming reports token usage on the `message_stop` event, which arrives after the `message_delta` event that carries `stop_reason`. The chunk converter attached a synthetic choice (`index 0`, empty delta, `finish_reason: None`) to every chunk, including that usage-only one. OpenAI-compatible providers put final usage on a trailing chunk with `choices: []` (OpenAI documents this for `stream_options.include_usage`), and the MiniMax fix in #1249 aligned MiniMax to the same shape. Code that picks up usage with `if not chunk.choices` therefore worked for OpenAI and silently saw no usage for Anthropic. Observed on `main` (last two chunks of a streamed `claude-sonnet-4-6` reply, null fields dropped): ``` 8 {'choices': [{'delta': {}, 'finish_reason': 'stop', 'index': 0}]} 9 {'choices': [{'delta': {}, 'index': 0}], 'usage': {'completion_tokens': 5, 'prompt_tokens': 13, 'total_tokens': 18}} ``` With this change chunk 9 becomes `{'choices': [], 'usage': {...}}`, identical in shape to OpenAI's trailing usage chunk. Chunks 1 to 8 are unchanged. Change: `_create_openai_chunk_from_anthropic_chunk` returns from the `MessageStopEvent` branch before the choice is appended. The stop event carries no delta and no stop reason, so nothing is lost. Tests: the two existing `message_stop` usage tests now assert `choices == []`; two new converter tests pin that the usage chunk arrives after the `finish_reason` chunk with empty choices, and that a raw `message_stop` without an accumulated message yields neither choices nor usage. A third test drives `acompletion(stream=True)` through a fake SDK message stream and reads the chunks the way an OpenAI-style consumer does (content and `finish_reason` from chunks with choices, usage from the chunk without). On `main` that test fails with `usage is None`; text and `finish_reason` already arrive correctly. Validation: - `uv run pytest tests/unit`: 2435 passed, 69 skipped - `uv run pre-commit run --all-files`: clean (ruff, ruff format, mypy strict, codespell) - Live streams against Anthropic (`claude-sonnet-4-6`) and OpenAI (`gpt-5-nano`, `include_usage`) printed chunk by chunk; usage chunk shapes match after the fix - `uv run pytest tests/integration -k anthropic` with real keys: 23 passed, 7 skipped. The skips are the existing capability skips (Anthropic has no batch, responses, moderation, or embeddings endpoints; one test targets Claude on Bedrock only, see #1184) Not changed, same pattern: Bedrock also attaches a synthetic choice to its `metadata` usage chunk (`src/any_llm/providers/bedrock/utils.py`). Gemini attaches usage to every content chunk and never emits a usage-only chunk. ## PR Type - 🐛 Bug Fix ## Relevant issues None open. Same shape as the MiniMax fix in #1249. ## Checklist - [x] I understand the code I am submitting. - [x] I have added unit tests that prove my fix/feature works - [x] I have run this code locally and verified it fixes the issue. - [x] New and existing tests pass locally - [x] Documentation was updated where necessary - [x] I have read and followed the [contribution guidelines](https://github.com/mozilla-ai/any-llm/blob/main/CONTRIBUTING.md) - [x] **AI Usage:** - [ ] No AI was used. - [ ] AI was used for drafting/refactoring. - [x] This is fully AI-generated. ## AI Usage Information - AI Model used: Claude Fable 5.1 (`claude-fable-5-1`) - AI Developer Tool used: Claude Code - Any other info you'd like to share: The bug was reported by an automated agent; the fix, tests, and this description were produced by Claude Code and reviewed by the submitter, who will answer reviewer questions personally. - [x] I am an AI Agent filling out this form (check box if true) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected streaming responses so the final usage-only chunk no longer includes an empty choice. * Improved compatibility with OpenAI-style streaming consumers by delivering usage after the finish reason. * Ensured message-stop events produce the expected empty choices list, including when no message content has been accumulated. * Standardised streaming completion behaviour so consumers receive text, a single stop reason, and final usage in the correct order. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Hareesh <hareeshbahuleyan@gmail.com>
Description
Fixes MiniMax streaming usage loss reported in #1185.
MiniMax returns token usage in a trailing streaming chunk with
choices=[].MinimaxProvider._convert_completion_response_async()previously forwardedonly chunks containing a choice delta, so it discarded this usage-only chunk
before the Messages bridge or downstream stream handler could observe it.
This change keeps the existing MiniMax filtering behavior while also forwarding
chunks where
chunk.usage is not None.The regression test verifies that:
choices=[]chunk with usage survives conversion;wrapper.
No usage is synthesized, and absent usage is not converted to zero. Non-streaming
behavior and other providers are unchanged.
Validation
Full local unit suite:
The warnings are existing provider exception deprecation warnings unrelated to
this change.
Full pre-commit suite passed, including Ruff, formatting, Mypy, codespell, and
repository file checks.
A live MiniMax-M3 streaming request was also verified with the patched provider.
The raw provider usage and normalized result matched:
No credentials, prompt text, response text, or request headers were retained.
PR Type
Relevant issues
Fixes #1185
Checklist
AI Usage Information
directed by the contributor. Codex implemented the change, added the
regression test, ran the local validation, and drafted this PR description.
The contributor reviewed the patch before submission.
When answering questions by the reviewer, please respond yourself, do not copy/paste the reviewer comments into an AI system and paste back its answer. We want to discuss with you, not your AI :)
Summary by CodeRabbit