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
135 changes: 121 additions & 14 deletions frontend/src/__tests__/recommendations.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Recommendations module tests
*/
import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline, loadColumnFilters, saveColumnFilters, resetColumnFiltersState, spGroupKey, groupSpCellKeys } from '../recommendations';
import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline, loadColumnFilters, saveColumnFilters, resetColumnFiltersState, spGroupKey, groupSpCellKeys, formatCapacity } from '../recommendations';
import type { CostPeriod } from '../state';

// Mock the api module
Expand Down Expand Up @@ -628,6 +628,46 @@ describe('Recommendations Module', () => {
expect(list?.innerHTML).toContain('us-east-1');
});

// #219 end-to-end render regression. Feeds the REAL API shape the fixed
// backend now emits: top-level vcpu/memory_gb (decoded from the nested
// ComputeDetails by buildRecommendationsResponse). Asserts the Capacity
// cell renders "8 vCPU / 32 GB" for a sized compute rec and "—" for a rec
// with neither field. Pre-fix, the backend never emitted these top-level
// fields, so the cell rendered "—" for every row in production while the
// formatCapacity unit tests stayed green on injected values the API never
// produced — this test ties the rendered cell to the real response shape.
test('renders Capacity cell from top-level vcpu/memory_gb (#219)', async () => {
(api.getRecommendations as jest.Mock).mockResolvedValue({
summary: {},
recommendations: [
{ id: 'rec-cap', provider: 'aws', service: 'ec2',
resource_type: 'm5.2xlarge', region: 'us-east-1',
count: 1, term: 1, savings: 100, upfront_cost: 500,
vcpu: 8, memory_gb: 32 },
{ id: 'rec-nocap', provider: 'aws', service: 'rds',
resource_type: 'db.t3.medium', region: 'us-east-1',
count: 1, term: 1, savings: 50, upfront_cost: 200 },
],
regions: ['us-east-1'],
});

await loadRecommendations();

const list = document.getElementById('recommendations-list');
// The capacity column is the cell immediately after resource_type.
const capCell = (recId: string): string | undefined => {
const checkbox = list?.querySelector<HTMLElement>(`input[data-rec-id="${recId}"]`);
const row = checkbox?.closest('tr');
const cells = Array.from(row?.querySelectorAll('td') ?? []);
const rtIdx = cells.findIndex((td) => td.textContent?.includes(recId === 'rec-cap' ? 'm5.2xlarge' : 'db.t3.medium'));
return cells[rtIdx + 1]?.textContent ?? undefined;
};

expect(capCell('rec-cap')).toBe('8 vCPU / 32 GB');
// Rec with no vcpu/memory_gb renders the em-dash placeholder, not "0".
expect(capCell('rec-nocap')).toBe('—');
});

test('shows empty-state message when no recommendations', async () => {
// Issue #700: zero rows now render an empty <tbody> with a hint cell
// rather than replacing the entire table with a <p>. The <thead> stays
Expand Down Expand Up @@ -973,15 +1013,15 @@ describe('Recommendations Module', () => {
});
});

test('renders sortable column headers with indicators (Bundle B + #282 + #317: 13 columns)', async () => {
test('renders sortable column headers with indicators (Bundle B + #282 + #317 + #219: 14 columns)', async () => {
await loadRecommendations();
const list = document.getElementById('recommendations-list');
// Bundle B + issue #282 + #317: every data column is sortable. 13 sortable data columns:
// provider, account, service, resource_type, region, count, term, payment,
// Bundle B + issue #282 + #317 + #219: every data column is sortable. 14 sortable data columns:
// provider, account, service, resource_type, capacity, region, count, term, payment,
// savings, upfront_cost, monthly_cost, on_demand_monthly, effective_savings_pct.
// The leading checkbox column is not sortable.
const sortables = list?.querySelectorAll('th.sortable');
expect(sortables?.length).toBe(13);
expect(sortables?.length).toBe(14);
// The default sort is savings desc → that header shows an active ▼.
const savingsHeader = list?.querySelector('th[data-sort="savings"]');
expect(savingsHeader?.innerHTML).toContain('active');
Expand Down Expand Up @@ -1024,9 +1064,9 @@ describe('Recommendations Module', () => {

const rows = document.querySelectorAll('tr.recommendation-row');
const paymentCells = Array.from(rows).map((row) => {
// Payment column is the 9th <td> (0-indexed: 0=checkbox, 1=provider,
// 2=account, 3=service, 4=resource_type, 5=region, 6=count, 7=term, 8=payment)
return row.querySelectorAll('td')[8]?.textContent?.trim() ?? '';
// Payment column is the 10th <td> (0-indexed: 0=checkbox, 1=provider,
// 2=account, 3=service, 4=resource_type, 5=capacity, 6=region, 7=count, 8=term, 9=payment)
return row.querySelectorAll('td')[9]?.textContent?.trim() ?? '';
});
expect(paymentCells).toContain('All Upfront');
expect(paymentCells).toContain('Partial Upfront');
Expand Down Expand Up @@ -1057,7 +1097,7 @@ describe('Recommendations Module', () => {
// Only the all-upfront rec should be rendered.
expect(rows.length).toBe(1);
// Payment cell text should be "All Upfront".
const paymentCell = rows[0]?.querySelectorAll('td')[8];
const paymentCell = rows[0]?.querySelectorAll('td')[9];
expect(paymentCell?.textContent?.trim()).toBe('All Upfront');

// Restore filter mock so it doesn't leak into subsequent tests.
Expand Down Expand Up @@ -1997,6 +2037,29 @@ describe('applyColumnFilters', () => {
});
expect(out.map(r => r.id)).toEqual(['b']);
});

test('capacity filter matches formatted "N vCPU / M GB" string', () => {
const recs: LocalRecommendation[] = [
{ ...rec({ id: 'a' }), vcpu: 8, memory_gb: 32 } as LocalRecommendation,
{ ...rec({ id: 'b' }), vcpu: 4, memory_gb: 16 } as LocalRecommendation,
{ ...rec({ id: 'c' }), vcpu: null, memory_gb: null } as unknown as LocalRecommendation,
];
const out = applyColumnFilters(recs, {
capacity: { kind: 'set', values: ['8 vCPU / 32 GB'] },
});
expect(out.map(r => r.id)).toEqual(['a']);
});

test('capacity filter with empty string matches recs without vcpu/memory_gb', () => {
const recs: LocalRecommendation[] = [
{ ...rec({ id: 'a' }), vcpu: 8, memory_gb: 32 } as LocalRecommendation,
{ ...rec({ id: 'b' }), vcpu: null, memory_gb: null } as unknown as LocalRecommendation,
];
const out = applyColumnFilters(recs, {
capacity: { kind: 'set', values: [''] },
});
expect(out.map(r => r.id)).toEqual(['b']);
});
});

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -2118,7 +2181,7 @@ describe('Bundle B: column header filter triggers', () => {
const buttons = document.querySelectorAll<HTMLButtonElement>('th .column-filter-btn[data-column]');
const cols = Array.from(buttons).map((b) => b.dataset['column']);
expect(cols.sort()).toEqual(
['account', 'count', 'effective_savings_pct', 'monthly_cost', 'on_demand_monthly', 'payment', 'provider', 'region', 'resource_type', 'savings', 'service', 'term', 'upfront_cost'].sort(),
['account', 'capacity', 'count', 'effective_savings_pct', 'monthly_cost', 'on_demand_monthly', 'payment', 'provider', 'region', 'resource_type', 'savings', 'service', 'term', 'upfront_cost'].sort(),
);
});

Expand Down Expand Up @@ -5368,12 +5431,13 @@ describe('Column visibility (issue #318)', () => {
// --- TOGGLEABLE_COLUMNS and COLUMN_DEFS ---

describe('COLUMN_DEFS and TOGGLEABLE_COLUMNS', () => {
test('COLUMN_DEFS contains all 14 column ids (13 data + usage_history sparkline)', () => {
test('COLUMN_DEFS contains all 15 column ids (13 data + capacity + usage_history sparkline)', () => {
const keys = COLUMN_DEFS.map((c) => c.key);
expect(keys).toContain('provider');
expect(keys).toContain('account');
expect(keys).toContain('service');
expect(keys).toContain('resource_type');
expect(keys).toContain('capacity');
expect(keys).toContain('region');
expect(keys).toContain('count');
expect(keys).toContain('term');
Expand All @@ -5385,7 +5449,12 @@ describe('Column visibility (issue #318)', () => {
expect(keys).toContain('effective_savings_pct');
// issue #239: usage_history sparkline column added
expect(keys).toContain('usage_history');
expect(COLUMN_DEFS.length).toBe(14);
expect(COLUMN_DEFS.length).toBe(15);
});

test('capacity column appears immediately after resource_type', () => {
const keys = COLUMN_DEFS.map((c) => c.key);
expect(keys.indexOf('capacity')).toBe(keys.indexOf('resource_type') + 1);
});

test('TOGGLEABLE_COLUMNS excludes fixed identity columns', () => {
Expand All @@ -5394,7 +5463,8 @@ describe('Column visibility (issue #318)', () => {
expect(keys).not.toContain('account');
expect(keys).not.toContain('service');
expect(keys).not.toContain('resource_type');
// All other 10 columns (9 original + usage_history) should be toggleable.
// All other 11 columns (9 original + capacity + usage_history) should be toggleable.
expect(keys).toContain('capacity');
expect(keys).toContain('region');
expect(keys).toContain('count');
expect(keys).toContain('term');
Expand All @@ -5405,11 +5475,48 @@ describe('Column visibility (issue #318)', () => {
expect(keys).toContain('on_demand_monthly');
expect(keys).toContain('effective_savings_pct');
expect(keys).toContain('usage_history');
expect(keys.length).toBe(10);
expect(keys.length).toBe(11);
});
});
});

// ---------------------------------------------------------------------------
// formatCapacity (closes #219)
// ---------------------------------------------------------------------------
describe('formatCapacity', () => {
test('returns formatted string when both vcpu and memory_gb are populated', () => {
expect(formatCapacity(8, 32)).toBe('8 vCPU / 32 GB');
});

test('handles fractional memory without trailing zeros', () => {
expect(formatCapacity(2, 0.5)).toBe('2 vCPU / 0.5 GB');
});

test('returns null when vcpu is null', () => {
expect(formatCapacity(null, 32)).toBeNull();
});

test('returns null when memory_gb is null', () => {
expect(formatCapacity(8, null)).toBeNull();
});

test('returns null when vcpu is undefined', () => {
expect(formatCapacity(undefined, 32)).toBeNull();
});

test('returns null when memory_gb is undefined', () => {
expect(formatCapacity(8, undefined)).toBeNull();
});

test('returns null when vcpu is 0 (unknown)', () => {
expect(formatCapacity(0, 32)).toBeNull();
});

test('returns null when memory_gb is 0 (unknown)', () => {
expect(formatCapacity(8, 0)).toBeNull();
});
});

// ---------------------------------------------------------------------------
// Issue #494: deterministic group sort on multi-variant cells.
//
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ export interface Recommendation {
// oldest-to-newest). Absent/null when the provider did not populate it
// (non-AWS providers or pre-#239 cached rows).
usage_history?: number[] | null;
// vcpu / memory_gb surface the compute size of the recommended instance
// type so the Capacity column can render "<vcpu> vCPU / <memory_gb> GB"
// (#219). Emitted top-level by the backend (buildRecommendationsResponse
// decodes the nested ComputeDetails and stamps them) only for compute recs
// with a known size; absent for non-compute services, legacy rows, or
// unknown sizes. formatCapacity renders absent/0 as "—".
vcpu?: number | null;
memory_gb?: number | null;
}

export interface RecommendationFilters {
Expand Down
19 changes: 19 additions & 0 deletions frontend/src/recommendations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,7 @@ export const COLUMN_DEFS: readonly ColumnDef[] = [
{ key: 'account', label: 'Account', kind: 'categorical' },
{ key: 'service', label: 'Service', kind: 'categorical' },
{ key: 'resource_type', label: 'Resource Type', kind: 'categorical' },
{ key: 'capacity', label: 'Capacity', kind: 'categorical' },
{ key: 'region', label: 'Region', kind: 'categorical' },
{ key: 'count', label: 'Count', kind: 'numeric' },
{ key: 'term', label: 'Term', kind: 'categorical' },
Expand Down Expand Up @@ -1638,12 +1639,24 @@ export function applyColumnFilters(
);
}

// formatCapacity renders the VCPU+MemoryGB pair from a ComputeDetails rec.
// Mirrors the Go-side ComputeDetails.GetDetailDescription format:
// "<vcpu> vCPU / <memoryGB> GB". Returns null when either field is absent or
// zero so callers can render a dash rather than a misleading "0 vCPU / 0 GB".
export function formatCapacity(vcpu: number | null | undefined, memoryGB: number | null | undefined): string | null {
if (!vcpu || !memoryGB) return null;
// String(n) trims trailing zeros for whole numbers (16, not 16.0)
// and preserves fractional precision (0.5), matching Go's %g format.
return `${vcpu} vCPU / ${String(memoryGB)} GB`;
}

function categoricalCellValue(r: LocalRecommendation, col: state.RecommendationsColumnId): string {
switch (col) {
case 'provider': return r.provider ?? '';
case 'account': return r.cloud_account_id ?? '';
case 'service': return r.service ?? '';
case 'resource_type': return r.resource_type ?? '';
case 'capacity': return formatCapacity(r.vcpu, r.memory_gb) ?? '';
case 'region': return r.region ?? '';
case 'term': return r.term == null ? '' : String(r.term);
case 'payment': return r.payment ?? '';
Expand Down Expand Up @@ -1683,6 +1696,7 @@ function numericCellValue(r: LocalRecommendation, col: state.RecommendationsColu
case 'account':
case 'service':
case 'resource_type':
case 'capacity':
case 'region':
case 'term':
case 'payment':
Expand Down Expand Up @@ -1725,6 +1739,7 @@ export function displayPrecision(col: state.RecommendationsColumnId, period: Cos
case 'account':
case 'service':
case 'resource_type':
case 'capacity':
case 'region':
case 'term':
case 'payment':
Expand Down Expand Up @@ -2747,6 +2762,10 @@ function renderColumnCell(key: state.RecommendationsColumnId, rec: LocalRecommen
return `<td><span class="service-badge">${escapeHtml(rec.service)}</span></td>`;
case 'resource_type':
return `<td title="${escapeHtml(rec.resource_type)}">${escapeHtml(rec.resource_type)}${rec.engine ? ` (${escapeHtml(rec.engine)})` : ''}${ctx.badge}</td>`;
case 'capacity': {
const cap = formatCapacity(rec.vcpu, rec.memory_gb);
return `<td>${cap !== null ? escapeHtml(cap) : '—'}</td>`;
}
case 'region':
return `<td>${escapeHtml(rec.region)}</td>`;
case 'count':
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Recommendation } from './api/types';
// Closed enumeration of column ids the per-column filters target.
// Typo-safety: misspellings at call sites become compile errors.
export type RecommendationsColumnId =
| 'provider' | 'account' | 'service' | 'resource_type' | 'region'
| 'provider' | 'account' | 'service' | 'resource_type' | 'capacity' | 'region'
| 'count' | 'term' | 'payment' | 'savings' | 'upfront_cost'
| 'monthly_cost' | 'on_demand_monthly' | 'effective_savings_pct'
| 'usage_history';
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ export interface LocalRecommendation {
// null or absent means the collector did not populate it (non-AWS providers
// or pre-#239 cached rows); the cell renders "—" in that case.
usage_history?: number[] | null;
// ComputeDetails fields surfaced by PR #810/#816/#833.
// null = provider catalogue did not return a value (renders as "—", not "0").
// Absent on non-compute recs (RDS, savings plans, etc.).
vcpu?: number | null;
memory_gb?: number | null;
}

export interface RecommendationsSummary {
Expand Down
48 changes: 47 additions & 1 deletion internal/api/handler_recommendations.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/LeanerCloud/CUDly/internal/auth"
"github.com/LeanerCloud/CUDly/internal/config"
"github.com/LeanerCloud/CUDly/pkg/common"
"github.com/aws/aws-lambda-go/events"
)

Expand Down Expand Up @@ -121,12 +122,23 @@ func (h *Handler) filterRecommendationsByAllowedAccounts(ctx context.Context, se
func buildRecommendationsResponse(recommendations []config.RecommendationRecord) *RecommendationsResponse {
regionSet := make(map[string]struct{})
var totalSavings, totalUpfront float64
for _, rec := range recommendations {
for i := range recommendations {
rec := &recommendations[i]
totalSavings += rec.Savings
totalUpfront += rec.UpfrontCost
if rec.Region != "" {
regionSet[rec.Region] = struct{}{}
}
// Surface the compute size (VCPU / MemoryGB) at the top level so
// the frontend Capacity column can render it without parsing the
// opaque Details blob. The canonical values live in the typed
// ComputeDetails nested inside Details; decode them here (the api
// layer may import pkg/common, config may not) and stamp the
// pointers when this is a compute rec with a known size. Non-compute
// recs, legacy rows with empty Details, and decode failures all
// leave the pointers nil, so JSON omits them and the frontend
// renders "—".
stampComputeCapacity(rec)
}

regions := make([]string, 0, len(regionSet))
Expand All @@ -151,6 +163,40 @@ func buildRecommendationsResponse(recommendations []config.RecommendationRecord)
}
}

// stampComputeCapacity decodes a recommendation's opaque Details blob and,
// when it is a ComputeDetails carrying a known instance size (VCPU > 0 &&
// MemoryGB > 0), stamps the top-level VCPU / MemoryGB pointers the frontend
// Capacity column reads (#219). For non-compute services, legacy rows with an
// empty Details, unknown sizes (0), or a decode error it leaves the pointers
// nil so the JSON omits them and the column renders "—" rather than a
// misleading "0 vCPU / 0 GB".
//
// VCPU and MemoryGB must both be present for the pair to be emitted: the
// frontend's formatCapacity treats a missing half as "absent" and renders a
// dash, so emitting one without the other would be inconsistent.
func stampComputeCapacity(rec *config.RecommendationRecord) {
details, err := common.DecodeServiceDetailsFor(rec.Service, rec.Details)
if err != nil || details == nil {
// Decode error or no typed Details for this service: leave the
// capacity fields absent. A malformed Details blob must not break
// the listing; the rest of the row is still valid.
return
}
compute, ok := details.(*common.ComputeDetails)
if !ok || compute == nil {
return
}
if compute.VCPU <= 0 || compute.MemoryGB <= 0 {
// Unknown size (converter didn't wire a catalogue lookup): keep
// both absent so the frontend renders "—".
return
}
vcpu := compute.VCPU
mem := compute.MemoryGB
rec.VCPU = &vcpu
rec.MemoryGB = &mem
}

// getRecommendationsFreshness returns the cache-freshness state (last
// successful collection timestamp + most recent collection error) so the
// frontend can render a "Data from <N> min ago" indicator and surface a
Expand Down
Loading
Loading