-
Notifications
You must be signed in to change notification settings - Fork 191
fix(chat): improve reasoning token estimation logic #1155
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2914,6 +2914,21 @@ chat.openapi(completions, async (c) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (calculatedPromptTokens || 0) + (calculatedCompletionTokens || 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Estimate reasoning tokens if not provided but reasoning content exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let calculatedReasoningTokens = reasoningTokens; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!reasoningTokens && fullReasoningContent) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| calculatedReasoningTokens = encode(fullReasoningContent).length; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Fallback to simple estimation if encoding fails | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Failed to encode reasoning text in streaming", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error instanceof Error ? error : new Error(String(error)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| calculatedReasoningTokens = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| estimateTokensFromContent(fullReasoningContent); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if the response finished successfully but has no content, tokens, or tool calls | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This indicates an empty response which should be marked as an error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Do this check BEFORE sending usage chunks to ensure proper event ordering | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -3129,7 +3144,7 @@ chat.openapi(completions, async (c) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| promptTokens: calculatedPromptTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| completionTokens: calculatedCompletionTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| totalTokens: calculatedTotalTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reasoningTokens: reasoningTokens, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reasoningTokens: calculatedReasoningTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cachedTokens: cachedTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasError: streamingError !== null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| errorDetails: streamingError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -3578,6 +3593,20 @@ chat.openapi(completions, async (c) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| completionTokens, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Estimate reasoning tokens if not provided but reasoning content exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let calculatedReasoningTokens = reasoningTokens; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!reasoningTokens && reasoningContent) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| calculatedReasoningTokens = encode(reasoningContent).length; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Fallback to simple estimation if encoding fails | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Failed to encode reasoning text", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error instanceof Error ? error : new Error(String(error)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| calculatedReasoningTokens = estimateTokensFromContent(reasoningContent); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3596
to
+3609
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use explicit null check to preserve provider-supplied zero values. Same issue as in the streaming path: the condition Apply this diff: // Estimate reasoning tokens if not provided but reasoning content exists
let calculatedReasoningTokens = reasoningTokens;
-if (!reasoningTokens && reasoningContent) {
+if (reasoningTokens == null && reasoningContent) {
try {
calculatedReasoningTokens = encode(reasoningContent).length;
} catch (error) {
// Fallback to simple estimation if encoding fails
logger.error(
"Failed to encode reasoning text",
error instanceof Error ? error : new Error(String(error)),
);
calculatedReasoningTokens = estimateTokensFromContent(reasoningContent);
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const costs = calculateCosts( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| usedModel, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| usedProvider, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -3679,7 +3708,7 @@ chat.openapi(completions, async (c) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (calculatedPromptTokens || 0) + (calculatedCompletionTokens || 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).toString(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reasoningTokens: reasoningTokens, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reasoningTokens: calculatedReasoningTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cachedTokens: cachedTokens?.toString() || null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasError: hasEmptyNonStreamingResponse, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| streamed: false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use explicit null check to preserve provider-supplied zero values.
The condition
!reasoningTokensevaluates to true whenreasoningTokensis0, which means the code will recalculate even if a provider explicitly returns0to indicate no reasoning tokens were used.Apply this diff to use a more explicit check:
📝 Committable suggestion
🤖 Prompt for AI Agents