ux(settings): clarify Savings Plans card covers SageMaker + Lambda (#22)#53
Merged
cristim merged 1 commit intoApr 25, 2026
Conversation
The "Compute Savings Plans" card in Settings → Purchasing actually governs defaults for all four SavingsPlansType values fetched by providers/aws/recommendations/parser_sp.go: ComputeSp (covers EC2, Fargate, Lambda), Ec2InstanceSp, SagemakerSp, and DatabaseSp. Users reported SageMaker and Lambda looked unrepresented in settings. Rename the card header to "Savings Plans" and add a small hint line listing the covered services, so the coverage is self-documenting. No extra cards are added: a Lambda-specific card would be misleading (Lambda SPs do not exist as a standalone product — Lambda usage is covered by Compute SP), and SageMaker SPs share the same term/payment knobs as the other types. The underlying aws-savingsplans-term and aws-savingsplans-payment inputs keep the same IDs so settings wiring and the existing save/load paths are unchanged.
Member
Author
|
@coderabbitai review |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 7 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Actions performedReview triggered.
|
Member
Author
|
AWS Sanity (Read-only Dry Run) passed on commit 00e7ae4. All checks green. |
Member
Author
|
Azure Sanity (Read-only Dry Run) passed on commit |
This was referenced Apr 25, 2026
cristim
added
triaged
cristim
added a commit
that referenced
this pull request
Apr 30, 2026
The AWS recommendations parser emits `sagemaker` as a distinct service (see providers/aws/recommendations/parser_sp.go:153, mapping to SagemakerSp), but the Recommendations page Service filter dropdown omitted it — users could see SageMaker recommendations in the unfiltered list but could not narrow to them. Adds `<option value="sagemaker">SageMaker</option>` under the AWS optgroup in the static `<select id="service-filter">`, alphabetically positioned between `redshift` and `savingsplans`. The option participates automatically in provider-based show/hide via the existing `updateServiceFilterVisibility` (it iterates optgroups, no per-option wiring needed). Lambda is intentionally NOT added in this PR: - The AWS recommendations parser does not emit `service=lambda`. Lambda commitments are surfaced under the existing `savingsplans` (Compute Savings Plans) umbrella, which the dropdown already exposes. - SageMaker Savings Plans is a distinct AWS product from Compute Savings Plans (different commitment scope + pricing), which is why SageMaker warrants its own filter value while Lambda does not. - This decision is correct under both possible resolutions of the still-open PR #71 (separate purchasing-defaults cards) and the already-merged PR #53 (Savings Plans is the umbrella for SM/Lambda). Adds a regression test in `frontend/src/__tests__/html.test.ts` that parses the real index.html and asserts SageMaker is present under the AWS optgroup, alongside the existing optgroup-presence test. Verification: - `npx jest` — 1252/1252 pass across 35 suites - `npx tsc --noEmit` — clean - `npm run build` — webpack production build succeeds Closes #77
cristim
added a commit
that referenced
this pull request
Apr 30, 2026
The AWS recommendations parser emits `sagemaker` as a distinct service (see providers/aws/recommendations/parser_sp.go:153, mapping to SagemakerSp), but the Recommendations page Service filter dropdown omitted it — users could see SageMaker recommendations in the unfiltered list but could not narrow to them. Adds `<option value="sagemaker">SageMaker</option>` under the AWS optgroup in the static `<select id="service-filter">`, alphabetically positioned between `redshift` and `savingsplans`. The option participates automatically in provider-based show/hide via the existing `updateServiceFilterVisibility` (it iterates optgroups, no per-option wiring needed). Lambda is intentionally NOT added in this PR: - The AWS recommendations parser does not emit `service=lambda`. Lambda commitments are surfaced under the existing `savingsplans` (Compute Savings Plans) umbrella, which the dropdown already exposes. - SageMaker Savings Plans is a distinct AWS product from Compute Savings Plans (different commitment scope + pricing), which is why SageMaker warrants its own filter value while Lambda does not. - This decision is correct under both possible resolutions of the still-open PR #71 (separate purchasing-defaults cards) and the already-merged PR #53 (Savings Plans is the umbrella for SM/Lambda). Adds a regression test in `frontend/src/__tests__/html.test.ts` that parses the real index.html and asserts SageMaker is present under the AWS optgroup, alongside the existing optgroup-presence test. Verification: - `npx jest` — 1252/1252 pass across 35 suites - `npx tsc --noEmit` — clean - `npm run build` — webpack production build succeeds Closes #77
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #22.
The "Compute Savings Plans" card in Settings → Purchasing actually drives defaults for all four Savings Plans types that
providers/aws/recommendations/parser_sp.gofetches on every collection:ComputeSp— covers EC2, Fargate, LambdaEc2InstanceSp— specific EC2 instance familiesSagemakerSp— SageMakerDatabaseSp— RDSAll four share the same term/payment vocabulary at the AWS API, so the UI intentionally exposes a single card rather than four near-identical rows. The old header "Compute Savings Plans" made that invisible to the reporter, who reasonably concluded SageMaker and Lambda had no purchasing defaults.
Change
.service-default-hintCSS rule instyles/settings.css.Why not separate Lambda / SageMaker cards (as the issue originally proposed):
ServiceConfigplumbing, a DB migration, and scheduler/UI changes — out of scope for a documentation fix and unwarranted without evidence users have divergent preferences per plan type.IDs
aws-savingsplans-termandaws-savingsplans-paymentare preserved, sosettings.tswiring and the existing save/load paths are unchanged.Test plan
npm run build(frontend) — clean webpack production buildnpm test(frontend) — 1242/1242 tests pass across 35 suitessettings.tswiring,settings.test.tscoverage, and the save/reset bar continue to work.service-default-card .service-default-hintso it can't bleed into other cardsScreenshot
(Text-only label tweak; the new hint renders as a single muted subtitle line immediately under the card title, above the Term/Payment selects.)