diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 98066916..7a8238de 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -1,7 +1,7 @@ /** * Recommendations module tests */ -import { loadRecommendations, openPurchaseModal, refreshRecommendations, setupRecommendationsHandlers } from '../recommendations'; +import { loadRecommendations, openPurchaseModal, refreshRecommendations, setupRecommendationsHandlers, clearRecommendationDetailCache } from '../recommendations'; // Mock the api module jest.mock('../api', () => ({ @@ -10,6 +10,21 @@ jest.mock('../api', () => ({ listAccounts: jest.fn().mockResolvedValue([]) })); +// Mock the per-id detail endpoint module so the drawer-fetch tests can +// assert on call shape without going through the apiRequest layer. +// Default resolution returns a benign empty payload so tests that +// merely open + close the drawer (and don't care about the detail +// fetch) don't trip on an undefined-promise return. +jest.mock('../api/recommendations', () => ({ + getRecommendationDetail: jest.fn().mockResolvedValue({ + id: 'rec-default', + usage_history: [], + confidence_bucket: 'low', + provenance_note: '', + }), + getRecommendationsFreshness: jest.fn().mockResolvedValue({ last_collected_at: null, last_collection_error: null }), +})); + // Mock state module jest.mock('../state', () => ({ getCurrentProvider: jest.fn().mockReturnValue('all'), @@ -458,6 +473,122 @@ describe('Recommendations Module', () => { document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); expect(document.querySelector('.detail-drawer')).toBeNull(); }); + + describe('drawer fetches detail from /api/recommendations/:id/detail (issue #44)', () => { + // The detail-fetch mock lives on the api/recommendations module + // so the test can assert call shape without round-tripping + // through apiRequest. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const recApi = require('../api/recommendations') as { getRecommendationDetail: jest.Mock }; + + beforeEach(() => { + // Real timers — the drawer's fetch uses microtasks (Promise + // resolution) which jest's fake timers don't auto-advance. + jest.useRealTimers(); + clearRecommendationDetailCache(); + recApi.getRecommendationDetail.mockReset(); + }); + + afterEach(() => { + jest.useFakeTimers(); + }); + + test('drawer fetches detail once per id and renders backend confidence + provenance', async () => { + recApi.getRecommendationDetail.mockResolvedValue({ + id: 'rec-15', + usage_history: [], + confidence_bucket: 'high', + provenance_note: 'AWS ec2 recommendation APIs · last collected 2026-04-24T12:00:00Z', + }); + + await loadRecommendations(); + const firstRow = document.querySelector('tr.recommendation-row'); + firstRow?.querySelectorAll('td')[3]?.click(); + + // Allow the .then() handler to run. + await Promise.resolve(); + await Promise.resolve(); + + expect(recApi.getRecommendationDetail).toHaveBeenCalledTimes(1); + // Default sort is savings desc → rec-15 ($1500) renders first. + expect(recApi.getRecommendationDetail).toHaveBeenCalledWith('rec-15'); + + const badge = document.querySelector('.detail-drawer .confidence-badge'); + expect(badge?.classList.contains('confidence-high')).toBe(true); + expect(badge?.textContent).toBe('High'); + + const provenance = document.querySelector('.detail-drawer .detail-drawer-note'); + expect(provenance?.textContent).toContain('last collected 2026-04-24T12:00:00Z'); + }); + + test('empty usage_history renders the "not yet available" placeholder, not a broken chart', async () => { + recApi.getRecommendationDetail.mockResolvedValue({ + id: 'rec-15', + usage_history: [], + confidence_bucket: 'medium', + provenance_note: 'AWS ec2 recommendation APIs.', + }); + + await loadRecommendations(); + const firstRow = document.querySelector('tr.recommendation-row'); + firstRow?.querySelectorAll('td')[3]?.click(); + await Promise.resolve(); + await Promise.resolve(); + + // No SVG sparkline — degraded path. + expect(document.querySelector('.detail-drawer-sparkline')).toBeNull(); + // Placeholder note present. + const usageNote = document.querySelector('.detail-drawer-usage .detail-drawer-note-muted'); + expect(usageNote?.textContent).toBe('Usage history not yet available.'); + }); + + test('non-empty usage_history renders an inline SVG sparkline', async () => { + recApi.getRecommendationDetail.mockResolvedValue({ + id: 'rec-15', + usage_history: [ + { timestamp: '2026-04-23T00:00:00Z', cpu_pct: 12, mem_pct: 30 }, + { timestamp: '2026-04-23T01:00:00Z', cpu_pct: 18, mem_pct: 32 }, + { timestamp: '2026-04-23T02:00:00Z', cpu_pct: 25, mem_pct: 40 }, + ], + confidence_bucket: 'high', + provenance_note: 'AWS ec2 recommendation APIs.', + }); + + await loadRecommendations(); + const firstRow = document.querySelector('tr.recommendation-row'); + firstRow?.querySelectorAll('td')[3]?.click(); + await Promise.resolve(); + await Promise.resolve(); + + const svg = document.querySelector('.detail-drawer-sparkline'); + expect(svg).not.toBeNull(); + // Two paths (CPU + memory). + expect(svg?.querySelectorAll('path').length).toBe(2); + }); + + test('repeated open of same drawer reuses the cached detail (one fetch per id)', async () => { + recApi.getRecommendationDetail.mockResolvedValue({ + id: 'rec-15', + usage_history: [], + confidence_bucket: 'low', + provenance_note: 'AWS ec2 recommendation APIs.', + }); + + await loadRecommendations(); + const firstRow = document.querySelector('tr.recommendation-row'); + firstRow?.querySelectorAll('td')[3]?.click(); + await Promise.resolve(); + await Promise.resolve(); + + // Close and re-open the same drawer. + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' })); + firstRow?.querySelectorAll('td')[3]?.click(); + await Promise.resolve(); + await Promise.resolve(); + + expect(recApi.getRecommendationDetail).toHaveBeenCalledTimes(1); + }); + }); }); describe('openPurchaseModal', () => { diff --git a/frontend/src/api/recommendations.ts b/frontend/src/api/recommendations.ts index 4c2bbdff..1992ef66 100644 --- a/frontend/src/api/recommendations.ts +++ b/frontend/src/api/recommendations.ts @@ -65,3 +65,50 @@ export interface RecommendationsFreshness { export async function getRecommendationsFreshness(): Promise { return apiRequest('/recommendations/freshness'); } + +/** + * A single sample in the per-recommendation usage time series. Mirrors + * `api.UsagePoint` in `internal/api/types.go`. The series is always + * ordered by `timestamp` ascending. cpu_pct / mem_pct are 0..100. + */ +export interface RecommendationUsagePoint { + timestamp: string; + cpu_pct: number; + mem_pct: number; +} + +/** + * Per-id drill-down payload backing the Recommendations row-click + * drawer. Issued by `GET /api/recommendations/:id/detail`. Mirrors + * `api.RecommendationDetailResponse` in the backend. + * + * `confidence_bucket` is the server-computed confidence tier and + * replaces the former client-side `confidenceBucketFor` heuristic. + * + * `provenance_note` is rendered verbatim — the backend already names + * the collector + last-collected timestamp. + * + * `usage_history` is `[]` until the collector starts persisting + * time-series utilisation per recommendation; the drawer renders a + * "Usage history not yet available" line in that case rather than a + * broken empty chart. + */ +export interface RecommendationDetail { + id: string; + usage_history: RecommendationUsagePoint[]; + confidence_bucket: 'low' | 'medium' | 'high'; + provenance_note: string; +} + +/** + * Fetch the per-id detail payload for the Recommendations drawer. + * Backend contract: `GET /api/recommendations/:id/detail`. Returns 404 + * for unknown ids (and for ids that exist but belong to accounts the + * caller is not allowed to see — the existence-disclosure-safe path). + * + * The id is path-encoded so ids containing reserved URL characters + * round-trip cleanly. + */ +export async function getRecommendationDetail(id: string): Promise { + return apiRequest(`/recommendations/${encodeURIComponent(id)}/detail`); +} diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 341dc616..63830867 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -4,9 +4,9 @@ import * as api from './api'; import * as state from './state'; -import { formatCurrency, formatTerm, escapeHtml, populateAccountFilter, formatRelativeTime } from './utils'; +import { formatCurrency, formatTerm, escapeHtml, populateAccountFilter } from './utils'; import { renderFreshness } from './freshness'; -import { getRecommendationsFreshness } from './api/recommendations'; +import { getRecommendationDetail, type RecommendationDetail } from './api/recommendations'; import { showToast } from './toast'; import { isPaymentSupported, type Provider as CompatProvider } from './lib/purchase-compatibility'; import type { RecommendationsResponse, LocalRecommendation, RecommendationsSummary } from './types'; @@ -302,76 +302,170 @@ function openDetailDrawer(rec: LocalRecommendation): void { }); drawer.appendChild(dl); - // Confidence bucket — computed client-side from the savings magnitude - // + instance count. A proper per-recommendation confidence score that - // accounts for historical usage variance needs a backend endpoint - // (tracked in known_issues/28_recommendations_detail_endpoint.md); - // this client-side heuristic gives users a directional signal in the - // meantime. - const bucket = confidenceBucketFor(rec); + // Confidence + provenance + usage history are sourced from the + // per-id detail endpoint (issue #44). The drawer renders a loading + // placeholder for each block immediately, then fills them in once + // the fetch resolves. The fetch is memoised per id for the drawer + // lifetime so re-opening the same row never re-hits the network + // within a single session (cache cleared on drawer close). const confidenceRow = document.createElement('dl'); confidenceRow.className = 'detail-drawer-fields'; const confDt = document.createElement('dt'); confDt.textContent = 'Confidence'; const confDd = document.createElement('dd'); const badge = document.createElement('span'); - badge.className = `confidence-badge confidence-${bucket}`; - badge.textContent = bucket.charAt(0).toUpperCase() + bucket.slice(1); + badge.className = 'confidence-badge confidence-loading'; + badge.textContent = '…'; confDd.appendChild(badge); confidenceRow.appendChild(confDt); confidenceRow.appendChild(confDd); drawer.appendChild(confidenceRow); - // Provenance — render immediately with a placeholder, then fill in - // asynchronously from /api/recommendations/freshness (the endpoint is - // already hit by the freshness pill so its response is cached on the - // network side; this fetch is fast and non-blocking to the drawer - // opening). const provenance = document.createElement('p'); provenance.className = 'detail-drawer-note'; - provenance.textContent = `Derived from ${providerDisplayName(rec.provider)} recommendation APIs. Last collection timing loading\u2026`; + provenance.textContent = 'Loading recommendation details\u2026'; drawer.appendChild(provenance); - void getRecommendationsFreshness() - .then((f) => { - const rel = f.last_collected_at ? formatRelativeTime(f.last_collected_at) : 'never'; - provenance.textContent = `Derived from ${providerDisplayName(rec.provider)} recommendation APIs. Last collected ${rel}.`; + + // Usage history container \u2014 replaced once the detail fetch resolves + // with either an inline SVG sparkline (when usage_history is + // non-empty) or a "not yet available" note (the documented default + // until the collector starts persisting time-series usage \u2014 + // known_issues/28). + const usageContainer = document.createElement('div'); + usageContainer.className = 'detail-drawer-usage'; + const usagePlaceholder = document.createElement('p'); + usagePlaceholder.className = 'detail-drawer-note detail-drawer-note-muted'; + usagePlaceholder.textContent = 'Loading usage history\u2026'; + usageContainer.appendChild(usagePlaceholder); + drawer.appendChild(usageContainer); + + const renderUsageEmptyNote = (): HTMLParagraphElement => { + const note = document.createElement('p'); + note.className = 'detail-drawer-note detail-drawer-note-muted'; + note.textContent = 'Usage history not yet available.'; + return note; + }; + + void fetchRecommendationDetail(rec.id) + .then((detail) => { + // Confidence: server-supplied bucket replaces the previous + // client-side heuristic so the label tracks the collector's + // view of the rec rather than a frontend approximation. + const bucket = detail.confidence_bucket; + badge.className = `confidence-badge confidence-${bucket}`; + badge.textContent = bucket.charAt(0).toUpperCase() + bucket.slice(1); + + // Provenance: rendered verbatim \u2014 the backend already names + // the collector + last-collected timestamp. + provenance.textContent = detail.provenance_note; + + // Usage history: render the sparkline when we have points, + // else a one-line note. The empty case is the documented + // default until the collector wiring follow-up lands. + const next = (detail.usage_history && detail.usage_history.length > 0) + ? renderUsageSparkline(detail.usage_history) + : renderUsageEmptyNote(); + usageContainer.replaceChildren(next); }) .catch(() => { - provenance.textContent = `Derived from ${providerDisplayName(rec.provider)} recommendation APIs.`; + // Detail-endpoint failure shouldn't blank the drawer \u2014 fall + // back to a minimal "details unavailable" state. Confidence + // badge is reset to an explicit Unknown rather than mis-claiming + // a bucket on a failed fetch. + badge.className = 'confidence-badge confidence-unknown'; + badge.textContent = 'Unknown'; + provenance.textContent = `Derived from ${providerDisplayName(rec.provider)} recommendation APIs. (Details temporarily unavailable.)`; + usageContainer.replaceChildren(renderUsageEmptyNote()); }); - // Usage-history drill-down still requires backend work — see - // known_issues/28_recommendations_detail_endpoint.md for the endpoint - // contract (GET /api/recommendations/:id/detail returning a usage - // series). - const usageNote = document.createElement('p'); - usageNote.className = 'detail-drawer-note detail-drawer-note-muted'; - usageNote.textContent = 'Usage history over the collection window is not yet available; the detail endpoint is tracked separately.'; - drawer.appendChild(usageNote); - document.body.appendChild(backdrop); document.body.appendChild(drawer); closeBtn.focus(); } -type ConfidenceBucket = 'low' | 'medium' | 'high'; +// detailFetchCache memoises in-flight + resolved detail fetches per id +// so re-opening the same row never re-hits the network within a single +// session. Cleared via clearRecommendationDetailCache() so a long-lived +// dashboard tab doesn't pin stale details indefinitely (the values +// evolve on every collector cycle). +const detailFetchCache = new Map>(); + +function fetchRecommendationDetail(id: string): Promise { + const existing = detailFetchCache.get(id); + if (existing) return existing; + const inflight = getRecommendationDetail(id); + detailFetchCache.set(id, inflight); + // On rejection, drop the cached promise so the next open retries. + inflight.catch(() => detailFetchCache.delete(id)); + return inflight; +} /** - * Client-side confidence heuristic. A real confidence score needs - * historical usage variance from the backend (tracked in known_issues - * #28); this directional bucket surfaces "probably a solid pick" vs - * "marginal" to users immediately based on savings magnitude + size - * of the target footprint. + * Clear the per-id detail-fetch cache. Exposed for tests + for any + * future explicit "refresh details" affordance. */ -function confidenceBucketFor(rec: LocalRecommendation): ConfidenceBucket { - const savings = rec.savings || 0; - const count = rec.count || 1; - // High: material monthly savings AND a non-trivial fleet — a single - // $1000/mo rec from one tiny instance is likely an outlier, so we - // require both signals. - if (savings >= 200 && count >= 3) return 'high'; - if (savings >= 50) return 'medium'; - return 'low'; +export function clearRecommendationDetailCache(): void { + detailFetchCache.clear(); +} + +/** + * Render a tiny inline SVG sparkline of the per-recommendation usage + * history. Two polylines (CPU + memory) over a shared time axis, no + * axes/legend — just enough to give a directional sense of utilisation + * over the collection window without pulling in a chart library. + */ +function renderUsageSparkline(points: ReadonlyArray<{ timestamp: string; cpu_pct: number; mem_pct: number }>): SVGSVGElement { + const svgNS = 'http://www.w3.org/2000/svg'; + const width = 280; + const height = 60; + const padX = 4; + const padY = 4; + const usableW = width - padX * 2; + const usableH = height - padY * 2; + + // Y axis is fixed to 0..100 since CPU/mem percentages have a known + // range — keeps the visual comparison stable across recs. + const yMax = 100; + const xStep = points.length > 1 ? usableW / (points.length - 1) : 0; + const project = (val: number, idx: number): [number, number] => { + const clamped = Math.max(0, Math.min(yMax, val)); + const x = padX + xStep * idx; + const y = padY + usableH * (1 - clamped / yMax); + return [x, y]; + }; + + const buildPath = (selector: (p: { cpu_pct: number; mem_pct: number }) => number): string => + points.map((p, i) => { + const [x, y] = project(selector(p), i); + return `${i === 0 ? 'M' : 'L'}${x.toFixed(1)},${y.toFixed(1)}`; + }).join(' '); + + const svg = document.createElementNS(svgNS, 'svg'); + svg.setAttribute('class', 'detail-drawer-sparkline'); + svg.setAttribute('viewBox', `0 0 ${width} ${height}`); + svg.setAttribute('width', String(width)); + svg.setAttribute('height', String(height)); + svg.setAttribute('role', 'img'); + svg.setAttribute('aria-label', `Usage history: ${points.length} samples, CPU and memory percent over time`); + + const cpu = document.createElementNS(svgNS, 'path'); + cpu.setAttribute('d', buildPath((p) => p.cpu_pct)); + cpu.setAttribute('fill', 'none'); + cpu.setAttribute('stroke', 'currentColor'); + cpu.setAttribute('stroke-width', '1.5'); + cpu.setAttribute('class', 'sparkline-cpu'); + svg.appendChild(cpu); + + const mem = document.createElementNS(svgNS, 'path'); + mem.setAttribute('d', buildPath((p) => p.mem_pct)); + mem.setAttribute('fill', 'none'); + mem.setAttribute('stroke', 'currentColor'); + mem.setAttribute('stroke-width', '1.5'); + mem.setAttribute('stroke-dasharray', '3,2'); + mem.setAttribute('class', 'sparkline-mem'); + svg.appendChild(mem); + + return svg; } function providerDisplayName(provider: string): string { diff --git a/internal/api/handler_recommendations.go b/internal/api/handler_recommendations.go index 2f985798..7a7ede85 100644 --- a/internal/api/handler_recommendations.go +++ b/internal/api/handler_recommendations.go @@ -4,6 +4,7 @@ package api import ( "context" "fmt" + "time" "github.com/LeanerCloud/CUDly/internal/auth" "github.com/LeanerCloud/CUDly/internal/config" @@ -148,3 +149,131 @@ func (h *Handler) getRecommendationsFreshness(ctx context.Context, req *events.L } return freshness, nil } + +// getRecommendationDetail returns the per-id drill-down payload backing +// the Recommendations row-click drawer. Surfaces the data that's already +// computed server-side (provider/service for provenance, savings + count +// for the confidence bucket) on a per-id GET so the listing payload +// stays compact for the common case where the drawer never opens. +// +// Contract documented in issue #44 + RecommendationDetailResponse in +// types.go. Gated by view:recommendations permission. Returns errNotFound +// on unknown id (404) and on ids that exist but belong to an account the +// caller is not allowed to see (matches the existence-disclosure pattern +// used by handler_accounts.go's account lookup). +// +// usage_history is intentionally empty in this first pass: the collector +// pipeline does not yet persist time-series utilisation per +// recommendation. Surfacing the missing field as an empty slice (rather +// than a 501) keeps the drawer functional today and means the day the +// collector starts populating it, the frontend automatically picks it +// up. The empty-slice case is documented in known_issues/28. +func (h *Handler) getRecommendationDetail(ctx context.Context, req *events.LambdaFunctionURLRequest, id string) (*RecommendationDetailResponse, error) { + // Authn/permission gate runs first so an unauthenticated caller + // can't probe id-shape validation to learn anything about the + // endpoint's existence. + session, err := h.requirePermission(ctx, req, "view", "recommendations") + if err != nil { + return nil, err + } + + if id == "" { + return nil, NewClientError(400, "recommendation id is required") + } + + // The recommendation cache doesn't expose a get-by-id, so we look it + // up from the unfiltered list. The list is already cached in + // Postgres (see store_postgres_recommendations.go) so this is a + // single round-trip; the in-memory linear scan is bounded by the + // catalogue size which is small (low thousands at the high end). + recs, err := h.scheduler.ListRecommendations(ctx, config.RecommendationFilter{}) + if err != nil { + return nil, fmt.Errorf("failed to list recommendations: %w", err) + } + + // Filter by allowed accounts FIRST, then look up by id within the + // filtered set. Doing it the other way around would leak existence + // of recommendations in accounts the caller can't see (a 404 vs + // 403 timing/wording diff would let an attacker probe the + // recommendation namespace across accounts). + visible, err := h.filterRecommendationsByAllowedAccounts(ctx, session, recs) + if err != nil { + return nil, err + } + + for i := range visible { + if visible[i].ID == id { + return h.buildRecommendationDetail(ctx, &visible[i]), nil + } + } + return nil, errNotFound +} + +// buildRecommendationDetail assembles the drawer payload from a single +// recommendation record + the global freshness state. The freshness +// lookup is best-effort; on error the provenance note degrades to its +// no-window form so the drawer still renders. +func (h *Handler) buildRecommendationDetail(ctx context.Context, rec *config.RecommendationRecord) *RecommendationDetailResponse { + return &RecommendationDetailResponse{ + ID: rec.ID, + UsageHistory: []UsagePoint{}, + ConfidenceBucket: confidenceBucketFor(rec.Savings, rec.Count), + ProvenanceNote: h.provenanceNoteFor(ctx, rec), + } +} + +// confidenceBucketFor mirrors the heuristic that previously lived +// client-side in frontend/src/recommendations.ts. Centralising it on +// the server lets future provider-specific tuning land in one place +// without a frontend deploy. Thresholds intentionally match the original +// shim 1:1 so the drawer label doesn't visibly shift on rollout. +// +// high = ≥ $200/mo savings AND ≥ 3-instance fleet (both signals) +// medium = ≥ $50/mo savings (single signal) +// low = otherwise +func confidenceBucketFor(savings float64, count int) string { + if count < 1 { + count = 1 + } + if savings >= 200 && count >= 3 { + return "high" + } + if savings >= 50 { + return "medium" + } + return "low" +} + +// provenanceNoteFor returns the short "Derived from … last collected …" +// string the drawer renders verbatim. Format mirrors the wording the +// frontend previously assembled inline so the rollout doesn't visibly +// change the drawer copy. Timestamp is RFC3339 UTC; the frontend can +// re-format relative if it wants — keeping the wire format absolute +// avoids ambiguity around the user's locale. +func (h *Handler) provenanceNoteFor(ctx context.Context, rec *config.RecommendationRecord) string { + provider := providerDisplayName(rec.Provider) + base := fmt.Sprintf("%s %s recommendation APIs", provider, rec.Service) + + freshness, err := h.config.GetRecommendationsFreshness(ctx) + if err != nil || freshness == nil || freshness.LastCollectedAt == nil { + return base + "." + } + ts := freshness.LastCollectedAt.UTC().Format(time.RFC3339) + return fmt.Sprintf("%s · last collected %s", base, ts) +} + +// providerDisplayName converts the lowercase provider slug used in +// storage to the canonical display casing used in user-facing strings. +// Mirrors the equivalent helper in frontend/src/recommendations.ts. +func providerDisplayName(provider string) string { + switch provider { + case "aws": + return "AWS" + case "azure": + return "Azure" + case "gcp": + return "GCP" + default: + return provider + } +} diff --git a/internal/api/handler_recommendations_test.go b/internal/api/handler_recommendations_test.go index a466235c..421c83ac 100644 --- a/internal/api/handler_recommendations_test.go +++ b/internal/api/handler_recommendations_test.go @@ -76,3 +76,138 @@ func TestHandler_getRecommendationsFreshness(t *testing.T) { assert.Nil(t, got) }) } + +func TestHandler_getRecommendationDetail(t *testing.T) { + ctx := context.Background() + now := time.Date(2026, 4, 24, 12, 0, 0, 0, time.UTC) + knownRecs := []config.RecommendationRecord{ + { + ID: "rec-known", + Provider: "aws", + Service: "ec2", + Region: "us-east-1", + ResourceType: "t3.large", + Count: 4, + Savings: 250, + }, + } + + t.Run("returns 400 on empty id (post-auth)", func(t *testing.T) { + // Auth gate runs before id validation so an unauthenticated + // caller doesn't learn the endpoint exists by probing id + // shape — the test exercises the authenticated path. + handler := &Handler{apiKey: "test-key"} + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"x-api-key": "test-key"}, + } + + got, err := handler.getRecommendationDetail(ctx, req, "") + require.Error(t, err) + assert.Nil(t, got) + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T", err) + assert.Equal(t, 400, ce.code) + }) + + t.Run("returns errNotFound on unknown id", func(t *testing.T) { + mockScheduler := new(MockScheduler) + mockScheduler.On("ListRecommendations", ctx, mock.Anything). + Return(knownRecs, nil) + + handler := &Handler{ + scheduler: mockScheduler, + apiKey: "test-key", + } + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"x-api-key": "test-key"}, + } + + got, err := handler.getRecommendationDetail(ctx, req, "rec-missing") + require.Error(t, err) + assert.Nil(t, got) + assert.True(t, IsNotFoundError(err), "expected 404 not-found, got %v", err) + }) + + t.Run("returns 200 with the expected shape for a known id", func(t *testing.T) { + mockScheduler := new(MockScheduler) + mockScheduler.On("ListRecommendations", ctx, mock.Anything). + Return(knownRecs, nil) + + mockStore := new(MockConfigStore) + mockStore.On("GetRecommendationsFreshness", ctx). + Return(&config.RecommendationsFreshness{LastCollectedAt: &now}, nil) + + handler := &Handler{ + scheduler: mockScheduler, + config: mockStore, + apiKey: "test-key", + } + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"x-api-key": "test-key"}, + } + + got, err := handler.getRecommendationDetail(ctx, req, "rec-known") + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "rec-known", got.ID) + // Empty (not nil) so json.Marshal renders [] rather than null — + // the frontend's `usage_history.length === 0` check needs the + // array form, not a null. + assert.NotNil(t, got.UsageHistory) + assert.Equal(t, 0, len(got.UsageHistory)) + // $250/mo savings + 4-instance fleet → high (matches the + // frontend shim's thresholds 1:1). + assert.Equal(t, "high", got.ConfidenceBucket) + assert.Contains(t, got.ProvenanceNote, "AWS") + assert.Contains(t, got.ProvenanceNote, "ec2") + assert.Contains(t, got.ProvenanceNote, "last collected") + }) + + t.Run("provenance degrades gracefully when freshness is unavailable", func(t *testing.T) { + mockScheduler := new(MockScheduler) + mockScheduler.On("ListRecommendations", ctx, mock.Anything). + Return(knownRecs, nil) + + mockStore := new(MockConfigStore) + mockStore.On("GetRecommendationsFreshness", ctx). + Return(nil, errors.New("db down")) + + handler := &Handler{ + scheduler: mockScheduler, + config: mockStore, + apiKey: "test-key", + } + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"x-api-key": "test-key"}, + } + + got, err := handler.getRecommendationDetail(ctx, req, "rec-known") + require.NoError(t, err) + require.NotNil(t, got) + // The drawer still renders a useful provenance line on + // freshness backend failure — drop the "last collected …" + // suffix rather than 500ing the whole detail call. + assert.NotEmpty(t, got.ProvenanceNote) + assert.NotContains(t, got.ProvenanceNote, "last collected") + }) +} + +func TestConfidenceBucketFor(t *testing.T) { + cases := []struct { + name string + savings float64 + count int + want string + }{ + {"high requires both signals", 250, 4, "high"}, + {"savings without fleet falls to medium", 250, 1, "medium"}, + {"medium on savings alone", 60, 1, "medium"}, + {"low when neither threshold is met", 10, 1, "low"}, + {"count clamped to >=1 — savings still drives bucket", 60, 0, "medium"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, confidenceBucketFor(tc.savings, tc.count)) + }) + } +} diff --git a/internal/api/router.go b/internal/api/router.go index 6a514531..b7aa500f 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -75,10 +75,13 @@ func (r *Router) registerRoutes() { // but we don't expose it unauthenticated. {ExactPath: "/api/commitment-options", Method: "GET", Handler: r.commitmentOptionsHandler, Auth: AuthUser}, - // Recommendations endpoints + // Recommendations endpoints. The /:id/detail suffix route uses + // the same prefix+suffix pattern as /api/plans/{id}/purchases + // below — extractParams strips both ends to recover the id. {ExactPath: "/api/recommendations", Method: "GET", Handler: r.getRecommendationsHandler}, {ExactPath: "/api/recommendations/freshness", Method: "GET", Handler: r.getRecommendationsFreshnessHandler}, {ExactPath: "/api/recommendations/refresh", Method: "POST", Handler: r.refreshRecommendationsHandler}, + {PathPrefix: "/api/recommendations/", PathSuffix: "/detail", Method: "GET", Handler: r.getRecommendationDetailHandler}, // Purchase plans endpoints {ExactPath: "/api/plans", Method: "GET", Handler: r.listPlansHandler}, @@ -314,6 +317,10 @@ func (r *Router) getRecommendationsFreshnessHandler(ctx context.Context, req *ev return r.h.getRecommendationsFreshness(ctx, req) } +func (r *Router) getRecommendationDetailHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.getRecommendationDetail(ctx, req, params["id"]) +} + func (r *Router) listPlansHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { return r.h.listPlans(ctx, req) } diff --git a/internal/api/types.go b/internal/api/types.go index ea92a04c..e523be5e 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -328,6 +328,39 @@ type RecommendationsResponse struct { Regions []string `json:"regions"` } +// UsagePoint is a single sample in the per-recommendation usage time +// series surfaced by GET /api/recommendations/:id/detail. The series is +// always ordered by Timestamp ascending. CPUPct/MemPct are 0..100. +// +// Empty in the current implementation: the collector pipeline does not +// yet persist time-series utilisation per recommendation. The endpoint +// returns the empty slice with a non-error status so the frontend can +// render a "Usage history not yet available" placeholder rather than a +// broken empty chart. See known_issues/28_recommendations_detail_endpoint.md +// for the full collector wiring follow-up. +type UsagePoint struct { + Timestamp string `json:"timestamp"` + CPUPct float64 `json:"cpu_pct"` + MemPct float64 `json:"mem_pct"` +} + +// RecommendationDetailResponse is the per-id payload backing the +// Recommendations row-click drawer. Contract documented in issue #44. +// +// ConfidenceBucket is "low" | "medium" | "high" — server-side mirror of +// the client-side heuristic that previously lived in +// frontend/src/recommendations.ts:confidenceBucketFor. Centralising it +// server-side lets future provider-specific tuning happen in one place. +// +// ProvenanceNote is a short human-readable string naming the collector +// + the freshness window. Rendered verbatim in the drawer. +type RecommendationDetailResponse struct { + ID string `json:"id"` + UsageHistory []UsagePoint `json:"usage_history"` + ConfidenceBucket string `json:"confidence_bucket"` + ProvenanceNote string `json:"provenance_note"` +} + // PlansResponse holds the purchase plans response type PlansResponse struct { Plans []config.PurchasePlan `json:"plans"`