Add spouseCalculations to ASR context

[dr-bizz]

Summary

This PR is a follow-up for #1680 to address Will's comments

Changes

• Added spouseCalculations to AdditionalSalaryRequestContext (mirroring existing calculations)
• Replaced all direct requestData?.latestAdditionalSalaryRequest?.calculations and requestData?.latestAdditionalSalaryRequest?.spouseCalculations access with context values
• Updated tests to provide calculations at the top level of mock context values

Test plan

• All 318 AdditionalSalaryRequest tests pass
• TypeScript compiles cleanly
• ESLint passes

[github-actions]

Bundle sizes [mpdx-react]

Compared against f7ae0e1

No significant changes found

[dr-bizz]

🤖 Multi-Agent Code Review Report

PR: #1682 — Add spouseCalculations to ASR context
Agents: 5 specialized reviewers (Architecture, Testing, UX, Financial, Standards)
Review Mode: Standard (smart selection)

---

📊 RISK ASSESSMENT

Risk Score: 7/15 — MEDIUM
Files Changed: 22 (+213 -103 lines)
Scope: Single feature area (AdditionalSalaryRequest)

Risk Factors: Custom hooks with business logic, financial calculations (salary caps, balances)

---

🚫 CRITICAL BLOCKERS (Severity 9-10)

None. All 5 agents confirmed no blocking issues. All financial calculations are preserved exactly through this refactor.

---

⚠️ IMPORTANT ISSUES (Severity 7-8)

1. Dual-path access: requestData + calculations both exposed on context

Severity: 8/10 (Architecture Agent)
File: AdditionalSalaryRequestContext.tsx:242-251

The context now exposes both requestData (full query result) and calculations/spouseCalculations (shortcuts). Some consumers still use requestData directly (e.g., EligibleDisplay, CapSubContent), while refactored components use the shortcuts. This creates two valid-looking paths for future developers.

Recommendation: This is expected for an incremental migration. Consider a follow-up to migrate remaining requestData.calculations consumers and eventually narrow the requestData exposure.

2. useMemo dependency array missing calculations/spouseCalculations

Severity: 7/10 (UX + Testing Agents)
File: AdditionalSalaryRequestContext.tsx:267-300

The contextValue useMemo includes calculations and spouseCalculations in the returned object but not in the dependency array. Both derive from requestData which IS in deps, so no stale data bug exists in practice. However, the deps array is technically incomplete.

Recommendation: Not blocking — requestData changing will trigger recalculation. But adding them explicitly would be more correct.

3. Asymmetric Pick types: combinedCap on calculations but not spouseCalculations

Severity: 7/10 (Architecture + Financial Agents)
File: AdditionalSalaryRequestContext.tsx:56-68

calculations picks 4 fields (including combinedCap), while spouseCalculations picks only 3 (excluding combinedCap). This matches the GraphQL query which doesn't fetch combinedCap for spouse — so it's correct. The Financial Agent notes combinedCap is not consumed by any calculation logic currently.

Recommendation: No action needed. The asymmetry matches the GraphQL schema.

---

💡 MEDIUM PRIORITY (Severity 5-7)

• Weak isSpouse test (Severity 5.5 — Testing + Standards): useAdditionalSalaryRequestForm.test.tsx line 296 — the renamed test 'should read isSpouse from context' only asserts values is defined, which passes trivially. The original test verified isSpouse was passed as a query variable.

• Redundant requestData in test mocks (Severity 3.5 — Testing + Financial): After the refactor, useSalaryCalculations reads calculations/spouseCalculations from context, but test mocks still include full requestData.latestAdditionalSalaryRequest.calculations. This is dead test data that could confuse future maintainers.

---

💭 SUGGESTIONS (Severity 3-5)

• Add context integration test verifying calculations/spouseCalculations are correctly populated from query data
• Clean up redundant requestData from test mocks in useSalaryCalculations.test.ts and useFormUserInfo.test.ts
• Consider adding a comment on the SpouseComponent ternary explaining the isSpouse perspective swap

---

💰 FINANCIAL ACCURACY VERIFICATION

The Financial Agent confirmed all calculations are preserved exactly:

Calculation	Status
totalAnnualSalary = grossAnnualSalary + pendingAsrAmount + total	✅ Preserved
spouseTotalAnnualSalary = spouseGrossAnnualSalary + spousePendingAsrAmount	✅ Preserved
exceedsCap, spouseExceedsCap, splitAsr logic	✅ Preserved
AT_CAP_TOLERANCE boundary logic	✅ Preserved
403b deduction calculations	✅ Not touched
staffAccountBalance / primaryAccountBalance flow	✅ Preserved

No arithmetic was changed. Only the data source was moved from requestData?.latestAdditionalSalaryRequest?.calculations to context shortcuts.

---

📋 STANDARDS COMPLIANCE

Standard	Status
File Naming	✅
Named Exports	✅
GraphQL	✅ N/A (no new operations)
Localization	✅
Testing	✅
Code Quality	✅

---

📝 REVIEW SUMMARY

Agent	Critical	Important	Suggestions	Confidence
🏗️ Architecture	0	3	3	High
🧪 Testing	0	4	2	High
👤 UX	0	1	0	High
💰 Financial	0	2	2	High
📋 Standards	0	0	1	High
Total	0	5 unique	4 unique	High

---

🎯 VERDICT

✅ APPROVE — Clean refactoring PR. No critical or high-priority blockers. All financial calculations verified preserved. The important issues noted are incremental improvement opportunities, not merge blockers.

---

🤖 Generated by MPDX Multi-Agent Review System v3.0 — 5 agents, standard mode

[dr-bizz]

We are doing this in the context, which is why we don't need to call useAdditionalSalaryRequestQuery here

[github-actions]

Preview branch generated at https://MPDX-9425-spouse-calculations.d3dytjb8adxkk5.amplifyapp.com

[wjames111]

Looks great thanks for the refactor! Sorry I didn't realize how many test files would need to change. I did notice there's a few mocks with requestData in them that I don't think are needed anymore.

[wjames111]

Do we still need these?

[wjames111]

Could we remove this now too?