Skip to content

fix(cost-insights): clarify chart and settings states - #4929

Merged
jeanduplessis merged 2 commits into
mainfrom
hasty-valley
Jul 31, 2026
Merged

fix(cost-insights): clarify chart and settings states#4929
jeanduplessis merged 2 commits into
mainfrom
hasty-valley

Conversation

@jeanduplessis

@jeanduplessis jeanduplessis commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Clarify Cost Insights chart states and reduce settings clutter when Spend Alerts are disabled.

Why this change is needed

The spend chart treated zero spend like unavailable data and displayed a floating peak label that obscured the chart. Spend Alert sub-settings also remained visible when their parent setting was off, making disabled controls appear active.

How this is addressed

  • Remove the floating peak-value label from spend-over-time charts.
  • Show a dedicated no-spend state when the selected period has no spend.
  • Describe zero-spend chart points as “No spend” in tooltips and accessibility labels.
  • Preserve partial and unavailable coverage indicators when spend exists.
  • Hide anomaly and threshold settings until Spend Alerts are enabled.

Human Verification

Helium 2026-07-31 16 02 12 Helium 2026-07-31 16 02 13

Reviewer Notes

Human Reviewer Flags

No notable items for human review beyond what the summary covers.

Code Reviewer Agent

Code Reviewer Notes
  • Saved anomaly and threshold values remain in form state while their controls are hidden, so enabling Spend Alerts restores the existing configuration.
  • Zero spend takes precedence over unavailable coverage in user-facing chart copy, while nonzero partial or unavailable evidence retains its coverage treatment.

Comment thread apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx Outdated
Comment thread apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx Outdated
Comment thread apps/web/src/components/cost-insights/settings/CostInsightsSettingsView.tsx Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

All four previously reported issues are fixed at e1103a5dc; the only remaining item is that turning Spend Alerts off now silently discards valid pending threshold edits on save.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/components/cost-insights/CostInsightsSettingsClient.tsx 271 With form.enabled false, handleSave submits the saved thresholds, so a valid pending edit (for example 50 -> 75) is dropped without feedback; this is also inconsistent with anomalyAlertsEnabled, which is still taken from form state while its control is hidden
Files Reviewed (4 files)
  • apps/web/src/components/cost-insights/CostInsightsSettingsClient.tsx - 1 issue
  • apps/web/src/components/cost-insights/CostInsightsOverviewClient.tsx - no issues
  • apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx - no issues
  • apps/web/src/components/cost-insights/settings/CostInsightsSettingsView.tsx - no issues

Notes

  • Verified against HEAD e1103a5dc8b26084c6200d286f02900c05a02726.
  • Resolved since the previous review: coverage is now checked before zero spend in both the accessibility label and the tooltip, hasIncompleteEvidence no longer depends on hasSpend (matching main), hidden threshold validations no longer disable Save, and manage_threshold deep links fall back to #spend-alert-options-hidden when Spend Alerts are off.
  • The top-7 -> top-0 gridline offset is consistent with removing the h-5 peak-label row, and the removed copy has no test, story, or other references.
  • No tests were run (read-only review mode); these components have no unit tests in apps/web.

Fix these issues in Kilo Cloud

Previous Review Summary (commit b93bd43)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit b93bd43)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The new zero-spend handling conflates unavailable coverage with confirmed $0.00 spend, so periods with missing billing data now read as "no spend" in the chart, tooltips, and screen-reader summary.

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx 39 Gating hasIncompleteEvidence on hasSpend hides the chart when all buckets are unavailable, showing "No spend in the selected period yet." and a $0.00 total. screen-reader summary for missing data
apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx 136 pointTotal === 0 precedes the coverage checks, and unavailable points are always null -> 0, making the unavailable aria label and tooltip unreachable while the dashed unavailable bar still renders; partial buckets with $0.00 known spend also lose the coverage note
apps/web/src/components/cost-insights/settings/CostInsightsSettingsView.tsx 111 Unmounting the threshold inputs hides their validation errors while hasValidationError still disables Save, so an invalid amount plus toggling Spend Alerts off leaves Save disabled with no visible cause

SUGGESTION

File Line Issue
apps/web/src/components/cost-insights/settings/CostInsightsSettingsView.tsx 143 manage_threshold alert deep links to #spend-threshold-* now resolve to a page without the threshold sections or the removed "Spend Alerts are off" hint
Files Reviewed (2 files)
  • apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx - 2 issues
  • apps/web/src/components/cost-insights/settings/CostInsightsSettingsView.tsx - 2 issues

Notes

  • Verified against HEAD b93bd439b89d1173cf797ca417fce3331522969a; no pre-existing review comments were present.
  • The gridline offset change from top-7 to top-0 is consistent with removing the h-5 peak-label row and no test or story references the removed strings.
  • No tests were run (read-only review mode).

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 52 · Output: 16.3K · Cached: 1.6M

Review guidance: REVIEW.md from base branch main

@pandemicsyn pandemicsyn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Local review bot basically flagged the same issue as kilo-code. Other than that. lgtm.

@jeanduplessis
jeanduplessis merged commit d030082 into main Jul 31, 2026
16 checks passed
@jeanduplessis
jeanduplessis deleted the hasty-valley branch July 31, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants