[Payment due @DylanDylann] [Odometer] Fix race condition#90445
[Payment due @DylanDylann] [Odometer] Fix race condition#90445jakubkalinski0 wants to merge 27 commits into
Conversation
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.
|
|
@carlosmiceli 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] |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@carlosmiceli Sorry, @DylanDylann will take care of review here |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@jakubkalinski0 Please add issue link too |
trjExpensify
left a comment
There was a problem hiding this comment.
Re-approving from product. 👍
Julesssss
left a comment
There was a problem hiding this comment.
Nice progress, generating a build for testing while the last review comments are resolved
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
@jakubkalinski0 Could you also adjust the position and style of the stitching error? I think it should be below the receipt
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-05-25.at.11.23.06.movAndroid: mWeb ChromeScreen.Recording.2026-05-25.at.11.31.08.moviOS: HybridAppScreen.Recording.2026-05-25.at.11.31.54.moviOS: mWeb SafariScreen.Recording.2026-05-25.at.11.33.28.movMacOS: Chrome / SafariScreen.Recording.2026-05-25.at.11.18.22.mov |
Good catch! |
|
🎯 @DylanDylann, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
Julesssss
left a comment
There was a problem hiding this comment.
One minor comment for reveiw

@DylanDylann
Explanation of Change
Refactor of the odometer receipt subsystem that supersedes the original bandaid fix. Previously,
useOdometerDraftHydrator,OdometerReceiptStitcher, andReceiptFileValidatorcould collide ontransaction.receipt: the validator finishing its check against an old source right after the stitcher wrote a new one would clear the fresh receipt. The earlier fix added self-healing refs (lastWrittenReceiptSource, stale-detection in the validator). This PR replaces that entanglement with composition:useOdometerReceiptStitcherhook composes the existinguseRestartOnOdometerImagesFailureinternally and owns only the stitch derivation as a small FSM (idle | stitching | ready | error). Verification + recovery logic is not duplicated - it stays inuseRestartOnOdometerImagesFailure.<ReceiptFileValidator />gains a flow-agnosticisReceiptReadyprop and skips the initial row while an upstream writer is mid-derivation. The bandaidisOdometerInitial && hasLiveOdometerImagebranch is removed, and so are the validator's earlier self-healing ref + staleness check - the standard Reactignoreguard handles the stale-async case directly.<OdometerReceiptStitcher />component is deleted - its responsibilities live inside the new hook.deriveOdometerReceipt(empty/single/stitch decision) andstitchTask(AbortController-cancellable canvas op with balanced telemetry span).useRestartOnOdometerImagesFailuredirectly - they need verification + recovery without receipt writes.Architecturally the two
useRestart*hooks are now siblings (useRestartOnReceiptFailurefor generic scan flows,useRestartOnOdometerImagesFailurefor odometer). Each receipt-derivation responsibility has a single owner.Diff overview
13 files changed · +946 / −151 - breakdown per cea8f39 commit
Tests vs. non-test source
So more than half of the diff is tests (35 new unit tests with descriptive setup helpers and §N matrix mocking).
Inside the 351 non-test insertions
useOdometerReceiptStitcher/index.ts(NEW)useOdometerReceiptStitcher/types.ts(NEW)stitchTask.ts(NEW)deriveOdometerReceipt.ts(NEW)OdometerReceipt/index.ts(NEW barrel)ReceiptFileValidator.tsx(modified)IOURequestStepConfirmation.tsx(modified)useRestartOnOdometerImagesFailure/index.ts(modified)useOdometerDraftHydrator.ts(modified)The hook is a single cross-platform file - no
.native.tsvariant. Platform differences are pushed down into the libs it composes:useRestartOnOdometerImagesFailurehas its own native stub that short-circuits blob verification (no blob: URLs on native), andstitchOdometerImagesalready has a Skia-based.native.tsimplementation. Adding an entry-point platform split here would have re-broken stitching on iOS/Android.Summary
Deletions (151 lines)
OdometerReceiptStitcher.tsx(DELETED)ReceiptFileValidator.tsx(bandaid branch + staleness check)IOURequestStepConfirmation.tsx(component JSX + locals + useState)useRestartOnOdometerImagesFailure/index.tsuseOdometerDraftHydrator.tsBottom line: real new logic added ~228 lines; real logic removed ~151 lines. Net new "real" logic: roughly +77 lines - mostly the FSM reducer + composition glue inside
useOdometerReceiptStitcher. The rest is testing + JSDoc infrastructure.Fixed Issues
$ #90819
PROPOSAL: N/A
Tests
Distancefield to enter edit mode.Saveto return to the confirmation step.Regression tests (optional)
Draft expense
+global create buttonPhoto replacement
+global create buttonClear draft data
+global create buttonBrowser refresh on confirmation (web only - Race A scenario)
ODOMETER_DRAFT), or - if no draft exists - that you are navigated back to the start of the odometer flow exactly once with no console errorsMulti-receipt split with mixed odometer + manual rows (Race B scenario)
Offline tests
Same as Tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
input.webm
Android: mWeb Chrome
input1.webm
iOS: Native
iOS: mWeb Safari
output.mp4
MacOS: Chrome / Safari
Screen.Recording.2026-05-21.at.22.37.38.mov