fix: Keep the sole GPS segment when a trip is stopped after one recorded point by TaduJR · Pull Request #101027 · Expensify/App

TaduJR · 2026-09-12T16:52:18Z

Explanation of Change

Standing still while tracking records only one location, because the app records a new location only after you move 100 m. When you tapped Stop, the app removed the last section of the trip if it had one location or none. That's right for a section added by Resume that recorded nothing useful, but it also happened when that section was the whole trip. The app treats a trip with no locations as one that never started, so the trip disappeared: no Resume or Save, no distance or addresses, and the button went back to Start.

Stop now only removes a short section when there's an earlier section to fall back on. So stopping without moving keeps the trip with Resume and Save, and tapping Save shows the existing "Can't create expense" message. The same applies when a trip is stopped from "View trip" after reopening the app, or by switching to Expensify Classic.

Keeping that one location makes a few new situations possible, and this PR handles them:

The map pins assumed every section has a start and an end, so a section with one location made the last pin show as a normal waypoint instead of the stop. Pins are now picked by where they sit in their section, on both the trip screen and the Edit stop screen.
A trip too short to trim, like one that never moved, no longer shows the Edit button.
A trip can now be discarded right after its first location, while its start address is still being looked up. When that lookup finished, it saved the address and the discarded trip came back. It now saves nothing if the trip is gone.
For a trip with a single location, the map now centers on it at the zoom used for a single pin, instead of trying to fit its view around a route that has no size.

This is a regression from #91418. The check that keeps a trip's only section was first lost in #88609, restored by #90237, then removed again by #91418, which also made Stop clear the only section. This PR restores the check and removes the clearing.

Fixed Issues

$ #100938
PROPOSAL:

Tests

Tap FAB, then Track distance, then the GPS tab.
Tap Start and allow any location prompts.
Wait until Distance shows 0.00, your address appears under Start, and Tracking distance... appears under Stop.
Without moving, tap Stop.
Verify the trip stays on screen: Distance 0.00, your address under both Start and Stop, a trash icon next to the distance, no pencil icon, and Resume and Save buttons.
Verify the map is centered on your location at a normal zoom where nearby streets are visible, not zoomed in all the way.
Drag the map away, then tap the crosshair button at the top right of the map. Verify the map returns to your location at the same normal zoom.
Tap Save. Verify a "Can't create expense" message says "You can't create an expense with the same start and stop location."
Tap Got it. Verify the trip is still on screen and no expense was created.

Verify that no errors appear in the JS console

Offline tests

Same as tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Screen.Recording.2026-09-12.at.7.51.01.PM.mov

Android: mWeb Chrome

iOS: Native

Screen.Recording.2026-09-12.at.7.58.04.PM.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

melvin-bot · 2026-09-12T16:52:24Z

@mkhutornyi 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]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c80098738

ℹ️ 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".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5c7fcff1f

ℹ️ 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".

chatgpt-codex-connector · 2026-09-12T19:29:00Z

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 72026b82e9

ℹ️ 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".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f5852e0a3

ℹ️ 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".

MelvinBot · 2026-09-12T22:06:51Z

🤖 Done — all five of my comments on this PR are now hidden as resolved:

Comment	Pass
5647386443	Initial review at `2c80098`
5647713052	Withdrew the refactor, raised the `.value` finding
5647815991	Re-review at `a5c7fcf`
5648186152	Re-review at `72026b8`
5648667453	Re-review at `8f5852e`

Correcting my last reply: I said there was nothing to resolve because these were top-level comments rather than inline review threads. That was wrong — GitHub's Hide comment → Resolved works on top-level comments too, and that's what I've applied.

Head is still 8f5852e, the commit my last pass cleared, so nothing has changed since. The two inline review threads from chatgpt-codex-connector were already resolved and I left them alone.

Next Steps: Reply with @MelvinBot <your feedback> if you want any of these unhidden.

view run · no recording available

…ded point

codecov · 2026-09-13T14:15:09Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
src/components/MapView/utils.ts	`92.10% <100.00%> (+2.91%)`	⬆️
src/libs/GPSDraftDetailsUtils.ts	`87.50% <100.00%> (+28.09%)`	⬆️
src/libs/actions/GPSDraftDetails.ts	`57.47% <100.00%> (+55.19%)`	⬆️
...OURequestStepDistanceGPS/useGPSWaypointMarkers.tsx	`100.00% <100.00%> (+100.00%)`	⬆️
...tup/backgroundLocationTrackingTask/index.native.ts	`82.05% <100.00%> (+79.34%)`	⬆️
...estStepDistanceGPS/Waypoints/EditGPSTripButton.tsx	`0.00% <0.00%> (ø)`
src/components/MapView/GPSMapView.tsx	`2.46% <0.00%> (-0.17%)`	⬇️
... and 173 files with indirect coverage changes

