feat(recommendations): per-rec deterministic seeding in multi-account fan-out (closes #197)

[cristim]

Summary

• FanOutBucket gains optional perRecPayments: Map<string, BulkPurchasePayment> populated for buckets spanning 2+ distinct cloud_account_id values; each rec's default is seeded from its account's AccountServiceOverride, falling back to the bucket-level payment.
• openFanOutModal now pre-fetches overrides for all accounts (was: sole-account buckets only), so per-rec seeding has the data it needs.
• renderFanOutBucketSection renders individual .fanout-per-rec-payment selects with data-rec-id when perRecPayments is set; the change handler mutates module state, and handleFanOutExecute uses perRecPayments.get(r.id) per POST.
• Two new tests (h, i) cover multi-account bucket seeding and dropdown-change propagation; all 321 tests pass.

Test plan

• cd frontend && npm ci && node_modules/.bin/jest --testPathPattern="recommendations.test.ts$" --no-coverage - 321 tests pass
• go build ./... - no backend breakage
• Select recs from two different AWS accounts, click bulk-purchase: fan-out modal shows per-rec Payment dropdowns inside the multi-account bucket; single-account buckets show only the bucket-level dropdown as before
• Change a per-rec dropdown, submit: each rec's POST body carries its individual payment value

🤖 Generated with claude-flow

Summary by CodeRabbit

New Features

• Added per-recommendation payment customization for multi-account bulk purchase operations.
• Per-recommendation payment dropdown controls now appear in the fan-out modal for multi-account buckets.
• Payment selection logic now respects account-specific service overrides when available.

[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 54 minutes and 48 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: 83d29892-63f1-477d-8567-acb3a7b460d5

📥 Commits

Reviewing files that changed from the base of the PR and between 4956d66 and d7c5764.

📒 Files selected for processing (3)

• frontend/src/__tests__/recommendations.test.ts
• frontend/src/app.ts
• frontend/src/recommendations.ts

📝 Walkthrough

Walkthrough

This PR extends the fan-out bulk purchase flow to support per-recommendation payment overrides in multi-account buckets. The interface adds a perRecPayments map, override prefetching resolves per-rec payment values, the modal UI renders per-rec payment dropdowns, and the submission logic uses those per-rec choices instead of defaulting all recommendations in a bucket to the same payment.

Changes

Per-Recommendation Payment Overrides for Multi-Account Fan-Out

Layer / File(s)	Summary
FanOutBucket contract and documentation frontend/src/recommendations.ts	FanOutBucket interface adds optional perRecPayments?: Map<string, BulkPurchasePayment> field; inline documentation clarifies multi-account bucket support for per-rec payment overrides.
Prefetch overrides and compute perRecPayments frontend/src/recommendations.ts	openFanOutModal now prefetches account service overrides for all distinct cloud_account_ids across buckets. When building FanOutBucket objects, multi-account buckets compute perRecPayments by resolving each rec's payment from its account override or bucket-level default. getFanOutBuckets() deep-copies perRecPayments to prevent caller mutations.
Modal UI: per-rec payment dropdowns frontend/src/recommendations.ts	renderFanOutBucketSection renders per-rec Payment dropdown controls when perRecPayments exists, seeded from the map (fallback to bucket payment when absent). Change handlers update both module state and the map entry for the affected recommendation.
Purchase submission with perRecPayments frontend/src/app.ts	handleFanOutExecute constructs the purchase POST payload by resolving each recommendation's payment from b.perRecPayments (keyed by rec.id) when available, otherwise falling back to b.payment.
Test coverage for multi-account per-rec payments frontend/src/__tests__/recommendations.test.ts	Test (h) verifies multi-account buckets build and render perRecPayments seeded from account service overrides. Test (i) verifies dropdown changes update the corresponding perRecPayments entry while leaving others unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• LeanerCloud/CUDly#600: Modifies fan-out purchase POST payload construction in handleFanOutExecute to preserve recommendation details; overlaps with this PR's recommendation object assembly in the same code path.
• LeanerCloud/CUDly#710: Changes fan-out payment logic in frontend/src/recommendations.ts to make payment resolution per-recommendation rather than bucket-wide; directly related to the perRecPayments structure introduced in this PR.
• LeanerCloud/CUDly#193: Modifies openFanOutModal and fan-out submission to consume account service overrides for recommendation payments; this PR extends that pattern from bucket-level seeding to per-recommendation overrides via perRecPayments.

Suggested labels

priority/p2, severity/medium, urgency/this-quarter, impact/many, effort/m

Poem

🐰 Per-rec payments hop into place,
No more buckets sharing one face—
Each recommendation chooses its own,
Multi-account dreams fully grown! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% 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 PR title clearly and concisely summarizes the main change: adding per-rec payment deterministic seeding support for multi-account fan-out, which directly matches the substantial additions to FanOutBucket, the modal UI, and payment handling logic throughout the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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/197-wave12

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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 full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[cristim]

pr-iterate: CR appears to be blocked by org-level usage limits (Your organization has run out of usage credits per CR's most recent reply on this PR), so no substantive review has landed despite two @coderabbitai review / full review pings. This PR is otherwise merge-ready: mergeStateStatus=CLEAN, CI green (5/5 checks pass), no conflicts. Re-invoke /pr-iterate 838 after the org's CR billing is topped up to get a substantive CR pass. To clear the org-wide marker after billing is fixed: rm ~/.claude/agent-comms/cr-rate-limit-deadline.txt.

[coderabbitai]

@cristim Thanks for the context note. I'll attempt a full review now — if the org's usage credits have been replenished since the last attempt, this should go through. Triggering below:

---

🧠 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.

✅ Action performed

Full review finished.

[cristim]

@coderabbitai review

[coderabbitai]

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 39 minutes and 28 seconds before sending another message.

[cristim]

@coderabbitai full review

---

Generated by Claude Code

[coderabbitai]

@cristim Triggering a full review of PR #838 now.

---

🧠 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-05-03T16:07:51.732Z
Learning: In the LeanerCloud/CUDly repository, approximately 50% docstring coverage is a pre-existing project-wide baseline. It should not be flagged as an actionable issue in individual PR reviews, as addressing it is out of scope for focused/surgical fixes.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:43:53.792Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), the Dashboard "Upcoming purchases" widget (`dashboard.ts`) has two Cancel entry points (card-level and modal) that both call `api.deletePlannedPurchase`. The `UpcomingPurchase` response DTO must include `CreatedByUserID *string` (omitempty, mirroring `PlannedPurchase`) so the frontend can gate these buttons via a `canCancelUpcomingPurchase` helper that mirrors `canManageScheduledPurchase` on the Plans page. Without this, the backend correctly 403s non-owners but the UI has no gate and the UX is broken. Regressions: `frontend/src/__tests__/dashboard-ownership-950.test.ts` (6 tests) and `TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID`. Fixed in commit 94326f6b9.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:28:28.139Z
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`.

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@frontend/src/recommendations.ts`:
- Around line 3401-3425: openFanOutModal() currently eagerly populates
perRecPayments for every rec, which makes the bucket-level dropdown a no-op for
rows that should fallback to seed.payment; change the logic so perRecPayments is
left undefined or only contains explicit overrides (e.g., when
overridesByAccount provides a different payment or the user has manually changed
a row) instead of setting every rec to bucketPayment, ensure the execute path
uses perRecPayments.get(rec.id) ?? seed.payment, and update the bucket-level
change handler to either remove per-rec entries that should follow the new
bucket default or sync visible per-rec selects for ids absent from the map; look
for symbols openFanOutModal, perRecPayments, seed.payment, overridesByAccount
and the bucket-level change handler and adjust them accordingly.

🪄 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: f828f1d6-1f03-4e2e-b74d-d5cff59c516f

📥 Commits

Reviewing files that changed from the base of the PR and between 4956d66 and 1b730df.

📒 Files selected for processing (3)

• frontend/src/__tests__/recommendations.test.ts
• frontend/src/app.ts
• frontend/src/recommendations.ts

[cristim]

CR follow-up: bucket-level Payment no-op for multi-account buckets

Addressed the outstanding CodeRabbit finding on frontend/src/recommendations.ts (openFanOutModal / per-rec Payment) in commit d7c5764.

Finding (Major / Quick win): "Bucket-level Payment is a no-op for multi-account buckets."
Disposition: valid, fixed.

• Root cause: openFanOutModal() eagerly wrote every rec into perRecPayments, and the execute path resolves payment as perRecPayments.get(rec.id) ?? b.payment (frontend/src/app.ts:485). With the map fully populated the ?? b.payment fallback was dead, so changing the bucket-level dropdown only mutated b.payment while unedited rows still posted their stale per-rec value.
• Fix:
  • Seed perRecPayments only with explicit overrides (recs whose payment differs from the bucket default); rows matching the default stay out of the map and follow b.payment.
  • Per-rec change handler now deletes the entry when the user re-selects the current bucket default, so the row resumes tracking the bucket-level control; otherwise it records the override.
  • Bucket-level change handler re-syncs visible per-rec selects that have no override to the new bucket payment, skipping rows whose service does not support that payment (mixed-SP buckets) so the displayed value never diverges from the POST payload.
• Tests: updated (i) to the override-only contract and added (j) asserting the bucket-level change propagates to the resolved payment and the visible selects for non-overridden recs. Both fail pre-fix; full recommendations.test.ts is green (322 passing). tsc --noEmit clean.

@coderabbitai full review

[coderabbitai]

@cristim The fix is well-reasoned — the sparse-map approach (override-only seeding) is exactly the right design: ?? b.payment stays live for every non-overridden row, so the bucket-level dropdown remains authoritative until a user explicitly diverges. The three-part co-ordination (seed sparse → per-rec handler prunes on revert → bucket handler re-syncs visible selects, skipping incompatible-payment rows) closes the loop cleanly without a redundant full re-render pass.

Triggering the full review now.

---

✏️ Learnings added

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

Timestamp: 2026-06-08T04:21:35.802Z
Learning: In LeanerCloud/CUDly `frontend/src/recommendations.ts` (PR `#838`, commit d7c5764), `perRecPayments` in `FanOutBucket` must be seeded **sparsely** (override-only: only recs whose resolved payment differs from the bucket-level default). The execute path in `frontend/src/app.ts` resolves each rec's payment as `perRecPayments.get(rec.id) ?? b.payment`; if the map is fully populated the fallback is dead and changing the bucket-level dropdown has no effect on unedited rows. The per-rec change handler deletes the map entry when the user reverts to the bucket default (resuming tracking via the fallback); the bucket-level change handler re-syncs visible per-rec selects that have no override, skipping rows whose service does not support the new payment (mixed-SP buckets). Tests (i) and (j) cover the override-only contract and bucket-level propagation respectively; 322 tests pass, `tsc --noEmit` clean.

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:38:19.433Z
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: 277
File: frontend/src/__tests__/recommendations.test.ts:2472-2476
Timestamp: 2026-05-05T07:46:01.902Z
Learning: In the CUDly frontend (`frontend/src/recommendations.ts`), `effectiveSavingsPct` intentionally has NO plausibility guard (no null return or warning for reconstructed percentages above per-term ceilings). The maintainer explicitly rejected that approach in PR `#277`. The fix for inflated percentages (issue `#274`) is purely a data-path fix: plumbing the provider's canonical `on_demand_cost` through to the frontend so the denominator is not reconstructed from potentially misleading `monthly_cost + savings + amortized`. Tests that pin the reconstructed >80% behavior are intentional documentation of the pre-fix path, not bugs.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T17:45:38.069Z
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-06T06:43:53.792Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), the Dashboard "Upcoming purchases" widget (`dashboard.ts`) has two Cancel entry points (card-level and modal) that both call `api.deletePlannedPurchase`. The `UpcomingPurchase` response DTO must include `CreatedByUserID *string` (omitempty, mirroring `PlannedPurchase`) so the frontend can gate these buttons via a `canCancelUpcomingPurchase` helper that mirrors `canManageScheduledPurchase` on the Plans page. Without this, the backend correctly 403s non-owners but the UI has no gate and the UX is broken. Regressions: `frontend/src/__tests__/dashboard-ownership-950.test.ts` (6 tests) and `TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID`. Fixed in commit 94326f6b9.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:28:28.139Z
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-07T17:38:19.433Z
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-07T18:28:28.139Z
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-06T06:41:06.841Z
Learning: In LeanerCloud/CUDly PR `#804`, `canRevokeCompletedRow` in `frontend/src/history.ts` must check `canAccess` (not just `getCurrentUser()` truthiness) to align with peer predicates `canCancelPendingRow`, `canApprovePendingRow`, and `canRetryFailedRow`. `revoke-own` and `revoke-any` were added to the closed `Action` union to make future UX-vs-RBAC drift a compile error.

---

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

Full review finished.

[cristim]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.