Skip to content

Improve scanned receipt quality without slowing the shutter - #100592

Merged
Julesssss merged 36 commits into
Expensify:mainfrom
callstack-internal:fix/receipt-capture-quality-99615
Oct 8, 2026
Merged

Julesssss merged 36 commits into
Expensify:mainfrom
callstack-internal:fix/receipt-capture-quality-99615

Conversation

@OlimpiaZurek

@OlimpiaZurek OlimpiaZurek commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

In the common case, portrait with flash off, the app doesn't take a photo. captureReceipt calls takeSnapshot, which saves the frame the preview is already showing, so the receipt misses exposure metering, per-shot focus and multi-frame stacking. That's the quality users are reporting.

Going back to takePhoto for that path would undo the latency win from #84951, #90774.

This keeps both. At the shutter, takeSnapshot runs as it does today and navigation happens on it, so the shutter feels the same. takePhoto runs alongside it, awaited by nothing, and when it lands it's rotated upright and written over the receipt file under the same filename.

The rest is the coordination that makes replacing a file under a live receipt safe:

  • Consumers learn about the upgrade through receiptUpgrades, keyed by durable name.
  • Uploads never wait for it. Payload preparation runs inside the sequential request queue, where waiting for a capture and a rotation would hold up every later write and every read parked on the queue's gate. The upload claims the bytes it is about to send, and an upgrade still preparing a better file gives up rather than rename underneath the read.
  • That leaves one wait: a swap already committed to its two renames. It is tracked apart from the preparation around it, which keeps a directory sweep and a full-image copy off the request path.
  • Only one full-resolution capture runs at a time. Abandoning a capture does not stop it, and vision-camera cannot cancel a takePhoto, so a second one started over a capture still running made the two contend and the next takeSnapshot paid for it. A shutter that would stack captures keeps its snapshot.
  • The camera session stays open until the capture in flight finishes. Closing it on capture cancels takePhoto with Camera is closed.
  • Replace receipt and skip confirmation both dismiss on capture, leaving no camera for the photo to finish in, so both opt out through canUpgradeReceiptQuality.
  • ReceiptStorage gained overwrite, which swaps a receipt's bytes under the same name and puts the original back if the swap fails.
  • The confirmation preview re-renders when an upgrade lands, matching what is on screen to what was uploaded.
  • The receipt keeps its snapshot when the photo fails, misses its 3 second deadline, or an upload claims the receipt first.

Flash and landscape captures already use takePhoto, and get no upgrade. On iOS they now take a 12 MP photo instead of ~3 MP, because the camera format is set once per session( That's about 2% of iOS captures) . Picking 12 MP only when an upgrade can run would reconfigure the session whenever flash or orientation changes. Multi-scan gets no upgrade and is otherwise unchanged.

Also added a telemetry span. ManualReceiptUpgrade records how each attempt ended in upgrade_outcome, with the resolution on the upgraded case, and upgrade_attempted on ManualReceiptCapture

Measurements(debug build)

The upgrade costs 16ms, 2.1%. It does not slow the camera: ManualReceiptCapture stayed flat at 109-118ms

Fixed Issues

$ #99615
#83503

Tests

  1. On Android and iOS, open Create > Scan, hold the phone in portrait with flash off, and capture a receipt.
  2. Verify the receipt is upright and correctly framed, not sideways or upside down.
  3. Stay on the confirmation page and verify the receipt visibly sharpens within a few seconds.
  4. Submit, open the receipt from the report, and verify it's sharper and better exposed than the same scene captured on main.
  5. Capture and submit immediately, before the receipt sharpens. Verify Submit responds with no pause and the expense has a valid receipt.
  6. Capture, return to the camera without waiting, and capture again four or five times. Verify every shutter stays as fast as the first.
  7. Capture with flash on, then again in landscape. Verify both capture and navigate normally and the receipt is correct.
  8. Capture several receipts with multi-scan and submit. Verify it behaves as on main.
  9. Retake a receipt from an existing expense through Edit receipt, and use the skip-confirmation flow. Verify both work with no errors.
  • Verify that no errors appear in the JS console

Offline tests

  1. Turn off the network, capture a receipt and submit.
  2. Verify the expense appears with its receipt.
  3. Reconnect and verify the expense uploads with a receipt that opens correctly.

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 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 shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • 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)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • 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)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • 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 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.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • 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

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@codex

