Repository navigation
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Pre-merge checks:
 |
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Issue #1727 requires both providers to adopt the cumulative message_delta.usage.output_tokens value for final task cost calculations. anthropic.ts and minimax.ts now replace the seed with that v… |
| Out of Scope Changes check | |
The changes are limited to the Anthropic and MiniMax output-token accumulators and focused provider regression tests. These changes directly support issue #1727. No unrelated behavior or scope expansi… |
| Regression Evidence | |
Focused provider-level regression coverage is present for both changed behaviors. anthropic.spec.ts and minimax.spec.ts stream a message_start seed of 1, a cumulative message_delta value, and … |
| Security Boundaries | |
No changed path crosses a security boundary. The only production changes in src/api/providers/anthropic.ts:295 and src/api/providers/minimax.ts:153 assign the provider-reported numeric `message_de… |
| Persistence Integrity | |
PASS. The pull request changes only in-memory output-token accounting in AnthropicHandler.createMessage and MiniMaxHandler.createMessage, plus tests. The changed assignment adopts `message_delta.u… |
| Lifecycle Resource Cleanup | |
The pull request changes only local output-token assignment inside the existing for await stream loops in AnthropicHandler.createMessage and MiniMaxHandler.createMessage, plus regression tests. … |
| Title check | |
The title clearly and concisely describes the main fix: using cumulative message_delta output tokens for Anthropic and MiniMax cost calculations. |
| Description check | |
The description includes the linked issue, implementation details, test procedure and results, completed checklist, documentation status, and additional notes. It is focused and sufficiently detailed … |
Full details: Linked Issues check
-
Explanation
Issue
#1727requires both providers to adopt the cumulativemessage_delta.usage.output_tokensvalue for final task cost calculations.anthropic.tsandminimax.tsnow replace the seed with that value and preserve the prior value when the delta value is zero or absent. Regression tests verify final costs with cumulative output counts instead of seed counts. The changes meet the coding requirements.
Full details: Out of Scope Changes check
-
Explanation
The changes are limited to the Anthropic and MiniMax output-token accumulators and focused provider regression tests. These changes directly support issue
#1727. No unrelated behavior or scope expansion is shown.
Full details: Regression Evidence
-
Explanation
Focused provider-level regression coverage is present for both changed behaviors.
anthropic.spec.tsandminimax.spec.tsstream amessage_startseed of 1, a cumulativemessage_deltavalue, and a later zero-token delta; each asserts the finaltotalCostuses the cumulative value and not the seed. This covers replacement semantics and the falsy fallback branch. The tests exercisecreateMessagethrough the final cost calculation, so they do not merely assert the local assignment. The PR changes no visible UI component, so no Playwright snapshot applies.
Full details: Security Boundaries
-
Explanation
No changed path crosses a security boundary. The only production changes in
src/api/providers/anthropic.ts:295andsrc/api/providers/minimax.ts:153assign the provider-reported numericmessage_delta.usage.output_tokensvalue to the local counter used bycalculateApiCostAnthropic. The value is used for cost reporting only; the diff does not expose secrets or PII, execute input, alter requests or credentials, or bypass approval and allowlist controls. The test-only changes add stream fixtures and cost assertions.
Full details: Persistence Integrity
-
Explanation
PASS. The pull request changes only in-memory output-token accounting in
AnthropicHandler.createMessageandMiniMaxHandler.createMessage, plus tests. The changed assignment adoptsmessage_delta.usage.output_tokensbefore calculating the final cost. No changed persistence path, write, transaction, rollback, or persisted-default propagation exists.
Full details: Lifecycle Resource Cleanup
-
Explanation
The pull request changes only local output-token assignment inside the existing
for awaitstream loops inAnthropicHandler.createMessageandMiniMaxHandler.createMessage, plus regression tests. The changed paths do not add or remove listeners, watchers, timers, tasks, providers, or other resources, and they do not alter cancellation, disposal, restart, or stream termination behavior. No changed lifecycle path can leak a resource or duplicate work.
Full details: Description check
-
Explanation
The description includes the linked issue, implementation details, test procedure and results, completed checklist, documentation status, and additional notes. It is focused and sufficiently detailed for review.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
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 help to get the list of available commands.
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Resolve the merge conflicts. The review sequence resumes after the branch is mergeable. Review-state labels are managed by this workflow; do not edit them manually. |
github-actions
Bot
added
coderabbit-review-active
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
awaiting-maintainer
github-actions
Bot
added
coderabbit-review-active
github-actions
Bot
added
awaiting-maintainer
github-actions
Bot
added
awaiting-maintainer
github-actions
Bot
added
has-conflicts
github-actions
Bot
added
the
awaiting-author
|
This PR has been awaiting author changes for 14 days and will be automatically closed in 7 days. Please address the review comments or leave a comment if you need more time. |
github-actions
Bot
added
the
stale-awaiting-author
|
Closing due to author inactivity after requested changes. Feel free to reopen once the requested changes have been addressed. |
Related GitHub Issue
Closes: #1727
Description
For Anthropic and MiniMax models, the task cost shown in the UI omits the output-token portion of the bill, because the streaming loop never adopts the cumulative
message_delta.usage.output_tokensinto the local accumulator used for the finaltotalCostchunk — only themessage_startseed (0/1) is counted.This PR adopts the cumulative value (replace, not add — the final
message_deltacarries the message total, per the pinned SDK's replace semantics) in bothsrc/api/providers/anthropic.tsandsrc/api/providers/minimax.ts, so the end-of-stream cost chunk reflects the real output tokens. (opencode-go.tsalready documents this exact trap;anthropic-vertex.tsnever emits atotalCostchunk and is unaffected.)Test Procedure
message_startusage{input:100, output:1}+message_deltausage{output_tokens:500}→ final usage chunk'stotalCostequalscalculateApiCostAnthropic(info, 100, 500), not the starved(info, 100, 1). Verified failing before the fix, passing after.vitest run api/providers/__tests__/anthropic.spec.ts api/providers/__tests__/minimax.spec.ts→ 99/99.pnpm run test:coverage:core→ exit 0 (162 files / 3015 passed).vitest run --coverage --config vitest.api.config.ts→ exit 0; changed lines covered (anthropic.ts:295, minimax.ts:153).pnpm run check-types→ 0 errors. ESLint on changed files → clean, suppression counts unchanged.Pre-Submission Checklist
Visual Snapshots
N/A.
Videos (interaction / animation only)
N/A.
Documentation Updates
Additional Notes
Pre-existing (out of scope): the consumer in
Task.tssums the cumulative delta yield into its own display accumulator, so displayedtokensOutcan read ~1 over — cosmetic; the cost figure this PR fixes is now exact.Get in Touch
GitHub: @myk1yt — please tag me here; I monitor notifications.