Restore Compare, fix card overflow, fill the API value card (1.3.1) - #94
Merged
Conversation
Compare never finished loading. "Align charts with provider reset boundaries" moved the charts onto reset-aligned windows and deleted the rolling comparison periods along the way, but the tab still waits for those periods before it renders, so it sat on "Comparing local history" until it timed out. The rolling windows are restored and scanned in the same pass as the reset-aligned ones. Their end is snapped to the minute, because a raw `now` moves every call and would defeat the chart cache. The test that asserted the empty list is replaced by one that fails when Compare has nothing to render. Long reset stamps ran across neighbouring usage cards: the detail line was `white-space: nowrap` in cards that can shrink to 118px, with no clipping. It wraps now, the dollar figure sits on its own line instead of breaking mid-value, and calendar windows carry dollars too, since showing them only on reset cards read as "no cost data" rather than "a different period". The Estimated API value card looked empty with one active provider: the single legend row stretched the full width, pushing its numbers to the far edge, and the change label collided with the ring. The legend is capped, idle providers stay listed at 0%, the label moved below the ring, and a seven-day trend fills the space beside it. Prepares 1.3.1, superseding the withdrawn 1.3.0.
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ceiling | b150fa9 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 05:30 AM |
📝 WalkthroughWalkthroughCeiling 1.3.1 adds rolling Compare windows, seven-day API-value trend data, updated usage-card presentation, zero-value provider slices, related tests, and synchronized release metadata. ChangesUsage charts and release update
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChartCommands
participant CostReport
participant BridgeModel
participant TotalApiValueCard
ChartCommands->>CostReport: scan daily and rolling usage windows
CostReport-->>ChartCommands: return token and API-value totals
ChartCommands-->>BridgeModel: populate lastSevenDays and comparisonPeriods
BridgeModel-->>TotalApiValueCard: provide provider trend data
TotalApiValueCard-->>TotalApiValueCard: aggregate and render trend bars
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/desktop-tauri/src-tauri/src/commands/chart.rs`:
- Around line 875-879: Capture a single refresh timestamp in the refresh
orchestration and thread it through the provider-building flow to the code
around comparison_period_specs, replacing the per-provider Utc::now() call.
Ensure every provider uses that same timestamp for comparison windows while
preserving the existing window extension behavior.
In `@apps/desktop-tauri/src/components/TotalApiValueCard.tsx`:
- Around line 257-280: Move the trend rendering block identified by the
api-value-card__trend class outside the !model.isEmpty branch so it remains
visible when the selected Today period has no data but trend contains prior-day
values. Preserve the existing trend condition and calculations, and add a
regression test covering an empty selected period with non-empty preceding
six-day trend data.
🪄 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: CHILL
Plan: Pro
Run ID: d03b3d14-e79b-4ecc-943b-988f64e3f107
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
CHANGELOG.mdapps/desktop-tauri/package.jsonapps/desktop-tauri/src-tauri/Cargo.tomlapps/desktop-tauri/src-tauri/src/commands/chart.rsapps/desktop-tauri/src-tauri/tauri.conf.jsonapps/desktop-tauri/src/components/TotalApiValueCard.tsxapps/desktop-tauri/src/lib/apiValueCard.test.tsapps/desktop-tauri/src/lib/apiValueCard.tsapps/desktop-tauri/src/styles.cssapps/desktop-tauri/src/surfaces/settings/providers/sections/charts/ChartsSection.test.tsxapps/desktop-tauri/src/surfaces/settings/providers/sections/charts/ChartsSection.tsxapps/desktop-tauri/src/types/bridge.tsrust/Cargo.tomlversion.env
Comment on lines
+875
to
+879
| // Compare reads shared rolling windows, which are independent of each | ||
| // provider's own reset boundaries. They are scanned alongside the | ||
| // reset-aligned windows so one pass over the logs serves both. | ||
| let (comparison_specs, comparison_windows) = comparison_period_specs(Utc::now()); | ||
| usage_windows.extend(comparison_windows); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use one refresh timestamp for all providers.
Line 878 captures Utc::now() inside each provider build. Builds that straddle a minute boundary get different rolling ranges, although ProviderComparison.tsx presents them as identical. Capture the timestamp once in the refresh orchestration and pass it here.
Proposed direction
- let (comparison_specs, comparison_windows) = comparison_period_specs(Utc::now());
+ let (comparison_specs, comparison_windows) = comparison_period_specs(comparison_now);🤖 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 `@apps/desktop-tauri/src-tauri/src/commands/chart.rs` around lines 875 - 879,
Capture a single refresh timestamp in the refresh orchestration and thread it
through the provider-building flow to the code around comparison_period_specs,
replacing the per-provider Utc::now() call. Ensure every provider uses that same
timestamp for comparison windows while preserving the existing window extension
behavior.
Comment on lines
+257
to
+280
| {trend.length > 0 && ( | ||
| <div className="api-value-card__trend"> | ||
| <div className="api-value-card__trend-head"> | ||
| <span>Last 7 days</span> | ||
| <strong>{formatUsd(trend.reduce((sum, day) => sum + day.value, 0))}</strong> | ||
| </div> | ||
| <div className="api-value-card__trend-bars"> | ||
| {trend.map((day, index) => ( | ||
| <span | ||
| key={day.date} | ||
| className="api-value-card__trend-bar" | ||
| data-today={index === trend.length - 1} | ||
| style={{ height: `${day.height}%` }} | ||
| title={`${day.label}: ${formatUsd(day.value)}`} | ||
| /> | ||
| ))} | ||
| </div> | ||
| <div className="api-value-card__trend-days"> | ||
| {trend.map((day) => ( | ||
| <span key={day.date}>{day.label}</span> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render the trend when the selected period is empty.
The trend is inside the !model.isEmpty branch. With no usage today but values during the preceding six days, the default Today view only shows “No data” and hides the new seven-day trend. Render it independently of the selected-period empty state and add this regression case.
🤖 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 `@apps/desktop-tauri/src/components/TotalApiValueCard.tsx` around lines 257 -
280, Move the trend rendering block identified by the api-value-card__trend
class outside the !model.isEmpty branch so it remains visible when the selected
Today period has no data but trend contains prior-day values. Preserve the
existing trend condition and calculations, and add a regression test covering an
empty selected period with non-empty preceding six-day trend data.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the three problems found in the withdrawn 1.3.0, and prepares 1.3.1.
1. Compare tab never loaded (regression, the reason 1.3.0 was pulled)
9897bd4a"Align charts with provider reset boundaries" moved the charts onto reset-aligned windows and deletedcomparison_period_specs()along the way, leavingcomparison_periods: Vec::new()plus a test asserting that empty state. ButProviderComparison.tsxstill gates rendering oncomparisonPeriods.length >= 2, so the tab retried 60x and gave up. Shipped in 1.3.0, absent from 1.2.1.Restored the rolling 5h/7d periods (current + prior window each), scanned in the same pass as the reset-aligned windows. Their end is snapped to the minute — a raw
nowmoves every call and would defeat the chart cache, which is likely why this was ripped out.CHART_CACHE_VERSIONbumped to 6 so entries cached at 5 (holding the empty list) can't keep the tab stuck after upgrade.The test asserting emptiness is replaced by one that fails when Compare has nothing to render, so this cannot silently rot again.
2. Text running across the usage cards
.usage-period > smallwaswhite-space: nowrapin a grid whose columns shrink to 118px, with no clipping — so long reset stamps ran straight across their neighbours. 1.3.0 made it worse by lengthening the copy.Detail lines wrap now, the dollar figure sits on its own line instead of breaking mid-value, and calendar windows carry dollars too (
sevenDayCost/thirtyDayCostalready existed) — showing$only on reset cards read as "no cost data" rather than "a different period".3. Estimated API value card looked empty
One active provider meant one legend row stretched across the full card, flinging its share and value to the far edge, and the change label collided with the ring stroke.
Legend capped at 186px, idle providers stay listed at
0%, the change label moved below the ring, and a seven-day trend (newlastSevenDaysseries, built from the same windowed scan) fills the space. Slices stay empty when nothing has data, so the existing empty-state behaviour is preserved.Verification
-D warningsclean,cargo fmt --checkclean.tsc --noEmitclean.styles.cssagainst the real component markup headlessly (not a mock) to confirm the layout, including the 4-card 5-hour + weekly + calendar case.local-check.ps1 -All -Version 1.3.1passes; release-doctor reports all five version locations at 1.3.1, with only the expected pre-tag warnings.Summary by CodeRabbit
New Features
Bug Fixes
Release