[IAmJSD]

Settings saved Azure and GCP service configs and per-account overrides under per-product ids (azure/vm, azure/sql, azure/cosmosdb, azure/redis, gcp/sql, gcp/memorystore) that no recommendation carries, so those defaults, filters and overrides never applied. This PR switches Settings to the ids recommendations use, migrates stored service configs on the next save, and flags overrides stored under an old id.

Refs #613
Partial: overrides stored under an old id are flagged but not migrated (they need a manual re-create, or a backend SQL migration), and Cosmos DB's azure/nosql is not in mapServiceSlug, so the new slug test allows it as one named exception.

What changed

• Traced the read path: Azure/GCP recommendations are stored with the provider's common.ServiceType slug (compute, relational-db, nosql, cache, search; see internal/scheduler/scheduler.go and the cloud-commitments-go Azure/GCP clients). The scheduler (config.ResolveAccountConfigsForRecs -> applyAccountOverrides) and the purchase modal's override seeding look up configs by that exact (provider, service) pair. Only azure/search and gcp/compute happened to match, so this was a live break, not just an inconsistency.
• New frontend/src/lib/settings-service-ids.ts: the current Azure/GCP ids, a legacy map (azure/vm->compute, azure/sql->relational-db, azure/cosmosdb->nosql, azure/redis->cache, gcp/sql->relational-db, gcp/memorystore->cache) and canonicalizeServiceConfigs(), which re-keys stored legacy rows onto the current id (a row already stored under the current id wins).
• frontend/src/settings.ts: SERVICE_FIELDS, AZURE_OVERRIDE_SERVICES, GCP_OVERRIDE_SERVICES and SERVICE_DISPLAY_NAMES use the current ids. Card DOM ids such as azure-vm-term are unchanged.
• loadGlobalSettings canonicalizes the GET /config services list and fills the cards through SERVICE_FIELDS instead of building DOM ids from the row's service id. A legacy azure/vm row still fills the VM card, and the next Save PUTs it as azure/compute, keeping its ramp_schedule and filters.
• The overrides panel shows a short warning under any override stored under an old id, below main's "Updated " stamp: "Old service id: never applied. Re-create it as azure/compute, then delete this row." One CSS rule in frontend/src/styles/settings.css.
• Existing tests updated to the new ids (settings-accounts, settings-gcp-storage-removed, settings-service-payment, settings); new frontend/src/__tests__/settings-service-ids.test.ts.

Why this approach

• The backend matches on the recommendation's service id, so changing the frontend ids is the only fix that needs no data migration.
• Keeping the card DOM ids avoids churn in index.html and the filter/term/payment wiring.
• Legacy service configs are migrated on read and save, so nothing already configured is lost.
• Legacy overrides are not migrated automatically: doing that silently while just viewing the panel would be a surprising write, and there is no clean one-click path without a backend helper. They are flagged instead; the current id is still offered in Add override, since the legacy row does not block it.

Screenshots

Screenshots were taken before this branch was merged with main; before = main at a14dc8f. After is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API.

Before	After

Azure account overrides modal with an azure/vm override. Before: the azure/vm row looks like any other, although no recommendation ever matches it. After: it carries an amber note saying it is never applied and naming azure/compute as the id to re-create it under; the azure/relational-db row (current id) has no note. On current main the service cell also shows the "Updated " stamp from #8, so the shipped cell has that stamp under the service name and the warning below it.

Verification

Run on this branch's head a2583a31:

• Merged with main at 5b52c4e; conflicts in frontend/src/settings.ts resolved, see the merge commit.
• Lint: 0 errors. Typecheck and build OK.
• Jest: 3875 of 3876 passed, 1 skipped.
• New settings-service-ids.test.ts (11 tests). Before the merge it passed with --randomize, and 4 of its tests failed against main's settings.ts. It reads mapServiceSlug from internal/purchase/execution.go and checks that every Azure/GCP id the cards PUT and the override modal offers is in it (with azure/nosql as the only allowed exception), and covers legacy load and save, the current-id-wins rule and the override warning.
• A --randomize run of all settings suites hit "Settings Module > saveGlobalSettings > shows error alert on failure" with seed -680793092; the same seed fails that test on main too (pre-existing order dependence).
• Playwright (e2e-local.sh): 200 passed. No new spec; the logic is covered by jest.
• Secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Notes for review

