[cristim]

What

Part 2 of the confirm-dialog series (stacked on #763; base is fix/ux-confirm-dialogs, retarget to main once #763 merges).

• New commitmentSummary.ts: shared stat band (Charged today / Monthly / Total over term) and commitment lines (count, service, type, term, payment, account, region). Total over term = upfront + monthly x 12 x years; shows "Not reported" / "Not available" instead of a fake $0 when the provider has no monthly cost.
• Execute now (app.ts): replaces the misleading "full upfront amount" sentence with the real numbers; focuses Cancel.
• Run now (plans.ts): title names the step and plan, body shows what is bought and the scheduled date, a step with no recommendations says nothing will be bought; a per-purchase in-flight guard prevents a second dialog or request from a second click.
• RI exchange execute (riexchange.ts): source and target instance types (from the loaded offerings or the suggested alternative) instead of offering UUIDs, quoted payment, hourly price change source -> target with an approximate monthly figure (730 h), Cancel focused.

Not in this PR: the multi-account fan-out modal totals (#711 bulk Total upfront $0), approver-side numbers, UX-08 button label. #711 and #701 are partly addressed (confirm dialogs only).

Refs #707, #709, #701, #711.

Verification

• tsc --noEmit clean, eslint clean on touched files, full jest 100 suites / 3161 passed (new: commitmentSummary totals/labels/truncation, Run now title+amount+double click, exchange confirm instance types and hourly delta).
• Playwright against the mock API (a scratch copy of the mock adds one planned purchase; the shared mock returns none). Before = origin/main e5ac75d, after = this head. Shots in /tmp/claude/ux-fixes/shots/: execute-now-{main,pr1b}-{1280,768,390}.png, run-now-{main,pr1b}-{1280,768,390}.png.
  • Execute now before: focus on "Execute Purchase Now", body without amounts, axe aria-dialog-name + color-contrast. After: focus on Cancel, "Charged today $8,400.00 / Monthly $0.00 / Total over term $8,400.00", axe color-contrast only.
  • Run now before: "Run purchase now? This will immediately execute the purchase.", focus on "Run now". After: 'Run step 2/5 of "Compute ladder" now?' with $5,400.00 and 4 x ec2 m5.large, focus on Cancel.
  • Remaining axe color-contrast is the token issue (fix(frontend): core color tokens fail WCAG AA contrast #696).
• RI exchange confirm: jest only (needs a live quote flow, not in the mock). Label as unit-level evidence.
• Observed, not fixed here: with two recommendations from different accounts the bulk path opens the fan-out modal (not confirmDialog), which still shows upfront only.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 19 billable files and costs up to $4.75.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 52 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 86 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: b7b81bdc-1601-43c9-9470-4020fec25942

---

📥 Commits

Reviewing files that changed from the base of the PR and between 2360338 and 7d66c1a.

---

📒 Files selected for processing (19)

• frontend/src/__tests__/commitmentSummary.test.ts
• frontend/src/__tests__/confirmDialog.test.ts
• frontend/src/__tests__/navigation.test.ts
• frontend/src/__tests__/plans.test.ts
• frontend/src/__tests__/purchase-modal-submit.test.ts
• frontend/src/__tests__/riexchange.test.ts
• frontend/src/__tests__/users.test.ts
• frontend/src/app.ts
• frontend/src/commitmentSummary.ts
• frontend/src/confirmDialog.ts
• frontend/src/modules/registrations.ts
• frontend/src/navigation.ts
• frontend/src/plans.ts
• frontend/src/riexchange.ts
• frontend/src/styles/components.css
• frontend/src/styles/forms.css
• frontend/src/users/userActions.ts
• frontend/tests-e2e/history-action-confirmation.spec.ts
• frontend/tests-e2e/ri-exchange-confirmation.spec.ts

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[cristim]

Independent review (rv-ux-visual): PR #764 at 0857a56fd2e7a420e2468ef797371d1fce47a8c6

Verdict: changes requested. One medium copy contradiction on the approval path. Everything else is verified and nothing regressed.

Stack: the base is fix/ux-confirm-dialogs at d7fb436 (#763). The PR moved twice while I reviewed:

• 37813354 to 1895d71: a rebase. The patch is byte-identical (GitHub compare e56aea5...3781335 vs 2524d41...1895d71, ignoring index lines). All live evidence below was re-run at 1895d71.
• 1895d71 to 0857a56: rebased onto d7fb436 (e2e-only change in fix(frontend): confirm dialog focuses the safe action and is accessible #763). The one source change is in riexchange.ts: exchange targets now read N × <type> (<offering type>, offering <id>), and the alternative fallback also gets (offering <id>), with the matching jest assertion. I reviewed this at source. Keeping the exact offering id next to the readable type is a good call on an irreversible exchange. app.ts, plans.ts, commitmentSummary.ts and components.css are unchanged, so the live evidence applies to this SHA. Both SHAs were built from tarballs (production webpack) and served through the same mock API, using Playwright Chromium at 1280/768/390 and axe-core 4.11 (WCAG A/AA). This is mock-based evidence. The RI exchange confirm was checked at source only (no quote flow in my mock), matching the author's unit-level label.

Money math checked at source

• commitmentTotals: termTotal = Σ(upfront + monthly × 12 × term), with term in years (utils.formatTerm). Any line with monthly === null makes Monthly and Total null, which is rendered as text, not a fake $0. Correct.
• Capacity: the summary uses the same recsWithPayment objects the POST sends. The backend treats capacity_percent as audit-only and validates the recs as already scaled (internal/api/handler_purchases.go normalizeCapacityPercent, validateCapacityConsistency). So "Charged today" is what is actually submitted at any Capacity %.

Verified live (base 2524d41 vs head 1895d71)

Dialog	Base	Head
Execute now (direct)	"This will charge the full upfront amount immediately."	intro + band Charged today $8,400.00 / Monthly $0.00 / Total over term $8,400.00 + "3 × ec2 m5.large · 3 Years, All upfront · AWS Staging, eu-west-1"; focus Cancel; reflex Enter sends no POST
Run now, step with recs	"Run purchase now? This will immediately execute the purchase."	"Run step 2/5 of "Compute ladder" now?", scheduled date, $5,400.00, 4 × ec2 m5.large; focus Cancel
Run now, step without recs	same generic copy as above (implies a buy)	"This step has no recommendations attached, so running it will not buy anything."
Run now, two clicks on the row button	2 stacked dialogs; confirming one leaves the second open	1 dialog, exactly 1 POST …/pp-1/run

• The band never overflows the dialog (scrollWidth - clientWidth = 0 at all widths). At 390 it wraps to 2 + 1.
• Dialog axe: approval clean. Execute and Run now show only color-contrast on the red confirm button (pre-existing, fix(frontend): core color tokens fail WCAG AA contrast #696).
• Pages: 11 pages x 3 widths pixel-diffed against base under the same mock. All are 0.00% except Admin Purchasing (0.2 to 0.7%, ±2px width). I reproduced that same delta by serving the same base build from a second server: the ladder timestamps are fixed at mock-server start. So it is an artifact of my harness, not this PR. Page axe results and horizontal-overflow numbers are otherwise identical, and there are no console errors.

Findings

1. Medium: the approval dialog says nothing is charged, then shows "Charged today $8,400.00" (frontend/src/app.ts approval branch, withIntro(...) + commitmentSummary.ts stat('Charged today', …)).
   • Live text: "This emails an approval request to the configured approver. Nothing is charged until the approver clicks the link in that email." followed by Charged today $8,400.00.
   • On the money path, two statements that contradict each other read as either a bug or a trap.
   • Fix: let buildCommitmentSummary take the first stat's label (or a mode: 'approval' | 'now'). Use "Upfront on approval" (or "Upfront") for the approval dialog, and keep "Charged today" for Execute now, Run now and the exchange. Add a jest case asserting the approval body does not contain "Charged today".
2. Low: a partial-upfront Run now step shows two of three stats as "Not reported" / "Not available".
   • Planned purchases carry no monthly field, so this is honest. But a 3-year partial-upfront commitment has a recurring fee, and the band's empty boxes do not say so.
   • Fix: when monthly === null and the payment is not all-upfront, replace the two boxes with one line: "Plus monthly charges for 3 years (amount not available for planned purchases)". Or drop the empty stats and keep the line.
3. Nit: the exchange monthly delta has no currency (source format in buildExchangeConfirmBody: `${CurrencyCode} ${src} → ${tgt} (${sign}${abs}/hr, about ${sign}${abs*730}/month)`, so the code appears once, before the source price only). Prefix the delta with the code, or format both through formatCurrency when the code is USD.
4. Nit: money here uses 2 decimals ($8,400.00) while the Opportunities and purchase modal behind it use 0 ($8,400). Fine to keep cents in a confirm, but it is the formatter split from audit M5 / fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739.

Out of scope, seen while testing: while the confirm is open, the purchase modal's own button behind it already reads "Sending...", because it is pre-disabled for #644 and reset on Cancel (audit UX-08 / #754).

Evidence (local, review host): /tmp/claude/ux-pr/shots-764/ (confirm-{approval,direct,runnow}-{base,head,head2}-{1280,768,390}.png, flows-*.json, axe-*.json, page shots and *-diff-*.png). Scripts: /tmp/claude/ux-run/{pr764-flows.js,pr764-dblclick.js,review-shoot.js,review-diff.js}.

[cristim]

Independent review, round 2 (rv-ux-visual): PR #764 at 108fddbd0a6c3ee679edc3f9f2755413e3c56f49

Verdict: changes requested (small). The medium, the low and nit 3 from round 1 are fixed and verified live. The new "monthly fees apply" hint is wrong for Azure all-upfront, which the frontend spells upfront. That is a two-line fix.

SHAs:

• fc4c1c2 holds the fixes.
• 108fddb is e2e-only (tests-e2e/ri-exchange-confirmation.spec.ts asserts 2 × m5.xlarge (No Upfront, offering …)). frontend/src is identical to fc4c1c2.
• Live evidence was taken at fc4c1c2 against base d7fb436 (served from the 2524d41 build, which has the same frontend/src).

Round 1 findings, re-verified live

1. Fixed: the approval dialog now reads "Upfront on approval $8,400.00". It shows directly under "Nothing is charged until the approver clicks the link…", and "Charged today" no longer appears there (1280, 768, 390). The Execute now dialog still says "Charged today".
2. Fixed: partial-upfront Run now shows "Monthly: Not reported (monthly fees apply)".
3. Fixed: the exchange delta carries the currency (+0.0250 USD/hr, about +18.25 USD/month, jest). I checked this at source; the exchange flow is not in my mock.
4. Nit (2 vs 0 decimals): unchanged, fine as part of fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739.

Still correct: focus on Cancel for Execute now and Run now; a reflex Enter sends no POST; the no-recommendations step says nothing will be bought; two clicks on Run now give 1 dialog and 1 POST …/run.

No regressions: 11 pages x 3 widths pixel-diffed against base, all 33 at 0.00%. Page axe and horizontal-overflow numbers are identical, with no console errors. (The round 1 timestamp noise is gone: my mock clock is now hour-rounded.)

New finding

5. Low: Azure all-upfront gets the "monthly fees apply" hint, and its commitment line reads a raw "upfront" (frontend/src/commitmentSummary.ts paymentLabel / monthlyUnknownText).
   • paymentLabel maps only all-upfront, partial-upfront and no-upfront. Azure's all-upfront value is upfront (recommendations.ts normalizeBulkPayment: 'upfront' -> 'all-upfront', "Azure canonical synonym"; commitmentOptions.ts:36 { value: 'upfront', label: 'Pay Upfront' }). So paymentLabel('upfront') !== 'All upfront', and every Azure all-upfront line with no monthly figure claims monthly fees. Every Run now step has no monthly figure.
   • Live, with an Azure planned purchase payment: 'upfront': "Run step 1/2 of "Azure VM reservations" now? … Charged today $3,000.00 · Monthly Not reported (monthly fees apply) · … 2 × compute Standard_D4_v3 · 1 Year, upfront · eastus".
   • It errs toward caution, but it is a false statement about money on a confirm that "cannot be undone".
   • Fix: add case 'upfront': to the all-upfront branch, plus case 'monthly': return 'Monthly';, so the line does not print lowercase. Better, reuse one map: this PR adds a third payment-label table next to PAYMENT_DISPLAY_LABELS (recommendations.ts, already has upfront and monthly) and paymentLabel in settings.ts:2958. Exporting one helper from utils.ts and using it in all three would close audit H4's duplicated-map point too, but that can be a follow-up. For this PR the two case lines plus a jest case for upfront with monthly: null are enough.

Evidence (local): /tmp/claude/ux-pr/shots-764/ (confirm-*-head3-*.png, confirm-runnow-azure-head3-1280.png, flows-head3.json, axe-{base5,head3}.json, *-diff-*.png). Scripts: /tmp/claude/ux-run/{pr764-flows.js,pr764-dblclick.js,pr764-azure.js}.

[cristim]

Independent review, round 3 (rv-ux-visual): PR #764 at 7d66c1a98d8fed4f53a8cff9932100025558682c

Verdict: approved (no blocking findings). CI had not reported any checks on this SHA when I posted, so the green-CI part of the gate is still open.

Delta since round 2 (108fddb): paymentLabel maps upfront (Azure all-upfront) to "All upfront" and monthly to "Monthly", with jest cases. That closes finding 5.

Verified live at 7d66c1a (production build, same mock and scripts):

• Azure all-upfront Run now: "Monthly: Not reported" with no "monthly fees apply" hint, and the line reads "2 × compute Standard_D4_v3 · 1 Year, All upfront · eastus".
• Partial-upfront Run now still shows "Not reported (monthly fees apply)". The approval dialog still shows "Upfront on approval"; Execute now still shows "Charged today".
• All 10 other dialog cases (approval, direct and Run now at 1280/768/390, plus the no-recommendations step) have the same title, body text and initial focus as round 2.
• Pages: 11 pages x 3 widths against base d7fb436 (same frontend/src as the 2524d41 build used), all 33 at 0.00%, with no console errors.

Follow-ups, not blocking:

• Three payment-label tables now exist: commitmentSummary.paymentLabel, recommendations.ts PAYMENT_DISPLAY_LABELS and settings.ts paymentLabel. Fold them into one utils.ts helper (audit H4 / fix(ladder): timeline shows raw backend values instead of formatted display #697).
• Money decimals (2 here vs 0 elsewhere) are tracked in fix(frontend): three money formatters, mixed placeholders, left-aligned money columns #739.

[cristim]

Superseded by #781: same commits replayed onto main after #763 was squash-merged. Leaving this open until the new PR merges.

[cristim]

Superseded by #781, merged as 6248db2