MPDX-9568 PDS Goal Calculator: section descriptions, field labels, and form fixes

[wjames111]

Description

Bundles UI/text refinements across the PDS Goal Calculator from six related Jira tickets, plus a small Pay Type bug fix.

• MPDX-9569 — Show the geographic location next to the multiplier amount in the Salary breakdown (e.g. 4.0% (Atlanta)), update the Gross Monthly Pay formula to include the location, and append each location's percentage to the options in the Location dropdown.
• MPDX-9568 — Replace the "Total Reimbursable Expenses" title info tooltip with a description below the heading explaining the $300/month minimum. On the Monthly Reimbursable grid, show the per-month maximums inline with the Ministry Cell Phone and Ministry Internet field names and add a description above the grid clarifying that amounts above the max will not be saved.
• MPDX-9567 — Remove the title-level info tooltips from the Annual / Monthly / Total Reimbursable sections (replaced with descriptive text) and add row-level info icons on Ministry Cell Phone / Ministry Internet explaining why those fields are prepopulated.
• MPDX-9564 — Split the Form Type helper text so the Default description and the Simple description render on separate lines.
• MPDX-9563 — Round the Average Hours Worked Per Week to two decimal places in the grid footer and when applying the value to Hours Worked.
• MPDX-9474 — Add descriptive headings at the top of the Reimbursable Expenses, Support Items (with Salary / Other subsections), and Summary Report steps.

Additional minor fix:

• Switching Pay Type between Salaried and Hourly now clears the Pay Rate, preventing stale hourly/salary values from carrying over to the new pay type.

Related Jira tickets: MPDX-9569, MPDX-9568, MPDX-9567, MPDX-9564, MPDX-9563, MPDX-9474.

Testing

Navigate to the PDS Goal Calculator (HR Tools → PDS Goal Calculator) and walk through each step:

Setup step

• Form Type helper text shows the Default sentence and Simple sentence on two separate lines.
• Open the Location dropdown — each option is suffixed with its percentage, e.g. Orlando, FL (6%).
• Switch Pay Type from Hourly → Salaried (and back) and confirm the Pay Rate field clears each time.
• In the Hours Per Week grid, enter values that don't divide evenly (e.g. Regular Week 40 hrs × 48 wks plus Travel 7 hrs × 4 wks) and confirm the footer average and the value applied to Hours Worked both show two decimal places (37.46).

Reimbursable Expenses step

• Confirm the step now has a heading and description at the top.
• Monthly section: shows a description below the heading; Ministry Cell Phone and Ministry Internet rows show (max $35/mo) / (max $30/mo) inline with the field name and an info icon whose tooltip reads "Pre-filled with the maximum allowed amount. Edit to a lower value if needed."
• Annual section: shows a description below the heading and no title info icon.
• Total Reimbursable Expenses: shows the $300 minimum description in place of the previous info tooltip; the floor still applies (totals below $300 snap to $300).

Support Items step

• Step has a heading and description, and both the Salary and Other subsections have their own descriptions.
• With a geographic location set (e.g. Atlanta), the Geographic Multiplier row shows (Atlanta) next to the percentage and the Gross Monthly Pay formula reads Monthly Base × (1 + Geographic Multiplier (Atlanta)).
• With no location set, the suffix and the location in the formula are both omitted.

Summary step

• Shows a heading and description above the summary table.

Checklist:

• I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
• I have applied the appropriate labels (Add the label "Preview" to automatically create a preview environment)
• I have run the Claude Code /pr-review command locally and fixed any relevant suggestions
• I have requested a review from another person on the project
• I have tested my changes in preview or in staging
• I have cleaned up my commit history

[github-actions]

Bundle sizes [mpdx-react]

Compared against 169bcfb

No significant changes found

[wjames111]

Multi-Agent Code Review — /quality:agent-review (standard mode)

Verdict: BLOCKERS_FOUND — 1 high-priority blocker, plus 7 medium-priority items and ~13 informational suggestions.

Agents: Architecture, Testing, Standards, UX, Financial Reporting (+ dependency-impact). Security and Data Integrity skipped (no auth/API/Apollo-cache paths touched).

Risk: 10/10 by formula (driven by file count — 10 production components touched), but the actual changes are narrow per-file (UI text + a few calc tweaks). Blast radius is fully contained in src/components/HrTools/PdsGoalCalculator/; no external API or index.ts re-exports affected. The titleTooltip → description interface rename has zero orphaned callers.

---

Highlights

🔴 High Priority Blocker (sev 8.0)

