Skip to content

fix(codex): estimate reasoning tokens from accumulated content when u… #357

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
+36 −6
Open

fix(codex): estimate reasoning tokens from accumulated content when u… #357

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions internal/translator/codex/openai/chat-completions/codex_openai_response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ var (

// ConvertCliToOpenAIParams holds parameters for response conversion.
type ConvertCliToOpenAIParams struct {
ResponseID string
CreatedAt int64
Model string
FunctionCallIndex int
ResponseID string
CreatedAt int64
Model string
FunctionCallIndex int
AccumulatedReasoningLen int // Track reasoning content length for token estimation
}

// ConvertCodexResponseToOpenAI translates a single chunk of a streaming response from the
Expand Down Expand Up @@ -90,15 +91,26 @@ func ConvertCodexResponseToOpenAI(_ context.Context, modelName string, originalR
if inputTokensResult := usageResult.Get("input_tokens"); inputTokensResult.Exists() {
template, _ = sjson.Set(template, "usage.prompt_tokens", inputTokensResult.Int())
}
if reasoningTokensResult := usageResult.Get("output_tokens_details.reasoning_tokens"); reasoningTokensResult.Exists() {
// Extract reasoning tokens from upstream, or estimate from accumulated content
reasoningTokensResult := usageResult.Get("output_tokens_details.reasoning_tokens")
if reasoningTokensResult.Exists() && reasoningTokensResult.Int() > 0 {
template, _ = sjson.Set(template, "usage.completion_tokens_details.reasoning_tokens", reasoningTokensResult.Int())
} else if (*param).(*ConvertCliToOpenAIParams).AccumulatedReasoningLen > 0 {
// Estimate reasoning tokens from accumulated content (approx 4 chars per token)
estimatedTokens := int64((*param).(*ConvertCliToOpenAIParams).AccumulatedReasoningLen / 4)
if estimatedTokens == 0 {
estimatedTokens = 1
}
Comment on lines +99 to +103
Copy link

@gemini-code-assist gemini-code-assist bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The magic number 4 is used for token estimation. It's better to define this as a constant, like const defaultCharsPerToken = 4, to improve readability and maintainability. Also, this token estimation logic is duplicated in the non-streaming function (ConvertCodexResponseToOpenAINonStream). You might consider extracting it into a helper function to avoid code duplication and ensure consistency.

Suggested change
// Estimate reasoning tokens from accumulated content (approx 4 chars per token)
estimatedTokens := int64((*param).(*ConvertCliToOpenAIParams).AccumulatedReasoningLen / 4)
if estimatedTokens == 0 {
estimatedTokens = 1
}
// Estimate reasoning tokens from accumulated content (approx 4 chars per token)
const defaultCharsPerToken = 4
estimatedTokens := int64((*param).(*ConvertCliToOpenAIParams).AccumulatedReasoningLen / defaultCharsPerToken)
if estimatedTokens == 0 {
estimatedTokens = 1
}

template, _ = sjson.Set(template, "usage.completion_tokens_details.reasoning_tokens", estimatedTokens)
}
}

if dataType == "response.reasoning_summary_text.delta" {
if deltaResult := rootResult.Get("delta"); deltaResult.Exists() {
template, _ = sjson.Set(template, "choices.0.delta.role", "assistant")
template, _ = sjson.Set(template, "choices.0.delta.reasoning_content", deltaResult.String())
// Accumulate reasoning content length for token estimation
(*param).(*ConvertCliToOpenAIParams).AccumulatedReasoningLen += len(deltaResult.String())
}
} else if dataType == "response.reasoning_summary_text.done" {
template, _ = sjson.Set(template, "choices.0.delta.role", "assistant")
Expand Down Expand Up @@ -194,6 +206,9 @@ func ConvertCodexResponseToOpenAINonStream(_ context.Context, _ string, original
template, _ = sjson.Set(template, "id", idResult.String())
}

// Track upstream reasoning tokens for later estimation
var upstreamReasoningTokens int64

// Extract and set usage metadata (token counts).
if usageResult := responseResult.Get("usage"); usageResult.Exists() {
if outputTokensResult := usageResult.Get("output_tokens"); outputTokensResult.Exists() {
Expand All @@ -206,7 +221,7 @@ func ConvertCodexResponseToOpenAINonStream(_ context.Context, _ string, original
template, _ = sjson.Set(template, "usage.prompt_tokens", inputTokensResult.Int())
}
if reasoningTokensResult := usageResult.Get("output_tokens_details.reasoning_tokens"); reasoningTokensResult.Exists() {
template, _ = sjson.Set(template, "usage.completion_tokens_details.reasoning_tokens", reasoningTokensResult.Int())
upstreamReasoningTokens = reasoningTokensResult.Int()
}
}

Expand Down Expand Up @@ -275,10 +290,25 @@ func ConvertCodexResponseToOpenAINonStream(_ context.Context, _ string, original
template, _ = sjson.Set(template, "choices.0.message.role", "assistant")
}

// Calculate reasoning tokens
var reasoningTokens int64
if upstreamReasoningTokens > 0 {
reasoningTokens = upstreamReasoningTokens
} else if reasoningText != "" {
// Estimate from reasoning content (approx 4 chars per token)
reasoningTokens = int64(len(reasoningText) / 4)
if reasoningTokens == 0 {
reasoningTokens = 1
}
Comment on lines +298 to +302
Copy link

@gemini-code-assist gemini-code-assist bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The magic number 4 is used here for token estimation, similar to the streaming implementation. To improve maintainability and ensure consistency, this should be a shared constant. As this logic is duplicated, consider creating a common helper function for token estimation.

			// Estimate from reasoning content (approx 4 chars per token)
			const defaultCharsPerToken = 4
			reasoningTokens = int64(len(reasoningText) / defaultCharsPerToken)
			if reasoningTokens == 0 {
				reasoningTokens = 1
			}

}

if reasoningText != "" {
template, _ = sjson.Set(template, "choices.0.message.reasoning_content", reasoningText)
template, _ = sjson.Set(template, "choices.0.message.role", "assistant")
}
if reasoningTokens > 0 {
template, _ = sjson.Set(template, "usage.completion_tokens_details.reasoning_tokens", reasoningTokens)
}

// Add tool calls if any
if len(toolCalls) > 0 {
Expand Down
Loading