fix(aws): fail loud on unknown payment-option/term/engine -- remove silent defaults

[cristim]

Summary

• H3 converters.go: add convertPaymentOptionE/convertTermInYearsE/convertLookbackPeriodE that error on unrecognized input; wire into SP recommendation path in parser_sp.go
• H4 elasticache/client.go: convertPaymentOption now returns (string, error) mirroring the RDS sibling
• H1/H2 savingsplans/client.go: convertPaymentOption and convertTermToSeconds now error on unknown/empty input (cherry-picked from merged fix(aws/savingsplans): paginate GetExistingCommitments via NextToken loop #1039 branch)
• M4 parser_services.go + rds/client.go: parseRDSDetails no longer defaults AZConfig to single-az when CE omits DeploymentOption; findOfferingID rejects empty AZConfig
• M5 parser_services.go: resolveEC2Tenancy errors on unrecognized tenancy values instead of silently collapsing to default
• M6 rds/client.go: normalizeEngineName errors for ambiguous Oracle/SQL Server/bare Aurora inputs instead of hardcoding edition guesses
• M1/M2/M3 ec2/client.go: replace raw string literals with SDK enum constants; buildEC2OfferingQuery errors on empty Platform; centralize Linux/UNIX into defaultEC2Platform constant

Regression tests added for all findings; each fails pre-fix and passes post-fix.

Closes #1084

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling for AWS recommendation conversions—invalid payment options, terms, and deployment configurations now explicitly fail with descriptive error messages instead of silently defaulting.
  • Enhanced validation for EC2 tenancy and RDS availability zone configurations to catch incomplete or unsupported inputs earlier.
  • Fixed Savings Plans offering lookup to validate parameters strictly, rejecting unknown or empty values.

• Tests

  • Added regression test coverage for error scenarios in AWS service configurations.

[coderabbitai]

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 28 minutes and 23 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7673b21f-24d6-49a7-bea7-e8ebc0e288e6

📥 Commits

Reviewing files that changed from the base of the PR and between 710cad1 and 681fd9e.

📒 Files selected for processing (4)

• providers/aws/recommendations/parser_services.go
• providers/aws/recommendations/parser_services_test.go
• providers/aws/services/rds/client.go
• providers/aws/services/rds/client_test.go

📝 Walkthrough

Walkthrough

This PR systematically replaces silent defaults with fail-loud error returns across AWS Cost Explorer enum converters and RI/Savings Plans offering lookup paths. New error-returning converter variants validate payment options, terms, and lookback periods; RDS and EC2 offering discovery now rejects missing or ambiguous metadata (AZConfig, engine name, platform, tenancy) instead of guessing; and Savings Plans offerings validation propagates conversion failures early.

Changes

Fail-loud AWS enum converters and stricter offering validation

Layer / File(s)	Summary
Cost Explorer enum converters with error variants providers/aws/recommendations/converters.go, providers/aws/recommendations/converters_test.go	Introduces convertPaymentOptionE, convertTermInYearsE, convertLookbackPeriodE that return errors on unsupported input; legacy wrappers (convertPaymentOption, convertTermInYears, convertLookbackPeriod) retained and delegate to error variants. Savings Plans converters (convertSavingsPlansPaymentOption, convertSavingsPlansTermInYears, convertSavingsPlansLookbackPeriod) now return (typedEnum, error) and use fail-loud variants. New tests added for error-returning variants and legacy wrapper behavior verified.
Savings Plans recommendation parser with enum validation providers/aws/recommendations/parser_sp.go	Updates getSavingsPlansRecommendations to validate PaymentOption, Term, and LookbackPeriod via error-returning converters; returns early with wrapped error on conversion failure instead of passing defaulted values to Cost Explorer API.
RDS stricter validation and offering discovery providers/aws/services/rds/client.go, providers/aws/services/rds/client_test.go	findOfferingID now fails if AZConfig is missing; normalizeEngineName errors on ambiguous Aurora/Oracle/SQL Server variants instead of guessing hardcoded editions; pagination refactored with fetchRDSOfferingPage helper to validate inputs per page, check context, enforce caps, and log progress. Tests added for AZConfig validation and ambiguous engine errors. parseRDSDetails no longer defaults AZConfig to "single-az" when DeploymentOption is omitted.
EC2 RI offering discovery and tenancy resolution providers/aws/services/ec2/client.go, providers/aws/services/ec2/client_test.go, providers/aws/recommendations/parser_services.go, providers/aws/recommendations/parser_services_test.go	buildEC2OfferingQuery returns error when Platform is empty instead of defaulting; resolveEC2Tenancy errors on unrecognized values including Dedicated Hosts. Introduces defaultEC2Platform constant and replaces offering-class string literals with AWS SDK typed enum types.OfferingClassTypeConvertible. Tests verify Platform validation and tenancy error handling.
Savings Plans offerings discovery with plan type and input validation providers/aws/services/savingsplans/client.go, providers/aws/services/savingsplans/client_test.go	Introduces resolveSPPlanType to validate recommendation plan type matching client scope and buildSPOfferingsInput to construct AWS API input with USD currency pinning and optional EC2 region filter. Refactored findOfferingID uses these helpers and fails early on unsupported/empty term or payment options. Tests expanded with error matrices for invalid payment options and terms.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• #1084: Directly implements the fail-loud fixes described in the linked issue across all AWS enum converters and offering lookup paths (payment options, terms, lookback periods, AZ config, engine names, tenancy, platform).

Possibly related PRs

• LeanerCloud/CUDly#599: Overlaps at EC2 tenancy parsing in providers/aws/recommendations/parser_services.go where main PR makes resolveEC2Tenancy fail-loud by returning error.
• LeanerCloud/CUDly#270: Both modify providers/aws/recommendations/parser_sp.go Savings Plans flow; main PR adds fail-loud enum validation, retrieved PR adds concurrency gating.
• LeanerCloud/CUDly#690: Overlaps on RDS/EC2 offering discovery refactors and stricter validation in find*OfferingID pagination and variant selection logic.

Suggested labels

bug

Poem

🐰 No more silent slips, we fail with pride,
Enums scream errors when inputs collide,
Platform and tenancy, engine and zone,
Now AWS defaults will not fly alone!
From RDS rigs to EC2 estates,
Fail-loud converters control our fates. 🚀

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(aws): fail loud on unknown payment-option/term/engine -- remove silent defaults' accurately and concisely summarizes the main change: converting silent defaults to fail-loud error handling in AWS provider converters.
Linked Issues check	✅ Passed	The PR fully addresses all coding requirements from issue #1084: added error-returning converter variants, wired them into SP paths, made converters fail on unknown/empty inputs, removed silent defaults in parsing functions, updated signatures to return errors, and added comprehensive regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly aligned with issue #1084 objectives: error handling in converters, removing silent defaults, replacing raw literals with SDK enums, adding regression tests, and refactoring for complexity reduction.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/aws-fail-loud-converters

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@providers/aws/recommendations/parser_services.go`:
- Around line 38-43: parseRDSDetails currently maps any non-nil
rdsDetails.DeploymentOption that's not "Multi-AZ" into "single-az", which
silently misclassifies unknown tokens; change parseRDSDetails so it explicitly
accepts only "Multi-AZ" and "Single-AZ" values: set rdsInfo.AZConfig =
"multi-az" for "Multi-AZ", "single-az" for "Single-AZ", and for any other
non-nil value return an error (or propagate one) instead of folding to
single-az; update the parseRDSDetails signature/callers to propagate the error
so providers/aws/services/rds/client.go consumes only validated AZConfig values.

In `@providers/aws/services/rds/client.go`:
- Around line 306-313: Validate that details.AZConfig contains one of the
expected enum values instead of only checking for empty string: in the RDS logic
(around details.AZConfig and paginateRDSOfferings) perform an explicit
check/switch against the allowed DeploymentOption values (e.g., the known
single-AZ and multi-AZ tokens your codebase expects) and return an error for any
unknown/typo value so you never silently treat invalid input as single-AZ;
update callers/consumers (e.g., where multiAZ is derived) to rely on this
validated value.
- Around line 580-593: normalizeEngineName currently rejects any engine string
containing "oracle" or "sqlserver"/"sql-server", which also blocks valid exact
edition tokens like "oracle-se2" or "sqlserver-se"; change the checks to only
error when the engineLower is exactly "oracle" or exactly
"sqlserver"/"sql-server" (i.e., when the caller omitted the edition), allowing
strings that include edition suffixes (e.g., "oracle-ee", "sqlserver-web") to
pass; update the conditional logic around engineLower in normalizeEngineName
(referencing variables engineLower and engine) so product-edition tokens are
accepted and only generic names produce the fmt.Errorf.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2b009b7f-830b-48c7-bcfb-0dfd1ccd9557

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc0899 and 710cad1.

📒 Files selected for processing (11)

• providers/aws/recommendations/converters.go
• providers/aws/recommendations/converters_test.go
• providers/aws/recommendations/parser_services.go
• providers/aws/recommendations/parser_services_test.go
• providers/aws/recommendations/parser_sp.go
• providers/aws/services/ec2/client.go
• providers/aws/services/ec2/client_test.go
• providers/aws/services/rds/client.go
• providers/aws/services/rds/client_test.go
• providers/aws/services/savingsplans/client.go
• providers/aws/services/savingsplans/client_test.go

[cristim]

Addressed CR pass (2026-06-07T19:21:35Z) findings (all 3 fixed in 681fd9e):

• parser_services.go ~38-43 (parseRDSDetails unknown DeploymentOption): fixed -- switch now explicitly accepts only Multi-AZ and Single-AZ; any other non-nil value returns an error instead of silently mapping to single-az. Regression test added (fails pre-fix, passes post-fix).

• rds/client.go ~306-313 (findOfferingID AZConfig enum validation): fixed -- replaced the empty-only check with a switch that adds a default error case for non-empty invalid values (e.g. typo-az). Previously a bad non-empty AZConfig fell through to multiAZ==false in paginateRDSOfferings, the same mis-buy as the old default. Regression test added.

• rds/client.go ~580-593 (normalizeEngineName Contains vs ==): fixed -- oracle and sqlserver checks changed from strings.Contains to == so edition-qualified tokens (oracle-se2, oracle-ee, sqlserver-se, sqlserver-web, sql-server-ex, etc.) pass through as valid RDS ProductDescription values. The previous Contains check contradicted the error message which told callers to supply those exact edition tokens. Updated existing test cases that incorrectly expected edition strings to error; added TestNormalizeEngineName_EditionTokensPassThrough covering 10 edition variants.

@coderabbitai review

[coderabbitai]

@cristim, thanks for the detailed summary — all three fixes look well-reasoned:

1. parseRDSDetails unknown DeploymentOption — making the switch exhaustive and failing on anything other than "Multi-AZ" / "Single-AZ" is exactly the right tightening.
2. findOfferingID AZConfig enum guard — the default: error branch on a non-empty invalid value closes the silent mis-buy path that the empty-only check missed.
3. normalizeEngineName == vs strings.Contains — this is the correct fix; using Contains was self-contradictory given that the error message already directed callers to supply the exact edition tokens.

Triggering a fresh review now.

---

✏️ Learnings added

Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/1085

Timestamp: 2026-06-07T19:32:26.165Z
Learning: In LeanerCloud/CUDly `providers/aws/recommendations/parser_services.go`, `parseRDSDetails` uses an exhaustive switch on `DeploymentOption`: "Multi-AZ" maps to "multi-az", "Single-AZ" maps to "single-az", nil leaves AZConfig unset, and any other non-nil value returns an error. Silent defaulting to "single-az" for unknown values was removed in PR `#1085` commit 681fd9e81. Regression test added.

---

Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/1085

Timestamp: 2026-06-07T19:32:26.165Z
Learning: In LeanerCloud/CUDly `providers/aws/services/rds/client.go`, `findOfferingID` validates AZConfig with a switch that has a `default: error` case covering non-empty invalid values (e.g. "typo-az"), not just an empty-string check. A bad non-empty AZConfig previously fell through to `multiAZ==false` in `paginateRDSOfferings` (same mis-buy as the old default). Fixed in PR `#1085` commit 681fd9e81. Regression test added.

---

Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/1085

Timestamp: 2026-06-07T19:32:26.165Z
Learning: In LeanerCloud/CUDly `providers/aws/services/rds/client.go`, `normalizeEngineName` uses `==` (not `strings.Contains`) for oracle and sqlserver checks. Edition-qualified tokens (oracle-se2, oracle-ee, sqlserver-se, sqlserver-web, sqlserver-ex, etc.) must pass through as valid RDS ProductDescription values. Using Contains was self-contradictory: the error message told callers to supply the exact edition tokens. Fixed in PR `#1085` commit 681fd9e81. `TestNormalizeEngineName_EditionTokensPassThrough` covers 10 edition variants.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T17:45:38.040Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/cloudsql/client.go`, `getSQLPricing` had the same per-hour/term-total unit mismatch as memorystore and cloudstorage: `commitmentPrice` (per-hour SKU rate) was passed directly to savings math that expected a term total, producing ~99.99% savings, and `HourlyRate` was set to `per-hour / hoursInTerm` (near-zero). Fixed in PR `#1047` commit 20590c6b4 (issue `#1078` folded in): `commitmentPriceTerm := commitmentPrice * hoursInTerm`; `HourlyRate = commitmentPrice` (raw per-hour). `convertGCPRecommendation` also hardcoded `rec.Term = "1yr"`, ignoring `params.Term`; fixed with same defaulting pattern as memorystore/cloudstorage. Regression tests: `TestGetSQLPricing_CommitmentPriceIsTermTotal` and `TestCloudSQLConvertGCPRecommendation_PropagatesTermFromParams`. PaymentOption was already correct (`if paymentOption == "" { paymentOption = "monthly" }`).

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T17:38:19.416Z
Learning: In LeanerCloud/CUDly GCP service converters (`convertGCPRecommendation` in memorystore/client.go and cloudstorage/client.go), `rec.Term` must be derived from `params.Term` with a `"1yr"` default — not hardcoded to `"1yr"`. Without this, 3-year callers always emit 1-year commitments even though the downstream `termYears` derivation from `rec.Term` is correct. Fixed in PR `#1047` commit c6280c390 (F2 for memorystore, F4 for cloudstorage). Regression tests: `TestConvertGCPRecommendation_PropagatesTermFromParams` and `TestCloudStorageConvertGCPRecommendation_PropagatesTermFromParams`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T17:38:19.416Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/computeengine/client.go`, `convertGCPRecommendation` must force `paymentOption = "monthly"` unconditionally (logging a WARN when a non-monthly value such as "upfront", "all-upfront", or "partial-upfront" is supplied) because GCP CUDs are monthly-only and any non-monthly value passed through would be a silent misconfiguration. Fixed in PR `#1047` commit c6280c390 (F1). Regression test: `TestConvertGCPRecommendation_NonMonthlyPaymentOptionForcedToMonthly`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:13:37.421Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/computeengine/client.go`, `convertGCPRecommendation` no longer hardcodes `Term = "1yr"` (fixed in PR `#1047` commit 8f9a787, H-3 audit finding). It propagates `params.Term` with a `"1yr"` default, validates via `termPlan`, and returns nil on unknown term. This is stricter than memorystore/cloudstorage/cloudsql (which default and continue): computeengine returns nil on unknown term because it is on the purchase path. Regression tests: `TestConvertGCPRecommendation_PropagatesParamsTerm`, `TestConvertGCPRecommendation_RejectsUnknownTerm`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:28:28.112Z
Learning: In LeanerCloud/CUDly `providers/gcp/recommendations.go`, `collectRegion` fans out to all four GCP service clients (computeengine, cloudsql, memorystore, cloudstorage) concurrently (fixed in PR `#1047` commit 95a390f, H-2 finding). `regionResult` gains `cache` and `storage` slices; merge is updated accordingly. The decision was to wire (not trim) memorystore and cloudstorage because both have complete `GetRecommendations` implementations and their no-op `PurchaseCommitment` paths are orthogonal to surfacing advisory recommendations. Regression tests: `TestRegionResult_HasCacheAndStorageFields` and `TestShouldIncludeService_Cache_Storage`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:28:28.112Z
Learning: In LeanerCloud/CUDly GCP service clients (computeengine, cloudsql, memorystore, cloudstorage), `GetRecommendations` filters out non-ACTIVE recommendations using the typed `recommenderpb.RecommendationStateInfo_ACTIVE` enum (CLAIMED/SUCCEEDED/FAILED/DISMISSED are skipped). State is accessed nil-safely via the proto getter. Fixed in PR `#1047` commit 95a390f (H-1 finding). Regression tests: `TestGetRecommendations_FiltersNonActiveStates` (5-rec mock, only ACTIVE survives) and `TestGetRecommendations_ActiveRecIncluded`. Existing mock tests in all four services had StateInfo added to satisfy the new filter.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:34:23.733Z
Learning: In LeanerCloud/CUDly Azure pricing extractors (compute, cache, database, cosmosdb, search, managedredis), Azure's Retail Prices API returns "1 Year" (singular) for 1-year reservation terms and "3 Years" (plural) for 3-year terms. All 6 pricing extractors use `azureTermString(termYears int) string` helper (added in PR `#1045` commit 5e938d9) to produce the correct singular/plural form for term matching. Prior to this fix, "%d Years" format produced "1 Years" which never matched Azure API responses, causing all 1-year reservation pricing lookups to silently fail. Regression tests: `TestExtractVMPricing_SingularOneYear`, `TestExtractVMPricing_PluralThreeYears`, `TestExtractRedisPricing_SingularOneYear`, `TestExtractRedisPricing_PluralThreeYears`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:13:37.421Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/computeengine/client.go`, `termPlan` returns `(string, error)` using `computepb.Commitment_TWELVE_MONTH.String()` and `computepb.Commitment_THIRTY_SIX_MONTH.String()` as the canonical plan strings (fixed in PR `#1047` commit 8f9a787). It accepts 1yr/1/12mo and 3yr/3/36mo; it errors on empty or unrecognized input (no silent 12-month default). Regression tests: `TestTermPlan_UsesSdkEnumConstants`, `TestTermPlan_RejectsUnknownTerm`, `TestTermPlan_AcceptsAllDocumentedForms`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-25T21:45:57.471Z
Learning: In this repository (CUDly), `internal/commitmentopts/normalizePayment` is unexported and AWS-spelling-focused (parses "All Upfront"/"ALL_UPFRONT" → canonical AWS tokens). `internal/api/validation.go:purchasePaymentWhitelist` is the purchase-execute boundary. `pkg/common/reservation_name.go:normalizeReservationPayment` produces short-form name segments. These are all distinct concerns from the cross-provider `NormalizePaymentOption` helper in `internal/config/validation.go`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:13:37.421Z
Learning: In LeanerCloud/CUDly `providers/gcp/services/computeengine/client.go`, the `memMBPerVCPU = 4096` const was removed in PR `#1047` commit 8f9a787. MEMORY MB is now extracted directly from the Recommender payload via `memoryMBFromOperationGroups` (sibling of `vcpuCountFromOperationGroups`). `buildInsertRequest` and `GroupCommitments` call `memoryMBFromDetails(rec)` which returns an error (no silent fallback) when `Details.MemoryGB` is absent. This ensures high-memory families (e.g. N2-highmem) get the correct MB from the payload rather than the GENERAL_PURPOSE approximation. Regression test: `TestBuildInsertRequest_RefusesMissingMemory`.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-25T21:45:57.471Z
Learning: In this repository (CUDly), GCP CUD commitments are inherently monthly-billed. The GCP CUD purchase API (`providers/gcp/services/computeengine/client.go:350-373`, `buildCommitmentRequests`) takes only a `Plan` field (TWELVE_MONTH / THIRTY_SIX_MONTH) and never reads a payment-option field. Therefore, the only valid payment option for GCP in `ValidPaymentOptionsByProvider` is `"monthly"`. The `NormalizePaymentOption` GCP branch should collapse any non-monthly token (including legacy `"upfront"`) to `"monthly"`, with a WARN log at the `scheduler.convertRecommendations` boundary.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.