HoursPerWeekGrid: displayed average can differ from applied average. HoursPerWeekGrid.tsx:493 uses .toFixed(2), but line 512 applies Math.round(x*100)/100. These diverge on .xx5 floating-point boundaries — verified with concrete inputs (totalHours=9.1 → user sees 0.17, but 0.18 is saved). The drifted value propagates through saveField → calculateSalaryTotals → goal totals. Also a documented Standards violation per .CLAUDE/rules/code-review.md (.toFixed in calculation paths). See inline comment for the fix. Cross-agent consensus (Financial, Architecture, Testing, UX, Standards).

🟡 Medium Priority (sev 5.0-6.9)

1. Missing test for multi-line Form Type helperText — SetupStep.tsx
2. Untested getLocationLabel 0%/undefined fallback branch — SetupStep.tsx
3. Pay Type TextField departs from AutosaveTextField pattern undocumented — SetupStep.tsx
4. Over-max reimbursable value retained in cell with red border despite "will not be saved" description — MonthlyReimbursableSection.tsx + ReimbursableExpensesGrid.tsx
5. Non-descriptive translation key t('({{location}})') — salaryBreakdown.tsx
6. Heading hierarchy: step + subsection both use variant="h6" (a11y) — multiple step components

All details + before/after code blocks are in the inline review comments below.

🟢 Suggestions (sev < 5)

• Pay Type switch silently clears payRate — consider visible inline feedback.
• Move SummaryReportStep heading above the loading guard.
• Extract a shared roundToCents helper to src/lib/intlFormat.ts (addresses root cause of the blocker — 3 sites already use this pattern).
• MonthlyReimbursableSection.fields rebuilt every render; getLocationLabel not memoized.
• 5 new heading+description blocks lack render tests (low risk — presentational JSX behind translation keys).
• InfoIcon aria-label may double-announce with the visible (max $35/mo) label.
• Various small testing edge cases (undefined max, negative multiplier, empty-string location).

Pre-existing (informational)

• HoursPerWeekGrid.test.tsx uses several await waitFor(() => getBy*) patterns where the project prefers findBy*. Not introduced by this PR.

Unresolved Debate

UX flagged Pay Type TextField's disabled-state visual treatment may differ from surrounding AutosaveTextField peers (sev 6.0). Architecture countered factually that AutosaveTextField has no saving adornment / spinner, so the visuals match (sev 3.5). The two arguments address different aspects. Recommendation: a visual check with isMutating=true will resolve it conclusively.

---

Generated by /quality:agent-review — multi-agent review with cross-examination debate. 5 agents + dependency impact. Mode: standard, model: opus.

[wjames111]

Multi-Agent Code Review — PR #1780

Mode: Standard · Agents: Architecture, Testing, Standards, UX, Financial Reporting (+ Coverage-Gap Review) · Risk: 10/10 (CRITICAL, breadth-driven across 13 non-test files in a single feature)

Verdict: APPROVED_WITH_SUGGESTIONS

No CRITICAL (≥9.0) or HIGH-priority (≥8.0) blockers were found. The PR is well-scoped, the WYSIWYS rounding in HoursPerWeekGrid is correctly implemented (display = applied = persisted to hoursWorkedPerWeek), and i18n discipline is generally strong (static keys, no template-literal interpolation inside t()).

Findings summary (post-debate consensus):

Tier	Count	Notes
Critical (≥9.0)	0	—
High (8.0–8.9)	0	—
Important (7.0–7.9)	1	Cannot be /dismiss'd — strongly recommended
Medium (5.0–6.9)	5	Block CLEAN verdict but not merge
Suggestions (<5.0)	~20	Informational only

⚠️ 1 Important finding (severity 7.0–7.9) — strongly recommended but doesn't block merge and can't be /dismiss'd. Consider addressing in this PR or creating a follow-up ticket.

Risk Assessment

Risk score is capped at 10/10 but the nature of the changes is low-blast-radius: 12 medium-risk component files in a single feature (PdsGoalCalculator) plus 1 shared lib (intlFormat.ts) with a backward-compatible signature extension. No auth, no API, no schema, no migrations.

Dependency Impact