TaduJR · 2026-09-13T17:24:59Z

@MelvinBot @codex review

chatgpt-codex-connector · 2026-09-13T17:28:22Z

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: d854560b4e

ℹ️ 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".

MelvinBot · 2026-09-13T17:31:38Z

🤖 Re-reviewed at d854560. The branch was rewritten since my last pass (8f5852e is no longer an ancestor — the three commits here are new), so this is a fresh read rather than a delta. CI is fully green. Two findings.

Test step 3 now contradicts the code

Step 3 still says to verify "trash and pencil icons next to the distance". Commit 77648b2 deliberately hides the pencil for exactly that trip: EditGPSTripButton now gates on canGpsTripBeTrimmed, and a stopped trip with one point has distanceInMeters === 0, so it returns null. QA will fail that step on correct behavior.

While you're in there — the steps only exercise the first commit. Nothing covers the map centering on a single point, the stop marker on a segment that holds one point, or the Edit button disappearing. QA Steps say "Same as tests", so both sections inherit the gap.

The rewrite dropped the `addressFromGpsPoint` fix

The previous revision changed GPSPointUtils.ts:21 to return address || null. That file is not in this PR any more, and main still has the bare return address.

It matters here specifically. Line 19 builds the address as formattedAddress ?? [name, city, region].filter(Boolean).join(', ') — a geocode result with a location object whose fields are all null yields '', not null. updateStartAddress gates on address !== null, so {value: '', type: 'address'} gets written. Then Waypoints/index.tsx:49-50 reads !address?.value, '' is falsy, and both rows render the '...' spinner with nothing to ever resolve them.

That's a latent bug on main, not one you introduced. But before this PR a one-point trip was deleted on Stop, so the stuck rows were never on screen. Keeping the trip is what makes it reachable — which is presumably why the earlier revision fixed it. Worth pulling back.

Nits

The caller-side if is now redundant. GPSDraftDetailsUtils.ts:143-147 guards on gpsPoints.length > 1, and removeLastSegment:68 now returns early on length <= 1 itself. Both branches of the outer if fall to the same return, so dropping it leaves behavior identical. Your earlier reason for keeping it — the sole segment continuing into the address logic — no longer applies to this code.
getSinglePointCoordinate([], undefined) returns undefined by accident. getBounds on empty input gives southWest: [Infinity, Infinity] and northEast: [-Infinity, -Infinity], so the equality check happens to fail. The test asserting this passes for a reason a reader would not guess. An explicit empty check would say it.
getBounds runs twice per render. GPSMapView.tsx:48 walks every coordinate, then getWaypointBounds walks them again on the same inputs — a third time in the focus effect. Computing the bounds once and deriving singlePointCoordinate from them would collapse all three.
The discard guard doesn't cover a restart. index.native.ts:82 catches the discard, but if a new trip is started during the lookup, updatedGpsDraftDetails is truthy and the stale tripSegmentIndex from line 72 writes the old trip's address onto the new one. Nearly always harmless since both points sit in the same place, but it's the same race the commit is about.

What I checked and found correct

canGpsTripBeTrimmed's threshold is exact, not approximate. > GPS_DISTANCE_INTERVAL_METERS matches Slider.tsx:33, where minRatio = GPS_DISTANCE_INTERVAL_METERS / totalDistanceMeters. At exactly one interval minRatio is 1, so the slider's min equals its max and there is genuinely nothing to trim. I went looking for an off-by-one here and there isn't one.

The .catch(() => undefined) on the draft read is unreachable, which makes the new guard unambiguous. getGpsDraftDetails wraps Onyx.connectWithoutView in a promise with no reject path, so !updatedGpsDraftDetails can only mean "no draft". Good — the branch can't silently swallow a read failure.

removeLastSegment owning the invariant is the change I asked for two passes ago, and the tests now assert real Onyx state instead of mock call counts, including the [[]] and resumed-segment cases.

