verify recommendations engine actually consumes per-account service overrides

Summary

PRs #72 (inline payment edit) and #106 (create modal) made account_service_overrides configurable through the UI. The schema and write path have existed since migration 000011_cloud_accounts.up.sql:78 (provider-scoped, account-scoped overrides for enabled/term/payment/coverage/ramp_schedule/include-exclude arrays). What hasn't been verified end-to-end is whether the recommendations engine actually consumes those overrides when generating per-account recommendations.

If the engine reads only the global service_configs rows, the entire override system is decorative — every PUT under /api/accounts/:id/service-overrides/... lands in the DB and disappears, while recommendations continue to use the global term/payment.

What needs verifying

For each plausible code path, trace whether the per-account override is read at the right point:

Recommendations collection (scheduler / on-demand) — internal/scheduler/scheduler.go populates RecommendationParams.Term/PaymentOption from globalCfg.DefaultTerm/DefaultPayment at ~scheduler.go:585-598 (per the PR feat(commitment-options): probe AWS reserved-offerings live & validate saves #54 exploration findings). Does it also consult account_service_overrides per account when iterating accounts? Grep for any read of account_service_overrides outside the API handlers.
Per-service recommendation generation — providers/aws/recommendations/parser_*.go files. Does the parser receive overridden term/payment via params, or does it read the global ServiceConfig directly?
Purchase execution — internal/purchase/execution.go. The execution path reads term/payment from RecommendationRecord.Term/PaymentOption (already stamped at recommendation time), not from config — so by then the override should already have been applied. The question is whether the override reached the recommendation record in the first place.
Coverage calculation — coverage is per-row on overrides. Does the engine apply the override or only the global default?
Disabled overrides — when enabled = false on an override row, does the engine skip that account's recommendations for that service, or does it ignore the flag?

Likely outcome of the trace: most of these read the global config and ignore overrides. If so, this issue's resolution is wiring per-account override lookup at the recommendation-collection point — one helper, called per (account_id, provider, service), that overlays the override's set fields on top of the global ServiceConfig.

Why this matters

Issues ux(settings): account override UIs should reuse commitmentOptions validity rules (e.g., RDS 3yr no-upfront) #107 (validity rules) and commitmentopts: probe Savings Plans offerings live (parity with RI probing from #54) #108 (SP probing) both assume the data on these rows actually matters. If it doesn't, both are window dressing.
Users who configure an override and see their recommendations unchanged would (correctly) consider this a P1 bug.
The override UIs landed (ux(settings): add payment option selector to AWS account overrides modal #72, feat(settings): create-override modal for AWS account overrides (closes #104) #106) without an integration test that proves the round trip — write override → recommendation reflects override.

Acceptance criteria

A documented trace (PR or issue comment) showing exactly which code path reads each per-account override field, and where the override merges with the global default.
An integration test in internal/scheduler/ or providers/aws/recommendations/ that:
- Seeds a global ServiceConfig with term=3, payment=all-upfront for (aws, rds)
- Seeds an account_service_overrides row for an account with term=1, payment=no-upfront on (aws, rds)
- Triggers recommendations collection for that account
- Asserts the resulting RecommendationRecord has term=1, payment=no-upfront (not the global values)
If the trace finds the override is not consumed today, file a follow-up bug under this issue's umbrella with the specific call sites that need updating, and add account_service_overrides to whichever lookup path is missing them.

Out of scope

Adding new override fields. The schema already covers the fields the UI surfaces.
UI changes — the goal is to verify (and if needed, fix) the backend integration, not change the front of house.

P1/severity:critical — if per-account overrides are not consumed by the recommendations engine (currently unverified), then every UI interaction since #72 and #106 has been a no-op on actual recommendations. This is potentially the most impactful bug in the overrides cluster: issues #107 and #108 both assume this data flows through to recommendations. Needs an urgent trace + integration test to confirm the round trip. (triage agent wave1-B)

Trace findings — overrides ARE decorative, confirmed

Audited every consumer of account_service_overrides reads + every reader of service_configs.Default* in the recommendation/purchase pipeline. Conclusion: the override system is fully implemented at the data layer (schema, CRUD API, UI in #72/#106, resolver in internal/config/resolver.go) but completely unwired from the recommendation pipeline. Every override an operator saves via the UI lands in the DB and stays there; the engine never reads it.

Where the override IS read today

$ rg 'GetAccountServiceOverride|ListAccountServiceOverrides' --type=go --no-tests
internal/api/handler_accounts.go:886    h.config.ListAccountServiceOverrides(ctx, id)         # serves GET …/service-overrides
internal/api/handler_accounts.go:921    h.config.GetAccountServiceOverride(ctx, ...)          # serves PUT …/service-overrides/:p/:s (read-modify-write)

Both call sites are in the override CRUD handlers themselves — read-your-own-overrides. Zero callers in the scheduler, the AWS/Azure/GCP providers, the recommendations parser, or the purchase manager.

Where `ResolveServiceConfig` is called

$ rg 'ResolveServiceConfig' --type=go --no-tests
internal/config/resolver.go:11    func ResolveServiceConfig(...)

The function is implemented and unit-tested (internal/config/resolver_test.go — 5 tests covering nil-override, all-fields-overridden, sparse, empty-slice-doesn't-override, no-mutation-of-global), but has zero production call sites. The merge logic that would produce a per-account-effective ServiceConfig is dead code.

Where the global ServiceConfig fields ARE read (i.e., where the override SHOULD have been merged in)

$ rg 'globalCfg\.Default(Term|Payment|Coverage)' --type=go --no-tests
internal/scheduler/scheduler.go:588    Term:           fmt.Sprintf("%dyr", globalCfg.DefaultTerm),
internal/scheduler/scheduler.go:589    PaymentOption:  globalCfg.DefaultPayment,
internal/api/handler_dashboard.go:124  if globalCfg != nil && globalCfg.DefaultCoverage > 0 {
internal/api/handler_config.go:72      svc.Term = cfg.DefaultTerm
internal/api/handler_config.go:73      svc.Payment = cfg.DefaultPayment
internal/purchase/manager.go:33,99     defaults.Term = cfg.DefaultTerm; defaults.Payment = cfg.DefaultPaymentOption

Six call sites where the global default is consulted; none of them look up the per-account override.

Architectural fork — where SHOULD the override be wired in?

The cloud APIs (Cost Explorer, Azure Advisor, GCP Recommender) typically return multiple recommendations per resource — one per (term, payment) combination. The current pipeline collects all combos and stores all of them. The override "filter" could plausibly live at three different points:

The acceptance criterion in the original issue ("trigger recommendations collection for that account → assert the resulting RecommendationRecord has term=1, payment=no-upfront") points at (A), but (B) is closer to current behaviour and avoids losing the comparison view. (C) is the smallest diff but doesn't satisfy the acceptance test.

Suggested next step

Pick the architecture (I'd lean (B) for behaviour-preservation), then a follow-up PR can:

Add ResolveServiceConfig calls at the chosen point.
Add Term / Payment to RecommendationFilter (currently only provider/service/region/account_ids).
Have ListStoredRecommendations consult the per-account override when account_ids is supplied.
Add the integration test from the acceptance criteria.

Happy to implement once the architecture call is made — leaving this issue open for that decision since picking unilaterally would either break the comparison view (A) or under-deliver vs the stated acceptance criterion (C).

I think the default override to just be pre-populated at purchase time, but allowing the user to select other options if desired

Investigation note for the purchase-time pre-fill design (request from 2026-04-29):

Override schema is fine. AccountServiceOverride (internal/config/types.go:450) carries Term, Payment, Coverage, Include/Exclude Engines/Regions/Types — every field a purchase form would need. Backend GET endpoint /api/accounts/:id/service-overrides already exists (handler at internal/api/handler_accounts.go:871) and the frontend client api.listAccountServiceOverrides(id) is wired (frontend/src/api/accounts.ts:155). No new endpoint needed.

But the Recommendations page has no per-rec purchase form to pre-populate. The two purchase paths today:

Per-row openPurchaseModal (recommendations.ts:1674): renders a read-only summary table — service, type, region, count, savings. No editable Term/Payment/Capacity fields. The rec's own term and payment (set by the scheduler at collection time) flow straight through to the executePurchase POST.
Bulk-purchase toolbar (bulk-purchase-payment + bulk-purchase-capacity): ONE Payment + ONE Capacity for the whole batch. Term is per-rec (the toolbar Term selector was removed in Bundle B per the comment at recommendations.ts:1125). The fan-out modal that follows splits the batch into buckets keyed by (provider, service, term) but every bucket inherits the toolbar's single Payment.

So "pre-populate at purchase time" maps to one of three plausible scopes — picking is a design choice, not an implementation choice:

(i) Toolbar-level seeding when bulk has a single (account, service): when every selected rec shares one (provider, account, service), look up that override and use its Payment as the toolbar default. When mixed, fall back. Smallest change. But only helps the single-cell case.
(ii) Per-bucket payment in the fan-out modal: extend the fan-out modal so each bucket has its own Payment dropdown, default-seeded from the bucket's override; the user can override each. Bigger UI change, but matches the "per-account override" intent more faithfully across mixed bulks.
(iii) Add an editable per-row preview to openPurchaseModal: turn that read-only summary into a tiny form (Term + Payment per row) seeded from the override. Biggest change; closest to the literal "open purchase modal → see prefilled defaults" reading.

Stopping here per the directive to avoid silently switching scope. Could you pick (i), (ii), (iii), or describe a fourth interpretation? Once the scope is clear I can resume.

(Recommendations themselves stay unfiltered in all three options, per your guidance.)

feat(recommendations): per-bucket payment seed from per-account servi…

feat(recommendations): editable per-row Term/Payment in single-bucket…

Implementation update — sub-options (ii) + (iii) are landed in PR #193 (force-pushed onto rebased base; CR re-pinged).

What's done in #193:

(ii) Per-bucket Payment dropdown in the fan-out modal, default-seeded from each bucket's per-account override when single-account.
(iii) Per-row editable Term + Payment in openPurchaseModal, default-seeded by override → rec.payment → fallback. Also fixes a pre-existing bug where handleExecutePurchase hardcoded 'all-upfront' for the single-bucket path.

Follow-up issues filed for the work that's NOT in #193 but came up during analysis/implementation:

feat(recommendations): option-B read-time filter for enabled / coverage / include-exclude per-account override fields #196 — option-B read-time filter for enabled / coverage / include-exclude lists (P1; those fields are still decorative until this lands).
feat(recommendations): per-rec seeding inside multi-account fan-out buckets #197 — per-rec seeding inside multi-account fan-out buckets (P3; cosmetic).
refactor(frontend): consolidate per-account override prefetch helper #198 — consolidate the duplicated per-account override prefetch helper (P3; cleanup).
bug(dashboard): potential savings headline ignores per-account coverage overrides #199 — dashboard "potential savings" headline ignoring per-account coverage overrides (P2; data-integrity).

Recommend keeping #111 open until #196 lands (it's the substantive remaining work). Closing PR #193 doesn't close this issue.

feat(recommendations): per-account service override pre-populated at …

feat(recommendations): apply per-account overrides at read time (#196) (

Wrap-up — closing the loop

The work proposed in this issue is complete across two PRs:

PR feat(recommendations): per-account service override pre-populated at purchase time (#111, ii+iii) #193 — purchase-time application of account_service_overrides.payment via the per-bucket fan-out modal (sub-option ii) and the per-row single-bucket modal (sub-option iii). Also fixed a pre-existing bug where handleExecutePurchase hardcoded payment: 'all-upfront' for the single-bucket path, silently dropping the toolbar's Payment.
PR feat(recommendations): apply per-account overrides at read time (#196) #200 — read-path application of the remaining override fields (enabled=false, coverage, include/exclude lists). The literal acceptance criterion from this issue ("seed override → recommendations reflect override") is exercised in internal/scheduler/scheduler_overrides_test.go.

Validation

Automated:

Backend: internal/config/recommendation_overrides_test.go (10 tests), internal/scheduler/scheduler_overrides_test.go (9 tests including the issue acceptance criterion), internal/api/handler_dashboard_test.go (8 cases for per-account coverage scaling).
Frontend: 9 new tests in frontend/src/__tests__/recommendations.test.ts covering both modals' override seed behaviour, plus 2 in settings-accounts.test.ts for refetch wiring.

Manual UI smoke (deployed instance, 2026-04-30): walked the full "set override → see effect at purchase time" path:

Created aws/ec2 → Partial Upfront override on the CUDly host AWS account
Single-bucket purchase modal: Payment dropdown pre-selected partial-upfront with (from account override) label rendered ✅
Multi-bucket fan-out modal (1yr + 3yr selection): both buckets pre-seeded partial-upfront with (from account override) label on each ✅
Reset override → both modals fall back to global default

GIF + zoomed screenshots from the smoke run captured locally; happy to share on request.

Follow-ups still open

feat(recommendations): per-rec seeding inside multi-account fan-out buckets #197 — per-rec seeding inside multi-account fan-out buckets (P3; cosmetic — toolbar fallback works correctly today)
refactor(frontend): consolidate per-account override prefetch helper #198 — consolidate the duplicated override-prefetch helper across openFanOutModal / openPurchaseModal (P3; cleanup)
bug(dashboard): potential savings headline ignores per-account coverage overrides #199 — dashboard "potential savings" headline scaling fix (P2; depends on feat(recommendations): apply per-account overrides at read time (#196) #200's plumbing which has now landed)

Closing.

fix(frontend/recs): address CR #253 findings — provider injection + c…

feat(frontend/recs): group rows by cell with savings range + collapse…

refactor(frontend): consolidate account-overrides prefetch helper (cl…

test(frontend): fix missing microtask ticks for openFanOutModal tests

refactor(frontend): consolidate account-overrides prefetch helper (cl…

Point	What changes	Pros	Cons
A. Collection-time (scheduler.go, fetchAndConvert)	Per account, look up override, only fetch / store the matching (term, payment) recs	Smaller cache, fewer Cost Explorer API calls	Loses the "what would 1yr-no-upfront save instead?" comparison view; recollect needed when an override changes
B. Read-time filter (handler_recommendations.go, ListRecommendations)	Cache stays full; per-account-resolved (term, payment) filters which cached recs are returned	Override changes take effect on the next page load with no recollect; comparison view still possible by ignoring the per-account filter	Read path becomes account-aware (currently it isn't); slightly more complex SQL
C. Purchase-time only (purchase/manager.go)	Listing shows all combos; the OVERRIDE only affects which (term, payment) the purchase actually buys	Minimal change — purchase manager already has a defaults concept	Recommendations UI continues to show all combos, ignoring the user's stated preference; Plan-driven scheduled purchases still need (A) or (B) to actually buy the right thing

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

verify recommendations engine actually consumes per-account service overrides #111

Summary

What needs verifying

Why this matters

Acceptance criteria

Out of scope

Trace findings — overrides ARE decorative, confirmed

Where the override IS read today

Where `ResolveServiceConfig` is called

Where the global ServiceConfig fields ARE read (i.e., where the override SHOULD have been merged in)

Architectural fork — where SHOULD the override be wired in?

Suggested next step

Wrap-up — closing the loop

Validation

Follow-ups still open

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Sunbelt Computer Software

PL/B Language Development and Support

verify recommendations engine actually consumes per-account service overrides #111

Description

Summary

What needs verifying

Why this matters

Acceptance criteria

Out of scope

Activity

cristim commented on Apr 28, 2026

cristim commented on Apr 28, 2026

Trace findings — overrides ARE decorative, confirmed

Where the override IS read today

Where ResolveServiceConfig is called

Where the global ServiceConfig fields ARE read (i.e., where the override SHOULD have been merged in)

Architectural fork — where SHOULD the override be wired in?

Suggested next step

cristim commented on Apr 29, 2026

cristim commented on Apr 29, 2026

cristim commented on Apr 29, 2026

cristim commented on Apr 29, 2026

Wrap-up — closing the loop

Validation

Follow-ups still open

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Where `ResolveServiceConfig` is called