• src/lib/intlFormat.ts — 144 importers across the app. numberFormat signature is extended with an optional options param (backward compatible). 9 files import numberFormat directly; signature change is non-breaking.
• All other changes are leaf or internally-encapsulated within src/components/HrTools/PdsGoalCalculator/**.

Cross-Examination Highlights

The debate round produced significant re-calibration:

• Silent over-max clipping (ReimbursableExpensesGrid.tsx): Financial (3.0) vs UX (6.0) vs Architecture (4.0 post-debate). UX argues description text ≠ in-context feedback per WCAG 3.3.1; Architecture/Financial argue the documented contract (section description + per-row tooltip + new test) is adequate. Consensus: medium-priority UX clarity concern, not a financial bug.
• Autosave divergence (HoursPerWeekGrid.tsx:287, 340): Financial originally 6.5; revised DOWN to 3.0 after verifying averageHoursPerWeek is a scratch column never read by salary math (downstream pay uses hoursWorkedPerWeek which receives the rounded value via Apply).
• Pay Type bypass of AutosaveTextField (SetupStep.tsx:214): Architecture's 6.5 revised DOWN to 4.5 (Yup rule is only .required() and MenuItem prevents empty value, so the validity gate is functionally inert here). BUT Testing's isMutating test gap went UP to 7.5 because it's now the only validity gate left.
• Translation nesting (MonthlyReimbursableSection.tsx:26-32): Standards revised DOWN to 3.5 (not a literal Checklist violation — uses {{}} placeholders, not template-literal interpolation). UX held at 6.5 (translator-grammar concern). Consensus: 6.0 (translator-context smell, not a hard violation).

Important Issues (Severity 7.0–7.9)

See inline comment on SetupStep.tsx:214.

Medium-Priority Issues (Severity 5.0–6.9)

See inline comments on:

• SetupStep.tsx:219 — Pay Type clears Pay Rate without warning
• MonthlyReimbursableSection.tsx:28 — buildCappedLabel translation nesting
• intlFormat.ts:16 — numberFormat options not unit-tested
• ReimbursableExpensesGrid.tsx:105 — Silent over-max clipping

Suggestions (Severity <5.0)

Lower-priority findings inline on ReimbursableExpensesGrid.tsx:133, HoursPerWeekGrid.tsx:122, HoursPerWeekGrid.tsx:521.

Other suggestions (not inlined — informational only)

• Step-intro duplication across SupportItemStep, SummaryReportStep, and ReimbursableExpensesStep — same <Box pb={4}><Typography variant="h6" component="h2">…</Typography><Typography pt={1}>…</Typography></Box> pattern in 3 files. Extract <StepIntro title description /> to Shared/. (3.5/10)
• OtherSection.tsx missing description test — sibling sections (Annual/Total/Monthly Reimbursable) added description-render assertions in this PR; OtherSection did not. (2.5/10)
• Heading hierarchy nit — ReimbursableExpensesGrid.tsx:181 and TotalReimbursableSection.tsx render variant="h6" without component="h3", producing a muddled outline under the step's h2. (4.0/10)
• AmountSuffix screen reader gap — salaryBreakdown.tsx:160-164 renders 0%(Atlanta) without whitespace separation in screen-reader output. (4.0/10)
• getLocationLabel template literal — SetupStep.tsx:87-93 builds Autocomplete label with `${location} (${percent})`; not a violation (wraps data only, no English copy) but inconsistent with neighboring t() usage. (2.5/10)
• geographicMultiplierSuffix template literal — salaryBreakdown.tsx:1194-1196 same pattern. (2.5/10)
• Description tests — SupportItemStep, SalarySection, SummaryReportStep, ReimbursableExpensesStep add new description copy without colocated assertions. Static i18n strings without branching are low-value to test, but SalarySection has no test file at all. (2.5/10)
• HoursPerWeekGrid locale-formatted average not tested — new numberFormat call with minimumFractionDigits: 2 lacks non-US-locale assertion. (3.5/10)

Pre-debate findings that were withdrawn

• HoursPerWeekGrid autosave divergence at lines 287/340 (severity 6.5) — Financial Agent verified averageHoursPerWeek is never consumed by salary math; downgraded to 3.0. A one-line fix would still tighten data hygiene for any future server-side consumer (PDFs, exports).

Standards Checklist

All mandatory items in .CLAUDE/rules/code-review.md pass:

• Named exports only · PascalCase file naming · Colocated tests ✓
• i18n via useTranslation/t(), static keys, {{}} interpolation ✓
• No any, no @ts-ignore, no new Date(), no debug output, no empty catch ✓
• Apollo optimisticResponse includes __typename and id (pre-existing pattern) ✓
• No .graphql/migration changes (N/A items) ✓

How to respond

• For severity <7 findings you disagree with, reply /dismiss: <reason> on the comment.
• For the Important finding (severity 7.5), address in this PR or open a follow-up ticket — it can't be /dismiss'd.

[github-actions]

AI Review Auto-Approval

Risk Level: MEDIUM (5/10)
Verdict: APPROVED_WITH_SUGGESTIONS (suggestions posted, no blockers)

This PR was auto-approved because:

• The multi-agent AI review determined it is medium risk
• No blocking issues were found
• All suggestions have been posted as review comments for the developer to consider

If you believe this PR needs human review, dismiss this approval and request a review manually.