useGPSWaypointMarkers — isLastWaypointSegmentStart uses findLast((segment) => segment.length > 0), which matches the segment.length < 1 skip inside getGPSWaypoints, so the index arithmetic stays aligned. The two trimmed cases I flagged as untested last round are now covered, including segmentIndex: 1.

Onyx array semantics. Onyx.merge replaces arrays rather than merging them, so stopGpsTrip writing its stale gpsPoints parameter can still clobber a point that lands during the awaits. Unchanged from main and not something this PR makes worse, so I'm not reopening it — noting it only because the backgroundLocationTrackingTask commit fixes the same class of race one file over.

CI. All 8 test jobs pass, along with typecheck, ESLint check, React Compiler Compliance, Oxfmt check, spellcheck, Storybook tests and both perf jobs. The only red checks are Check independent approval and checklist, neither of which reflects the code.

I did not run the app — this flow is native-only and isn't reproducible on web, so I reviewed statically against green CI.

Next Steps: Reply with @MelvinBot <your feedback> to refine this review.

view run · no recording available

TaduJR · 2026-09-13T18:11:29Z

@MelvinBot The bug is already on main branch. I can make a follow-up. Addressing here will bloat up the PR. For now make the comment your wrote as resolved including all the others.

MelvinBot · 2026-09-13T18:14:21Z

🤖 Nothing left to resolve — and my main point is already fixed on your branch.

All three inline review threads are already resolved, and no review is in a blocking state. My review was posted as a top-level PR comment, which GitHub gives no resolve control for. Treat it as closed out — I'm not holding anything open.

You already applied the cleanup I asked for. At src/libs/actions/GPSDraftDetails.ts:66-70, removeLastSegment now early-returns on gpsPoints.length <= 1. The clear-to-[[]] branch that was the actual bug is gone, so the invariant lives in one place. That was the only thing I flagged as "should fix."

Agreed on the follow-up for the rest — the remaining ... endpoint edge cases predate this PR, and widening the diff to chase them here is the wrong trade.

One optional leftover whenever you touch this next: the if (gpsPoints.length > 1) guard at src/libs/GPSDraftDetailsUtils.ts:144 is now redundant with the one inside removeLastSegment. Harmless as defense in depth — just not worth a push on its own.

view run · no recording available

garrettmknight

Looks good from a product perspective.

MelvinBot · 2026-09-14T17:33:44Z

Regression review

