Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/src/__tests__/money-format.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('money formatting (issue #739)', () => {
it('groups thousands in every cost period', () => {
expect(formatCostForPeriod(10288, 'yearly')).toBe('$123,456');
expect(formatCostForPeriod(100000, 'daily')).toBe('$3,333.33');
expect(formatCostForPeriod(2000000, 'hourly')).toBe('$2,777.7778');
expect(formatCostForPeriod(2000000, 'hourly')).toBe('$2,739.7260');
expect(formatCostForPeriod(1234.4, 'monthly')).toBe('$1,234');
});
});
22 changes: 13 additions & 9 deletions frontend/src/__tests__/recommendations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, displaySavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline, loadColumnFilters, saveColumnFilters, resetColumnFiltersState, spGroupKey, groupSpCellKeys, formatCapacity, scaleRecForCapacity, weightedDisplaySavingsPct } from '../recommendations';
import type { CostPeriod } from '../state';
import { HOURS_PER_MONTH } from '../lib/hours-per-month';

// Mock the api module
jest.mock('../api', () => ({
Expand Down Expand Up @@ -5813,8 +5814,11 @@ describe('Issue #319: scaleCost', () => {
expect(scaleCost(100, 'monthly')).toBe(100);
});

test('hourly factor is 1/720', () => {
expect(scaleCost(720, 'hourly')).toBeCloseTo(1.0);
// Issues #337/#394: hourly uses the backend's 730-hour month, not 720.
test('hourly factor is 1/730, the shared hours-per-month', () => {
expect(HOURS_PER_MONTH).toBe(730);
expect(scaleCost(730, 'hourly')).toBeCloseTo(1.0, 10);
expect(scaleCost(720, 'hourly')).not.toBeCloseTo(1.0, 4);
});

test('daily factor is 1/30', () => {
Expand Down Expand Up @@ -5851,7 +5855,7 @@ describe('Issue #319: formatCostForPeriod', () => {
});

test('hourly uses 4 decimal places', () => {
expect(formatCostForPeriod(720, 'hourly')).toMatch(/\$1\.0000/);
expect(formatCostForPeriod(730, 'hourly')).toMatch(/\$1\.0000/);
});

test('daily uses 2 decimal places', () => {
Expand Down Expand Up @@ -6003,13 +6007,13 @@ describe('Issue #319: table cells scale with period', () => {
(state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({});
});

test('hourly: savings cell shows $1.0000', async () => {
test('hourly: savings cell shows $0.9863 (720 / 730 hours)', async () => {
(state.getCostPeriod as jest.Mock).mockReturnValue('hourly');
await loadRecommendations();
const rows = document.querySelectorAll('tr.recommendation-row');
expect(rows.length).toBeGreaterThan(0);
const savingsCell = rows[0]!.querySelector('.savings');
expect(savingsCell?.textContent).toMatch(/\$1\.0000/);
expect(savingsCell?.textContent).toMatch(/\$0\.9863/);
});

test('daily: savings cell shows $24.00', async () => {
Expand Down Expand Up @@ -6047,7 +6051,7 @@ describe('Issue #319: table cells scale with period', () => {
(state.getRecommendations as jest.Mock).mockReturnValue([recWithUpfront]);
await loadRecommendations();
const rows = document.querySelectorAll('tr.recommendation-row');
// We just verify "1000" appears somewhere but "0.0014" (1000/720) does not
// We just verify "1000" appears somewhere but "0.0014" (1000/730) does not
const rowText = rows[0]!.textContent ?? '';
expect(rowText).toContain('1000');
expect(rowText).not.toMatch(/0\.0014/);
Expand Down Expand Up @@ -6099,8 +6103,8 @@ describe('Issue #319: table cells scale with period', () => {

test('hourly numeric filter compares the scaled (per-hour) savings, not the raw monthly value', async () => {
// Both recs would pass a ">0.75" filter against raw monthly_cost (360
// and 720 are both > 0.75). After hourly scaling (÷720), only r-high
// (1.0) passes; r-low (0.5) is filtered out. If numericCellValue were
// and 720 are both > 0.75). After hourly scaling (÷730), only r-high
// (0.986) passes; r-low (0.493) is filtered out. If numericCellValue were
// to drop the scaleCost call, this test would surface the regression.
const recLow = { ...mockRec, id: 'r-low', resource_type: 'low', savings: 360, monthly_cost: 360 };
const recHigh = { ...mockRec, id: 'r-high', resource_type: 'high', savings: 720, monthly_cost: 720 };
Expand Down Expand Up @@ -7609,7 +7613,7 @@ describe('Issue #484: numeric filter matches the displayed rounded value', () =>
// display rounds to PERIOD_DECIMALS.hourly (4) decimals.
//
// raw monthly = 123.456789 → daily = monthly/30 = 4.1152263; hourly =
// monthly/720 = 0.171467... Use a monthly that's clean enough to keep
// monthly/730 = 0.169119... Use a monthly that's clean enough to keep
// the assertions readable while still distinguishing exact rounding.
const recs = [
{ id: 'r-exact', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 123.456789, upfront_cost: 500 },
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/lib/hours-per-month.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Hours in a month for every hourly <-> monthly cost conversion in the UI.
*
* 730 is AWS's canonical convention: 8760 hours in a 365-day year divided by
* 12 months. The backend uses the same figure (ricost.HoursPerMonth in
* internal/ricost/monthly_cost.go), so an hourly figure shown here matches the
* monthly figures the API derives. Keep the two in sync (issues #337, #394).
*/
export const HOURS_PER_MONTH = 730;
5 changes: 3 additions & 2 deletions frontend/src/modules/savings-history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { formatTrendAxisTick, upsertChart } from './chart-utils';
import { formatCompactCurrency, formatCurrency } from '../utils';
import { setChartSummary, renderChartTable, removeChartTable } from '../lib/chart-a11y';
import { applyChartDefaults, chartColor, withAlpha } from '../lib/chart-theme';
import { HOURS_PER_MONTH } from '../lib/hours-per-month';

// Register Chart.js components
Chart.register(...registerables);
Expand All @@ -26,8 +27,8 @@ const API_UNIT = 'monthly' as const;

export type SavingsUnit = 'hourly' | 'monthly' | 'yearly';

// Conversion factors relative to monthly as the canonical unit.
const HOURS_PER_MONTH = 730; // 365.25 * 24 / 12
// Conversion factors relative to monthly as the canonical unit. Hourly uses
// the shared 730-hour month (365 * 24 / 12), the same as the backend.
const MONTHS_PER_YEAR = 12;

/**
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/recommendations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function canActOnRecommendations(): boolean {
import { parseNumericFilter, applyColumnFilters as applyColumnFiltersLib } from './lib/column-filters';
import { fetchOverridesForAccounts } from './lib/overrides';
import { ownValue } from './lib/own-property';
import { HOURS_PER_MONTH } from './lib/hours-per-month';
// Re-export the shared primitives so existing consumers that import from
// recommendations.ts keep working without import-path churn (issue #166).
export { parseNumericFilter } from './lib/column-filters';
Expand Down Expand Up @@ -1726,9 +1727,12 @@ export function onDemandMonthly(r: LocalRecommendation): number | null {
// Cost-period scaling (issue #319)
//

/** Conversion factors relative to a monthly base. */
/**
* Conversion factors relative to a monthly base. Hourly uses the 730-hour
* month the backend uses (issues #337, #394); daily stays a 30-day month.
*/
const PERIOD_FACTOR: Record<CostPeriod, number> = {
hourly: 1 / 720, // 24 × 30 hrs/mo
hourly: 1 / HOURS_PER_MONTH,
daily: 1 / 30,
monthly: 1,
yearly: 12,
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/riexchange-confirm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
*/

import type { ExchangeQuoteSummary, RIExchangeHistoryRecord } from './api';

// Hours in an average month; only used for the approximate monthly delta.
const HOURS_PER_MONTH = 730;
import { HOURS_PER_MONTH } from './lib/hours-per-month';

export interface ExchangeConfirmParts {
quote: ExchangeQuoteSummary;
Expand Down
Loading