Properly apply commuter exclusions when merging expenses

[Gonals]

Explanation of Change

Fixed Issues

$ #98343
PROPOSAL:

Tests

In an account with the beta and in 2 workspaces:

• Enable distance rates in both workspaces

• Enable commuter deductions and set the deduction to 1 mile in one of the (workspace A)

• Create a map distance expense in each of those workspaces. Make sure they have different mileage.

• Open Expense A and click merge. Select Expense B for the merge.
  Confirm that:

• If Expense A Merchant and Expense A report are selected, the exclusion is applied.

• If Expense A Merchant and Expense B report are selected, the exclusion is NOT applied.

• If Expense B Merchant and Expense A report are selected, the exclusion is applied.

• If Expense B Merchant and Expense B report are selected, the exclusion is NOT applied.

• Verify that no errors appear in the JS console

Offline tests

Same

QA Steps

Same as tests

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If new assets were added or existing ones were modified, I verified that:
  • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
  • The assets load correctly across all supported platforms.
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

[melvin-bot]

@marufsharifi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[marufsharifi]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • I verified the steps for Staging and/or Production testing are in the QA steps section
  • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
• I checked that screenshots or videos are included for tests on all platforms
• [ x I included screenshots or videos for tests on all platforms
• I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
• I verified tests pass on all platforms & I tested again on:
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see Reviewing the code)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I verified that this PR follows the guidelines as stated in the Review Guidelines
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

Screen.Recording.2026-08-12.at.8.57.11.PM.mov

Android: mWeb Chrome

Recording_20260812_185222.mp4

iOS: HybridApp

Screen.Recording.2026-08-12.at.9.51.57.PM.mov

iOS: mWeb Safari

Screen.Recording.2026-08-12.at.9.56.09.PM.mov

MacOS: Chrome / Safari

Screencast.From.2026-08-12.18-42-34.webm

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16da227b36

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[marufsharifi]

Proportional re-scaling can drift ±1¢ from a rate-based recompute.

amount / billedDistance reconstructs an implied per-unit rate from an already-rounded cents amount, then re-rounds. For rates with sub-cent precision that don't divide evenly, this can land 1¢ off from DistanceRequestUtils.getDistanceRequestAmount(reimbursableMeters, unit, rate) (the rate-based path used everywhere else for distance). Most rates match exactly (verified 0.67/mi, 0.70/mi), so this is minor — but it's only safe if the backend recomputes the merged amount from the rate. If the FE amount is authoritative on merge, please confirm the drift is acceptable; otherwise compute from the rate for an exact result.

The newBilledDistance === undefined guard (rather than !newBilledDistance) is a nice touch — it correctly lets a 0 reimbursable distance drive the amount to 0 on a full exclusion. 👍

[Gonals]

I think this is fine? Maybe I'm misunderstanding

[marufsharifi]

@MelvinBot Review

[MelvinBot]

🤖 Reviewed the commuter-exclusion merge logic. The core fix (thread the destination workspace through getMergeFieldUpdatedValues and re-scale the amount to the reimbursable distance) is sound and well-tested for the interactive handleSelect path. But I can confirm one correctness gap that reverses the PR's intent, plus two smaller issues worth resolving before merge.

1. Auto-merge (no-conflict) path strips the exclusion and can wipe customUnit — confirmed 🔴

Your own inline note on this is correct — I verified it end to end. getMergeableDataAndConflictFields calls getMergeFieldUpdatedValues twice without destinationPolicy:

• MergeTransactionUtils.ts:290 — reportID auto-merge (both txns already share a report)
• MergeTransactionUtils.ts:321 — generic non-conflict branch (includes merchant for distance)

With destinationPolicy === undefined, getCommuterExclusionCustomUnitUpdate computes commuterExclusion = 0 → the clear branch → nulls commuterExclusion/reimbursableDistance, and getAmountForBilledDistance scales the amount back up to the full distance. Merging two duplicate distance expenses on a workspace that excludes a commuter mile has no conflict fields, so setupMergeTransactionData jumps straight to confirmation and the merged expense loses its exclusion and over-reimburses — the exact opposite of this fix.

