fix(usage): improve prompt savings estimates - #618
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRewrite savings now derive avoided input costs from observed blended rates, exclude separable fixed charges, and fall back to pricing when needed. Recalculation, tests, usage documentation, dashboard calculations and messaging, and settings help are updated. ChangesRewrite savings accounting and dashboard
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UsageEntry
participant ApplyRewriteSavings
participant EntryInputSegments
participant rewriteCostSaved
UsageEntry->>ApplyRewriteSavings: provide input cost and rewrite tokens saved
ApplyRewriteSavings->>EntryInputSegments: derive cache-aware input segments
EntryInputSegments-->>rewriteCostSaved: return input parts
ApplyRewriteSavings->>rewriteCostSaved: calculate avoided input cost
rewriteCostSaved-->>UsageEntry: store RewriteCostSaved
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Confidence Score: 5/5Safe to merge based on the exercised dashboard persistence and usage-pricing flows. The reviewed changes have no remaining defects: direct runtime reproductions and focused regression tests exercised the changed date persistence, rollover, cache-rate, fixed-charge, tier-pricing, and historical recalculation behavior successfully. Files Needing Attention: No files require follow-up attention.
What T-Rex did
Reviews (2): Last reviewed commit: "test(usage): consolidate fixed charge ca..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/usage/savings_test.go (1)
95-199: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMake the new fixed-charge scenarios table-driven.
These cases vary inputs and expectations but repeat the same setup, invocation, and assertion flow. Put them in a case table so provider-specific pricing behavior remains easy to extend consistently.
As per coding guidelines, “Add or update table-driven tests for behavior changes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/usage/savings_test.go` around lines 95 - 199, Convert the three fixed-charge tests—image, audio, and per-character cases—into a table-driven subtest using shared setup, ApplyRewriteSavings invocation, and savings assertions. Keep each case’s provider, pricing, raw-data key, input cost, and expected savings distinct, while leaving the unseparated fixed-charge and tier-crossing tests unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/usage/savings.go`:
- Around line 89-93: The per-character input branch in the savings calculation
validates pricing but does not apply its charge. Update the logic around
extractInt and pricing.PerCharacterInput to add the character-based input cost
to fixedCost, while preserving the existing missing-pricing failure behavior and
observed-rate calculation.
---
Outside diff comments:
In `@internal/usage/savings_test.go`:
- Around line 95-199: Convert the three fixed-charge tests—image, audio, and
per-character cases—into a table-driven subtest using shared setup,
ApplyRewriteSavings invocation, and savings assertions. Keep each case’s
provider, pricing, raw-data key, input cost, and expected savings distinct,
while leaving the unseparated fixed-charge and tier-crossing tests unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 029e911d-f36c-4246-9921-2281916c0a57
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-AqKCz9Vp.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (6)
internal/usage/savings.gointernal/usage/savings_test.gointernal/usage/usage.goweb/dashboard/src/pages/settings/PricingRecalculation.svelteweb/dashboard/src/pages/usage/usage-helpers.jsweb/dashboard/tests/usage.test.js
Summary
Testing
go test ./...git diff --checkSummary by CodeRabbit
Bug Fixes
Usage Dashboard
Documentation
RewriteCostSaved, including how the rate is derived and when it’s unavailable.