Skip to content

fix(cost-insights): show spend in partial chart periods#4643

Merged
jeanduplessis merged 3 commits into
mainfrom
jdp/cost-insights-charts
Jul 20, 2026
Merged

fix(cost-insights): show spend in partial chart periods#4643
jeanduplessis merged 3 commits into
mainfrom
jdp/cost-insights-charts

Conversation

@jeanduplessis

@jeanduplessis jeanduplessis commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Show known Cost Insights spend when a chart period has incomplete rollup coverage.

Why this change is needed

Recent query optimizations stopped loading canonical data for hours before rollup coverage. Daily and weekly chart buckets can therefore contain both covered and unavailable hours.

The presenter treated those partial buckets as having no spend, so the chart rendered dashed zero-value bars even when captured spend appeared in the contributor list.

How this is addressed

  • Preserve spend totals from covered hours in partial chart buckets.
  • Render partial spend as a visible bar with a dashed outline.
  • Label partial values as known minimum spend instead of complete totals.
  • Keep fully unavailable periods empty.
  • Describe anomaly detection as unusually high spend without implying a configured alert fired.
  • Update the evidence contract and presenter regression coverage.

Human Verification

  • The local code-review workflow passed all eight quality focuses with no findings.
  • React Doctor scanned the changed React code and reported no issues.

Reviewer Notes

Human Reviewer Flags

  • Partial-period values intentionally represent a lower bound. The chart shows captured spend while retaining incomplete-coverage warnings.

Code Reviewer Agent

Code Reviewer Notes
  • Partial evidence carries numeric category values; only unavailable evidence retains null values.
  • Tooltips and screen-reader labels distinguish known partial spend from complete totals.
  • The focused Jest suite could not run because the local PostgreSQL service and Docker daemon were unavailable. Type checking, linting, formatting, and static React analysis passed.

@kilo-code-bot

kilo-code-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Since the previous review, SpendEvidencePoint was refactored to split the combined 'complete' | 'partial' discriminant into two separate union members with identical field shapes; this is a type-only change with no effect on runtime behavior, and no logic, security, or runtime issues were found.

Files Reviewed (4 files)
  • apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx (unchanged since last review)
  • apps/web/src/lib/cost-insights/presenter.test.ts (unchanged since last review)
  • apps/web/src/lib/cost-insights/presenter.ts (unchanged since last review)
  • apps/web/src/components/cost-insights/types.ts - discriminated union split, no behavior change
Previous Review Summaries (2 snapshots, latest commit b856eca)

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

Previous review (commit b856eca)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Since the previous review, the only change is a single-line copy update in the anomaly tooltip label (removing the implication that a configured alert fired); no logic, security, or runtime issues found.

Files Reviewed (4 files)
  • apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx (unchanged since last review)
  • apps/web/src/components/cost-insights/types.ts (unchanged since last review)
  • apps/web/src/lib/cost-insights/presenter.test.ts (unchanged since last review)
  • apps/web/src/lib/cost-insights/presenter.ts - label text change only

Previous review (commit f3d195d)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the presenter/type/component changes that surface known spend for partially-covered chart periods; the discriminated-union change is consumed only within these files and the updated regression test matches the new behavior, with no security, runtime, logic, or breaking-API issues found in the changed lines.

Files Reviewed (4 files)
  • apps/web/src/components/cost-insights/overview/SpendEvidenceCard.tsx
  • apps/web/src/components/cost-insights/types.ts
  • apps/web/src/lib/cost-insights/presenter.test.ts
  • apps/web/src/lib/cost-insights/presenter.ts

Reviewed by claude-sonnet-5 · Input: 26 · Output: 5.1K · Cached: 509.1K

Review guidance: REVIEW.md from base branch main

@jeanduplessis
jeanduplessis merged commit c6ff4c7 into main Jul 20, 2026
15 checks passed
@jeanduplessis
jeanduplessis deleted the jdp/cost-insights-charts branch July 20, 2026 19:06
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