There's a second, sharper edge on the same path: the reportID branch sets updatedValues.customUnit = commuterExclusionUpdate (a partial update designed to be applied through Onyx.merge's deep merge). In the auto-merge path it's instead applied with a shallow Object.assign, so {} (or a nulls-only object) replaces the full customUnit a prior merchant iteration populated — dropping quantity/rate/waypoints entirely. Codex flagged this same spot independently.

Fix: thread the surviving workspace into both call sites (in the no-conflict path the expense stays on the same report, so it's the selected txn's policy — destinationPolicy: selectedPolicy at L321, targetTransactionPolicy at L290). Neither new test covers this — both drive getMergeFieldUpdatedValues/handleSelect with destinationPolicy set — so please add a unit test that merges two identical exclusion-bearing distance expenses and asserts the exclusion + reimbursable amount survive.

2. Manual / odometer distances shouldn't be excluded — confirmed 🟡

The merchant and reportID branches gate on isDistanceRequest(transaction), which includes manual and odometer distances. But the canonical exclusion path, getTransactionCommuterExclusionData, explicitly bails for DISTANCE_MANUAL/DISTANCE_ODOMETER before applying anything. The new extracted getPolicyCommuterExclusionForDistance only checks the method, so merging a manual/odometer distance expense onto an excluding workspace would wrongly shave the commuter distance off an amount that should stay whole. Add the same iouRequestType guard here (or fold it into the shared helper).

3. Amount re-scaling can drift ±1¢ — acceptable if the backend recomputes 🟢

getAmountForBilledDistance reconstructs an implied per-unit rate from an already-rounded cents amount and re-rounds, which can land 1¢ off from the rate-based getDistanceRequestAmount used elsewhere. This is only safe if the backend recomputes the merged amount from the rate on save. If the FE amount is authoritative on merge, compute from the rate instead. (The newBilledDistance === undefined guard letting a 0 reimbursable distance drive the amount to 0 is correct — nice.)

Nits

• Codex's CONSISTENCY-16 on the em dash in the getCommuterExclusionCustomUnitUpdate JSDoc is a fair call — worth splitting into two sentences.

Item 1 is a blocker (silent over-reimbursement on duplicate distance merges); 2 should be fixed; 3 just needs confirmation that the backend re-derives the amount.

How I verified item 1

Traced getMergeableDataAndConflictFields → both getMergeFieldUpdatedValues call sites omit destinationPolicy. MERGE_FIELDS orders merchant (2nd) before reportID (last), so for fully-duplicate distance expenses the merchant generic branch first sets mergeableData.customUnit (with commuterExclusion/reimbursable nulled because destinationPolicy is undefined), then the reportID branch's shallow Object.assign replaces it with the partial ({}) update. Both new tests pass destinationPolicy, so this branch is uncovered.

---

This is an automated review — flagging item 1 as a likely blocker, but defer to your judgment on the final call.

[Gonals]

🤖 Reviewed the commuter-exclusion merge logic. The core fix (thread the destination workspace through getMergeFieldUpdatedValues and re-scale the amount to the reimbursable distance) is sound and well-tested for the interactive handleSelect path. But I can confirm one correctness gap that reverses the PR's intent, plus two smaller issues worth resolving before merge.

1. Auto-merge (no-conflict) path strips the exclusion and can wipe customUnit — confirmed 🔴

Your own inline note on this is correct — I verified it end to end. getMergeableDataAndConflictFields calls getMergeFieldUpdatedValues twice without destinationPolicy:

• MergeTransactionUtils.ts:290 — reportID auto-merge (both txns already share a report)
• MergeTransactionUtils.ts:321 — generic non-conflict branch (includes merchant for distance)

With destinationPolicy === undefined, getCommuterExclusionCustomUnitUpdate computes commuterExclusion = 0 → the clear branch → nulls commuterExclusion/reimbursableDistance, and getAmountForBilledDistance scales the amount back up to the full distance. Merging two duplicate distance expenses on a workspace that excludes a commuter mile has no conflict fields, so setupMergeTransactionData jumps straight to confirmation and the merged expense loses its exclusion and over-reimburses — the exact opposite of this fix.

There's a second, sharper edge on the same path: the reportID branch sets updatedValues.customUnit = commuterExclusionUpdate (a partial update designed to be applied through Onyx.merge's deep merge). In the auto-merge path it's instead applied with a shallow Object.assign, so {} (or a nulls-only object) replaces the full customUnit a prior merchant iteration populated — dropping quantity/rate/waypoints entirely. Codex flagged this same spot independently.

Fix: thread the surviving workspace into both call sites (in the no-conflict path the expense stays on the same report, so it's the selected txn's policy — destinationPolicy: selectedPolicy at L321, targetTransactionPolicy at L290). Neither new test covers this — both drive getMergeFieldUpdatedValues/handleSelect with destinationPolicy set — so please add a unit test that merges two identical exclusion-bearing distance expenses and asserts the exclusion + reimbursable amount survive.

2. Manual / odometer distances shouldn't be excluded — confirmed 🟡

The merchant and reportID branches gate on isDistanceRequest(transaction), which includes manual and odometer distances. But the canonical exclusion path, getTransactionCommuterExclusionData, explicitly bails for DISTANCE_MANUAL/DISTANCE_ODOMETER before applying anything. The new extracted getPolicyCommuterExclusionForDistance only checks the method, so merging a manual/odometer distance expense onto an excluding workspace would wrongly shave the commuter distance off an amount that should stay whole. Add the same iouRequestType guard here (or fold it into the shared helper).

3. Amount re-scaling can drift ±1¢ — acceptable if the backend recomputes 🟢

getAmountForBilledDistance reconstructs an implied per-unit rate from an already-rounded cents amount and re-rounds, which can land 1¢ off from the rate-based getDistanceRequestAmount used elsewhere. This is only safe if the backend recomputes the merged amount from the rate on save. If the FE amount is authoritative on merge, compute from the rate instead. (The newBilledDistance === undefined guard letting a 0 reimbursable distance drive the amount to 0 is correct — nice.)

Nits

• Codex's CONSISTENCY-16 on the em dash in the getCommuterExclusionCustomUnitUpdate JSDoc is a fair call — worth splitting into two sentences.

Item 1 is a blocker (silent over-reimbursement on duplicate distance merges); 2 should be fixed; 3 just needs confirmation that the backend re-derives the amount.

How I verified item 1
Traced getMergeableDataAndConflictFields → both getMergeFieldUpdatedValues call sites omit destinationPolicy. MERGE_FIELDS orders merchant (2nd) before reportID (last), so for fully-duplicate distance expenses the merchant generic branch first sets mergeableData.customUnit (with commuterExclusion/reimbursable nulled because destinationPolicy is undefined), then the reportID branch's shallow Object.assign replaces it with the partial ({}) update. Both new tests pass destinationPolicy, so this branch is uncovered.

This is an automated review — flagging item 1 as a likely blocker, but defer to your judgment on the final call.

Let's try to avoid heavily agent-based reviews for the last human-based reviews.

Feel free to use agents, but let's make sure the review is actually written by the reviewer. This leaves the owner to analyze and determine whether the review makes sense, which defeats the purpose of a reviewer

[Gonals]

Ready for another round

[marufsharifi]

Thanks, i am checking again.

[marufsharifi]

Bug: merging two distance expenses loses the trip distance and rate

What happens: When two identical map-distance expenses on the same report are merged (and the merge completes automatically with no conflicts to resolve), the merged expense loses its distance and rate.

RF-1.mp4

[marufsharifi]

Bug: manual distance expense gets a commuter deduction it shouldn't when merged

What happens: When a manually-entered (or odometer) distance expense is merged onto a workspace that has a commuter exclusion, the merged expense gets a commuter deduction applied. A manual distance expense created fresh on that same workspace would NOT get this deduction — the deduction is only meant for map-based distance. So merging produces a different result than creating the expense normally.

RF-2.mov

[JmillsExpensify]

LGTM

[marufsharifi]

@Gonals, could you please check the above comments, thanks.

[marufsharifi]

@Gonals, Gentle bump. thanks.

[Gonals]

@Gonals, Gentle bump. thanks.

Been ooo for a long week. Cleaning this up and getting it ready for review

[Gonals]

@marufsharifi, ready for another look