perf: drop fabricated timestamp writes from setOptimisticTransactionThread#88732
Draft
adhorodyski wants to merge 1 commit intoExpensify:mainfrom
Draft
perf: drop fabricated timestamp writes from setOptimisticTransactionThread#88732adhorodyski wants to merge 1 commit intoExpensify:mainfrom
adhorodyski wants to merge 1 commit intoExpensify:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
TLDR: On every transaction-row tap,
setOptimisticTransactionThreadwriteslastReadTime/lastVisibleActionCreatedwithDateUtils.getDBTime()intoreport_${id}. Because that timestamp is monotonic, the merge ALWAYS produces a diff →report_collection subscribers fire → 6 derivations recompute (sortedReportActions,outstandingReportsByPolicyID,visibleReportActions,openAndSubmittedReportsByPolicyID,todos,reportAttributes). Measured cost: 156ms–2685ms per tap, sitting before navigation starts → user-perceived press-to-mount jank.Dropping the two timestamp writes. Structural fields stay; Onyx dedupes them on existing threads → zero cascade.
Why this is safe
Those two lines are armor around an unrelated fix in the same commit (
e84c0ad4dbdin PR #72001). That commit narrowed theCREATED-placeholder gate inReportActionsView.tsx:The gate is the real fix. The timestamp writes were redundant defensive armor. Keeping the gate, dropping the armor.
lastReadTimecontinues to be written post-mount byreadNewestActioninReportActionsList.handleReportChangeMarkAsRead(gated onisUnread() + isVisible + scroll-near-bottom).lastVisibleActionCreatedis action-derived — set by action-creating flows (IOU, Task, etc.) with real action timestamps. Neither needs a nav-time fabrication.Evidence
ff567683672, 2025-08-22) wrote onlyparentReportID+parentReportActionID. No timestamps.e84c0ad4dbd(2025-09-19, "fix optimistic create report transaction") alongside theReportActionsViewgate change. This PR effectively reverts just the timestamp additions from that commit.report_fan-out problem this PR kills at the source.Measured impact (warm transaction thread re-tap, offline, 5 trials, iOS)
ManualOpenReportspan medianFixed Issues
$ #88595
PROPOSAL:
Tests
Offline tests
Covered in QA steps (offline warm-tap is the primary validation path).
QA Steps
Existing transaction thread — LHN unread dot:
MoneyRequestReportView.ReportActionsListmounts (post-nav), not instantly on press. Matches chat-flow behavior.Arrow navigation in RHP (prev/next):
Optimistic thread creation (new, never-navigated thread):
ReportActionsViewgate (unchanged) should still suppress the issue.Sort ordering in LHN after tap:
lastVisibleActionCreated = nowcould cause artificial reordering.Offline warm-tap, same thread 3× in a row (the perf validation):
ManualOpenReportspans andOnyxDerivedCompute_*spans.report_${reportID}merge withhasChanged: truepre-nav; no 6-derivation cascade on taps 2 and 3.Sentry production metric to watch post-merge:
OnyxDerivedCompute_outstandingReportsByPolicyIDp50/p95 for warm usersOnyxDerivedCompute_visibleReportActionsp50/p95ManualOpenReportwithis_transaction_thread: true— watch for p50 dropPR 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari