From fa8b7f32b65b725b517f790e27cf7cfe7f5db10e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 21:00:47 +0200 Subject: [PATCH 1/3] docs(purchase): update savings-plans alias TODO for #95 -- deferred to 2026-10-30 PR #94 (merged 2026-04-30) renamed the "savings-plans" slug to "savingsplans". The ~6-month purchase_executions retention window means the "savings-plans" alias in mapSavingsPlansSlug and newDetailsForService cannot be dropped until 2026-10-30 at earliest. Update both TODO comments to reference #95, record the PR #94 merge date, state the earliest safe drop date, and include the full removal checklist (DB verification query, exact files/lines, test flip). The alias itself is unchanged. Closes #95 --- internal/purchase/execution.go | 22 ++++++++++++++-------- pkg/common/service_details_codec.go | 9 +++++---- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 0eefefcee..25cecd1af 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -1083,14 +1083,20 @@ func mapServiceSlug(service string) (common.ServiceType, bool) { // four per-plan-type slugs in both spellings — pulled out of mapServiceType // to keep that switch under the gocyclo budget. // -// TODO(#85): once purchase_executions JSONB rows persisted before the -// "savingsplans" rename (~6-month retention window) have aged out, the -// "savings-plans"-spelled aliases below can be removed and only the -// dash-free spellings ("savingsplans", "savingsplans-compute", etc.) -// need be matched here. The umbrella rename happened in PR #94; the -// per-plan-type slugs were always dash-form on the wire so their -// "savingsplans-*" aliases are forward-compat for any future -// frontend-canonical normalisation. +// TODO(#95): PR #94 (merged 2026-04-30) renamed "savings-plans" -> +// "savingsplans". purchase_executions rows have a ~6-month retention window, +// so the earliest safe drop date is 2026-10-30. Until then the +// "savings-plans"-spelled aliases below must stay so Lambda-scheduled +// executions persisted before PR #94 still map correctly on retry/approval. +// To drop: verify zero rows with +// SELECT id FROM purchase_executions +// WHERE recommendations::text LIKE '%"service":"savings-plans"%' +// LIMIT 1; +// then remove every "savings-plans*" key from this map and the matching +// case in pkg/common/service_details_codec.go: newDetailsForService, and +// flip the coverage_extra_test.go case (line ~60) to assert +// ServiceType("savings-plans") (default-arm pass-through). The +// "savingsplans-*" keys are forward-compat and should stay. func mapSavingsPlansSlug(service string) (common.ServiceType, bool) { slugs := map[string]common.ServiceType{ "savings-plans": common.ServiceSavingsPlans, diff --git a/pkg/common/service_details_codec.go b/pkg/common/service_details_codec.go index 75a43f75e..3d39d7b49 100644 --- a/pkg/common/service_details_codec.go +++ b/pkg/common/service_details_codec.go @@ -119,15 +119,16 @@ func newDetailsForService(service string) (ServiceDetails, bool) { case string(ServiceElastiCache), string(ServiceCache): return &CacheDetails{}, true - // AWS Savings Plans — all umbrella + per-plan-type slugs use + // AWS Savings Plans -- all umbrella + per-plan-type slugs use // SavingsPlanDetails. The dash-free spellings are the canonical // values of ServiceSavingsPlans / ServiceSavingsPlansCompute etc.; // "savings-plans" (with a dash) is the legacy umbrella alias that // internal/purchase/execution.go: mapSavingsPlansSlug still // recognises so purchase_executions JSONB rows persisted before - // the rename in PR #94 still resolve. Recognising it here means a - // legacy direct-execute approval still decodes against the right - // type. + // the rename in PR #94 (merged 2026-04-30) still resolve. + // TODO(#95): drop "savings-plans" here once the ~6-month retention + // window has passed (earliest 2026-10-30). See execution.go + // mapSavingsPlansSlug for the full removal checklist. case string(ServiceSavingsPlans), string(ServiceSavingsPlansCompute), string(ServiceSavingsPlansEC2Instance), From 2061784f7e14186b0009c79c40856dc2b7977d1f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 11 Jun 2026 03:33:02 -0700 Subject: [PATCH 2/3] style(purchase): gofmt savings-plans TODO comment block CodeRabbit flagged the failing go-fmt pre-commit gate on this PR: the SQL verification snippet added to the mapSavingsPlansSlug TODO used space-indented comment lines, which gofmt normalizes to a tab-indented code block delimited by blank comment lines. Run gofmt -w on internal/purchase/execution.go; the TODO content (query, dates, removal checklist) is semantically unchanged. --- internal/purchase/execution.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 25cecd1af..db6f2581b 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -1089,9 +1089,11 @@ func mapServiceSlug(service string) (common.ServiceType, bool) { // "savings-plans"-spelled aliases below must stay so Lambda-scheduled // executions persisted before PR #94 still map correctly on retry/approval. // To drop: verify zero rows with -// SELECT id FROM purchase_executions -// WHERE recommendations::text LIKE '%"service":"savings-plans"%' -// LIMIT 1; +// +// SELECT id FROM purchase_executions +// WHERE recommendations::text LIKE '%"service":"savings-plans"%' +// LIMIT 1; +// // then remove every "savings-plans*" key from this map and the matching // case in pkg/common/service_details_codec.go: newDetailsForService, and // flip the coverage_extra_test.go case (line ~60) to assert From c2f5aeb92cbb19ee1582897f59b8cf3c6fd16208 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 23:08:20 +0200 Subject: [PATCH 3/3] style(purchase): US-spell misspell linter findings in touched files Fix British spellings flagged by golangci-lint misspell in the two files touched by the #95 TODO-update commit: execution.go and service_details_codec.go. Changes are comment-only. behaviour -> behavior, recognise/normalise/honour -> recognize/normalize/honor --- internal/purchase/execution.go | 4 ++-- pkg/common/service_details_codec.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index db6f2581b..655800e9f 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -1027,7 +1027,7 @@ func (m *Manager) executeSinglePurchase(ctx context.Context, rec config.Recommen // canonical hyphenated slugs (compute, relational-db, cache, search, // data-warehouse) and the legacy AWS-only slugs (ec2, rds, elasticache, // opensearch, redshift, memorydb) are recognized; everything else passes -// through verbatim. Savings Plans slugs are normalised by mapSavingsPlansSlug. +// through verbatim. Savings Plans slugs are normalized by mapSavingsPlansSlug. func (m *Manager) mapServiceType(service string) common.ServiceType { if svc, ok := mapSavingsPlansSlug(service); ok { return svc @@ -1077,7 +1077,7 @@ func mapServiceSlug(service string) (common.ServiceType, bool) { return svc, ok } -// mapSavingsPlansSlug normalises both the canonical hyphenated SP slugs and +// mapSavingsPlansSlug normalizes both the canonical hyphenated SP slugs and // the dash-free spellings the frontend has historically sent into the // matching common.ServiceType. The map covers the legacy umbrella plus the // four per-plan-type slugs in both spellings — pulled out of mapServiceType diff --git a/pkg/common/service_details_codec.go b/pkg/common/service_details_codec.go index 3d39d7b49..c37268918 100644 --- a/pkg/common/service_details_codec.go +++ b/pkg/common/service_details_codec.go @@ -60,7 +60,7 @@ func MarshalServiceDetails(details ServiceDetails) (json.RawMessage, error) { // "savingsplans", "savings-plans-compute" and the dash-free spellings — // see internal/purchase/execution.go: mapServiceType / mapSavingsPlansSlug). // -// Behaviour: +// Behavior: // - raw is empty AND service maps to a known *Details type → returns a // zero-valued typed pointer (the legacy / pre-#453 fallback). The // downstream service client's buildOfferingFilters tolerates zero- @@ -79,7 +79,7 @@ func DecodeServiceDetailsFor(service string, raw json.RawMessage) (ServiceDetail if !ok { // Service has no *Details type — nothing to decode. Empty raw // payload is fine; a non-empty payload on a service we don't - // recognise is suspicious but tolerated (writers and readers + // recognize is suspicious but tolerated (writers and readers // might be on different versions during a rolling deploy). return nil, nil } @@ -124,7 +124,7 @@ func newDetailsForService(service string) (ServiceDetails, bool) { // values of ServiceSavingsPlans / ServiceSavingsPlansCompute etc.; // "savings-plans" (with a dash) is the legacy umbrella alias that // internal/purchase/execution.go: mapSavingsPlansSlug still - // recognises so purchase_executions JSONB rows persisted before + // recognizes so purchase_executions JSONB rows persisted before // the rename in PR #94 (merged 2026-04-30) still resolve. // TODO(#95): drop "savings-plans" here once the ~6-month retention // window has passed (earliest 2026-10-30). See execution.go @@ -139,7 +139,7 @@ func newDetailsForService(service string) (ServiceDetails, bool) { // Services that don't currently type-assert Details in // findOfferingID (OpenSearch / Redshift / MemoryDB read rec.ResourceType - // directly). We still recognise OpenSearch and Redshift here so a + // directly). We still recognize OpenSearch and Redshift here so a // forthcoming refactor that starts asserting can flip the table // value without changing call sites. MemoryDB has no dedicated // *Details type yet, so it's intentionally absent — callers see