iframe-proxy

cristim · 2026-05-04T07:15:47Z

Summary

Closes #225 and #226. This PR merges both issues into a single feature since they're tightly coupled (cell grouping is the visual anchor that makes savings range legible).

Cell grouping (feat(frontend/recs): group rec rows by physical-resource cell so variants are visually adjacent #226): Recommendation rows are now grouped by physical-resource cell (provider|account|service|region|resource_type|engine) — the same key as fix(frontend/recs): enforce one-variant-per-cell radio-button selection (select-all currently 6x over-commits) #224's radio enforcement. Multi-variant cells render as a collapsed summary row with a chevron; single-variant cells render flat (no overhead). Expand-all toggle added to the filter-status bar.
Savings range (feat(frontend/recs): show savings range per cell + overall page-level range #225): The per-cell summary row shows $X – $Y/mo savings range across all (term, payment) variants. Collapses to $X when all variants are uniform. When a variant is selected (radio from fix(frontend/recs): enforce one-variant-per-cell radio-button selection (select-all currently 6x over-commits) #224), the summary row shows that variant's value with (+N variants) instead of the range. A page-level range banner at the top of the table shows the aggregate across all visible cells.

Design choices (UX call — flag anything to change in CR comments)

Collapsed by default — cleaner first impression. Most users only need to see the best option per resource, not all 6 variants on first load.
Summary row layout — provider + account + service in the first 4 columns; a single wide content cell (colspan) with the chevron, resource identity, and range. This keeps the identity readable without wrapping. The identity cell has colspan=8 (the remaining columns after the 4 identity ones).
Expand-All button location — in the filter-status bar (above the table), not in the bottom sticky toolbar. Keeps it near the table context rather than near the purchase controls.
Sort operates at the cell level — cells sort by the selected variant's column value if one is selected; otherwise by the max value for numeric columns (e.g. max-savings). String columns are uniform within a cell by definition (they're the cell key).
Single-variant cells render flat — no group overhead (no summary row, no chevron, no indentation). Per feat(frontend/recs): group rec rows by physical-resource cell so variants are visually adjacent #226's acceptance criteria.
CSS approach — variant rows use padding-left: 2rem on all cells except the checkbox column. Summary rows have a slightly distinct background (#f3f6fb). No new CSS framework or component library.
css-modules.d.ts addition — fixes a pre-existing TypeScript 5.9 strict-mode error on the import './styles.css' side-effect import in index.ts. The base branch had this error but CI passed because of bundler resolution; adding the declaration makes local tsc --noEmit clean as well.

Out of scope

Effective savings % column (feat(frontend/recs): add Effective savings % column (amortized over term) #221) — separate issue, separate PR.
Sparkline (feat(recommendations): populate usage_history + surface per-row sparkline in rec list #239) — separate issue.
Column-filter primitives (refactor(frontend): extract column-filter primitives into a shared module — adopt on Plans / History / RI Exchange #166) — separate issue.
Savings-vs-selected delta ("You've picked options that save $X; max possible is $Y") — listed as a bonus in feat(frontend/recs): show savings range per cell + overall page-level range #225. Deferred — the per-cell range data is now available to implement this in a follow-up.

Test plan

All 1452 existing tests still pass (npm test)
TypeScript --noEmit clean
npm run build compiles without errors
16 new tests added:
- groupRecsByCell pure helper: 2 tests
- cellSummary pure helper: 3 tests
- pageLevelRange pure helper: 2 tests
- DOM integration: collapsed-by-default, chevron expand, page banner, single-variant flat, expand-all button, expand-all functional: 6 tests
- Existing fix(frontend/recs): enforce one-variant-per-cell radio-button selection (select-all currently 6x over-commits) #224 radio enforcement test updated to expand the cell first (collapsed-by-default means checkboxes aren't in the DOM until expanded)

🤖 Generated with claude-flow

Summary by CodeRabbit

Release Notes

New Features
- Recommendations table now groups variants by physical cell with collapsible summary rows
- Added "Expand all / Collapse all" toggle to manage multi-variant cell visibility
- Displays "Recommended range" banner for pages with multiple variant options

coderabbitai · 2026-05-04T07:15:53Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1ef16732-12e2-44d8-89ae-3b0e06c18cbd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR refactors the recommendations table from flat per-row rendering to a cell-grouped model. Multi-variant cells now render a summary row by default and expand on demand, with helpers computing per-cell and page-level savings ranges. An "Expand all" toggle was added to the filter bar, and new TypeScript/CSS definitions support the grouped UI.

Changes

Cell Grouping & Expansion UX

Layer / File(s)	Summary
State Management `frontend/src/recommendations.ts` (lines 29–36, 52–60)	Module-level state `expandedCells` tracks which multi-variant cells are expanded, and `lastVisibleGroupKeys` records the most recent visible group ordering for "Expand all" behavior. `resetExpandedCells()` clears both for test isolation.
Core Grouping & Summaries `frontend/src/recommendations.ts` (lines 221–367)	New exported helpers: `groupRecsByCell()` groups recommendations by cell key; `cellSummary()` and `pageLevelRange()` compute per-cell and page-level min/max envelopes for savings/upfront/term. Non-exported `groupsInSortOrder()` sorts groups by cell identity. Old `sortedRecommendations()` flat-sort helper removed (line 487–490).
Table Rendering Refactor `frontend/src/recommendations.ts` (lines 1465–1581)	`buildListMarkup()` now groups recommendations via `groupRecsByCell()`, sorts groups via `groupsInSortOrder()`, renders single-variant rows flat, and multi-variant cells as a collapsed summary row with an optional expandable chevron. `buildVariantRowMarkup()` renders individual variant rows when expanded. `lastVisibleGroupKeys` is populated here and used by the expand-all button.
UI Controls & Integration `frontend/src/recommendations.ts` (lines 1162–1205, 2301–2328, 2429–2443)	Filter status bar now includes an "Expand all / Collapse all" toggle that expands/collapses visible multi-variant cells. Render ordering ensures `buildListMarkup()` runs before `renderFilterStatusBar()` so `lastVisibleGroupKeys` is populated. Chevron click handler toggles cell expansion and re-renders the table.
Styling `frontend/src/styles/components.css` (lines 869–946)	New CSS rules for the grouped table UX: expand-all toggle styling, page-level range banner, collapsed cell summary rows with hover states, chevron/identity/range styling, variant row nesting with padding/background.
TypeScript Module Declaration `frontend/src/css-modules.d.ts` (lines 1–7)	Added ambient module declaration for `*.css` to allow side-effect CSS imports under TypeScript 5.9+.
Tests & Validation `frontend/src/__tests__/recommendations.test.ts` (lines 4–4, 2117–2122, 2570–2766)	Test imports expanded to include grouping/range helpers. Issue `#224` test now explicitly expands multi-variant cells via chevron before selecting variants. New Issues `#225/`#226 test block validates pure helpers (`groupRecsByCell`, `cellSummary`, `pageLevelRange`) and integrated DOM behavior: summary-row rendering, per-cell expansion, range banner presence, and global expand-all toggle.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

Issue #225: Directly addressed by implementation of cellSummary(), pageLevelRange(), and groupRecsByCell() for per-cell and page-level savings range display.
Issue #226: Directly addressed by cell-grouping refactor and expand/collapse UI controls added to recommendations table rendering.

Possibly related PRs

PR #243: Overlapping changes to frontend/src/recommendations.ts and exports of pickBestVariantPerCell and seedGlobalDefaults functions.
PR #231: Related through shared modifications to per-cell selection logic and pickBestVariantPerCell function in frontend/src/recommendations.ts.

Suggested labels

effort/xl

🐰 Cells collapse with quiet grace,
Expanding on demand—what a place!
Variants grouped, ranges shown,
This table's grown and fully grown!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and clearly describes the main changes: grouping rows by cell with savings range display and collapse/expand functionality, and directly references the closed issues.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
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 feat/issues-225-226-cell-grouping

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

cristim · 2026-05-04T07:16:33Z

@coderabbitai review

coderabbitai · 2026-05-04T07:16:38Z

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

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/recommendations.ts`:
- Around line 1510-1522: The default "visible" purchase target must be computed
at cell (group) granularity rather than per-recommendation element: update the
logic that builds the default visible selection (currently using
recommendations/selectedRecs and lastVisibleGroupKeys) to treat each group key
in sortedKeys as one visible target when groups.get(key)?.length > 1 (i.e. use
groupRecsByCell(groups) and lastVisibleGroupKeys as the single selectable unit),
so the action text and default submit operate on one summary row per cell
instead of on every sibling variant; adjust any code that derives the count or
list of visible targets (where selectedRecs or the expand-all behavior is
computed) to use the group key list (lastVisibleGroupKeys or sortedKeys filtered
for multi-variant groups) rather than the raw recommendations array.
- Around line 1467-1493: The code injects rec.provider raw into class names and
text in buildVariantRowMarkup (and the similar block at 1559-1563), so
sanitize/whitelist it before interpolation: validate rec.provider against an
allowed provider list or normalize it to a safe token (e.g., map to lowercase
alphanumerics/hyphens, fallback 'unknown-provider') and use that sanitized token
for the CSS class while still escaping the displayed text with
escapeHtml(rec.provider) or a mapped display name; replace direct uses of
rec.provider and rec.provider.toUpperCase() in the template with the sanitized
class token and escaped display string respectively, and apply the same change
to the other analogous function/markup block.

🪄 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: c4724c87-8107-4905-b6d3-4f0113a5fd9a

📥 Commits

Reviewing files that changed from the base of the PR and between bc25a4d and 9a693d8.

📒 Files selected for processing (4)

frontend/src/__tests__/recommendations.test.ts
frontend/src/css-modules.d.ts
frontend/src/recommendations.ts
frontend/src/styles/components.css

…ell purchase target - Add providerBadgeClass() helper that whitelists provider to aws|azure|gcp before injecting into CSS class attributes; use escapeHtml(providerDisplayName()) for the badge display text. Applied in both the summary row and variant row markup paths. - resolvePurchaseTarget() now uses pickBestVariantPerCell(visible) for the default (no-selection) path so the purchase target is exactly one rec per physical resource, matching the collapsed grouped table view. Submitting all visible variants of a multi-variant cell would create conflicting reservations. - updateBottomActionBox() derives visibleCellCount from pickBestVariantPerCell for the default path and shows "N cells visible" rather than "N visible" to match the cell-grouped table count. - Update affected tests: #111 (a)/(b)/(c)/(d) fixtures changed to use distinct resource_type values per rec so each rec occupies its own cell and all appear in the default purchase target (triggering fan-out). Bundle B summary text assertions updated to match "cells visible" format.

cristim · 2026-05-04T07:35:13Z

CodeRabbit review — round 2 response

Addressed both actionable findings from the round 1 review:

Finding 1 — provider injection into CSS class (security)

Added providerBadgeClass() helper that whitelists rec.provider against aws|azure|gcp before injecting into class attributes. Non-whitelisted values fall back to '' (no badge class).
Badge display text is now escapeHtml(providerDisplayName(rec.provider)) in both the summary row and variant row markup paths.

Finding 2 — cell purchase target regression

resolvePurchaseTarget() now uses pickBestVariantPerCell(visible) for the default (no-selection) path. This ensures the purchase target is exactly one rec per physical resource — matching what the user sees in the collapsed grouped table. Submitting all visible variants of a multi-variant cell would create conflicting reservations for the same resource, violating the one-variant-per-cell contract from fix(frontend/recs): enforce one-variant-per-cell radio-button selection (select-all currently 6x over-commits) #224.
updateBottomActionBox() derives visibleCellCount from pickBestVariantPerCell for the default path and shows "N cells visible" to match the cell-grouped table count.

Test updates

verify recommendations engine actually consumes per-account service overrides #111 (a)/(b)/(c)/(d) fixtures: changed to use distinct resource_type values per rec so each rec occupies its own cell. With pickBestVariantPerCell for the default target, same-cell recs (same account + same resource type) now collapse to a single target — tests that expected fan-out needed different cells.
Bundle B summary text assertions updated to match the new "cells visible" format.

All 1452 tests pass, TypeScript clean, build clean.

@coderabbitai review

coderabbitai · 2026-05-04T07:35:28Z

Tip

For best results, initiate chat on the files or code changes.

@cristim Thanks for the detailed round 2 summary. Both fixes look well-reasoned:

The providerBadgeClass() whitelist approach is the right pattern for safe CSS-class injection — falling back to '' for unknown providers avoids any class-name pollution or injection risk.
Using pickBestVariantPerCell(visible) for the default purchase-target path correctly enforces the one-variant-per-cell contract established in #224, and the test fixture updates (distinct resource_type per rec) ensure those tests remain meaningful.

Triggering a fresh review now.

[review]

…/expand (closes #225, closes #226) - Group recommendation rows by physical-resource cell (provider|account|service| region|resource_type|engine) using the same cellKey as #224's radio enforcement. - Multi-variant cells render as a collapsed summary row by default (chevron to expand); single-variant cells render flat with no group overhead. - Per-cell summary row shows savings range ($X – $Y/mo), upfront range, and term range; collapses to a single value when all variants are uniform. - When a variant is selected (radio from #224), the summary row shows that variant's values with a "+N variants" affordance instead of the range. - Page-level range banner at the top of the table: "Recommended range: $X – $Y/mo across N cells" (sum of per-cell min/max across visible cells). - Expand-All/Collapse-All toggle added to the filter-status bar; only shown when at least one multi-variant cell is present. - Sort operates at cell level: groups ordered by selected-variant value if one is selected in the cell, else by max value across variants. String columns use the shared cell-identity value. Within a cell, variants sort by (term ASC, payment fixed-order: no-upfront < partial < all-upfront < monthly). - Filter operates at cell level: any variant matching a filter keeps the entire cell visible. - Adds css-modules.d.ts to fix a pre-existing TS 5.9 strict-mode error on the `import './styles.css'` side-effect import in index.ts. - Removes now-unused sortedRecommendations flat-list sort helper (superseded by groupsInSortOrder which reuses the same SORTABLE_* column maps). - Exports resetExpandedCells() for test isolation of module-level expand state. - Adds 16 new tests (6 pure-helper, 6 DOM integration, 4 expand/collapse) and updates the existing #224 radio-enforcement test to expand the cell first.

…ell purchase target - Add providerBadgeClass() helper that whitelists provider to aws|azure|gcp before injecting into CSS class attributes; use escapeHtml(providerDisplayName()) for the badge display text. Applied in both the summary row and variant row markup paths. - resolvePurchaseTarget() now uses pickBestVariantPerCell(visible) for the default (no-selection) path so the purchase target is exactly one rec per physical resource, matching the collapsed grouped table view. Submitting all visible variants of a multi-variant cell would create conflicting reservations. - updateBottomActionBox() derives visibleCellCount from pickBestVariantPerCell for the default path and shows "N cells visible" rather than "N visible" to match the cell-grouped table count. - Update affected tests: #111 (a)/(b)/(c)/(d) fixtures changed to use distinct resource_type values per rec so each rec occupies its own cell and all appear in the default purchase target (triggering fan-out). Bundle B summary text assertions updated to match "cells visible" format.

cristim · 2026-05-04T09:39:37Z

Rebased onto current feat/multicloud-web-frontend (which now includes PR #254's monthly_cost fix — null-aware rendering, NaN for numeric filters, POSITIVE_INFINITY for sort). Resolved conflicts in frontend/src/recommendations.ts: kept #253's cell-grouping structure (rows.join(''), buildVariantRowMarkup, expand/collapse, groupRecsByCell, cellSummary, pageLevelRange, groupsInSortOrder) + adopted #254's corrected null-aware monthly_cost data path (monthly_cost != null ? formatCurrency(...) : '—' in buildVariantRowMarkup; ?? Number.POSITIVE_INFINITY in sort; ?? Number.NaN in filter). TypeScript clean, 140 tests pass, webpack build clean. @coderabbitai review

coderabbitai · 2026-05-04T09:39:50Z

cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-quarter Within the quarter impact/many Affects most users effort/m Days type/feat New capability triaged Item has been triaged labels May 4, 2026

coderabbitai Bot reviewed May 4, 2026

View reviewed changes

Comment thread frontend/src/recommendations.ts

Comment thread frontend/src/recommendations.ts

cristim added 2 commits May 4, 2026 11:38

cristim force-pushed the feat/issues-225-226-cell-grouping branch from e22afd6 to 5201c69 Compare May 4, 2026 09:39

cristim merged commit bd331c4 into feat/multicloud-web-frontend May 4, 2026
6 checks passed

cristim mentioned this pull request May 13, 2026

feat(frontend/recs): show savings range per cell + overall page-level range #225

Closed

coderabbitai Bot mentioned this pull request May 19, 2026

fix(ux/opportunities): deterministic sort on Term/Payment/Cost/Effective % cells #496

Merged

4 of 5 tasks

This was referenced May 28, 2026

feat(frontend/recs): group rec rows by physical-resource cell so variants are visually adjacent #226

Closed

feat(recommendations): populate usage_history + surface per-row sparkline in rec list #239

Closed

This was referenced May 28, 2026

ux(frontend/recs): enlarge cell-expand chevron + reposition (closes #280) #798

Merged

feat(recommendations): group per-plan-type SP rows in table (closes #135) #807

Merged

cristim deleted the feat/issues-225-226-cell-grouping branch June 3, 2026 21:54

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

cristim commented May 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design choices (UX call — flag anything to change in CR comments)

Out of scope

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Uh oh!

cristim commented May 4, 2026

Uh oh!

coderabbitai Bot commented May 4, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cristim commented May 4, 2026

CodeRabbit review — round 2 response

Uh oh!

coderabbitai Bot commented May 4, 2026

Uh oh!

cristim commented May 4, 2026

Uh oh!

coderabbitai Bot commented May 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented May 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 4, 2026 •

edited

Loading