• lib/plan-service-ids.ts (and its copy in plans.ts) has a second legacy-id table for plans. The two differ on purpose (for example gcp/sql vs gcp/cloudsql, and azure/nosql is not a plan service), so they are not shared here.
• Cosmos DB is saved as azure/nosql because that is what Cosmos recommendations carry (common.ServiceNoSQL). mapServiceSlug does not list nosql, so Cosmos cannot go through the plan/purchase slug path. That is a separate backend gap.
• Old rows stay in the database. service_configs rows under old ids are effectively replaced on the next Settings save but not deleted (there is no DELETE endpoint for service configs), and old overrides need a manual re-create and delete. A backend SQL migration (UPDATE service_configs / account_service_overrides SET service = <new id> where the row is legacy and none exists under the new id) would clean this up for good; that is out of scope for a frontend-only branch.
• The warning copy and its amber color (--cudly-warn-fg) need design sign-off.
• SERVICE_DISPLAY_NAMES labels are unchanged; only their keys moved.
• Azure/GCP deploys are paused, so a live check (a real Azure/GCP recommendation filtered by a Settings override) has to wait until they resume.

Overlaps

• chore(frontend): standardize loading indicators and add slow/empty fallbacks #186 touches an adjacent import line in settings.ts (trivial textual conflict).
• Merges cleanly with the other open branches touching settings.ts, settings.css or the settings tests (feat(frontend): Admin Purchasing policies page is very long with a mid-page partial load #190, fix(frontend): low-severity copy, toast, confirm and stale-state fixes #728, fix(frontend): dialogs bypassing openModal lack focus handling, Escape and alert roles #731, fix(frontend): status badges use four shapes and conflicting colors per status #737, chore(frontend): add typography foundation (line-height, heading scale, font inherit) #741, chore(frontend): unify four modal systems and five card styles, reuse kpi-tile #743, fix(frontend): raw transport and developer errors reach users instead of friendly messages #748, chore(frontend): replace mixed Unicode glyphs with one SVG icon set #751).
• Related: fix(frontend): the GCP Storage term setting is offered although Cloud Storage has no commitment product #543 already removed the GCP storage card; this branch updates that test's expected GCP list to compute/relational-db/cache.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 8 billable files and costs up to $2.00.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 5b52c4e
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: b03a3338-a1f7-4bd0-b299-f38a37371af7

Commits

Reviewing files that changed from the base of the PR and between 5b52c4e and a2583a3.

Files selected for processing: 8

• frontend/src/__tests__/settings-accounts.test.ts
• frontend/src/__tests__/settings-gcp-storage-removed.test.ts
• frontend/src/__tests__/settings-service-ids.test.ts
• frontend/src/__tests__/settings-service-payment.test.ts
• frontend/src/__tests__/settings.test.ts
• frontend/src/lib/settings-service-ids.ts
• frontend/src/settings.ts
• frontend/src/styles/settings.css

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE a2583a3120ed574841884f924092a0b7f1a6a6c8

Blocking:

• none
  Non-blocking:
• frontend/src/lib/settings-service-ids.ts:13-26: a second legacy-id table exists in lib/plan-service-ids.ts (with a copy at plans.ts:690, from feat(plans): add date-draft and canonical service id libraries #1067). The two disagree, for valid domain reasons: gcp/sql vs gcp/cloudsql, and azure/nosql is absent from CANONICAL_PLAN_SERVICES.azure. Add a cross-reference comment or share one canonical-id list.
• frontend/src/settings.ts:3155: SERVICE_FIELDS.find runs once per row, O(n*m) on about 17 rows, which is negligible. A row with no card now hits an explicit continue and behaves the same as before.
• main has moved 8 commits past the merge parent (5b52c4e -> 51aa5b1). None touch settings, and merge-tree is clean.
  Attacked: The merge diffed against ^2 (main) is identical to origin/main...HEAD, so none of main's settings.ts, settings.css or test changes were dropped. The one real conflict was the loadOverridesPanel service cell. ux(settings): expose updated_at + audit log for service overrides and configs #8's buildLastUpdatedStamp is kept, and the fix(frontend): settings service overrides still use the old Azure/GCP service ids #613 hint is appended after it via textContent, so there is no XSS. Against ^1, the branch lib and tests are unchanged. --cudly-warn-fg survives fix(frontend): replace undefined CSS tokens and inline MFA panel styles #1382's token cleanup. On the backend, validateServicePath and ServiceConfig.validateService have no allowlist, so relational-db, nosql and cache are accepted. The Azure and GCP commitmentOptions use _default, so no term or payment constraints are lost. Grepped for leftover vm/sql/memorystore ids: only intentional legacy fixtures remain. Test edits only rename ids. Jest (settings-service-ids, settings-accounts, settings-gcp-storage-removed, settings-service-payment, settings, plan-service-ids): 257/257.

[IAmJSD]

Local verification at a2583a31 merged onto current main (818e11e8; merge-tree 640adedb; Linux, Node, headless Chromium):

• This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
• The combined tree of every UI PR in this merge wave (tree dc090cc2, main plus 17 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4097 passed, 4098 total), and the full Playwright suite (232 passed).
• CI is green on a2583a31 (CI Success, Run pre-commit hooks).
• The independent review above approves this exact head.
• Nothing was applied to any cloud.

[IAmJSD]

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at a2583a31.