-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fixed usage handling in OpenAI.ts
#9134
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
base: main
Are you sure you want to change the base?
Conversation
The presence of usage doesn't always mean it's the last chunk.
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
1 similar comment
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
|
All contributors have signed the CLA ✍️ ✅ |
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.
1 issue found across 1 file
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/openai-adapters/src/apis/OpenAI.ts">
<violation number="1" location="packages/openai-adapters/src/apis/OpenAI.ts:277">
P1: `finish_reason` is not a direct property on `ChatCompletionChunk` - it's on each choice in `choices[0].finish_reason`. This condition will always be false, causing usage data to never be captured.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| for await (const result of response) { | ||
| // Check if this chunk contains usage information | ||
| if (result.usage) { | ||
| if (result.usage && result.finish_reason === "stop") { |
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.
P1: finish_reason is not a direct property on ChatCompletionChunk - it's on each choice in choices[0].finish_reason. This condition will always be false, causing usage data to never be captured.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/openai-adapters/src/apis/OpenAI.ts, line 277:
<comment>`finish_reason` is not a direct property on `ChatCompletionChunk` - it's on each choice in `choices[0].finish_reason`. This condition will always be false, causing usage data to never be captured.</comment>
<file context>
@@ -274,7 +274,7 @@ export class OpenAIApi implements BaseLlmApi {
for await (const result of response) {
// Check if this chunk contains usage information
- if (result.usage) {
+ if (result.usage && result.finish_reason === "stop") {
// Store it to emit after all content chunks
lastChunkWithUsage = result;
</file context>
| if (result.usage && result.finish_reason === "stop") { | |
| if (result.usage && result.choices?.[0]?.finish_reason === "stop") { |
✅ Addressed in c2ded53
Bug Fix Requiredcubic correctly identified an issue: The current code: if (result.usage && result.finish_reason === "stop")Should be: if (result.usage && result.choices?.[0]?.finish_reason === "stop")I've created a branch with the fix: patch-1...continuedev:continue:fix-usage-finish-reason-access You can either:
The fix aligns with how other parts of the codebase access chunk properties (e.g., line 389: |
|
I have read the CLA Document and I hereby sign the CLA |

The presence of
usagedoesn't always mean it's the last chunk.Summary by cubic
Remove special handling of usage in streaming: yield every chunk as it arrives instead of deferring usage to the end. This avoids misordering when usage appears mid-stream.
Written for commit c2ded53. Summary will update automatically on new commits.