perf: scope reportAttributes error propagation to changed chats#96406
Draft
adhorodyski wants to merge 1 commit into
Draft
perf: scope reportAttributes error propagation to changed chats#96406adhorodyski wants to merge 1 commit into
adhorodyski wants to merge 1 commit into
Conversation
The propagate-errors-to-parent-chat pass walked every report on every compute, even when the incremental pass only touched one report. Keep a module-level chat -> children index (patched from report deltas, rebuilt on full recomputes, same pattern as reportTransactionsAndViolations) and re-evaluate propagation only for chats recomputed in the pass. Every path that enqueues a child already enqueues its parent chat; the one gap (policy-tag updates) is closed by moving the parent-chat enqueue after all branches. A child that is deleted or moves to another chat now re-enqueues the chat it left, fixing a stale error dot that main keeps until the old chat is next recomputed. Untouched errored chats keep their entry by reference instead of being restamped every compute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
The
reportAttributesOnyx derived value recomputes on every change to any of its 14 dependency keys (every chat message, transaction, or violation tick). Its per-report pass is incremental, but the "propagate errors from IOU reports to parent chats" pass that runs after it walked all reports on every compute (building two full maps) and re-stamped every errored chat each pass. On large accounts (30-50k reports) this is a constant O(all reports) tax per tick — the hottest remaining full scan after PRs #95944 and #95931.Fix: a module-level chat→children index (
childToChat/childrenByChat) is rebuilt on full recomputes and patched incrementally fromsourceValuesreport deltas — the same pattern already used by the sibling derived valuereportTransactionsAndViolations.ts(PR #65247). The propagation pass now only re-checks chats present indataToIterateinstead of every report in the account.Safety invariant: every code path that enqueues a child report for recompute must also enqueue its parent chat. This already held for report/transaction/violation updates; the parent-chat enqueue was moved to run after all update branches so it also covers the policy-tags path. A child that is deleted or moves to another chat now re-enqueues the chat it left, so that chat's propagated error is re-evaluated instead of going stale.
Net effect: full recomputes behave exactly as before (
dataToIterate= all reports); single-report/incremental updates on large accounts no longer pay the full-account scan.Fixed Issues
$
PROPOSAL:
Tests
tests/unit/reportAttributesTest.ts: 27/27 passing (21 existing + 6 new covering: parent flagged by errored child; parent stays flagged while a sibling is errored; parent unflagged when all children clear; flag moves when a child moves chats; flag cleared when an errored child is deleted; unrelated errored chat entries carried by reference on single-report updates). 3 of the new tests fail againstmain's implementation — they document the behavior change (stale-dot fix) this PR makes.eslint0 errors (seatbelt budget unchanged at 24 — new tests written cast-free),typecheck-tsgoclean for the changed files.Offline tests
Same unit suite as above — the propagation logic is offline-first client code that runs entirely from local Onyx state, with no network dependency.
QA Steps
Same as tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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