Skip to content

Only write non-zero AdditionalCounts keys in OpenAI chat addUsage - #737

Merged
George Adams (gdams) merged 1 commit into
microsoft:mainfrom
PratikDhanaveFork:openai-nonzero-additional-counts
Jul 29, 2026
Merged

Only write non-zero AdditionalCounts keys in OpenAI chat addUsage#737
George Adams (gdams) merged 1 commit into
microsoft:mainfrom
PratikDhanaveFork:openai-nonzero-additional-counts

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

What

addUsage in provider/openaiprovider/chat.go unconditionally allocated AdditionalCounts and wrote the four audio/prediction token details regardless of value. Every text-only response therefore carried four zero-valued entries (PromptTokensDetails.AudioTokens, CompletionTokensDetails.AudioTokens, .AcceptedPredictionTokens, .RejectedPredictionTokens).

This change guards each key with a small helper so it is added only when non-zero, leaving AdditionalCounts nil in the common text-only case.

Why

  • Parity with the sibling Anthropic provider, which already guards its extra count (if usage.CacheCreationInputTokens != 0 { ... }) before allocating AdditionalCounts.
  • Parity with the Python SDK, which writes each usage detail only when the underlying token value is truthy.
  • Downstream impact: otelprovider's hasUsage returns true whenever len(AdditionalCounts) > 0, so all-zero OpenAI usage previously reported hasUsage == true and emitted four zero-valued additional-count span attributes for plain text responses. With the guard, text-only responses no longer inflate the usage map or the span.

How it is tested

Existing black-box tests in chat_test.go are updated to reflect the corrected behavior: text-only responses now expect a nil AdditionalCounts, and the audio-token case asserts only the two non-zero keys are present (the two zero prediction-token keys are dropped). These assertions fail before the fix and pass after. go build ./..., go vet ./provider/openaiprovider/..., and go test ./provider/openaiprovider/... all pass.

Copilot AI review requested due to automatic review settings July 24, 2026 03:47
@PratikDhanave
PratikDhanave (PratikDhanave) requested a review from a team as a code owner July 24, 2026 03:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request refines OpenAI chat usage reporting so UsageDetails.AdditionalCounts is only allocated and populated when OpenAI provides non-zero “detail” token counts, avoiding noisy zero-valued attributes for text-only responses and improving downstream OpenTelemetry span emission behavior.

Changes:

  • Updated addUsage to lazily allocate AdditionalCounts and only write non-zero usage-detail keys.
  • Updated OpenAI provider black-box tests to expect AdditionalCounts == nil for text-only responses and to assert only non-zero additional-count keys are present.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
provider/openaiprovider/chat.go Lazily populates UsageDetails.AdditionalCounts with non-zero OpenAI usage-detail counts only.
provider/openaiprovider/chat_test.go Adjusts expectations to match the new “only non-zero additional counts” behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the parity-approved Go API consistency review found no parity issues label Jul 24, 2026
addUsage unconditionally allocated AdditionalCounts and wrote the audio and
prediction token details, so every text-only response carried four zero
entries. This made otelprovider's hasUsage report true and emit four
zero-valued additional-count span attributes for plain text responses.

Guard each detail so keys are added only when non-zero, leaving
AdditionalCounts nil in the common text-only case. This matches the
sibling Anthropic provider (which guards CacheCreationInputTokens) and the
Python SDK, which writes each detail only when truthy.
@PratikDhanave
PratikDhanave (PratikDhanave) force-pushed the openai-nonzero-additional-counts branch from cabd2ad to 502056a Compare July 24, 2026 09:30
@github-actions

Copy link
Copy Markdown
Contributor

Parity Review — parity-approved

This PR makes an internal behavioral fix to the unexported addUsage helper in provider/openaiprovider/chat.go. No exported function or method signatures change.

Cross-repo consistency assessment:

  • Go Anthropic provider: Already uses the same guard pattern (if usage.CacheCreationInputTokens != 0 { ... }) before allocating AdditionalCounts. This PR brings the OpenAI provider into alignment with that existing pattern.
  • Python SDK: The PR description cites that Python writes each usage detail only when the underlying token value is truthy — the Go fix mirrors that behavior.
  • Behavioral effect: AdditionalCounts will now be nil (not an empty non-nil map) for text-only responses, which correctly prevents otelprovider's hasUsage from returning true on zero-valued entries. This is the expected semantics.

API surface: No exported symbols were added, removed, or changed — UsageContent, AdditionalCounts, and all public provider types remain identical. The public-api-change label is not warranted.

This change preserves and improves cross-SDK semantic parity. No consistency issues found.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent · 38.3 AIC · ⌖ 5.58 AIC · ⊞ 5.9K ·

@gdams
George Adams (gdams) added this pull request to the merge queue Jul 29, 2026
Merged via the queue into microsoft:main with commit e9fa53f Jul 29, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parity-approved Go API consistency review found no parity issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants