[IAmJSD]

Home and Opportunities compute Potential Monthly Savings the same way but over different rows: Opportunities hides recommendations for providers switched off in Settings > Enabled Providers (#463) and applies the user's saved column filters, while Home summed every provider and took its "N recommendations" count from the backend total. This PR makes Home honor Enabled Providers through a shared helper, so both cards show the same figure, and labels the Opportunities card when column filters narrow it.

Refs #180
Partial: the exact disagreement in the review (different ranges for the same 47 recommendations) could not be reproduced on current code; this fixes the two scope differences that exist today.

What changed

• New frontend/src/lib/enabled-providers.ts: filterByEnabledProviders, permissive when the list is empty or absent. Used by both pages.
• frontend/src/dashboard.ts: loadDashboard fetches /api/config alongside the other calls (a failure falls back to no filter, as on Opportunities) and filters recommendations before the KPI range and the per-service chart. The savings tile's "N recommendations" now counts the rows it summed, and only falls back to the backend total if the recommendations fetch failed.
• frontend/src/recommendations.ts: loadRecommendations uses the shared helper. renderRecommendationsSummary takes the loaded count and adds a scope line under the savings value whenever a column filter is active (not a selection). When the loaded list is complete it reads "Filtered: X of Y recommendations". A single-value service or region filter is sent to the server (fix(frontend): RI-exchange account chip sends CloudAccount UUID where AWS and Azure handlers expect the provider account ID #162), so the loaded list is already narrowed and the unfiltered total isn't known; then the line names that filter instead of a total, e.g. "Filtered: 6 recommendations (Service: ec2)", plus "+N more filters" for any other active column filters. loadRecommendations records which filters the fetch pushed.
• Tests: 4 new cases in src/__tests__/dashboard.test.ts; in src/__tests__/recommendations.test.ts, scope-line assertions in the existing fix(rekey): any zero-key decrypt failure counts as already re-keyed, so the run succeeds #272 filter test plus a new case where the service filter is sent to the server and the response holds only matching rows; and a new Playwright spec tests-e2e/savings-kpi-parity.spec.ts. The e2e recommendations mock (tests-e2e/fixtures/recs.ts) now applies service and region the way the backend does.

Why this approach

Making Home honor the same Enabled Providers setting fixes the disagreement at its source, and one shared helper keeps the two screens from drifting again. Column filters are a deliberate user choice on Opportunities, so rather than forcing Home to read them (or Opportunities to ignore them), the narrower scope is labeled on the card, which is the issue's second acceptable outcome. Moving the computation server-side would need a backend change (the summary endpoint still sums every variant); this stays frontend-only.

Screenshots

Before is main at a14dc8f2, after is this branch, both taken before current main was merged in. They are cropped to the KPI cards, which main's later changes (including #1335's per-service chart) don't touch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API, with a fixture of 19 recommendations across AWS, Azure and GCP and Enabled Providers set to AWS only.

Before	After

1. Home. Before: $3,320 and 19 recommendations, summing Azure and GCP even though only AWS is enabled. After: $2,260 and 11 recommendations, the same scope as Opportunities.
2. Opportunities with a leftover Service = ec2 column filter. Before: "Potential Monthly Savings $1,760" with no hint that it is narrower than Home. After: the card adds "Filtered: 6 of 11 recommendations". This is the first load, before the persisted filter is applied to the fetch. Later loads (returning to Opportunities, switching provider or account) send service=ec2 to the server, and the line then reads "Filtered: 6 recommendations (Service: ec2)".
3. Opportunities for the same data without filters (unchanged by this branch): 11 recommendations and $2,260, matching Home after the fix.

Verification

Run on this branch's head 0be9d788, which has main 51aa5b13 merged in:

• npm run lint: 0 errors (existing warnings only). Typecheck and build pass.
• Jest: 135 suites, 3920 passed, 1 skipped. The new server-side-filter case in recommendations.test.ts fails without the scope-line change (no label is rendered) and passes with it. New dashboard.test.ts cases: disabled providers dropped before the range and count; empty list means no filter; config fetch failure means no filter and the dashboard still renders; recommendations fetch failure falls back to the backend count.
• New tests pass with --randomize. A whole-file randomized run of recommendations.test.ts has one order-dependent failure in the savings-trend "daily interval" test, which also fails on an unmodified copy of that code.
• Playwright (e2e-local.sh): 211 passed, including savings-kpi-parity.spec.ts (Home and Opportunities show the same dollar figure and count with Azure and GCP disabled; a column-filtered Opportunities card shows the scope line; after leaving and returning to Opportunities, the reload sends service=ec2 and the card still shows "Filtered: 6 recommendations (Service: ec2)"). That last test fails without the scope-line change.
• Secrets scan clean (planted key flagged). scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Notes for review

• Home now makes one extra GET /api/config per load (Opportunities already does the same).
• Copy: "Filtered: 6 of 11 recommendations", or "Filtered: 6 recommendations (Service: ec2)" when the filter was applied server-side, under the Opportunities savings value. The wording is a suggestion. The server-side case doesn't show an "of Y" total because the API returns only the filtered rows and their count; getting the unfiltered total would need another request.
• Pre-existing (fix(frontend): RI-exchange account chip sends CloudAccount UUID where AWS and Azure handlers expect the provider account ID #162), not changed here: "Clear filters" re-renders without re-fetching, so after a server-side service/region filter the list stays narrowed until the next load. The scope line follows the rows that were actually fetched, so in that state it still says "Filtered ... (Service: ec2)".
• The Home per-service chart now also excludes disabled providers' recommendations, but its "Current / Committed" layer still comes from the backend summary's by_service, which does not know about Enabled Providers. Fixing that needs a backend change if it matters.
• Opportunities still scales by the cost-period selector. When it is not monthly the card title already changes (for example "Potential Savings /yr"), so that difference is labeled.
• The exact $210-326 vs $227-358 numbers from the August review could not be reproduced. Enabled Providers and leftover column filters are the two scope differences present in current code, hence Refs rather than Closes.

Overlaps

• fix(frontend): "Potential savings range per service" legend and bar-color encoding mismatch #189 has landed on main (fix(frontend): color the per-service savings chart by layer #1335) and is merged into this branch. The only conflict in that merge was a chart-utils import line, from fix(dashboard): include the year in the savings-trend tooltip title #1323.
• fix(frontend): dashboard charts have no text alternative and layers differ only by shade #744 and fix(frontend): charts rebuilt on every change; dashboard trend has no stale guard #747 are still open and also edit frontend/src/dashboard.ts (fix(frontend): dashboard charts have no text alternative and layers differ only by shade #744 the per-service chart, fix(frontend): charts rebuilt on every change; dashboard trend has no stale guard #747 chart rebuilds and the trend loader). This branch only touches loadDashboard and renderDashboardSummary, so conflicts should be small and textual. The dynamic aria-label from fix(frontend): dashboard charts have no text alternative and layers differ only by shade #744 reads the same filtered recommendations.

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]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• 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): REQUEST_CHANGES 0dc969487f7e15aba27c80d9e70948635d20c363

Blocking:

• frontend/src/recommendations.ts:920 (+ call site :4893): The "Filtered: X of Y" label only shows when variantCount < loadedCount. But loadedCount = loadedRecs.length, and loadedRecs has already been narrowed on the server. loadRecommendations (:589-605) sends single-value service/region column filters as ?service=/?region= (fix(frontend): RI-exchange account chip sends CloudAccount UUID where AWS and Azure handlers expect the provider account ID #162 pushdown, already on the branch base a14dc8f), and the backend applies them in SQL. Failure: the user has a leftover Service=ec2 filter and returns to Opportunities, switches provider or account, or picks one value in the popover (commitAndRefresh -> loadRecommendations). The fetch returns only ec2 rows, so variantCount == loadedCount and no label is shown. The card then shows a narrower sum than Home with no explanation, which is the fix(frontend): Potential Monthly Savings differs between Home and Opportunities for the same recommendations #180 symptom again. Today it only works on the first load, because persisted filters are seeded after the fetch. The e2e mock (tests-e2e/fixtures/recs.ts) ignores service, and the jest test filters on the client side, so neither catches this. Fix: base "filtered" on whether any column filter is active, not on the count comparison. Add a test with a server-pushed service filter.
  Non-blocking:
• frontend/src/dashboard.ts:176: if the recs fetch is fulfilled but returns null or an empty body, recsCount is 0, so the tile reads "--" / "0 recommendations" while the backend total may be higher. Decide the fallback from the data, not from the fulfilled status.
• frontend/src/dashboard.ts:277: nothingTracked still uses the backend total_recommendations, which ignores Enabled Providers. If every rec belongs to a disabled provider, the tile says "0 recommendations" while coverage shows a % instead of "No services tracked".
• In the per-service chart, the Current/Committed layer (main fix(frontend): color the per-service savings chart by layer #1335) still comes from unfiltered by_service. The body already notes this.
  Attacked: Compared origin/main...HEAD line by line. The merge was checked against both parents. The only conflict was the chart-utils import, and main's formatTrendTooltipTitle (fix(dashboard): include the year in the savings-trend tooltip title #1323) was kept. fix(frontend): color the per-service savings chart by layer #1335's layer chart, skeleton and error paths are intact. I traced the Opportunities load path (filter seeding order, server pushdown, commitAndRefresh) through to the Go store (store_postgres_recommendations.go) to confirm the missing label. Only numbers go into innerHTML, so there is no XSS. Main has no duplicate enabled-providers helper. Jest for dashboard and recommendations passed at HEAD in a scratch worktree (545 passed, 1 skipped).

[IAmJSD]

On the blocking re-review finding above: fixed in 0be9d788, with main merged at 88b58f99.

• How "filtered" is decided: the card now decides "filtered" from the active column filters and from the service/region filters the last fetch sent to the server. It no longer compares the visible and loaded counts.
• Server-side filter: a Service=ec2 filter that was pushed to the server now shows "Filtered: 6 recommendations (Service: ec2)", plus "+N more filter(s)" for any other active column filters. There's no "of Y" here, because the response only carries the filtered rows (TotalCount is their count) and getting the full total would take another request. The complete-list case still reads "Filtered: X of Y recommendations". The label text is escaped.
• Tests:
  • New jest case: the server returns only ec2 rows for service: 'ec2', and the label must show.
  • The e2e mock now applies service and region the way the backend does.
  • New Playwright test: Opportunities to Home and back, with the reload sending service=ec2.
  • Both new tests fail on the old code.
• Checks at 0be9d788: jest 3920 passed, Playwright 211 passed, and lint, typecheck, build and git-secrets all pass.
• Pre-existing behavior, not changed here: "Clear filters" re-renders without a refetch (fix(frontend): RI-exchange account chip sends CloudAccount UUID where AWS and Azure handlers expect the provider account ID #162), so after a server-side filter the list stays narrowed until the next load. The label stays accurate in that state.

This branch will merge main again before landing, and that head gets a fresh review.

[IAmJSD]

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

Blocking:

• none
  Non-blocking:
• frontend/src/dashboard.ts:194: recsCount is chosen from the fetch status, not from the data. If the recs fetch succeeds but returns null or an empty body, the tile reads "— / 0 recommendations" while the backend total may be higher. This was raised in the last review and is still open.
• frontend/src/dashboard.ts:328: nothingTracked still reads the backend total_recommendations, which ignores Enabled Providers. If every rec belongs to a disabled provider, the tile shows "0 recommendations" while Coverage shows a % instead of "No services tracked". Also, renderHomeOnboarding now shows the first-run card when every rec is provider-disabled. That matches the Opportunities first-run check, but the card's wording may mislead.
• frontend/src/recommendations.ts savingsScopeText: always uses the plural, so one row reads "Filtered: 1 recommendations (Service: ec2)". Home has the same "1 recommendations" pattern.
• recommendations.ts renderRecommendationsSummary sets data-testid after renderKpiTiles by querying the DOM. This works, but it is a test-only hook bolted onto chore(frontend): unify four modal systems and five card styles, reuse kpi-tile #743's shared builder. A future KpiTileSpec field such as detailTestId would be cleaner.
  Attacked: Read origin/main...HEAD in full. The previous blocking issue (a server-pushed service/region filter hid the "Filtered" label) is really fixed: savingsScopeText keys off loadedServerFilters plus the active column filters, and the e2e mock now applies ?service/?region. For merge ac03cbb I diffed against both parents. In recommendations.ts, main's chore(frontend): unify four modal systems and five card styles, reuse kpi-tile #743 renderKpiTiles is kept intact, and the scope line moves from the escaped innerHTML

  to the tile detail. buildKpiTile sets it with textContent, so there is still no XSS. In dashboard.ts only the imports conflicted. For the earlier merge 2c19d18: main's NO_VALUE, the five-way allSettled with listAccountsMinimal, and renderHomeOnboarding all survived. 6982f14 only switches the expected value '--' to main's em dash. Main has no duplicate enabled-providers helper. In a scratch worktree, jest dashboard + recommendations at HEAD: 567 passed, 1 skipped.

[IAmJSD]

Local verification at ac03cbbd merged onto current main (10f2c8af; merge-tree fd3582b4; 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 ee0a6278, main plus 3 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4400 passed, 4401 total), and the full Playwright suite (264 passed).
• CI is green on ac03cbbd (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 ac03cbbd.