@codecov

codecov Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/CONST/index.ts 95.86% <ø> (ø)
...ationListFooter/hooks/useReceiptThumbnailSource.ts 100.00% <100.00%> (ø)
src/hooks/useLocalReceiptThumbnail.ts 88.63% <100.00%> (+7.68%) ⬆️
src/hooks/useReceiptUpgradeCount.ts 100.00% <100.00%> (ø)
...s/fileDownload/validateReceiptFile/index.native.ts 80.00% <100.00%> (+80.00%) ⬆️
src/libs/getImageRecyclingKey.ts 100.00% <100.00%> (ø)
src/libs/prepareRequestPayload/index.native.ts 67.30% <100.00%> (+2.72%) ⬆️
...epScan/components/NavigationAwareCamera/Camera.tsx 100.00% <100.00%> (+83.33%) ⬆️
.../IOURequestStepScan/components/ScanEditReceipt.tsx 91.66% <ø> (ø)
...equestStepScan/components/ScanSkipConfirmation.tsx 91.17% <ø> (ø)
... and 9 more
... and 25 files with indirect coverage changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0ce58a8b6

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

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 858832d8f4

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

Comment thread src/libs/ReceiptStorage/index.native.ts Resolved
@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95ad1748f0

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

Comment thread src/libs/ReceiptStorage/index.native.ts Resolved
@staszekscp staszekscp mentioned this pull request Sep 9, 2026
40 tasks
@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5a8fcd368

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

Comment thread src/libs/ReceiptStorage/index.native.ts Resolved
@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bb309633fe

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

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@codex review

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@MelvinBot review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a5679ec4e

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

MelvinBot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

🤖 Code review — the approach (fire-and-forget takePhoto, swap in place under the same durable name) is sound and the atomic-swap + interrupted-swap recovery in ReceiptStorage is genuinely well built. Three things I'd want resolved before merge, then a list of smaller ones.

1. The replace-receipt flow probably never gets the upgrade — but still pays the delay

ScanEditReceipt.tsx:99 calls navigateBack() synchronously, which pops the scan screen. shouldStayActiveWhenBlurred only saves a screen that stays mounted — which is the global-create case, where you navigate forward. A popped screen unmounts <Camera>, which closes the session, which is exactly what cancels takePhoto with Camera is closed.

So on the one flow you added RECEIPT_UPGRADE_WAIT_MS for, the still likely loses a race with the pop animation. Can you confirm on device? The Tests section has no replace-receipt step, and nothing in the unit tests covers it.

2. Deferring replaceReceipt can silently lose the replacement

setMoneyRequestReceipt writes the new source into the transaction immediately, but replaceReceipt — which sets pendingFields.receipt and enqueues the API request — is now held back for up to 10s. In that window:

  • there's no pending indicator, so the UI shows a replaced receipt with nothing in flight;
  • if the app is killed or crashes, the request was never queued. Local state shows a receipt the server doesn't have, with no error and no retry path.

Suggestion: keep the action synchronous and do the waiting where the file is actually read — the key === 'file' branch of prepareRequestPayload/index.native.ts, mirroring what the receipt branch now gets from locate. That preserves the optimistic write and the offline queue entry.

Separately, that .then() has no .catch().

3. locate()'s 10s wait stalls the whole sequential queue

prepareRequestPayload is awaited by HttpUtils.xhr (HttpUtils.ts:259), and the SequentialQueue awaits one request at a time. So UPGRADE_WAIT_TIMEOUT_MS doesn't just delay the receipt upload — it blocks every other queued write behind it. Worst case is 10s of frozen writes for a capture that was only ever supposed to be a quality nicety. Given the capture deadline is 3s, is 10s the right cap here?

Smaller things

startPhotoCapture overwrites pendingPhotoRef without discarding the previous pending photo — usePhotoUpgrade.ts:150. That's exactly the two-in-flight case the captureCountRef comment anticipates, so if a second shutter fires before the first is consumed, the first temp file leaks. startSpan(SPAN_RECEIPT_PHOTO_UPGRADE) is keyed by name too, so the second capture orphans the first span and the outcome attribute lands on the wrong one.