I reviewed the diff against the surrounding GPS code. No blocking regression — the core change (keeping a trip's sole segment) is correct and the newly reachable [[onePoint]] state holds up almost everywhere. Five issues worth a look, ranked.

1. The Start and Stop rows can spin `...` forever on a one-point trip — medium

For a one-point segment, stopGpsTrip returns at GPSDraftDetailsUtils.ts:142-147 before setEndWaypointAddress. That's right — the one point is both start and stop — but it means the row text comes entirely from the start-address lookup, which runs once and is fire-and-forget at backgroundLocationTrackingTask/index.native.ts:56.

Failure scenario: record the first point, and the reverse geocode never finishes because Android tears down the headless JS context (backgrounded / app killed). Relaunch, tap View trip → stopGpsTrip → one-point branch → still no address. Nothing re-fetches it. shouldShowLoadingEndAddress at Waypoints/index.tsx:49 stays true, so both rows show a permanent spinner. Resume doesn't help — it geocodes the new segment, not point 0.

Pre-PR this was invisible: the trip was wiped, so the whole block was hidden. Now it's a stuck screen whose only exit is the trash icon.

Why it doesn't hit the normal path

addressFromGpsPoint catches and returns null, and updateStartAddress falls back to coordinates, so a failed geocode is fine. Only a lookup that never resolves leaves the point address-less. Narrow, but it's the one state this PR makes visible.

2. `zoomLevel` is passed to `Mapbox.Camera` ungated — low, latent

GPSMapView.tsx:245 passes zoomLevel={waypointsZoomLevel} unconditionally, while the sibling defaultSettings.zoomLevel at :199 is gated on shouldFollowFallbackLocation. When the fallback wins, centerCoordinate becomes the fallback location but zoomLevel stays SINGLE_MARKER_ZOOM. Harmless only because DEFAULT_ZOOM and SINGLE_MARKER_ZOOM are both 15. Mirror the :199 ternary so it can't drift.

3. `getSinglePointCoordinate` recomputes on every render — low

GPSMapView.tsx:48 runs getBounds over every waypoint plus the whole flattened route on every render, before any guard. The old code only did that inside getWaypointBounds, which short-circuited on userInteractedWithMap. During tracking, onUserLocationUpdate re-renders continuously and the route keeps growing, so this is a new per-render Math.min(...coords) sweep over the full trip. A useMemo on [waypoints, directionCoordinates] covers it.

4. The `<= 1` guard is now duplicated, and one copy is dead — low

removeLastSegment at actions/GPSDraftDetails.ts:66-70 now no-ops for a single segment, and its only production caller already guards with gpsPoints.length > 1. So the branch is unreachable. Not a bug, but the exported function silently changed contract (it used to clear to [[]]) while being double-guarded. Pick one place to own the rule.

5. Marker type during the transient two-single-point state — cosmetic

Stop at one point, Resume, and Onyx briefly holds [[p0],[p1]] while tracking. isLastWaypointSegmentStart at useGPSWaypointMarkers.tsx:23 is true, so p1 renders as a generic WAYPOINT; the old parity rule treated it as an end and hid it while tracking. The new rule is still the right trade — it genuinely fixes the [[p0],[p1,p2]] case the old parity check got wrong.

Follow-up, not this PR

useUpdateGpsTripOnReconnect.ts:72 is missing the exact guard you added to the background task. getGpsPoints(undefined) returns [[]], so the ?? gpsPoints fallback on that line is dead code, and a trip discarded while the reconnect geocode is in flight gets resurrected by the updateGpsPoints at :89 as a draft with no reportID/isTracking/unit. Pre-existing, but it's the same bug you just fixed next door.

What I checked and found clean

Camera / follow mode. @rnmapbox/maps 10.3.2 buildNativeStop returns null whenever followUserLocation is true (node_modules/@rnmapbox/maps/src/components/Camera.tsx:299-311), so the new centerCoordinate/zoomLevel are discarded while tracking — native saw stop=null both before and after. Fabric's diffAttributePayloads deep-compares, so the fresh array identity each render produces no native update. "Map follows me while I walk" is not affected.
Trim gate. canGpsTripBeTrimmed's > GPS_DISTANCE_INTERVAL_METERS threshold exactly matches Slider.tsx's minRatio = 100 / totalDistanceMeters, so the pencil is hidden precisely when the slider couldn't move. applyTrimmedTrip writes modifiedDistance, never distanceInMeters, so an already-trimmed trip keeps its Edit button and can be un-trimmed.
Trimmed-trip markers. calculateTrimmedEndPoint always returns precedingPointIndex >= 0, so getTrimmedGpsTrip can't produce a one-point last segment. The stop pin never disappears on the Edit stop screen.
Single-point route rendering. [[[lng,lat]]] flows through convertSegmentedRouteToSingleSegmentRoute correctly, and Direction filters segment.length >= 2, so no 1-coordinate LineString reaches Mapbox.
getBounds on empty input returns [±Infinity], and Infinity === -Infinity is false, so getSinglePointCoordinate correctly returns undefined.
Save being blocked by the zero-distance modal is intentional per your own test step 8, not a regression.
stopGpsTrip after Resume correctly collapses [[p0],[p1]] back to [[p0]]; Onyx replaces arrays wholesale, so no index-merge artifacts.

Note on web verification

I couldn't run a browser check — the test session's page went blank during an idle gap before the run started, and agent-device web has no console access to diagnose it. It wouldn't have covered this PR anyway: the GPS step renders a static "use the mobile app" placeholder on web (IOURequestStepDistanceGPS/index.tsx), and the only shared file touched, MapView/utils.ts, is a pure additive export that nothing on web calls. All findings above are from code analysis. Verifying #1 needs a device.

Next Steps: Reply with @MelvinBot <your feedback> to refine this review.

view run · no recording available

…-GPS-Save-and-Resume-buttons-not-present-after-tapping-Stop-without-moving

TaduJR · 2026-09-15T12:20:10Z

Same as above. The are pre-existing planned as follow-ups.

We can continue with the review.

…-GPS-Save-and-Resume-buttons-not-present-after-tapping-Stop-without-moving

TaduJR · 2026-09-17T10:02:14Z

@MelvinBot @codex review

MelvinBot · 2026-09-17T10:14:36Z

🤖 Re-reviewed at 259baa1. No new findings — the code is unchanged since my last pass.

The three commits since d854560 are all merges of main. The net diff against main is exactly the same 7 source files and 5 test files I cleared last time, and main has not touched any of them, so nothing drifted. CI is green: all 8 test jobs, typecheck, ESLint, React Compiler compliance, and both codecov gates.

What I verified this round

I went deeper on the camera path, which I had only traced shallowly before. It holds up.

buildNativeStop in @rnmapbox/maps returns null whenever followUserLocation is set, so the new centerCoordinate / zoomLevel props at src/components/MapView/GPSMapView.tsx:244-245 are inert while a trip is recording and only take effect once isTrackingGPS flips false. Nothing fights between "follow the puck" and "center the single point."
The crosshair path for a one-point trip resolves correctly. centerMap falls to its else branch (one waypoint, one direction coordinate), clears userInteractedWithMap, and the re-render flips the Camera props from undefined back to that point at zoom 15 — which is what makes test step 7 pass. Before this PR the same tap produced zero-area bounds and zoomed all the way in.
getSinglePointCoordinate([], undefined) can't misfire: empty bounds are [Infinity, Infinity] and [-Infinity, -Infinity], never equal.
SINGLE_MARKER_ZOOM is the same treatment MapView.tsx:169-173 already gives a lone waypoint, so the GPS map now matches the rest of that folder.

I also confirmed the newly reachable [[onePoint]] state can't leave a trimmed segment of length 1: calculateTrimmedEndPoint always returns precedingPointIndex >= 0, so a trimmed segment always holds at least two points and isLastWaypointSegmentStart stays false on the Edit stop screen. The two trimmed-trip cases in useGPSWaypointMarkersTest.ts pin that, closing the trim-screen coverage gap I raised at 72026b8.

Nit: the reason in `canGpsTripBeTrimmed` isn't the reason it works

src/libs/GPSDraftDetailsUtils.ts:194 says trimming "cannot shorten a trip below one location interval." It can — calculateTrimmedEndPoint interpolates between the two recorded points, so a 120 m trip trims to 30 m fine.

The check is still correct, just for a different reason: addGpsPoints only keeps a point that is at least 100 m from the previous one, and a segment's first point adds 0 m — so a stopped trip with two or more points always measures 100 m or more. distanceInMeters <= 100 therefore only ever means the one-point trip you're targeting. distanceInMeters > 0 would say that directly and wouldn't depend on the interval constant.

Strictly, > also hides Edit for a trip measuring exactly 100.000 m. That's reachable on paper and not in practice with float geodesics, so it isn't worth a push — but the comment is worth fixing so a later edit doesn't trust the wrong rationale.

The items you deferred earlier (the ... endpoint edge cases, and the addressFromGpsPoint empty-string fix that lives on main) I'm leaving alone per your follow-up plan. Nothing held open from my side.

view run

mkhutornyi · 2026-09-17T19:24:28Z

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
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 any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- 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 and each prop has a /** comment above it */
- The file is named correctly
- 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
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- 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 any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
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-09-18.at.10.16.03.AM.mov

Android: mWeb Chrome

iOS: HybridApp

Screen.Recording.2026-09-18.at.10.16.52.AM.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

MelvinBot · 2026-09-18T09:19:32Z

🤖 I traced every state the "keep the sole segment" change makes newly reachable. No functional regression found in the diff — CI is green, and the two riskiest-looking changes hold up (details below). Three things are worth a decision before merge.

1. A zero-distance trip now lives until the user explicitly discards it

stopGpsTrip keeps [[p1]] (GPSDraftDetailsUtils.ts:142-146), so isTripStopped stays true forever. The only reset is the trash icon (DiscardGPSTripButton.tsx:44), and the draft survives app restarts and a forced re-auth (SignInRedirect.ts:104).

Before, an accidental Start → Stop self-cleared and the tab was back at Start. Now that user gets Resume / Save on a 0.00 trip on every later visit — including when they open Track distance from a different chat, since the draft is a single global key. Abandoned real trips already behaved this way, so this is the fix's intended cost, not a defect. Please confirm it matches the expectation in Android - GPS - "Save" and "Resume" buttons not present after tapping "Stop" without moving.

2. The discard-race guard also swallows a failed read

index.native.ts:82 bails on any falsy result, but the value comes from getGpsDraftDetails().catch(() => undefined) — so "trip discarded" and "read threw" are indistinguishable. That matters more now: for a one-point trip stopGpsTrip returns before writing any end address, so this function is the only writer of that point's address, and both the Start and Stop rows read from it. If it bails, both sit on the ... spinner until a network reconnect. The old code fell back to the in-hand gpsPoints and still landed the address. Worth splitting the two cases and keeping the fallback on the catch path.

3. Two guards for one rule

gpsPoints.length > 1 at GPSDraftDetailsUtils.ts:144 is dead — removeLastSegment already no-ops at length <= 1 (GPSDraftDetails.ts:68). Keep one so the rule has one home.

What I checked that came back clean

New isEnd rule is never worse than the old parity rule. For every reachable state, a last non-empty segment of length 1 only happens when it is the trip's only segment: stopGpsTrip drops a one-point last segment whenever an earlier one exists, and getTrimmedGpsTrip always rebuilds the trimmed segment with ≥ 2 points (slice(0, preceding + 1) plus the interpolated point, precedingPointIndex >= 0). The "View trip" and switch-to-Classic paths both route through stopGpsTrip, so neither leaves a stale one-point tail.
The new zoomLevel / centerCoordinate props can't fight follow-puck while tracking. During the first 100 m of every trip there is exactly one waypoint, so singlePointCoordinate is set while followUserLocation is still true — but rnmapbox's buildNativeStop returns null whenever followUserLocation is set, so the stop is never issued. SINGLE_MARKER_ZOOM and DEFAULT_ZOOM are both 15, so there is no visible zoom change either.
The pre-Start map is unaffected. getSinglePointCoordinate([], undefined) returns undefined because Math.min() is Infinity and Math.max() is -Infinity, so the exact-equality check fails. Load-bearing and subtle — good that MapViewUtilsTest pins it.
Hiding the pencil below one interval matches the slider. canGpsTripBeTrimmed uses > GPS_DISTANCE_INTERVAL_METERS, and Slider clamps to minRatio = 100 / totalDistanceMeters, so a trip at or under 100 m had nothing to trim anyway. Multi-point trips always clear 100 m, since addGpsPoints only records a point once it is that far from the previous one.
Nothing else keys off a non-tracking draft. FloatingGPSButton, AccountSwitcher, useSwitchToDelegator, MergeResultPage, and useInitialSettingsPageMenuData all read through isTrackingSelector, so the surviving stopped trip doesn't block account switching, delegation, or merges.
Save still guards zero distance via distanceInMeters === 0 in GPSButtons, which is what produces the "Can't create expense" modal the test steps expect.

4. Minor: a one-point trip renders only the green Start pin

With one waypoint entry, isEnd is false and markerType falls through to START_WAYPOINT (useGPSWaypointMarkers.tsx:28-36) — so there is no stop pin, while the list below shows both a Start and a Stop row with the same address. Defensible, but worth a design confirmation since test step 5 describes both rows.

One caveat on verification: this flow is native-only — the tracking task and the GPS step are index.native.*, so there is no web surface to exercise and I could not run the test steps in a browser. Steps 1-9 still need a device or emulator pass.

Next Steps: Reply with @MelvinBot <your feedback> to dig into any of these.

view run

MelvinBot · 2026-09-18T09:24:53Z

🤖 Code quality review — the diff is clean, well-commented, and unusually well-tested for a fix this size. The comments explain why rather than what, which is exactly right. Five things I'd tighten, roughly in order of value.

1. "The last segment has one point" is now expressed twice, in two incompatible ways

GPSDraftDetailsUtils.ts:118 already owns this concept as isLastSegmentEmptyOrHasOnlyOnePoint. The new useGPSWaypointMarkers.tsx:23 re-derives a different variant inline — findLast((segment) => segment.length > 0)?.length === 1, which skips empty segments where the util counts them as a hit.

Two spellings of the same idea that deliberately disagree is the kind of thing that rots. Both are really asking a question about trip shape, so both belong in GPSDraftDetailsUtils next to getGPSWaypoints — which is where the segment structure is already known. A named predicate there also lets line 28 read as prose instead of a double negative.

While you're there: isLastWaypointSegmentStart names a role but the expression measures a length. Something like lastSegmentHasOnlyOnePoint says what it actually checks.

2. The "can't trim below one interval" rule now lives in two files

GPSDraftDetailsUtils.ts:195 encodes it as distanceInMeters > GPS_DISTANCE_INTERVAL_METERS; Slider.tsx:33 encodes the same rule as GPS_DISTANCE_INTERVAL_METERS / totalDistanceMeters. They agree today — that's why hiding the pencil is correct — but nothing keeps them in step. One exported helper both can call keeps "the shortest trip you can trim" in a single place.

3. GPSMapView's camera now resolves across four overlapping expressions

singlePointCoordinate (line 48) feeds the focus effect, getWaypointBounds, defaultSettings, and two Camera props, each with its own ternary. The tell is line 245: zoomLevel={waypointsZoomLevel} does not mirror the shouldFollowFallbackLocation ternary that line 244 uses for centerCoordinate, nor line 199 in defaultSettings. It's harmless — shouldFollowFallbackLocation requires noWaypoints, which rules out a single point — but a reader has to prove that invariant to know it's safe.

Deriving one camera target with an explicit precedence chain (fallback → single point → bounds → follow user) and reading every consumer off it would collapse all four sites and make the asymmetry impossible.

4. `getSinglePointCoordinate`'s most load-bearing case is undocumented

The doc on utils.ts:129 covers "spans an area" but not "no coordinates at all" — which is the case that keeps the pre-Start map from centring on a garbage coordinate. It works only because getBounds returns Math.min() → Infinity and Math.max() → -Infinity, so the equality check fails. That's an implicit dependency on another function's degenerate output. An explicit early return on empty input (or a sentence in the doc) makes it intentional. Good that MapViewUtilsTest already pins the behavior.

5. Redundant plumbing in the background task

At index.native.ts:78-79, OnyxEntry<T> is T | undefined, so ?? undefined is a no-op and the intermediate gpsDraftDetailsPromiseResult name adds nothing — one line does it. (The pattern is copied from the task body above; no need to extend it.) This is also where the catch-vs-empty conflation I flagged in my regression review lives, so both are worth fixing in one pass.

Smaller nits

Two guards for one rule (repeat from the regression review): gpsPoints.length > 1 at GPSDraftDetailsUtils.ts:144 is dead, since removeLastSegment already no-ops at length <= 1. The rationale comment is duplicated in both places too, which is the same smell showing up in prose.
Import alias: the PR rewrote EditGPSTripButton.tsx:16, so it's a free moment to switch @src/libs/GPSDraftDetailsUtils → @libs/GPSDraftDetailsUtils. The file already uses @libs/ on lines 10-11, and the repo is 3165 files to 27 in favour of @libs/.
Tests: 'mi' as Unit appears in both new test files — CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES types correctly with no cast. The three near-identical GpsDraftDetails factories (makeDraft, stoppedTrip, trackingDraft) could share one fixture, though duplication in tests is explicitly exempt from the duplication standard, so take it or leave it.
useGPSWaypointMarkers now walks the trip twice per render (getGPSWaypoints calls getTrimmedGpsTrip internally, then line 23 calls it again), and the screen calls it a third time for directionCoordinates. Trips are small so this isn't a perf problem — it's the same signal as item 1, that the shape derivation wants to live in one place.

What I liked

Every comment explains the reason, not the mechanics — // A trip discarded during the lookup is gone, and writing its address onto the points read before would bring it back is a model of the style.
Test coverage matches the risk: the discard-during-lookup race is driven with a real held promise (holdAddressLookup) rather than mocked timers, and stopGpsTrip is covered across all four segment shapes including the empty-trip case.
canGpsTripBeTrimmed replacing a raw isTripStopped call at the button site moves a policy decision out of the component and into the utils — right direction.
No magic numbers introduced; both new zoom and distance values come from existing constants.

Next Steps: Reply with @MelvinBot <your feedback> to go deeper on any of these.

view run

mkhutornyi · 2026-09-18T09:43:15Z

+        // Dropping the sole segment would leave no points, which reads as a trip that never started
+        if (gpsPoints.length > 1) {
+            removeLastSegment(gpsPoints);
+        }


So this part is the core fix - restores exactly the guard that #90237 added and #91418 deleted.

The other 6 files

A stopped trip with exactly one point was previously an unreachable state, so everything downstream of isTripStopped now has to handle it. 3 of 4 are genuine consequences; one is scope creep.

Change Verdict

useGPSWaypointMarkers changes replaces the index % 2 === 1 end-marker parity check with "last non-empty segment has >1 point" Needed. Parity assumed every segment yields 2 waypoints. A surviving 1-point segment yields 1, so [[p1],[p2,p3]] (start → stop → resume → move → stop) produced an odd count and lost its end marker. Newly reachable because of the fix.

GPSMapView + MapView/utils changes getSinglePointCoordinate, center at fixed zoom instead of fitBounds Needed. While tracking, shouldFollowUserLocation is true so bounds are ignored. After Stop with 1 waypoint it flips false, and getBounds returns southWest === northEast — fitBounds on a zero-area box max-zooms the camera. Only reachable post-fix.

canGpsTripBeTrimmed + EditGPSTripButton changes hides Edit when distanceInMeters <= 100 Needed, but indirect. Edit was gated on isTripStopped, so it now appears on a 0-distance trip. The distance threshold is a proxy; getTotalGpsTripPoints(...) > 1 says what's actually meant and avoids hiding Edit on a real 2-point trip measuring exactly 100 m.

backgroundLocationTrackingTask.updateStartAddress changes bail out if the draft vanished during reverse geocoding Scope creep, borderline. Pre-existing discard-resurrection race. Arguably newly reachable (DiscardGPSTripButton is also gated on isTripStopped, so "start → stop → discard while the start-point geocode is in flight" only exists after the fix), but it's a distinct bug and belongs in its own PR.

mkhutornyi · 2026-09-18T09:44:33Z

                    defaultSettings={defaultSettings}
-                    centerCoordinate={shouldFollowFallbackLocation ? centerCoordinate : undefined}
+                    centerCoordinate={shouldFollowFallbackLocation ? centerCoordinate : waypointsCenterCoordinate}
+                    zoomLevel={waypointsZoomLevel}


Worth verifying on device

This adds zoomLevel={waypointsZoomLevel} to the Camera - a prop it never had.
During tracking with one point recorded, singlePointCoordinate is truthy, so the Camera now gets centerCoordinate + zoomLevel: 15 while followUserLocation is also true.
rnmapbox should let follow-mode win, but controlled camera props fighting follow-mode has been flaky historically.
Confirm the map still tracks the user normally between the first and second recorded point.

OSBotify · 2026-09-18T15:42:36Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-09-18T15:49:15Z

🚧 AndrewGable has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-09-18T15:56:23Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #101027.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-6062a4b-7288901-96942956bddc3ed0c7ebd4388cfb586cb24ba2d9/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-6062a4b-7288901-0dca4e412f671b5dab721bc89637abf6e891b191/index.html

Web 🕸️
https://101027.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-18T20:19:55Z

🚀 Deployed to staging by https://github.com/AndrewGable in version: 9.4.90-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

MelvinBot · 2026-09-18T20:28:46Z

🤖 Yes, help site changes are required. Draft PR: Docs updates for E/A#101027 GPS trip stopped without moving

Two statements in docs/articles/new-expensify/reports-and-expenses/Distance-Expenses.md no longer match what this PR ships:

The edit FAQ was unconditional. It said you can always tap the pencil Edit icon after Stop. EditGPSTripButton.tsx now renders nothing unless canGpsTripBeTrimmed is true, which requires the trip to be longer than the 100 m location interval.
Stopping without moving was undocumented. That trip now stays on screen with Resume and Save, and Save shows Can't create expense. Nothing on the help site described that state.

What the docs PR changes

Qualified the FAQ Can I edit a GPS trip after I stop tracking? with "as long as the trip is long enough to trim", plus a paragraph explaining the pencil Edit icon is absent on a very short trip.
Added a FAQ, What happens if I tap Stop without moving?, covering the trip staying on screen, Resume and Save, the trash icon, and the Can't create expense message with Got it.
Added keywords for the searches this state creates.

I checked the other GPS-related articles. Create-an-Expense.md lists the GPS steps only through Save and links to Distance Expenses for the rest, so it stays accurate and needed no edit.

Every UI label was verified against the strings the app renders in src/languages/en.ts:10316-10340 — Start, Stop, Resume, Save, Edit stop, Total distance, Can't create expense, Got it. The GPS screens are mobile only, so they can't be opened on web to screenshot. I also couldn't run cspell locally (sandbox blocked it); CI will check spelling on the docs PR.

The PR is a draft, labelled HelpDot, and assigned to you.

@TaduJR, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

TaduJR requested review from a team as code owners September 12, 2026 16:52

melvin-bot Bot assigned TaduJR Sep 12, 2026

melvin-bot Bot requested review from garrettmknight and mkhutornyi and removed request for a team September 12, 2026 16:52