-
Notifications
You must be signed in to change notification settings - Fork 1
fix: [DecimalError] Invalid argument: getUsage() crashes when token data contains objects #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adding CLAUDE.md with task information for AI processing. This file will be removed when the task is complete. Issue: #119
- Add safe() function to sanitize numeric inputs before passing to Decimal.js - Wrap all token values with safe() to handle NaN, Infinity, and object values - Add try-catch around cost calculation to prevent crashes - Add comprehensive tests for safe() and getUsage() edge cases Fixes #119
…ins objects Add safe() wrapper function to sanitize numeric inputs before passing to Decimal.js. Wrap all token calculations with safe() and add try-catch around cost computation. This fixes crashes when AI providers return unexpected token usage data. Resolves #119 Related to upstream anomalyco/opencode#6161
…rash Added direct Decimal.js crash demonstration at the start of the experiment to show the exact error message: [DecimalError] Invalid argument: [object Object]
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This reverts commit e9e3e25.
|
|
Can we use lodash for that? And, let's also check for similar logic in Rust. Also double check if it possible to get this all tested. |
|
🤖 AI Work Session Started Starting automated work session at 2026-01-12T21:55:18.577Z The PR has been converted to draft mode while work is in progress. This comment marks the beginning of an AI work session. Please wait working session to finish, and provide your feedback. |
Response to FeedbackThanks for the review! I've investigated your questions: 1. Can we use lodash for that?I researched this thoroughly:
Recommendation: lodash's The current implementation uses native
If you prefer using lodash for consistency, I can add 2. Check for similar logic in Rust✅ Investigated: The Rust code in 3. Double check if it's possible to get this all tested✅ Confirmed: All 21 unit tests pass: Tests cover:
Would you like me to:
Please let me know your preference! |
|
|
Ok, just make me But we still should print warning to logs in verbose mode to see actual passed object, also add more lazy loging debug output as we do in other files for --verbose mode, so next time it will happen, we will be able to identify the real root cause. Because we actually getting I don't like |
|
🤖 AI Work Session Started Starting automated work session at 2026-01-12T23:08:45.141Z The PR has been converted to draft mode while work is in progress. This comment marks the beginning of an AI work session. Please wait working session to finish, and provide your feedback. |
…dling Per PR review feedback: - Renamed safe() to toDecimal() as requested - toDecimal() returns Decimal(NaN) instead of converting to 0 - Added verbose mode logging to show actual values when invalid data received - Added safeTokenValue() helper for token extraction with debug logging - Updated tests to verify toDecimal() behavior (returns Decimal(NaN)) - Updated experiment script to demonstrate the new implementation The verbose logging will help identify the root cause when objects or other unexpected values are passed from AI provider APIs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
✅ Implementation CompleteAll requested changes from feedback have been implemented: Changes Made
Test ResultsCI StatusAll checks passing on ubuntu, macos, and windows. Ready for your review! 🚀 |
|
|
Now, we should use That condition should removed. Also double check that we print input data for toDecimal function on all stages, so we can catch the real problem in the future by enabling verbose mode. |
|
🤖 AI Work Session Started Starting automated work session at 2026-01-13T07:45:55.277Z The PR has been converted to draft mode while work is in progress. This comment marks the beginning of an AI work session. Please wait working session to finish, and provide your feedback. |
Per PR feedback, the toDecimal() function now: 1. Uses try-catch instead of type checking - because Decimal.js supports strings and more, so we only return Decimal(NaN) when the constructor actually throws an error 2. Logs input data at all stages in verbose mode to help identify root causes of invalid data in the future 3. Accepts valid numeric strings like "42" (Decimal.js feature) 4. Properly handles Infinity/-Infinity (Decimal.js accepts them) Updated tests to reflect the new behavior: - Numeric strings now create valid Decimals - Infinity creates Decimal(Infinity), not Decimal(NaN) - Only values that throw DecimalError become Decimal(NaN) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implementation CompleteI've implemented the changes per your feedback: Changes Made
Test ResultsAll 29 unit tests pass with the updated behavior. Ready for your review! |
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost estimation:
Now working session is ended, feel free to review and add any feedback on the solution draft. |
🔄 Auto-restart 1/3Detected uncommitted changes from previous run. Starting new session to review and commit them. Uncommitted files: Auto-restart will stop after changes are committed or after 2 more iterations. Please wait until working session will end and give your feedback. |
🔄 Auto-restart 2/3Detected uncommitted changes from previous run. Starting new session to review and commit them. Uncommitted files: Auto-restart will stop after changes are committed or after 1 more iteration. Please wait until working session will end and give your feedback. |
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost estimation:
Now working session is ended, feel free to review and add any feedback on the solution draft. |
Function should work exactly the same ways as toDecimal, and should be named |
|
🤖 AI Work Session Started Starting automated work session at 2026-01-13T08:08:10.247Z The PR has been converted to draft mode while work is in progress. This comment marks the beginning of an AI work session. Please wait working session to finish, and provide your feedback. |
Per reviewer feedback: - Renamed safeTokenValue to toNumber for consistency with toDecimal - Uses try-catch pattern instead of type checking (like toDecimal) - Returns NaN on conversion errors (not 0) - Added verbose logging at all stages (input, success, error) - Updated getUsage to use safeNum helper for NaN/Infinity handling - Added 19 new tests for toNumber function The toNumber function follows the same pattern as toDecimal: - Logs input in verbose mode - Attempts conversion with try-catch - Returns NaN on failure (logged in verbose mode) - Supports numbers and valid numeric strings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implementation CompleteI've implemented all the requested changes: Changes per feedback
Test ResultsAll 48 tests pass, including 19 new tests for the CI StatusAll CI checks passing on ubuntu, macos, and windows. Ready for your review! |
|
Summary
Fixes the
[DecimalError] Invalid argument: [object Object]crash inSession.getUsage()when token usage data from AI providers contains unexpected values like objects,NaN, orInfinityinstead of numbers.This issue was encountered when using the agent with certain models (e.g.,
opencode/grok-code) where the token usage data may contain unexpected structures.Fixes #119
Root Cause
The
getUsage()function was passing raw token values directly todecimal.jsconstructors, which throws errors when receiving non-numeric data:Solution
Per PR review feedback, implemented two safe conversion functions:
1.
toDecimal()- Safe Decimal constructionUses try-catch for Decimal construction with verbose logging:
2.
toNumber()- Safe number conversion (NEW)Per latest feedback, renamed
safeTokenValuetotoNumberwith the same pattern astoDecimal:3. Verbose logging at all stages
When
--verboseflag is enabled, detailed logging shows:This helps identify when API response formats change or contain unexpected data structures.
Changes
js/src/session/index.tstoDecimal()with try-catch and verbose logging, renamedsafeTokenValuetotoNumberwith same patternjs/tests/session-usage.test.tstoDecimal(),toNumber(), andgetUsage()edge casesTesting
getUsage()edge casesTest output:
Related
AI-assisted implementation with verbose debugging support