Rotated temp file leaks when overwrite fails — the catch in upgradeReceiptWithPhoto discards photo.path, but not uprightPath when the rotate succeeded and only the swap failed.

upgradeCounts never shrinks — receiptUpgrades.ts grows one entry per upgraded receipt for the life of the process. Small, but unbounded.

No sweep for .staged / .backup leftovers — if the app dies after the final rename but before discardLeftover(backupPath), a full-size .backup sits in the receipts folder forever. Nothing enumerates that folder to clean it, and anything that later does would see it as a receipt.

getUprightRotation always assumes 90° clockwise when the decoded frame is landscape (rotatePhotoToUpright.ts:38). PhotoFile.orientation distinguishes landscape-left from landscape-right; guessing 90 puts the opposite-sensor case 180° out. Did you test both landscape holds?

Shutter sound — enableShutterSound: false is honoured on most devices but forced on in JP/KR. Today the snapshot path is silent; adding takePhoto alongside it would give those users an audible shutter they didn't have. Worth a check.

#upgradedN fragment on a file:// URI — this relies on the image loader ignoring the fragment when opening the file while keeping it in the cache key. Confirmed on iOS as well as Android?

Coverage — Codecov flags drops in ScanEditReceipt.tsx, CameraViewport.tsx and NavigationAwareCamera/Camera.tsx, which are the three files carrying the new lifecycle logic. useReceiptUpgradeCount and the thumbnail cache-bust have no tests either.

PR body — all five Screenshots/Videos sections are empty despite the checklist being fully ticked, and the Tests list is misnumbered (1, 2, 4, 5, 6).

What I verified and what I didn't

Read the full diff plus surrounding code on 8a5679ec4e: the capture handler, usePhotoUpgrade, receiptUpgrades, the native ReceiptStorage swap, prepareRequestPayload, HttpUtils.xhr, replaceReceipt, and the thumbnail hooks.

One thing I checked and found fine: I expected the #upgradedN cache-bust to serve a stale thumbnail, since thumbnailCache is keyed on the unchanged sourceUri. It doesn't — precacheReceiptImage maps sourceUri → sourceUri for the camera path, so the busted URI points at the real receipt file, and the generateThumbnail fallback only runs on the gallery path where upgradeCount is 0.

I did not exercise this on a device. Points 1, 4 (shutter sound) and the fragment question all need a device to settle. I tried a web smoke test of the shared hook changes, but the browser session went inert before it could run, and file upload isn't supported on that harness anyway — so treat this as a code review only, with no runtime verification behind it.


view run · no recording available

@mkhutornyi

Copy link
Copy Markdown
Contributor

From AI analysis:

1. Scanned receipts are less detailed on iPhone when you submit straight away

iOS only. Needs this build and the current production App Store build.

  1. Take a receipt with small print (a long itemised grocery receipt is ideal). Keep it flat under steady, even light and don't move it between runs.
  2. On the production app, tap + → Scan.
  3. Frame the whole receipt and tap the shutter.
  4. When the confirmation screen opens, tap Submit immediately, without pausing.
  5. Open the new expense, tap the receipt to open it full screen, pinch to zoom into the smallest line of print, screenshot.
  6. Repeat steps 2–5 on this build, same receipt, distance and lighting.
  7. Compare the two zoomed screenshots.

Expected: small print equally legible in both builds.
Actual: softer and harder to read on this build.

Submitting immediately is required: the fast capture is uploaded before the full-resolution replacement lands, and it is the fast capture whose quality dropped. Pausing on the confirmation screen hides it. Scanning two receipts in quick succession also triggers it.

Camera/index.native.tsx:87-100 — photoResolution carries priority 2 and videoResolution only 1 (getCameraFormat.ts:94, priority: filters.length - index), so the new 12 MP iOS still target decides the format and the snapshot inherits that format's video size. On iOS takeSnapshot returns a video frame, so the snapshot resolution moves with it.

Fix: keep the iOS photoResolution target at the 6 MP value, or order videoResolution ahead of photoResolution so the snapshot size is the one that is pinned.


2. With rotation lock on, a sideways-held receipt is saved on its side

Both platforms. Needs this build and the current production build.

  1. Turn on screen rotation lock. iPhone: Control Centre, padlock-with-arrow. Android: Quick Settings, Auto-rotate off.
  2. Confirm it works — turning the phone no longer rotates the app.
  3. On the production app, tap + → Scan, then tap the flash icon to turn the flash on.
  4. Turn the phone sideways while the app stays upright, and frame a wide receipt filling the sideways view.
  5. Tap the shutter, then open the receipt full screen.
  6. Repeat steps 3–5 on this build.

Expected: receipt upright and readable, as on production.
Actual: receipt saved rotated a quarter turn.

On iPhone this also happens with the flash off. Turning rotation lock off makes it go away on both platforms.

Camera/index.native.tsx:270-276 — outputOrientation="preview" ties every capture to the screen. vision-camera's own doc for the prop: "If the device rotation is locked, the user cannot take photos or videos in different orientations." The default "device" follows the accelerometer, so production captures upright here.

Fix: revert to the default, or, if the trade-off against the flat-hold case is intended, say so in the comment — rotation lock is a common setting and this is the scenario it breaks.


3. After ~50 scans in one sitting, early receipts go back to looking blurry

One continuous session. Don't force-quit or restart the app, or the counter resets.

  1. Tap + → Scan.
  2. Photograph a receipt with visible small print.
  3. On the confirmation screen, wait until the picture visibly sharpens, or about 5 seconds. Do not tap Submit.
  4. Tap back to return to the camera.
  5. Repeat steps 2–4 another 50 times, 51 scans in total. The same physical receipt is fine.
  6. Open the first receipt you scanned, full screen, and zoom in.

Expected: still the sharp version from step 3.
Actual: back to the blurry capture, although the sharp picture is still on the device.

Tapping Submit, or backing out before the picture sharpens, cancels the improvement for that receipt so it won't count toward the 51.

receiptUpgrades.ts:47-48 — the cap evicts keys().next().value, and Map.set on an existing key does not reorder, so eviction is insertion-order rather than least-recently-used and a receipt still on screen can go first. Its count drops to 0, the plain URL comes back, and expo-image serves the pre-swap bytes it cached at first load.

Fix: delete then set the key in recordUpgrade so the map stays in recency order.

@mkhutornyi

Copy link
Copy Markdown
Contributor

@OlimpiaZurek Can you please try to reproduce #100592 (comment) on your side?

Bug #2 is true on android orientation-locked, flash-on but not blocker since already happening on main when flash-off.

@mkhutornyi

Copy link
Copy Markdown
Contributor

Bug: scan expense is not created. stuck on confirmation screen after tapping "Create expense" button.

Not consistently reproducible.

bug.mp4

@mkhutornyi

Copy link
Copy Markdown
Contributor

When add more receipt to existing scan expense, bottom view is cut off.
This also happens on production so not blocker. Worth creating separate issue and fix.

Screenshot 2026-10-07 at 10 38 51 AM Screenshot 2026-10-07 at 10 38 59 AM
bug.mp4

@MelvinBot

Copy link
Copy Markdown
Contributor

Final review

Verdict: No blockers. The claim/swap coordination holds up, CI is green, and earlier review threads are resolved. I recommend one fix before merge (1) and a few small cleanups.

Regressions

  1. Crop or rotate on a fresh draft can use the wrong pixels (medium-low). ReceiptCropView measures the image size once per URI. If the upgrade lands after the user opens crop, saveCrop applies a rect measured on the snapshot to the larger photo, so the saved crop covers only part of the receipt. Rotate and crop can also read the file between the two renames. Before this PR, the bytes behind a durable name never changed. Fix: call ReceiptStorage.settle(name) before you enter crop or rotate, so the receipt is claimed and any committed swap finishes first.
  2. outputOrientation="preview" also applies to the flows that opt out (low). Camera/index.native.tsx:273-276 changes multi-scan, Replace receipt and skip-confirmation too. A user with portrait rotation lock who turns the phone sideways for a wide receipt now gets a sideways image, where device used to return it upright. If that trade-off is intended, please add it to the PR description.
  3. The iOS 12 MP format reaches more flows than the description says (low). Camera/index.native.tsx:92-100 also gives multi-scan and Replace receipt 4032x3024 photos when flash is on or the phone is in landscape. So "multi-scan is otherwise unchanged" isn't quite accurate. Video resolution now only breaks ties, so please confirm on a device that the iOS snapshot (still the fallback for every receipt that isn't upgraded) didn't drop below 2016x1512.

Code quality

  • Free native image memory in rotatePhotoToUpright.ts:44-47. Neither the manipulator context nor the rendered image is released. On a 12 MP iPhone photo that holds about 2×48 MB until GC runs, which can't see native memory. processPickedAssets/index.native.ts already releases both with releaseQuietly, so follow that pattern.
  • ReceiptStorage/types.ts deletes the existing JSDoc, including the type summary and the "Valid for this launch only, so never store the result" warning on toLocalUri. The six new members have no docs. Please restore the old comments and document overwrite, locate, settle and recheckAfterSwap, since how they wait and claim isn't obvious.
  • Dead export. isUpgrading in receiptUpgrades.ts:76-78 is only used in tests.
  • Misleading telemetry. In usePhotoUpgrade.ts:285-290, a rotate failure that isn't a timeout (for example, ImageSize.getSize rejects) is reported as swap_failed. That will skew the upgrade_outcome data.
Checked and sound
  • Claim vs swap: nothing is awaited between shouldAbort() and committedSwaps.set, and settle claims and looks up synchronously, so no upload can read mid-rename.
  • The upgrade is registered before onCapture. finishUpgrade runs in .finally. Every step before overwrite has a deadline. Temp files (photo, rotated copy, staged, backup) are cleaned up on every path I traced.
  • prepareRequestPayload only adds a wait for a committed two-rename swap. It never waits for capture or rotation, so the request queue isn't held.
  • getImageRecyclingKey: no other caller builds a # fragment, so stripping it has no effect outside receipts.
  • Web ReceiptStorage stubs keep web behavior unchanged. The odometer camera keeps the CameraViewport defaults.
  • The ManualReceiptUpgrade span ends on every path.
  • Very unlikely edge case: if the second rename and its restore both fail while an upload is waiting, locate doesn't retry the backup, so that request goes out without a receipt (index.native.ts:98-101). It takes two filesystem failures in a row, so it's not worth blocking on.

I didn't test this in the app. The change is native-only, and this run could only test on web.

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

Not blocker but there's tiny difference on preview on cofirmation screen:

android.mp4
diff.mov

I was able to reproduce it, but not consistently. It shows up when the sharper photo replaces the snapshot. On Android the snapshot is a screenshot of the camera preview, and the full photo frames the scene slightly differently, so the same receipt sits a few pixels off. It happens once per scan, and the image stays still afterwards. I tried expo-image's transition to soften it, but on Android it fades in from blank, which looked worse, so I left it as it is.

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

@OlimpiaZurek Can you please try to reproduce #100592 (comment) on your side?

Bug #2 is true on android orientation-locked, flash-on but not blocker since already happening on main when flash-off.

Yes, that's the trade-off of outputOrientation="preview". It fixes the much more common case of a phone held flat over a receipt being saved sideways, which "device" gets wrong.

@OlimpiaZurek

Copy link
Copy Markdown
Contributor Author

Bug: scan expense is not created. stuck on confirmation screen after tapping "Create expense" button.

Not consistently reproducible.

bug.mp4

I couldn't reproduce it on my side. I'm also not sure it's related to these changes, since this PR doesn't touch the submit or location flow.

@mkhutornyi

Copy link
Copy Markdown
Contributor

Looks like no blockers remaining.
But still be on a look out for deploy blockers that will be caught in staging QA

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

This comment has been minimized.

@Julesssss
Julesssss merged commit b9626f2 into Expensify:main Oct 8, 2026
37 of 38 checks passed
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

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

@OSBotify OSBotify mentioned this pull request Oct 9, 2026
74 tasks done
@OSBotify

OSBotify commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/Julesssss in version: 9.5.7-0 🚀

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

@MelvinBot

Copy link
Copy Markdown
Contributor

No help site update needed. This PR improves the quality of scanned receipt photos in the background without changing any step, label, or workflow, so no sentence in an existing article is now incorrect.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.5.7-3 🚀

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

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants