cost dialog: enrich with session stats, per-model percentages, and formatting fixes#2046
Conversation
…rmatting fixes Enhance the /cost dialog with richer session insights: - Show total token count (input + output combined) in the Total section - Display session duration and message count in the header - Show reasoning tokens when the model reports them (e.g. o1) - Add average cost per 1K tokens and average cost per message metrics - Show cost percentage breakdown per model and per message - Display cache hit rate and per-entry cached token counts - Include model name in the plain-text clipboard copy output Code quality improvements: - Extract shared stat computation (totalStats, actualMessageCount, plainTextLine) to eliminate duplication between styled and plain-text renderers - Fix slice aliasing bug in applyScrolling (use slices.Clone) - Fix formatCost/formatCostPadded silently swallowing negative values - Fix formatDuration producing garbled output for negative durations Assisted-By: docker-agent
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Assessment: 🟡 NEEDS ATTENTION
The PR successfully adds valuable session insights and formatting improvements to the cost dialog. However, 1 confirmed bug was found in the newly added cache hit rate calculation that will produce incorrect percentages when cache writes are present.
Findings
- 1 MEDIUM severity issue requiring attention
- 0 CRITICAL issues
Cache Hit Rate Formula
The cache hit rate calculation should exclude CacheWriteTokens from the denominator. Cache write tokens represent tokens written to cache for future use, not tokens that were candidates for cache hits. Including them artificially deflates the hit rate percentage.
For example:
- With CachedInputTokens=700, InputTokens=300, CacheWriteTokens=200
- Current formula: 700/(700+300+200) = 58.3%
- Correct formula: 700/(700+300) = 70%
Consider changing line 185 from:
float64(d.total.CachedInputTokens)/float64(totalIn)*100to:
float64(d.total.CachedInputTokens)/float64(d.total.CachedInputTokens + d.total.InputTokens)*100| stats = append(stats, stat{"avg cost/1K tokens:", formatCost(d.total.cost / float64(tok) * 1000)}) | ||
| } | ||
| if totalIn := d.total.totalInput(); totalIn > 0 && d.total.CachedInputTokens > 0 { | ||
| stats = append(stats, stat{"cache hit rate:", fmt.Sprintf("%.0f%%", float64(d.total.CachedInputTokens)/float64(totalIn)*100)}) |
There was a problem hiding this comment.
MEDIUM: Cache hit rate calculation includes CacheWriteTokens in denominator
The denominator totalIn includes CacheWriteTokens, which artificially inflates the base and produces an incorrect (lower) percentage.
The semantically correct cache hit rate should be:
float64(d.total.CachedInputTokens) / float64(d.total.CachedInputTokens + d.total.InputTokens) * 100CacheWriteTokens represent tokens written to the cache for future use, not tokens that were candidates for cache hits, so they shouldn't be included in the denominator.
Example:
- CachedInputTokens=700, InputTokens=300, CacheWriteTokens=200
- Current: 700/1200 = 58.3% ❌
- Correct: 700/1000 = 70% ✅
1 participant
Summary
Enhance the
/costdialog with richer session insights and fix several edge-case bugs.New features
Code quality improvements
totalStats,actualMessageCount,plainTextLine) to eliminate duplication between styled and plain-text renderersapplyScrolling(useslices.Clone)formatCost/formatCostPaddedsilently swallowing negative valuesformatDurationproducing garbled output for negative durations