Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Apr 15, 2025

Important

Enhances OpenAI cache tracking and cost estimation by updating model handling and tests in openai-native.ts and openai-native.test.ts.

  • Behavior:
    • Updates OpenAiNativeHandler in openai-native.ts to include cache read tokens in cost calculation using calculateApiCostOpenAI.
    • Adds cacheReadsPrice to openAiNativeModels in api.ts for cost estimation.
  • Tests:
    • Modifies tests in openai-native.test.ts to verify individual response fields and usage data, including totalCost.
    • Uses type assertions to avoid TypeScript errors in tests.
  • Misc:
    • Refactors handleStreamResponse and related functions in openai-native.ts to pass model information for accurate cost calculation.

This description was created by Ellipsis for 1c814a9. It will automatically update as commits are pushed.

monotykamary and others added 3 commits April 14, 2025 22:19
Set correct cacheReadsPrice (cached input price) for gpt-4.1, gpt-4.1 mini, and gpt-4.1 nano based on official OpenAI pricing. No changes to cacheWritesPrice as per current OpenAI documentation. This ensures prompt caching costs are accurately reflected for these models in cost calculations.
@changeset-bot
Copy link

changeset-bot bot commented Apr 15, 2025

⚠️ No Changeset found

Latest commit: 1c814a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 15, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 15, 2025
// Check usage data fields but use toBeCloseTo for floating point comparison
expect(results[1].type).toBe("usage")
// Use type assertion to avoid TypeScript errors
expect((results[1] as any).inputTokens).toBe(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider defining proper result types to eliminate repeated 'as any' assertions, enhancing maintainability.

// Check usage data fields but use toBeCloseTo for floating point comparison
expect(results[3].type).toBe("usage")
// Use type assertion to avoid TypeScript errors
expect((results[3] as any).inputTokens).toBe(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider replacing the any cast with a more specific type or unknown to improve type safety in these assertions.

// Check usage data fields but use toBeCloseTo for floating point comparison
expect(results[1].type).toBe("usage")
// Use type assertion to avoid TypeScript errors
expect((results[1] as any).inputTokens).toBe(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

Repeated type assertions for usage properties appear in multiple tests. Consider extracting these assertions into a helper function to reduce duplication and improve maintainability.

@mrubens mrubens merged commit a64cab9 into main Apr 15, 2025
@mrubens mrubens deleted the fix_openai_caching branch April 15, 2025 02:55
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants