From 6691e522057f36facd1478e134074e24e5bf059e Mon Sep 17 00:00:00 2001 From: Issa Nimaga Date: Wed, 29 Apr 2026 10:53:34 +0900 Subject: [PATCH 01/26] Add Concierge draft streaming infrastructure Squashed extraction of the foundation from https://github.com/Expensify/App/pull/87110 (`issa/add-concierge-draft-streaming`): adds Pusher draft event types, the per-report ConciergeDraftContext, the conciergeDraftState reducer that builds a synthetic ReportAction from streaming events, the synthetic-action splice in ReportActionsList, and the ReportScreen wrapping. Reconciles Issa's branch with main's actionIndexMap performance optimization (now built from renderedVisibleReportActions so the synthetic-bubble index resolves correctly during a Concierge trickle). Used here only to back the client-side trickle of pregenerated Concierge responses (#626938). Issa's PR will rebase on top once it merges. Co-authored-by: Issa Nimaga --- src/libs/Pusher/EventType.ts | 5 + src/libs/Pusher/types.ts | 20 +++ src/pages/inbox/ConciergeDraftContext.tsx | 132 +++++++++++++++++ src/pages/inbox/ReportScreen.tsx | 17 ++- src/pages/inbox/conciergeDraftState.ts | 85 +++++++++++ src/pages/inbox/report/ReportActionsList.tsx | 80 ++++++++--- .../pages/inbox/conciergeDraftState.test.ts | 134 ++++++++++++++++++ 7 files changed, 447 insertions(+), 26 deletions(-) create mode 100644 src/pages/inbox/ConciergeDraftContext.tsx create mode 100644 src/pages/inbox/conciergeDraftState.ts create mode 100644 tests/unit/pages/inbox/conciergeDraftState.test.ts diff --git a/src/libs/Pusher/EventType.ts b/src/libs/Pusher/EventType.ts index 2298662040d6..2e59fa68f5aa 100644 --- a/src/libs/Pusher/EventType.ts +++ b/src/libs/Pusher/EventType.ts @@ -9,6 +9,11 @@ export default { USER_IS_TYPING: 'client-userIsTyping', MULTIPLE_EVENTS: 'multipleEvents', CONCIERGE_REASONING: 'conciergeReasoning', + CONCIERGE_DRAFT_STARTED: 'conciergeDraftStarted', + CONCIERGE_DRAFT_UPDATED: 'conciergeDraftUpdated', + CONCIERGE_DRAFT_COMPLETED: 'conciergeDraftCompleted', + CONCIERGE_DRAFT_FAILED: 'conciergeDraftFailed', + CONCIERGE_DRAFT_CLEARED: 'conciergeDraftCleared', // An event that the server sends back to the client in response to a "ping" API command PONG: 'pong', diff --git a/src/libs/Pusher/types.ts b/src/libs/Pusher/types.ts index ad9fa1d4f154..b7cf87d69e7e 100644 --- a/src/libs/Pusher/types.ts +++ b/src/libs/Pusher/types.ts @@ -39,11 +39,30 @@ type ConciergeReasoningEvent = { loopCount: number; }; +type ConciergeDraftEvent = { + reportID: string; + reportActionID: string; + streamSessionID: string; + sequence: number; + status: 'started' | 'updated' | 'completed' | 'failed' | 'cleared'; + created: string; + bodyMarkdown?: string; + finalRenderedHTML?: string; + startedAt?: string; + terminalReason?: string; + updatedAt?: string; +}; + type PusherEventMap = { [TYPE.USER_IS_TYPING]: UserIsTypingEvent; [TYPE.USER_IS_LEAVING_ROOM]: UserIsLeavingRoomEvent; [TYPE.PONG]: PingPongEvent; [TYPE.CONCIERGE_REASONING]: ConciergeReasoningEvent; + [TYPE.CONCIERGE_DRAFT_STARTED]: ConciergeDraftEvent; + [TYPE.CONCIERGE_DRAFT_UPDATED]: ConciergeDraftEvent; + [TYPE.CONCIERGE_DRAFT_COMPLETED]: ConciergeDraftEvent; + [TYPE.CONCIERGE_DRAFT_FAILED]: ConciergeDraftEvent; + [TYPE.CONCIERGE_DRAFT_CLEARED]: ConciergeDraftEvent; }; type EventData = {chunk?: string; id?: string; index?: number; final?: boolean} & (EventName extends keyof PusherEventMap @@ -103,6 +122,7 @@ export type { UserIsLeavingRoomEvent, PingPongEvent, ConciergeReasoningEvent, + ConciergeDraftEvent, EventData, EventCallbackError, ChunkedDataEvents, diff --git a/src/pages/inbox/ConciergeDraftContext.tsx b/src/pages/inbox/ConciergeDraftContext.tsx new file mode 100644 index 000000000000..977ca7928486 --- /dev/null +++ b/src/pages/inbox/ConciergeDraftContext.tsx @@ -0,0 +1,132 @@ +import {getReportChatType} from '@selectors/Report'; +import React, {createContext, useCallback, useContext, useEffect, useMemo, useState} from 'react'; +import useOnyx from '@hooks/useOnyx'; +import {getReportChannelName} from '@libs/actions/Report'; +import Log from '@libs/Log'; +import Pusher from '@libs/Pusher'; +import type {ConciergeDraftEvent} from '@libs/Pusher/types'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {ReportAction} from '@src/types/onyx'; +import type {ConciergeDraft} from './conciergeDraftState'; +import {applyConciergeDraftEvent} from './conciergeDraftState'; + +type ConciergeDraftState = { + draftReportAction: ReportAction | null; + hasActiveDraft: boolean; +}; + +type ConciergeDraftActions = { + clearDraft: () => void; +}; + +const defaultState: ConciergeDraftState = { + draftReportAction: null, + hasActiveDraft: false, +}; + +const defaultActions: ConciergeDraftActions = { + clearDraft: () => {}, +}; + +const ConciergeDraftStateContext = createContext(defaultState); +const ConciergeDraftActionsContext = createContext(defaultActions); + +function ConciergeDraftProvider({reportID, children}: React.PropsWithChildren<{reportID: string | undefined}>) { + const [chatType] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {selector: getReportChatType}); + const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID); + const isConciergeChat = reportID === conciergeReportID; + const isAdmin = chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS; + const isAgentZeroChat = isConciergeChat || isAdmin; + + if (!reportID || !isAgentZeroChat) { + return children; + } + + return ( + + {children} + + ); +} + +function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{reportID: string}>) { + const [draft, setDraft] = useState(null); + + const clearDraft = useCallback(() => { + setDraft(null); + }, []); + + useEffect(() => { + const channelName = getReportChannelName(reportID); + const handleResubscribe = () => { + clearDraft(); + }; + const eventTypes = [ + Pusher.TYPE.CONCIERGE_DRAFT_STARTED, + Pusher.TYPE.CONCIERGE_DRAFT_UPDATED, + Pusher.TYPE.CONCIERGE_DRAFT_COMPLETED, + Pusher.TYPE.CONCIERGE_DRAFT_FAILED, + Pusher.TYPE.CONCIERGE_DRAFT_CLEARED, + ] as const; + + const subscriptions = eventTypes.map((eventType) => { + const listener = Pusher.subscribe( + channelName, + eventType, + (eventData) => { + const conciergeDraftEvent = eventData as ConciergeDraftEvent; + setDraft((currentDraft) => applyConciergeDraftEvent(currentDraft, conciergeDraftEvent, reportID)); + }, + handleResubscribe, + ); + + listener.catch((error: unknown) => { + Log.hmmm('Failed to subscribe to Pusher concierge draft events', {eventType, reportID, error}); + }); + + return listener; + }); + + return () => { + for (const subscription of subscriptions) { + subscription.unsubscribe(); + } + }; + }, [clearDraft, reportID]); + + const stateValue = useMemo( + () => ({ + draftReportAction: draft?.reportAction ?? null, + hasActiveDraft: !!draft?.reportAction, + }), + [draft?.reportAction], + ); + + const actionsValue = useMemo( + () => ({ + clearDraft, + }), + [clearDraft], + ); + + return ( + + {children} + + ); +} + +function useConciergeDraft(): ConciergeDraftState { + return useContext(ConciergeDraftStateContext); +} + +function useConciergeDraftActions(): ConciergeDraftActions { + return useContext(ConciergeDraftActionsContext); +} + +export {ConciergeDraftProvider, useConciergeDraft, useConciergeDraftActions}; +export type {ConciergeDraftState, ConciergeDraftActions}; diff --git a/src/pages/inbox/ReportScreen.tsx b/src/pages/inbox/ReportScreen.tsx index c543b3e91f65..33b7b1ca871f 100644 --- a/src/pages/inbox/ReportScreen.tsx +++ b/src/pages/inbox/ReportScreen.tsx @@ -21,6 +21,7 @@ import CONST from '@src/CONST'; import SCREENS from '@src/SCREENS'; import AccountManagerBanner from './AccountManagerBanner'; import {AgentZeroStatusProvider} from './AgentZeroStatusContext'; +import {ConciergeDraftProvider} from './ConciergeDraftContext'; import DeleteTransactionNavigateBackHandler from './DeleteTransactionNavigateBackHandler'; import LinkedActionNotFoundGuard from './LinkedActionNotFoundGuard'; import ReactionListWrapper from './ReactionListWrapper'; @@ -143,13 +144,15 @@ function ReportScreen({route, navigation}: ReportScreenProps) { {!shouldDeferNonEssentials && } - - - {shouldDeferNonEssentials ? : } - + + + + {shouldDeferNonEssentials ? : } + + diff --git a/src/pages/inbox/conciergeDraftState.ts b/src/pages/inbox/conciergeDraftState.ts new file mode 100644 index 000000000000..ecf078571dc3 --- /dev/null +++ b/src/pages/inbox/conciergeDraftState.ts @@ -0,0 +1,85 @@ +import Parser from '@libs/Parser'; +import type {ConciergeDraftEvent} from '@libs/Pusher/types'; +import {getParsedComment} from '@libs/ReportUtils'; +import CONST from '@src/CONST'; +import type {ReportAction} from '@src/types/onyx'; + +type ConciergeDraft = { + reportAction: ReportAction; + sequence: number; + status: ConciergeDraftEvent['status']; + streamSessionID: string; + terminalReason?: string; +}; + +type BuildConciergeDraftReportActionParams = { + bodyMarkdown?: string; + created: string; + finalRenderedHTML?: string; + reportActionID: string; + reportID: string; +}; + +function buildConciergeDraftReportAction({bodyMarkdown, created, finalRenderedHTML, reportActionID, reportID}: BuildConciergeDraftReportActionParams): ReportAction | null { + const html = finalRenderedHTML ?? (bodyMarkdown ? getParsedComment(bodyMarkdown, {reportID}) : ''); + + if (!html) { + return null; + } + + return { + reportActionID, + reportID, + actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, + actorAccountID: CONST.ACCOUNT_ID.CONCIERGE, + person: [{style: 'strong', text: CONST.CONCIERGE_DISPLAY_NAME, type: 'TEXT'}], + created, + message: [{type: CONST.REPORT.MESSAGE.TYPE.COMMENT, html, text: Parser.htmlToText(html)}], + originalMessage: {html, whisperedTo: []}, + shouldShow: true, + } as ReportAction; +} + +function applyConciergeDraftEvent(currentDraft: ConciergeDraft | null, event: ConciergeDraftEvent, reportID: string): ConciergeDraft | null { + if (event.reportID !== reportID) { + return currentDraft; + } + + const isSameStreamSession = currentDraft?.streamSessionID === event.streamSessionID; + + if (isSameStreamSession && event.sequence <= currentDraft.sequence) { + return currentDraft; + } + + if (!isSameStreamSession && currentDraft && event.status !== 'started' && event.status !== 'updated') { + return currentDraft; + } + + if (event.status === 'failed' || event.status === 'cleared') { + return isSameStreamSession ? null : currentDraft; + } + + const nextReportAction = + buildConciergeDraftReportAction({ + bodyMarkdown: event.bodyMarkdown, + created: event.created, + finalRenderedHTML: event.finalRenderedHTML, + reportActionID: event.reportActionID, + reportID: event.reportID, + }) ?? currentDraft?.reportAction; + + if (!nextReportAction) { + return currentDraft; + } + + return { + reportAction: nextReportAction, + sequence: event.sequence, + status: event.status, + streamSessionID: event.streamSessionID, + terminalReason: event.terminalReason, + }; +} + +export {applyConciergeDraftEvent, buildConciergeDraftReportAction}; +export type {ConciergeDraft}; diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx index 0b48a323e6b9..fbad8ce9ea3b 100644 --- a/src/pages/inbox/report/ReportActionsList.tsx +++ b/src/pages/inbox/report/ReportActionsList.tsx @@ -10,6 +10,7 @@ import {renderScrollComponent as renderActionSheetAwareScrollView} from '@compon import Button from '@components/Button'; import InvertedFlashList from '@components/FlashList/InvertedFlashList'; import getShowScrollIndicator from '@components/FlashList/InvertedFlashList/getShowScrollIndicator'; +import {AUTOSCROLL_TO_TOP_THRESHOLD} from '@components/FlatList/hooks/useFlatListScrollKey'; import {usePersonalDetails} from '@components/OnyxListItemProvider'; import ReportActionsSkeletonView from '@components/ReportActionsSkeletonView'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; @@ -37,6 +38,8 @@ import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types'; import { getFirstVisibleReportActionID, + getReportActionMessage, + getSortedReportActions, isConsecutiveActionMadeByPreviousActor, isCurrentActionUnread, isDeletedParentAction, @@ -64,6 +67,7 @@ import { import Visibility from '@libs/Visibility'; import type {ReportsSplitNavigatorParamList} from '@navigation/types'; import ConciergeThinkingMessage from '@pages/home/report/ConciergeThinkingMessage'; +import {useConciergeDraft, useConciergeDraftActions} from '@pages/inbox/ConciergeDraftContext'; import {ActionListContext} from '@pages/inbox/ReportScreenContext'; import variables from '@styles/variables'; import {openReport, readNewestAction, subscribeToNewActionEvent} from '@userActions/Report'; @@ -187,6 +191,8 @@ function ReportActionsList({ const route = useRoute>(); const reportScrollManager = useReportScrollManager(); const {scrollOffsetRef} = useContext(ActionListContext); + const {draftReportAction, hasActiveDraft} = useConciergeDraft(); + const {clearDraft} = useConciergeDraftActions(); const userActiveSince = useRef(DateUtils.getDBTime()); const lastMessageTime = useRef(null); const [isVisible, setIsVisible] = useState(Visibility.isVisible); @@ -216,7 +222,18 @@ function ReportActionsList({ const isTransactionThreadReport = useMemo(() => isTransactionThread(parentReportAction) && !isSentMoneyReportAction(parentReportAction), [parentReportAction]); const isMoneyRequestOrInvoiceReport = useMemo(() => isMoneyRequestReport(report) || isInvoiceReport(report), [report]); const shouldFocusToTopOnMount = useMemo(() => isTransactionThreadReport || isMoneyRequestOrInvoiceReport, [isMoneyRequestOrInvoiceReport, isTransactionThreadReport]); - const topReportAction = sortedVisibleReportActions.at(-1); + const renderedVisibleReportActions = useMemo(() => { + if (!draftReportAction || sortedVisibleReportActions.some((action) => action.reportActionID === draftReportAction.reportActionID)) { + return sortedVisibleReportActions; + } + + return getSortedReportActions([...sortedVisibleReportActions, draftReportAction], true); + }, [draftReportAction, sortedVisibleReportActions]); + const draftMessageHTML = draftReportAction ? getReportActionMessage(draftReportAction)?.html : undefined; + const isSyntheticDraftVisible = !!draftReportAction && !sortedVisibleReportActions.some((action) => action.reportActionID === draftReportAction.reportActionID); + const draftAutoScrollKey = isSyntheticDraftVisible ? `${draftReportAction.reportActionID}:${draftMessageHTML ?? ''}` : ''; + const previousDraftAutoScrollKey = usePrevious(draftAutoScrollKey); + const topReportAction = renderedVisibleReportActions.at(-1); const [shouldScrollToEndAfterLayout, setShouldScrollToEndAfterLayout] = useState(shouldFocusToTopOnMount && !linkedReportActionID); const scrollEndTimerRef = useRef | undefined>(undefined); const isAnonymousUser = useIsAnonymousUser(); @@ -272,6 +289,14 @@ function ReportActionsList({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [reportLastReadTime]); + useEffect(() => { + if (!draftReportAction || !sortedVisibleReportActions.some((action) => action.reportActionID === draftReportAction.reportActionID)) { + return; + } + + clearDraft(); + }, [clearDraft, draftReportAction, sortedVisibleReportActions]); + const prevUnreadMarkerReportActionID = useRef(null); /** @@ -393,6 +418,21 @@ function ReportActionsList({ resetKey: linkedReportActionID, }); + useEffect(() => { + if (!draftAutoScrollKey || previousDraftAutoScrollKey === draftAutoScrollKey) { + return; + } + + if (scrollOffsetRef.current >= AUTOSCROLL_TO_TOP_THRESHOLD || !hasNewestReportActionRef.current) { + return; + } + + setIsFloatingMessageCounterVisible(false); + requestAnimationFrame(() => { + reportScrollManager.scrollToBottom(); + }); + }, [draftAutoScrollKey, previousDraftAutoScrollKey, reportScrollManager, scrollOffsetRef, setIsFloatingMessageCounterVisible]); + useEffect(() => { const shouldTriggerScroll = shouldFocusToTopOnMount && prevHasCreatedActionAdded && !hasCreatedActionAdded; if (!shouldTriggerScroll) { @@ -654,7 +694,7 @@ function ReportActionsList({ linkedReportActionID, shouldScrollToEndAfterLayout, hasCreatedActionAdded, - sortedVisibleReportActionsLength: sortedVisibleReportActions.length, + sortedVisibleReportActionsLength: renderedVisibleReportActions.length, isOffline, getInitialNumToRender, }); @@ -665,7 +705,7 @@ function ReportActionsList({ linkedReportActionID, shouldScrollToEndAfterLayout, hasCreatedActionAdded, - sortedVisibleReportActions.length, + renderedVisibleReportActions.length, isOffline, ]); @@ -681,7 +721,7 @@ function ReportActionsList({ const firstVisibleReportActionID = useMemo(() => getFirstVisibleReportActionID(sortedReportActions, isOffline), [sortedReportActions, isOffline]); const shouldUseThreadDividerLine = useMemo(() => { - const topReport = sortedVisibleReportActions.length > 0 ? sortedVisibleReportActions.at(sortedVisibleReportActions.length - 1) : null; + const topReport = renderedVisibleReportActions.length > 0 ? renderedVisibleReportActions.at(renderedVisibleReportActions.length - 1) : null; if (topReport && topReport.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED) { return false; @@ -696,17 +736,19 @@ function ReportActionsList({ } return isExpenseReport(report) || isIOUReport(report) || isInvoiceReport(report); - }, [parentReportAction, report, sortedVisibleReportActions]); + }, [parentReportAction, renderedVisibleReportActions, report]); // Precompute a reportActionID → index map so renderItem can resolve the real index in O(1) - // instead of scanning sortedVisibleReportActions with indexOf on every render. + // instead of scanning renderedVisibleReportActions with indexOf on every render. Build from + // renderedVisibleReportActions (which includes the synthetic draft when active) so the + // synthetic-bubble index resolves correctly during a Concierge trickle. const actionIndexMap = useMemo(() => { const map = new Map(); - for (const [i, action] of sortedVisibleReportActions.entries()) { + for (const [i, action] of renderedVisibleReportActions.entries()) { map.set(action.reportActionID, i); } return map; - }, [sortedVisibleReportActions]); + }, [renderedVisibleReportActions]); const renderItem = useCallback( ({item: reportAction, index}: ListRenderItemInfo) => { @@ -729,12 +771,12 @@ function ReportActionsList({ transactionThreadReport={transactionThreadReport} linkedReportActionID={linkedReportActionID} displayAsGroup={ - !isConsecutiveChronosAutomaticTimerAction(sortedVisibleReportActions, safeIndex, chatIncludesChronosWithID(reportAction?.reportID), isOffline) && - isConsecutiveActionMadeByPreviousActor(sortedVisibleReportActions, safeIndex, isOffline) + !isConsecutiveChronosAutomaticTimerAction(renderedVisibleReportActions, safeIndex, chatIncludesChronosWithID(reportAction?.reportID), isOffline) && + isConsecutiveActionMadeByPreviousActor(renderedVisibleReportActions, safeIndex, isOffline) } shouldHideThreadDividerLine={shouldHideThreadDividerLine} shouldDisplayNewMarker={reportAction.reportActionID === unreadMarkerReportActionID} - shouldDisplayReplyDivider={sortedVisibleReportActions.length > 1} + shouldDisplayReplyDivider={renderedVisibleReportActions.length > 1} isFirstVisibleReportAction={firstVisibleReportActionID === reportAction.reportActionID} shouldUseThreadDividerLine={shouldUseThreadDividerLine} userWalletTierName={userWalletTierName} @@ -773,7 +815,7 @@ function ReportActionsList({ transactionThreadReport, linkedReportActionID, actionIndexMap, - sortedVisibleReportActions, + renderedVisibleReportActions, shouldHideThreadDividerLine, unreadMarkerReportActionID, firstVisibleReportActionID, @@ -800,8 +842,8 @@ function ReportActionsList({ // Native mobile does not render updates flatlist the changes even though component did update called. // To notify there something changes we can use extraData prop to flatlist const extraData = useMemo( - () => [shouldUseNarrowLayout ? unreadMarkerReportActionID : undefined, isArchivedNonExpenseReport(report, isReportArchived)], - [unreadMarkerReportActionID, shouldUseNarrowLayout, report, isReportArchived], + () => [shouldUseNarrowLayout ? unreadMarkerReportActionID : undefined, isArchivedNonExpenseReport(report, isReportArchived), draftReportAction?.reportActionID, draftMessageHTML], + [draftMessageHTML, draftReportAction?.reportActionID, unreadMarkerReportActionID, shouldUseNarrowLayout, report, isReportArchived], ); const hideComposer = !canUserPerformWriteAction(report, isReportArchived); const shouldShowReportRecipientLocalTime = canShowReportRecipientLocalTime(personalDetailsList, report, currentUserAccountID) && !isComposerFullSize; @@ -836,14 +878,14 @@ function ReportActionsList({ return ( <> - + {!hasActiveDraft && } ); - }, [canShowHeader, report, retryLoadNewerChatsError]); + }, [canShowHeader, hasActiveDraft, report, retryLoadNewerChatsError]); const shouldShowSkeleton = isOffline && !sortedVisibleReportActions.some((action) => action.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED); @@ -856,7 +898,7 @@ function ReportActionsList({ }, [shouldShowSkeleton]); const renderTopReportActions = useCallback(() => { - const previewItems = sortedVisibleReportActions.slice(initialNumToRender ? -initialNumToRender : 0).reverse(); + const previewItems = renderedVisibleReportActions.slice(initialNumToRender ? -initialNumToRender : 0).reverse(); return ( <> @@ -873,7 +915,7 @@ function ReportActionsList({ ); - }, [actionIndexMap, hideComposer, initialNumToRender, renderItem, shouldShowReportRecipientLocalTime, sortedVisibleReportActions, styles]); + }, [actionIndexMap, hideComposer, initialNumToRender, renderItem, shouldShowReportRecipientLocalTime, renderedVisibleReportActions, styles]); const onStartReached = useCallback(() => { if (!isSearchTopmostFullScreenRoute()) { @@ -906,7 +948,7 @@ function ReportActionsList({ ref={reportScrollManager.ref} testID="report-actions-list" style={styles.overscrollBehaviorContain} - data={sortedVisibleReportActions} + data={renderedVisibleReportActions} renderItem={renderItem} keyExtractor={keyExtractor} drawDistance={1500} diff --git a/tests/unit/pages/inbox/conciergeDraftState.test.ts b/tests/unit/pages/inbox/conciergeDraftState.test.ts new file mode 100644 index 000000000000..e83e7125eb89 --- /dev/null +++ b/tests/unit/pages/inbox/conciergeDraftState.test.ts @@ -0,0 +1,134 @@ +import {applyConciergeDraftEvent} from '@pages/inbox/conciergeDraftState'; +import CONST from '@src/CONST'; + +const REPORT_ID = '123'; +const REPORT_ACTION_ID = '456'; +const CREATED = '2026-04-03 10:00:00.000'; +const STREAM_SESSION_ID = 'stream-session-1'; + +function createDraftEvent(overrides?: Partial[1]>) { + return { + reportID: REPORT_ID, + reportActionID: REPORT_ACTION_ID, + streamSessionID: STREAM_SESSION_ID, + sequence: 1, + status: 'started' as const, + created: CREATED, + bodyMarkdown: 'Hello, **world**!', + ...overrides, + }; +} + +function getFirstMessageHTML(draft: ReturnType) { + const message = draft?.reportAction.message; + + if (!Array.isArray(message)) { + return undefined; + } + + return message.at(0)?.html; +} + +function getFirstMessageText(draft: ReturnType) { + const message = draft?.reportAction.message; + + if (!Array.isArray(message)) { + return undefined; + } + + return message.at(0)?.text; +} + +describe('conciergeDraftState', () => { + it('should create a synthetic Concierge draft action from the first streamed snapshot', () => { + const draft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + + expect(draft?.reportAction.reportActionID).toBe(REPORT_ACTION_ID); + expect(draft?.reportAction.actorAccountID).toBe(CONST.ACCOUNT_ID.CONCIERGE); + expect(draft?.reportAction.created).toBe(CREATED); + expect(getFirstMessageHTML(draft)).toContain('world'); + expect(getFirstMessageText(draft)).toBe('Hello, *world*!'); + }); + + it('should update the same draft session when a newer sequence arrives', () => { + const initialDraft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + const updatedDraft = applyConciergeDraftEvent( + initialDraft, + createDraftEvent({ + sequence: 2, + status: 'updated', + bodyMarkdown: 'Hello, **streaming** world!', + }), + REPORT_ID, + ); + + expect(updatedDraft?.sequence).toBe(2); + expect(getFirstMessageHTML(updatedDraft)).toContain('streaming'); + }); + + it('should ignore stale events from the same stream session', () => { + const initialDraft = applyConciergeDraftEvent(null, createDraftEvent({sequence: 3}), REPORT_ID); + const staleDraft = applyConciergeDraftEvent( + initialDraft, + createDraftEvent({ + sequence: 2, + status: 'updated', + bodyMarkdown: 'This should be ignored', + }), + REPORT_ID, + ); + + expect(staleDraft).toBe(initialDraft); + expect(getFirstMessageText(staleDraft)).toBe('Hello, *world*!'); + }); + + it('should keep the draft visible through completion and prefer finalRenderedHTML when provided', () => { + const initialDraft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + const completedDraft = applyConciergeDraftEvent( + initialDraft, + createDraftEvent({ + sequence: 2, + status: 'completed', + finalRenderedHTML: 'Server rendered', + bodyMarkdown: undefined, + }), + REPORT_ID, + ); + + expect(completedDraft?.status).toBe('completed'); + expect(getFirstMessageHTML(completedDraft)).toBe('Server rendered'); + expect(getFirstMessageText(completedDraft)).toBe('Server rendered'); + }); + + it('should clear the active draft when the same stream session fails', () => { + const initialDraft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + const failedDraft = applyConciergeDraftEvent( + initialDraft, + createDraftEvent({ + sequence: 2, + status: 'failed', + terminalReason: 'lostLease', + bodyMarkdown: undefined, + }), + REPORT_ID, + ); + + expect(failedDraft).toBeNull(); + }); + + it('should ignore events for a different report', () => { + const initialDraft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + const otherReportDraft = applyConciergeDraftEvent( + initialDraft, + createDraftEvent({ + reportID: '999', + sequence: 2, + status: 'updated', + bodyMarkdown: 'Different report', + }), + REPORT_ID, + ); + + expect(otherReportDraft).toBe(initialDraft); + }); +}); From e5cbb77a93540effac1fd6ece6f0f3cca2872831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Mon, 27 Apr 2026 13:09:11 +0900 Subject: [PATCH 02/26] Trickle pregenerated Concierge response over follow-up generation window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the single 4s setTimeout in usePendingConciergeResponse with a decelerating ease-out reveal driven into Issa's ConciergeDraftContext reducer (PR #87110) so the synthetic report-action graft and reconciliation behave identically to server-driven streaming. The pregenerated HTML originates client-side from , so the trickle is purely view-local — REPORT_ACTIONS is written exactly once at trickle completion via applyPendingConciergeAction, preserving LHN previews / push / search snapshots until the bubble is fully formed. When the canonical reportComment lands mid-trickle the loop accelerates the remaining reveal to ~1.5s instead of snapping the synthetic bubble closed. Single-chunk payloads keep the original binary reveal so 1–2 sentence answers don't feel artificially stretched. Tokenizer splits at top-level child boundaries to keep inline atomics (mentions, emoji, anchors, code) whole. Fixes https://github.com/Expensify/Expensify/issues/626938 Stacked on https://github.com/Expensify/App/pull/87110 --- src/hooks/usePendingConciergeResponse.ts | 166 ++++++++++++++++-- src/libs/ReportActionFollowupUtils/index.ts | 4 +- .../tokenizeForReveal.ts | 26 +++ src/pages/inbox/ConciergeDraftContext.tsx | 18 +- .../tokenizeForRevealTest.ts | 54 ++++++ 5 files changed, 248 insertions(+), 20 deletions(-) create mode 100644 src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts create mode 100644 tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 2fc5b1e2147c..d29b9fa255b3 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -1,42 +1,172 @@ -import {useEffect} from 'react'; +import {useCallback, useEffect, useMemo, useRef} from 'react'; +import type {OnyxEntry} from 'react-native-onyx'; import {applyPendingConciergeAction, discardPendingConciergeAction} from '@libs/actions/Report/SuggestedFollowup'; +import Log from '@libs/Log'; +import {rand64} from '@libs/NumberUtils'; +import type {ConciergeDraftEvent} from '@libs/Pusher/types'; +import tokenizeForReveal from '@libs/ReportActionFollowupUtils/tokenizeForReveal'; +import {getReportActionHtml} from '@libs/ReportActionsUtils'; +import {useConciergeDraftActions} from '@pages/inbox/ConciergeDraftContext'; import ONYXKEYS from '@src/ONYXKEYS'; +import type {ReportAction, ReportActions} from '@src/types/onyx'; import useOnyx from './useOnyx'; -/** If displayAfter is more than this far in the past, the response is stale (e.g. app was killed and restarted) */ +/** If displayAfter is more than this far in the past, the response is stale (e.g. app was killed and restarted). */ const STALE_THRESHOLD_MS = 10_000; +/** Default trickle duration. Roughly matches the p50 follow-up generation window so the reveal lands close to when followups arrive. */ +const DEFAULT_STREAM_DURATION_MS = 30_000; +/** Trickle tick cadence. ~75 updates over the 30s default — enough granularity for a smooth ease-out, sparse enough to keep render churn low. */ +const TICK_INTERVAL_MS = 400; +/** Hard cap on running trickle. If the loop is still alive past this, force completion to avoid pinning a synthetic bubble forever. */ +const TRICKLE_HARD_CAP_MS = 60_000; +/** Once the real reportComment lands in REPORT_ACTIONS, finish the remaining reveal within this window. */ +const ACCELERATED_REMAINING_MS = 1_500; + +function easeOut(t: number): number { + const clamped = Math.max(0, Math.min(1, t)); + return 1 - (1 - clamped) ** 2; +} /** - * Processes pending concierge responses stored in Onyx for a given report. - * When a pending response exists, schedules the action to be moved to REPORT_ACTIONS - * after the remaining delay, with automatic cleanup on unmount via useEffect. + * Manages the lifecycle of a pending pregenerated Concierge response. + * + * For payloads with multiple top-level chunks the hook trickles the body in + * progressively over `DEFAULT_STREAM_DURATION_MS` (decelerating ease-out), + * dispatching `conciergeDraftStarted/updated/completed` events into Issa's + * existing `ConciergeDraftContext` reducer (PR #87110) so the synthetic + * report action splice and reconciliation behave identically to server-driven + * streaming. Canonical Onyx state (`REPORT_ACTIONS`) is only written at the + * end via `applyPendingConciergeAction`, preserving LHN previews / push + * notifications / search snapshots until the trickle finishes. + * + * Single-chunk payloads fall back to the original binary reveal at + * `displayAfter` so short replies (1–2 sentence answers) don't get stretched + * artificially. + * + * Reconciliation: if the real `reportComment` arrives mid-trickle, the loop + * accelerates the remaining reveal so the synthetic bubble lands within + * `ACCELERATED_REMAINING_MS` instead of snapping closed. */ function usePendingConciergeResponse(reportID: string | undefined) { const [pendingResponse] = useOnyx(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`); + const reportActionID = pendingResponse?.reportAction?.reportActionID; + const fullHtml = pendingResponse?.reportAction ? getReportActionHtml(pendingResponse.reportAction) : ''; + const persistedActionSelector = useCallback( + (actions: OnyxEntry): ReportAction | undefined => (reportActionID && actions ? actions[reportActionID] : undefined), + [reportActionID], + ); + const [persistedAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {selector: persistedActionSelector}); + const {dispatchLocalDraftEvent} = useConciergeDraftActions(); + + const tokens = useMemo(() => tokenizeForReveal(fullHtml), [fullHtml]); + const accelerateRef = useRef<((nowMs: number) => void) | null>(null); + // Reconciliation: when the canonical reportComment lands in REPORT_ACTIONS + // mid-trickle, fire the running loop's accelerator so the remaining reveal + // finishes in ~1.5s instead of snapping the synthetic bubble closed. useEffect(() => { - if (!pendingResponse) { + if (!persistedAction || !accelerateRef.current) { return; } + accelerateRef.current(Date.now()); + }, [persistedAction]); - const remaining = pendingResponse.displayAfter - Date.now(); + useEffect(() => { + if (!pendingResponse || !reportID || !reportActionID) { + return; + } + const {reportAction, displayAfter} = pendingResponse; + const remainingDelay = displayAfter - Date.now(); - // If the pending response is stale (e.g. app was killed/restarted), discard it - // instead of displaying a phantom message that was never confirmed by the server. - if (remaining < -STALE_THRESHOLD_MS) { + if (remainingDelay < -STALE_THRESHOLD_MS) { discardPendingConciergeAction(reportID); return; } - const timer = setTimeout( - () => { - applyPendingConciergeAction(reportID, pendingResponse.reportAction); - }, - Math.max(0, remaining), - ); + // Single-chunk payloads keep the original binary reveal — trickling a + // 1–2 sentence answer over 30s would feel broken. + const shouldTrickle = tokens.length >= 3 && !!fullHtml; + if (!shouldTrickle) { + const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay)); + return () => clearTimeout(timer); + } + + const session = rand64(); + let sequence = 0; + let intervalID: ReturnType | null = null; + let trickleStart = 0; + let effectiveDuration = DEFAULT_STREAM_DURATION_MS; + let lastStage = 0; + let cancelled = false; + + const dispatch = (status: ConciergeDraftEvent['status'], finalRenderedHTML: string) => { + if (cancelled) { + return; + } + sequence += 1; + dispatchLocalDraftEvent({ + reportID, + reportActionID, + streamSessionID: session, + sequence, + status, + created: reportAction.created, + finalRenderedHTML, + }); + }; + + const completeAndApply = () => { + if (intervalID) { + clearInterval(intervalID); + intervalID = null; + } + dispatch('completed', tokens.at(-1) ?? fullHtml); + applyPendingConciergeAction(reportID, reportAction); + }; + + accelerateRef.current = (nowMs: number) => { + if (!intervalID || trickleStart === 0) { + return; + } + const elapsed = nowMs - trickleStart; + // Compressing effectiveDuration is what makes progress hit 1 within + // ACCELERATED_REMAINING_MS — the next tick observes progress >= 1 + // and runs completeAndApply via the normal path. + effectiveDuration = elapsed + ACCELERATED_REMAINING_MS; + Log.info('[ConciergeTrickle] accelerated due to persisted action arrival', false, {reportActionID, elapsed}); + }; + + const startTrickle = () => { + if (cancelled) { + return; + } + trickleStart = Date.now(); + dispatch('started', tokens.at(1) ?? ''); + lastStage = 1; + intervalID = setInterval(() => { + const elapsed = Date.now() - trickleStart; + const progress = easeOut(elapsed / effectiveDuration); + const stage = Math.min(tokens.length - 1, Math.max(0, Math.ceil(progress * (tokens.length - 1)))); + if (stage > lastStage) { + lastStage = stage; + dispatch('updated', tokens.at(stage) ?? ''); + } + if (progress >= 1 || elapsed >= TRICKLE_HARD_CAP_MS) { + completeAndApply(); + } + }, TICK_INTERVAL_MS); + }; - return () => clearTimeout(timer); - }, [pendingResponse, reportID]); + const startTimer = setTimeout(startTrickle, Math.max(0, remainingDelay)); + return () => { + cancelled = true; + clearTimeout(startTimer); + if (intervalID) { + clearInterval(intervalID); + } + accelerateRef.current = null; + }; + }, [pendingResponse, reportID, reportActionID, fullHtml, tokens, dispatchLocalDraftEvent]); } export default usePendingConciergeResponse; diff --git a/src/libs/ReportActionFollowupUtils/index.ts b/src/libs/ReportActionFollowupUtils/index.ts index dd591ea2a79c..c0091c2f451c 100644 --- a/src/libs/ReportActionFollowupUtils/index.ts +++ b/src/libs/ReportActionFollowupUtils/index.ts @@ -3,6 +3,7 @@ import {DomUtils, parseDocument} from 'htmlparser2'; import {getReportActionMessage, isActionOfType} from '@libs/ReportActionsUtils'; import CONST from '@src/CONST'; import type {OnyxInputOrEntry, ReportAction} from '@src/types/onyx'; +import tokenizeForReveal from './tokenizeForReveal'; type Followup = { text: string; @@ -58,5 +59,6 @@ function parseFollowupsFromHtml(html: string): Followup[] | null { return {text, response}; }); } -export {containsActionableFollowUps, parseFollowupsFromHtml}; + +export {containsActionableFollowUps, parseFollowupsFromHtml, tokenizeForReveal}; export type {Followup}; diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts new file mode 100644 index 000000000000..766e62ea1225 --- /dev/null +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -0,0 +1,26 @@ +import render from 'dom-serializer'; +import {parseDocument} from 'htmlparser2'; + +/** + * Pre-computes a list of progressive HTML stages for a pregenerated Concierge + * response, so usePendingConciergeResponse can trickle the body in over the + * follow-up generation window without re-parsing on every tick. + * + * Splits at top-level child boundaries so atomic inline elements (mentions, + * emoji, anchors, code) never render half-parsed. Stage 0 is empty; the final + * stage equals the rendered children. The hook walks indices 0..N-1 over the + * stream duration. + */ +function tokenizeForReveal(html: string): string[] { + if (!html) { + return ['']; + } + const doc = parseDocument(html); + const stages: string[] = ['']; + for (let n = 1; n <= doc.children.length; n++) { + stages.push(render(doc.children.slice(0, n))); + } + return stages; +} + +export default tokenizeForReveal; diff --git a/src/pages/inbox/ConciergeDraftContext.tsx b/src/pages/inbox/ConciergeDraftContext.tsx index 977ca7928486..7745d634c69b 100644 --- a/src/pages/inbox/ConciergeDraftContext.tsx +++ b/src/pages/inbox/ConciergeDraftContext.tsx @@ -18,6 +18,13 @@ type ConciergeDraftState = { type ConciergeDraftActions = { clearDraft: () => void; + /** + * Apply a draft event from a non-Pusher source (e.g. the local pacer in + * usePendingConciergeResponse for pregenerated replies whose body is + * already on the client). Uses the same reducer as the Pusher path so the + * reportActionID-based reconciliation continues to work unchanged. + */ + dispatchLocalDraftEvent: (event: ConciergeDraftEvent) => void; }; const defaultState: ConciergeDraftState = { @@ -27,6 +34,7 @@ const defaultState: ConciergeDraftState = { const defaultActions: ConciergeDraftActions = { clearDraft: () => {}, + dispatchLocalDraftEvent: () => {}, }; const ConciergeDraftStateContext = createContext(defaultState); @@ -60,6 +68,13 @@ function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{repor setDraft(null); }, []); + const dispatchLocalDraftEvent = useCallback( + (event: ConciergeDraftEvent) => { + setDraft((currentDraft) => applyConciergeDraftEvent(currentDraft, event, reportID)); + }, + [reportID], + ); + useEffect(() => { const channelName = getReportChannelName(reportID); const handleResubscribe = () => { @@ -109,8 +124,9 @@ function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{repor const actionsValue = useMemo( () => ({ clearDraft, + dispatchLocalDraftEvent, }), - [clearDraft], + [clearDraft, dispatchLocalDraftEvent], ); return ( diff --git a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts new file mode 100644 index 000000000000..ddb7d51c0045 --- /dev/null +++ b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts @@ -0,0 +1,54 @@ +import tokenizeForReveal from '@libs/ReportActionFollowupUtils/tokenizeForReveal'; + +describe('tokenizeForReveal', () => { + it('returns just the empty stage for an empty input', () => { + expect(tokenizeForReveal('')).toEqual(['']); + }); + + it('returns one content stage for single top-level child (binary reveal fallback)', () => { + const html = '

Just one paragraph.

'; + const stages = tokenizeForReveal(html); + + // [empty, full]. The hook treats this as a binary reveal because length < 3. + expect(stages).toHaveLength(2); + expect(stages.at(0)).toBe(''); + expect(stages.at(-1)).toBe(html); + }); + + it('produces progressive prefixes split at top-level boundaries', () => { + const html = '

First.

Second.

  • Third.
'; + const stages = tokenizeForReveal(html); + + expect(stages).toEqual(['', '

First.

', '

First.

Second.

', html]); + }); + + it('keeps inline atomics whole (mention, emoji, anchor, code stay intact in their parent)', () => { + // Each top-level

is an atomic unit so its inline children never render half-parsed. + // htmlparser2's parseDocument lowercases attribute names — that matches the rest of the + // codebase (see parseFollowupsFromHtml) so we accept it as the canonical shape. + const html = '

Hi @daniel

Status:

'; + const stages = tokenizeForReveal(html); + + expect(stages.at(1)).toContain('mention-user'); + expect(stages.at(1)).toContain('@daniel'); + // No stage shows a partially-rendered mention or emoji. + for (const stage of stages) { + const openMentions = (stage.match(//g) ?? []).length; + expect(openMentions).toBe(closeMentions); + + const openEmojis = (stage.match(//g) ?? []).length; + const closeEmojis = (stage.match(/<\/emoji>/g) ?? []).length; + expect(openEmojis).toBe(closeEmojis); + } + }); + + it('preserves the typical Concierge response shape (paragraphs + bullet list)', () => { + const html = "

To set up QuickBooks, here's what to do:

  1. Go to Settings.
  2. Click Connections.
  3. Pick QBO.

Let me know if you hit a snag.

"; + const stages = tokenizeForReveal(html); + + // 3 top-level children => 4 stages (empty + 3 progressive). + expect(stages).toHaveLength(4); + expect(stages.at(-1)).toBe(html); + }); +}); From 5e96c4cdaa8e128cf1c3b2f6c950453f7db7dda3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Mon, 27 Apr 2026 14:33:37 +0900 Subject: [PATCH 03/26] Add lifecycle telemetry to Concierge trickle (start + complete) Two structured Log.info events bracket each trickle so VictoriaLogs can answer Phase 2 tuning questions and detect regressions: - [ConciergeTrickle] start: {reportActionID, tokenCount, durationMs} - [ConciergeTrickle] complete: {reportActionID, reason, tokenCount, durationMs, totalElapsedMs, arrivedAtProgress, arrivedAtElapsedMs} reason in {natural, accelerated, stale_cap} attributes the completion path; arrivedAtProgress + arrivedAtElapsedMs (defined when accelerated) record where in the trickle the canonical reportComment landed - the single most useful signal for tuning the duration heuristic. Replaces the standalone "[ConciergeTrickle] accelerated" log: that data is now carried by the complete event's reason='accelerated' branch + arrivedAtProgress field. Also enables: stale-cap firing-rate alerts (backend-health signal), client/server cross-correlation by reportActionID against PAZR logs, mobile JS-timer-throttle detection via arrivedAtProgress skew, and A/B-test readiness without per-experiment instrumentation. Lifecycle pairing (start + complete) makes orphaned trickles detectable as starts without matching completes. --- src/hooks/usePendingConciergeResponse.ts | 29 +++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index d29b9fa255b3..bd087ebcd9a7 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -98,6 +98,11 @@ function usePendingConciergeResponse(reportID: string | undefined) { let effectiveDuration = DEFAULT_STREAM_DURATION_MS; let lastStage = 0; let cancelled = false; + // Telemetry: track whether/when acceleration fired so the [complete] log + // can attribute the completion reason and the arrival point of the canonical + // reportComment. Drives Phase 2 duration tuning + regression detection. + let arrivedAtProgress: number | undefined; + let arrivedAtElapsedMs: number | undefined; const dispatch = (status: ConciergeDraftEvent['status'], finalRenderedHTML: string) => { if (cancelled) { @@ -120,6 +125,22 @@ function usePendingConciergeResponse(reportID: string | undefined) { clearInterval(intervalID); intervalID = null; } + const totalElapsedMs = trickleStart === 0 ? 0 : Date.now() - trickleStart; + let reason: 'natural' | 'accelerated' | 'stale_cap' = 'natural'; + if (arrivedAtProgress !== undefined) { + reason = 'accelerated'; + } else if (totalElapsedMs >= TRICKLE_HARD_CAP_MS) { + reason = 'stale_cap'; + } + Log.info('[ConciergeTrickle] complete', false, { + reportActionID, + reason, + tokenCount: tokens.length, + durationMs: effectiveDuration, + totalElapsedMs, + arrivedAtProgress, + arrivedAtElapsedMs, + }); dispatch('completed', tokens.at(-1) ?? fullHtml); applyPendingConciergeAction(reportID, reportAction); }; @@ -132,8 +153,9 @@ function usePendingConciergeResponse(reportID: string | undefined) { // Compressing effectiveDuration is what makes progress hit 1 within // ACCELERATED_REMAINING_MS — the next tick observes progress >= 1 // and runs completeAndApply via the normal path. + arrivedAtProgress = easeOut(elapsed / effectiveDuration); + arrivedAtElapsedMs = elapsed; effectiveDuration = elapsed + ACCELERATED_REMAINING_MS; - Log.info('[ConciergeTrickle] accelerated due to persisted action arrival', false, {reportActionID, elapsed}); }; const startTrickle = () => { @@ -141,6 +163,11 @@ function usePendingConciergeResponse(reportID: string | undefined) { return; } trickleStart = Date.now(); + Log.info('[ConciergeTrickle] start', false, { + reportActionID, + tokenCount: tokens.length, + durationMs: effectiveDuration, + }); dispatch('started', tokens.at(1) ?? ''); lastStage = 1; intervalID = setInterval(() => { From 22ef7547ffba4827933aeb3cf28973b35b1b6839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Mon, 27 Apr 2026 18:09:16 +0900 Subject: [PATCH 04/26] Word-level tokenizer: ChatGPT-style reveal + bold/em recursion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the top-level-element tokenizer with a DOM-walking word-level one. Text nodes split per word/whitespace run; formatting wrappers (, , , , , etc.) recurse so bold appears as words become bold rather than as a single jump. Atomic visual primitives (, , , , , ) stay whole — no half-rendered mention or partial emoji codepoint. Implementation: collect anchors in document order (one per word run or atomic-tag subtree), then for each anchor render a partial node forest where the path branch is shallow-cloned with truncated children and sibling references stay unchanged so dom-serializer renders them as-is. shouldTrickle threshold bumps from 3 to 20 anchors so single-sentence replies (~10-18 anchors) still get the binary 4s reveal — multi- paragraph replies clear 20 easily and get the smooth trickle. Tests cover: empty, plain word reveal, bold-recursion (wrapper opens around partial words), mention atomicity (no partial mentions), emoji / anchor / code atomicity, multi-paragraph monotonic growth, realistic Xero-shape >= 20 anchors. --- src/hooks/usePendingConciergeResponse.ts | 9 +- .../tokenizeForReveal.ts | 132 ++++++++++++++++-- .../tokenizeForRevealTest.ts | 101 ++++++++++---- 3 files changed, 205 insertions(+), 37 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index bd087ebcd9a7..47a07e8a94f8 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -83,9 +83,12 @@ function usePendingConciergeResponse(reportID: string | undefined) { return; } - // Single-chunk payloads keep the original binary reveal — trickling a - // 1–2 sentence answer over 30s would feel broken. - const shouldTrickle = tokens.length >= 3 && !!fullHtml; + // Threshold matches word-level tokenization — a typical 1–2 sentence + // reply produces ~10–18 anchors and feels stretched if trickled over + // 30s, so binary reveal is preserved for those. Multi-paragraph + // replies (intro + list + closing) clear 20 anchors easily and get + // the smooth ChatGPT-style stream. + const shouldTrickle = tokens.length >= 20 && !!fullHtml; if (!shouldTrickle) { const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay)); return () => clearTimeout(timer); diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts index 766e62ea1225..6dff8f637688 100644 --- a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -1,24 +1,140 @@ import render from 'dom-serializer'; +import {ElementType} from 'domelementtype'; +import type {AnyNode, Document, Element as DomElement} from 'domhandler'; import {parseDocument} from 'htmlparser2'; +/** + * Tags rendered atomically — splitting their content mid-element looks broken + * (e.g. half a mention, a partial emoji codepoint, a link with no closing tag). + * Mentions/emoji/img are visual primitives, is a structural target with a + * URL, is a literal text run that shouldn't reflow as words trickle in. + */ +const ATOMIC_TAGS = new Set(['mention-user', 'mention-report', 'emoji', 'a', 'code', 'img']); + +/** + * One reveal point. `path` is the index path from the doc root to the anchor + * node; `textEndIdx` (when present) is the character offset to truncate that + * node's text data to. + */ +type Anchor = { + path: number[]; + textEndIdx?: number; +}; + +/** + * Walk the DOM in document order, emitting one anchor per "word or whitespace + * run" inside text nodes and one anchor for each whole atomic-tag subtree. + * Block / formatting elements (

,

    , , , ...) recurse so + * their inner words become anchors with the wrapper preserved on render — + * that's what makes bold appear as words become bold rather than as a single + * jump. + */ +function collectAnchors(doc: Document): Anchor[] { + const anchors: Anchor[] = []; + const visit = (node: AnyNode, path: number[]): void => { + if (node.type === ElementType.Text) { + const text = node.data; + if (text.length === 0) { + return; + } + // Word-or-whitespace runs — each run advances the visible content + // by one perceptual unit. Splitting on \s alone would coalesce + // multiple words around a single space; this keeps the reveal + // granular without breaking on character boundaries. + const re = /\S+|\s+/g; + let match = re.exec(text); + while (match !== null) { + anchors.push({path: path.slice(), textEndIdx: match.index + match[0].length}); + match = re.exec(text); + } + return; + } + if (node.type !== ElementType.Tag) { + return; + } + const elem = node; + const tagName = elem.name.toLowerCase(); + if (ATOMIC_TAGS.has(tagName)) { + anchors.push({path: path.slice()}); + return; + } + if (!elem.children || elem.children.length === 0) { + anchors.push({path: path.slice()}); + return; + } + for (let i = 0; i < elem.children.length; i++) { + const child = elem.children.at(i); + if (child) { + visit(child, [...path, i]); + } + } + }; + for (let i = 0; i < doc.children.length; i++) { + const child = doc.children.at(i); + if (child) { + visit(child, [i]); + } + } + return anchors; +} + +/** + * Build a partial node forest containing every node up to (and including) + * `anchor`, with any text-node leaf truncated to the anchor's offset. + * + * Reconstructs only the path elements: siblings before the path branch are + * cloned by reference (rendered fully), the path branch is shallow-cloned + * with its `children` array replaced. Sibling references are unchanged so + * dom-serializer renders them via their existing structure — no parent/next + * pointer surgery needed. + */ +function buildClippedNodes(doc: Document, anchor: Anchor): AnyNode[] { + const clip = (siblings: readonly AnyNode[], depth: number): AnyNode[] => { + const idx = anchor.path.at(depth) ?? 0; + const isLeaf = depth === anchor.path.length - 1; + const before = siblings.slice(0, idx); + const stopNode = siblings.at(idx); + if (!stopNode) { + return [...before]; + } + if (isLeaf) { + if (stopNode.type === ElementType.Text && anchor.textEndIdx !== undefined) { + const truncated = {...stopNode, data: stopNode.data.slice(0, anchor.textEndIdx)} as unknown as AnyNode; + return [...before, truncated]; + } + return [...before, stopNode]; + } + const elem = stopNode as DomElement; + const innerChildren = clip(elem.children as AnyNode[], depth + 1); + const partialElem = {...elem, children: innerChildren} as unknown as AnyNode; + return [...before, partialElem]; + }; + return clip(doc.children as AnyNode[], 0); +} + /** * Pre-computes a list of progressive HTML stages for a pregenerated Concierge - * response, so usePendingConciergeResponse can trickle the body in over the - * follow-up generation window without re-parsing on every tick. + * response — usePendingConciergeResponse trickles them in over the follow-up + * generation window so the chat keeps moving while the server works. + * + * Word-level granularity within text nodes (ChatGPT-style streaming feel), + * recursing into formatting wrappers like / so bold appears as + * words become bold rather than in one jump. Atomic visual primitives + * (mentions, emoji, links, code, images) stay whole — no half-rendered + * mention or partial emoji codepoint. * - * Splits at top-level child boundaries so atomic inline elements (mentions, - * emoji, anchors, code) never render half-parsed. Stage 0 is empty; the final - * stage equals the rendered children. The hook walks indices 0..N-1 over the - * stream duration. + * Stage 0 is empty; the final stage equals the rendered children. The hook + * walks indices 0..N-1 across the trickle duration via an ease-out curve. */ function tokenizeForReveal(html: string): string[] { if (!html) { return ['']; } const doc = parseDocument(html); + const anchors = collectAnchors(doc); const stages: string[] = ['']; - for (let n = 1; n <= doc.children.length; n++) { - stages.push(render(doc.children.slice(0, n))); + for (const anchor of anchors) { + stages.push(render(buildClippedNodes(doc, anchor))); } return stages; } diff --git a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts index ddb7d51c0045..45bb3fe14257 100644 --- a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts +++ b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts @@ -5,50 +5,99 @@ describe('tokenizeForReveal', () => { expect(tokenizeForReveal('')).toEqual(['']); }); - it('returns one content stage for single top-level child (binary reveal fallback)', () => { - const html = '

    Just one paragraph.

    '; + it('reveals plain text word-by-word inside a single block', () => { + const html = '

    The quick brown fox.

    '; const stages = tokenizeForReveal(html); - // [empty, full]. The hook treats this as a binary reveal because length < 3. - expect(stages).toHaveLength(2); expect(stages.at(0)).toBe(''); - expect(stages.at(-1)).toBe(html); + expect(stages.at(-1)).toBe('

    The quick brown fox.

    '); + // Word-level reveal yields one stage per word + whitespace run. + expect(stages.length).toBeGreaterThanOrEqual(5); }); - it('produces progressive prefixes split at top-level boundaries', () => { - const html = '

    First.

    Second.

    • Third.
    '; + it('preserves the wrapper while revealing words inside it (formatting applies as text grows)', () => { + const html = '

    Status: important warning for you.

    '; const stages = tokenizeForReveal(html); - expect(stages).toEqual(['', '

    First.

    ', '

    First.

    Second.

    ', html]); + // At some intermediate stage we should see the bold wrapper opened + // around a partial word — proves we're recursing into the formatting + // element rather than treating it as atomic. + const hasOpenStrong = stages.some((s) => /[^<]*<\/strong>/.test(s) && !s.includes('important warning')); + expect(hasOpenStrong).toBe(true); + + // Every stage must keep the tag balanced. + for (const stage of stages) { + const opens = (stage.match(//g) ?? []).length; + const closes = (stage.match(/<\/strong>/g) ?? []).length; + expect(opens).toBe(closes); + } + + expect(stages.at(-1)).toBe('

    Status: important warning for you.

    '); }); - it('keeps inline atomics whole (mention, emoji, anchor, code stay intact in their parent)', () => { - // Each top-level

    is an atomic unit so its inline children never render half-parsed. - // htmlparser2's parseDocument lowercases attribute names — that matches the rest of the - // codebase (see parseFollowupsFromHtml) so we accept it as the canonical shape. - const html = '

    Hi @daniel

    Status:

    '; + it('keeps mention-user atomic (never half-rendered)', () => { + const html = '

    Hi @daniel welcome.

    '; const stages = tokenizeForReveal(html); - expect(stages.at(1)).toContain('mention-user'); - expect(stages.at(1)).toContain('@daniel'); - // No stage shows a partially-rendered mention or emoji. for (const stage of stages) { - const openMentions = (stage.match(//g) ?? []).length; - expect(openMentions).toBe(closeMentions); + const opens = (stage.match(//g) ?? []).length; + expect(opens).toBe(closes); + } + + // Partial text inside the mention-user wrapper would indicate broken atomicity. + expect(stages.some((s) => /]*>@dani(?!e)/.test(s))).toBe(false); + }); + + it('keeps emoji atomic', () => { + const html = '

    Status: all good.

    '; + const stages = tokenizeForReveal(html); + + for (const stage of stages) { + const opens = (stage.match(//g) ?? []).length; + const closes = (stage.match(/<\/emoji>/g) ?? []).length; + expect(opens).toBe(closes); + } + }); + + it('keeps anchor atomic so the URL and text appear together', () => { + const html = '
    '; + const stages = tokenizeForReveal(html); - const openEmojis = (stage.match(//g) ?? []).length; - const closeEmojis = (stage.match(/<\/emoji>/g) ?? []).length; - expect(openEmojis).toBe(closeEmojis); + const anchorContents = stages.flatMap((s) => Array.from(s.matchAll(/]*>([^<]*)<\/a>/g)).map((m) => m[1])); + for (const content of anchorContents) { + expect(content).toBe('our docs'); } }); - it('preserves the typical Concierge response shape (paragraphs + bullet list)', () => { - const html = "

    To set up QuickBooks, here's what to do:

    1. Go to Settings.
    2. Click Connections.
    3. Pick QBO.

    Let me know if you hit a snag.

    "; + it('keeps code atomic', () => { + const html = '

    Run npm install to start.

    '; + const stages = tokenizeForReveal(html); + + const codeContents = stages.flatMap((s) => Array.from(s.matchAll(/([^<]*)<\/code>/g)).map((m) => m[1])); + for (const content of codeContents) { + expect(content).toBe('npm install'); + } + }); + + it('reveals each top-level child progressively (multi-paragraph + list shape)', () => { + const html = '

    Intro paragraph here.

    1. First step.
    2. Second step.

    Closing line.

    '; const stages = tokenizeForReveal(html); - // 3 top-level children => 4 stages (empty + 3 progressive). - expect(stages).toHaveLength(4); expect(stages.at(-1)).toBe(html); + + // Stages must monotonically grow: textual content length never decreases. + const lengths = stages.map((s) => s.replaceAll(/<[^>]+>/g, '').length); + for (let i = 1; i < lengths.length; i++) { + expect(lengths.at(i) ?? 0).toBeGreaterThanOrEqual(lengths.at(i - 1) ?? 0); + } + }); + + it('produces enough anchors that a typical multi-paragraph response trickles smoothly (>=20 stages)', () => { + // Realistic shape — Xero-style 5-step instructions. + const html = + '

    Here is how to connect Xero as your accounting integration:

    1. In the left-hand menu, go to Settings.
    2. Click More features, then click Accounting.
    3. Click Set up next to Xero.
    4. Log in to Xero as an administrator.
    5. Confirm the connection.

    This imports your charts of accounts and tracking categories.

    '; + const stages = tokenizeForReveal(html); + expect(stages.length).toBeGreaterThanOrEqual(20); }); }); From b6ce8155a4523a1f67287dc6a37e59dd662858db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Mon, 27 Apr 2026 22:08:05 +0900 Subject: [PATCH 05/26] Concierge trickle: drop tick cadence 400ms -> 150ms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 400ms ticks left perceptible gaps between word reveals at the ~ChatGPT/Claude streaming pace. 150ms keeps the trailing edge always moving — the gap between dispatches drops below the threshold where the eye perceives discrete jumps. Re-render cost goes 4x but RNW's draft-bubble re-render is one list item, not the whole list. Same total trickle duration (DEFAULT_STREAM_DURATION_MS = 30s); ease-out curve unchanged. Only the loop sampling rate changes. --- src/hooks/usePendingConciergeResponse.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 47a07e8a94f8..d22252daa575 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -15,8 +15,10 @@ import useOnyx from './useOnyx'; const STALE_THRESHOLD_MS = 10_000; /** Default trickle duration. Roughly matches the p50 follow-up generation window so the reveal lands close to when followups arrive. */ const DEFAULT_STREAM_DURATION_MS = 30_000; -/** Trickle tick cadence. ~75 updates over the 30s default — enough granularity for a smooth ease-out, sparse enough to keep render churn low. */ -const TICK_INTERVAL_MS = 400; +/** Trickle tick cadence. 150ms keeps each visible chunk small enough to feel + * continuous (closer to the ChatGPT/Claude streaming feel) without spamming + * dispatches faster than RNW can comfortably re-render the synthetic bubble. */ +const TICK_INTERVAL_MS = 150; /** Hard cap on running trickle. If the loop is still alive past this, force completion to avoid pinning a synthetic bubble forever. */ const TRICKLE_HARD_CAP_MS = 60_000; /** Once the real reportComment lands in REPORT_ACTIONS, finish the remaining reveal within this window. */ From cf2c4e0e72b5d24fa564b741b939009b34f1458a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Tue, 28 Apr 2026 10:59:50 +0900 Subject: [PATCH 06/26] Tighten comments per Expensidev style guide Drop PR/companion-PR references, author attribution, and forward-looking "Phase 2"/"Drives" prose in trickle-related comments. Compress 4+ line blocks down to 1-3 lines without losing the non-obvious WHY. --- src/hooks/usePendingConciergeResponse.ts | 36 ++++--------- .../tokenizeForReveal.ts | 50 ++++++------------- src/pages/inbox/ConciergeDraftContext.tsx | 7 ++- 3 files changed, 29 insertions(+), 64 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index d22252daa575..7790935115b3 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -30,24 +30,11 @@ function easeOut(t: number): number { } /** - * Manages the lifecycle of a pending pregenerated Concierge response. - * - * For payloads with multiple top-level chunks the hook trickles the body in - * progressively over `DEFAULT_STREAM_DURATION_MS` (decelerating ease-out), - * dispatching `conciergeDraftStarted/updated/completed` events into Issa's - * existing `ConciergeDraftContext` reducer (PR #87110) so the synthetic - * report action splice and reconciliation behave identically to server-driven - * streaming. Canonical Onyx state (`REPORT_ACTIONS`) is only written at the - * end via `applyPendingConciergeAction`, preserving LHN previews / push - * notifications / search snapshots until the trickle finishes. - * - * Single-chunk payloads fall back to the original binary reveal at - * `displayAfter` so short replies (1–2 sentence answers) don't get stretched - * artificially. - * - * Reconciliation: if the real `reportComment` arrives mid-trickle, the loop - * accelerates the remaining reveal so the synthetic bubble lands within - * `ACCELERATED_REMAINING_MS` instead of snapping closed. + * Multi-chunk payloads trickle into the `ConciergeDraftContext` reducer over + * `DEFAULT_STREAM_DURATION_MS` so the synthetic report action behaves + * identically to server-driven streaming. `REPORT_ACTIONS` is written only + * at completion, preserving LHN previews / push / search snapshots. + * Single-chunk payloads keep the binary reveal at `displayAfter`. */ function usePendingConciergeResponse(reportID: string | undefined) { const [pendingResponse] = useOnyx(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`); @@ -85,11 +72,9 @@ function usePendingConciergeResponse(reportID: string | undefined) { return; } - // Threshold matches word-level tokenization — a typical 1–2 sentence - // reply produces ~10–18 anchors and feels stretched if trickled over - // 30s, so binary reveal is preserved for those. Multi-paragraph - // replies (intro + list + closing) clear 20 anchors easily and get - // the smooth ChatGPT-style stream. + // Short replies (~10–18 anchors) feel stretched if trickled over 30s, + // so they keep the binary reveal; multi-paragraph replies clear the + // threshold and get the smooth trickle. const shouldTrickle = tokens.length >= 20 && !!fullHtml; if (!shouldTrickle) { const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay)); @@ -103,9 +88,8 @@ function usePendingConciergeResponse(reportID: string | undefined) { let effectiveDuration = DEFAULT_STREAM_DURATION_MS; let lastStage = 0; let cancelled = false; - // Telemetry: track whether/when acceleration fired so the [complete] log - // can attribute the completion reason and the arrival point of the canonical - // reportComment. Drives Phase 2 duration tuning + regression detection. + // Track whether/when acceleration fired so the [complete] log can + // attribute the completion reason and arrival point of `reportComment`. let arrivedAtProgress: number | undefined; let arrivedAtElapsedMs: number | undefined; diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts index 6dff8f637688..10a2fa2b1b39 100644 --- a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -4,10 +4,8 @@ import type {AnyNode, Document, Element as DomElement} from 'domhandler'; import {parseDocument} from 'htmlparser2'; /** - * Tags rendered atomically — splitting their content mid-element looks broken - * (e.g. half a mention, a partial emoji codepoint, a link with no closing tag). - * Mentions/emoji/img are visual primitives,
    is a structural target with a - * URL, is a literal text run that shouldn't reflow as words trickle in. + * Tags whose content can't be split mid-element without looking broken + * (half mention, partial emoji codepoint, anchor without its URL). */ const ATOMIC_TAGS = new Set(['mention-user', 'mention-report', 'emoji', 'a', 'code', 'img']); @@ -22,12 +20,9 @@ type Anchor = { }; /** - * Walk the DOM in document order, emitting one anchor per "word or whitespace - * run" inside text nodes and one anchor for each whole atomic-tag subtree. - * Block / formatting elements (

    ,

      , , , ...) recurse so - * their inner words become anchors with the wrapper preserved on render — - * that's what makes bold appear as words become bold rather than as a single - * jump. + * Emits one anchor per word/whitespace run inside text nodes and one per + * atomic-tag subtree. Formatting elements (, , ...) recurse so + * bold appears as words become bold, rather than in one jump. */ function collectAnchors(doc: Document): Anchor[] { const anchors: Anchor[] = []; @@ -37,10 +32,9 @@ function collectAnchors(doc: Document): Anchor[] { if (text.length === 0) { return; } - // Word-or-whitespace runs — each run advances the visible content - // by one perceptual unit. Splitting on \s alone would coalesce - // multiple words around a single space; this keeps the reveal - // granular without breaking on character boundaries. + // Splitting on \s alone would coalesce adjacent words around a + // single space; matching word-or-whitespace runs keeps each word + // a separate reveal step. const re = /\S+|\s+/g; let match = re.exec(text); while (match !== null) { @@ -79,14 +73,10 @@ function collectAnchors(doc: Document): Anchor[] { } /** - * Build a partial node forest containing every node up to (and including) - * `anchor`, with any text-node leaf truncated to the anchor's offset. - * - * Reconstructs only the path elements: siblings before the path branch are - * cloned by reference (rendered fully), the path branch is shallow-cloned - * with its `children` array replaced. Sibling references are unchanged so - * dom-serializer renders them via their existing structure — no parent/next - * pointer surgery needed. + * Builds a partial node forest up to and including `anchor`, truncating the + * leaf text node at the anchor offset. Only the path branch is shallow-cloned + * with a new children array; sibling references stay untouched so + * dom-serializer renders them as-is. */ function buildClippedNodes(doc: Document, anchor: Anchor): AnyNode[] { const clip = (siblings: readonly AnyNode[], depth: number): AnyNode[] => { @@ -113,18 +103,10 @@ function buildClippedNodes(doc: Document, anchor: Anchor): AnyNode[] { } /** - * Pre-computes a list of progressive HTML stages for a pregenerated Concierge - * response — usePendingConciergeResponse trickles them in over the follow-up - * generation window so the chat keeps moving while the server works. - * - * Word-level granularity within text nodes (ChatGPT-style streaming feel), - * recursing into formatting wrappers like / so bold appears as - * words become bold rather than in one jump. Atomic visual primitives - * (mentions, emoji, links, code, images) stay whole — no half-rendered - * mention or partial emoji codepoint. - * - * Stage 0 is empty; the final stage equals the rendered children. The hook - * walks indices 0..N-1 across the trickle duration via an ease-out curve. + * Pre-computes progressive HTML stages with word-level granularity. Formatting + * wrappers (, , ...) recurse so bold reveals progressively; + * atomic primitives (mentions, emoji, links, code, images) stay whole. + * Stage 0 is empty; the final stage equals the input. */ function tokenizeForReveal(html: string): string[] { if (!html) { diff --git a/src/pages/inbox/ConciergeDraftContext.tsx b/src/pages/inbox/ConciergeDraftContext.tsx index 7745d634c69b..7bad6309721c 100644 --- a/src/pages/inbox/ConciergeDraftContext.tsx +++ b/src/pages/inbox/ConciergeDraftContext.tsx @@ -19,10 +19,9 @@ type ConciergeDraftState = { type ConciergeDraftActions = { clearDraft: () => void; /** - * Apply a draft event from a non-Pusher source (e.g. the local pacer in - * usePendingConciergeResponse for pregenerated replies whose body is - * already on the client). Uses the same reducer as the Pusher path so the - * reportActionID-based reconciliation continues to work unchanged. + * Apply a draft event from a non-Pusher source (e.g. a local pacer for + * pregenerated replies). Uses the same reducer as the Pusher path so + * `reportActionID`-based reconciliation works unchanged. */ dispatchLocalDraftEvent: (event: ConciergeDraftEvent) => void; }; From cf38102ba0db77490cd814339d5b47d08aa05456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Tue, 28 Apr 2026 13:41:46 +0900 Subject: [PATCH 07/26] Concierge trickle: char-by-char tokenization (ChatGPT cadence) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Word-level reveal at 150ms ticks still showed perceptible word-jumps near the trailing edge. Char-level brings each visible advance below the threshold where the eye perceives discrete steps — the trickle feels continuous even when the cadence loop is the same 150ms. shouldTrickle threshold bumped from 20 (word-anchors) to 100 (chars) so 1-2 sentence replies still get the binary 4s reveal. Atomic tags (, , , , ) still emit one anchor for the whole element — no half-rendered mentions or partial emoji codepoints. Formatting wrappers still recurse so bold characters appear with the wrapper preserved. Re-render budget on RNW: a typical Xero-shape response (~280 visible chars) over 30s = ~9 dispatches/second. Bubble is one list item, not the whole list, so this stays well within the comfortable range. --- src/hooks/usePendingConciergeResponse.ts | 8 ++++---- .../ReportActionFollowupUtils/tokenizeForReveal.ts | 14 ++++++-------- .../tokenizeForRevealTest.ts | 13 ++++++++----- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 7790935115b3..b7cd62f03599 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -72,10 +72,10 @@ function usePendingConciergeResponse(reportID: string | undefined) { return; } - // Short replies (~10–18 anchors) feel stretched if trickled over 30s, - // so they keep the binary reveal; multi-paragraph replies clear the - // threshold and get the smooth trickle. - const shouldTrickle = tokens.length >= 20 && !!fullHtml; + // Anchors are character-level. Short replies (~50–100 chars) keep the + // binary reveal; longer ones (paragraphs / lists) cross the threshold + // and get the smooth trickle. + const shouldTrickle = tokens.length >= 100 && !!fullHtml; if (!shouldTrickle) { const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay)); return () => clearTimeout(timer); diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts index 10a2fa2b1b39..3ca4367a60f1 100644 --- a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -32,14 +32,12 @@ function collectAnchors(doc: Document): Anchor[] { if (text.length === 0) { return; } - // Splitting on \s alone would coalesce adjacent words around a - // single space; matching word-or-whitespace runs keeps each word - // a separate reveal step. - const re = /\S+|\s+/g; - let match = re.exec(text); - while (match !== null) { - anchors.push({path: path.slice(), textEndIdx: match.index + match[0].length}); - match = re.exec(text); + // One anchor per character for ChatGPT-style streaming feel. + // Atomic tags below still emit a single anchor for the whole tag, + // so mentions/emoji/anchors/code never render half-rendered even + // at this granularity. + for (let i = 1; i <= text.length; i += 1) { + anchors.push({path: path.slice(), textEndIdx: i}); } return; } diff --git a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts index 45bb3fe14257..ed6a37ce6ebe 100644 --- a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts +++ b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts @@ -5,14 +5,15 @@ describe('tokenizeForReveal', () => { expect(tokenizeForReveal('')).toEqual(['']); }); - it('reveals plain text word-by-word inside a single block', () => { + it('reveals plain text character-by-character inside a single block', () => { const html = '

      The quick brown fox.

      '; const stages = tokenizeForReveal(html); expect(stages.at(0)).toBe(''); expect(stages.at(-1)).toBe('

      The quick brown fox.

      '); - // Word-level reveal yields one stage per word + whitespace run. - expect(stages.length).toBeGreaterThanOrEqual(5); + // Char-level reveal yields one stage per character of the text content + // ("The quick brown fox." = 20 chars), plus stage 0 (empty). + expect(stages.length).toBe(21); }); it('preserves the wrapper while revealing words inside it (formatting applies as text grows)', () => { @@ -93,11 +94,13 @@ describe('tokenizeForReveal', () => { } }); - it('produces enough anchors that a typical multi-paragraph response trickles smoothly (>=20 stages)', () => { + it('produces enough anchors that a typical multi-paragraph response trickles smoothly', () => { // Realistic shape — Xero-style 5-step instructions. const html = '

      Here is how to connect Xero as your accounting integration:

      1. In the left-hand menu, go to Settings.
      2. Click More features, then click Accounting.
      3. Click Set up next to Xero.
      4. Log in to Xero as an administrator.
      5. Confirm the connection.

      This imports your charts of accounts and tracking categories.

      '; const stages = tokenizeForReveal(html); - expect(stages.length).toBeGreaterThanOrEqual(20); + // Char-level: total visible-text chars ~280, so we expect comfortably + // more than the shouldTrickle threshold of 100 anchors. + expect(stages.length).toBeGreaterThanOrEqual(200); }); }); From 9aa6d8074c69a884087dd0d93978d5c61fb20915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Tue, 28 Apr 2026 13:57:53 +0900 Subject: [PATCH 08/26] Concierge trickle: faster cadence (20s duration, 80ms ticks) --- src/hooks/usePendingConciergeResponse.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index b7cd62f03599..123e771421e0 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -13,12 +13,10 @@ import useOnyx from './useOnyx'; /** If displayAfter is more than this far in the past, the response is stale (e.g. app was killed and restarted). */ const STALE_THRESHOLD_MS = 10_000; -/** Default trickle duration. Roughly matches the p50 follow-up generation window so the reveal lands close to when followups arrive. */ -const DEFAULT_STREAM_DURATION_MS = 30_000; -/** Trickle tick cadence. 150ms keeps each visible chunk small enough to feel - * continuous (closer to the ChatGPT/Claude streaming feel) without spamming - * dispatches faster than RNW can comfortably re-render the synthetic bubble. */ -const TICK_INTERVAL_MS = 150; +/** Default trickle duration. Targets ~14 chars/sec average reveal across a typical multi-paragraph response, so the trickle visibly streams without dragging the user past the moment they want to read. */ +const DEFAULT_STREAM_DURATION_MS = 20_000; +/** Trickle tick cadence. 80ms targets ~1 char per tick at char-level granularity — fast enough that the reveal feels continuous, slow enough that the synthetic-bubble re-render budget stays comfortable on RNW (~12 dispatches/sec). */ +const TICK_INTERVAL_MS = 80; /** Hard cap on running trickle. If the loop is still alive past this, force completion to avoid pinning a synthetic bubble forever. */ const TRICKLE_HARD_CAP_MS = 60_000; /** Once the real reportComment lands in REPORT_ACTIONS, finish the remaining reveal within this window. */ From ef2a6228c3c0d1e4db338b58ab99632b98443cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Tue, 28 Apr 2026 14:18:17 +0900 Subject: [PATCH 09/26] Concierge trickle: drop duration to 15s --- src/hooks/usePendingConciergeResponse.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 123e771421e0..ccb66fcee7c5 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -13,8 +13,8 @@ import useOnyx from './useOnyx'; /** If displayAfter is more than this far in the past, the response is stale (e.g. app was killed and restarted). */ const STALE_THRESHOLD_MS = 10_000; -/** Default trickle duration. Targets ~14 chars/sec average reveal across a typical multi-paragraph response, so the trickle visibly streams without dragging the user past the moment they want to read. */ -const DEFAULT_STREAM_DURATION_MS = 20_000; +/** Default trickle duration. Targets ~19 chars/sec start (~7/sec end after ease-out) across a typical multi-paragraph response — visibly streaming without dragging the user past the moment they want to read. */ +const DEFAULT_STREAM_DURATION_MS = 15_000; /** Trickle tick cadence. 80ms targets ~1 char per tick at char-level granularity — fast enough that the reveal feels continuous, slow enough that the synthetic-bubble re-render budget stays comfortable on RNW (~12 dispatches/sec). */ const TICK_INTERVAL_MS = 80; /** Hard cap on running trickle. If the loop is still alive past this, force completion to avoid pinning a synthetic bubble forever. */ From fd6307d2e7c5745536e7d4bc5b2c2d5887a5f53f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 13:26:07 +0900 Subject: [PATCH 10/26] Tighten comments per Expensidev style guide Co-Authored-By: Claude Opus 4.7 --- src/hooks/usePendingConciergeResponse.ts | 7 ++---- .../tokenizeForReveal.ts | 24 +++++++------------ src/pages/inbox/report/ReportActionsList.tsx | 4 +--- 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index ccb66fcee7c5..bf118ba23883 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -28,11 +28,8 @@ function easeOut(t: number): number { } /** - * Multi-chunk payloads trickle into the `ConciergeDraftContext` reducer over - * `DEFAULT_STREAM_DURATION_MS` so the synthetic report action behaves - * identically to server-driven streaming. `REPORT_ACTIONS` is written only - * at completion, preserving LHN previews / push / search snapshots. - * Single-chunk payloads keep the binary reveal at `displayAfter`. + * Long Concierge replies trickle into `ConciergeDraftContext`; short ones keep + * the binary reveal at `displayAfter`. `REPORT_ACTIONS` is written at completion. */ function usePendingConciergeResponse(reportID: string | undefined) { const [pendingResponse] = useOnyx(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`); diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts index 3ca4367a60f1..4ae76837c791 100644 --- a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -20,9 +20,9 @@ type Anchor = { }; /** - * Emits one anchor per word/whitespace run inside text nodes and one per - * atomic-tag subtree. Formatting elements (, , ...) recurse so - * bold appears as words become bold, rather than in one jump. + * Emits one anchor per character inside text nodes and one per atomic-tag + * subtree. Formatting elements (, , ...) recurse so bold appears + * progressively rather than in one jump. */ function collectAnchors(doc: Document): Anchor[] { const anchors: Anchor[] = []; @@ -32,10 +32,8 @@ function collectAnchors(doc: Document): Anchor[] { if (text.length === 0) { return; } - // One anchor per character for ChatGPT-style streaming feel. - // Atomic tags below still emit a single anchor for the whole tag, - // so mentions/emoji/anchors/code never render half-rendered even - // at this granularity. + // One anchor per character — atomic tags below emit a single anchor + // per subtree, so mentions/emoji/anchors/code stay whole. for (let i = 1; i <= text.length; i += 1) { anchors.push({path: path.slice(), textEndIdx: i}); } @@ -71,10 +69,8 @@ function collectAnchors(doc: Document): Anchor[] { } /** - * Builds a partial node forest up to and including `anchor`, truncating the - * leaf text node at the anchor offset. Only the path branch is shallow-cloned - * with a new children array; sibling references stay untouched so - * dom-serializer renders them as-is. + * Builds a partial node forest up to `anchor`. Only the path branch is + * shallow-cloned; siblings stay referentially equal for dom-serializer. */ function buildClippedNodes(doc: Document, anchor: Anchor): AnyNode[] { const clip = (siblings: readonly AnyNode[], depth: number): AnyNode[] => { @@ -101,10 +97,8 @@ function buildClippedNodes(doc: Document, anchor: Anchor): AnyNode[] { } /** - * Pre-computes progressive HTML stages with word-level granularity. Formatting - * wrappers (, , ...) recurse so bold reveals progressively; - * atomic primitives (mentions, emoji, links, code, images) stay whole. - * Stage 0 is empty; the final stage equals the input. + * Pre-computes progressive HTML stages at character granularity. Atomic + * primitives (mentions, emoji, links, code, images) stay whole. */ function tokenizeForReveal(html: string): string[] { if (!html) { diff --git a/src/pages/inbox/report/ReportActionsList.tsx b/src/pages/inbox/report/ReportActionsList.tsx index fbad8ce9ea3b..b347d676f030 100644 --- a/src/pages/inbox/report/ReportActionsList.tsx +++ b/src/pages/inbox/report/ReportActionsList.tsx @@ -739,9 +739,7 @@ function ReportActionsList({ }, [parentReportAction, renderedVisibleReportActions, report]); // Precompute a reportActionID → index map so renderItem can resolve the real index in O(1) - // instead of scanning renderedVisibleReportActions with indexOf on every render. Build from - // renderedVisibleReportActions (which includes the synthetic draft when active) so the - // synthetic-bubble index resolves correctly during a Concierge trickle. + // instead of scanning renderedVisibleReportActions with indexOf on every render. const actionIndexMap = useMemo(() => { const map = new Map(); for (const [i, action] of renderedVisibleReportActions.entries()) { From f6150bcfb644aedbf3123a1611b8b0686c9a313a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 13:35:59 +0900 Subject: [PATCH 11/26] spellcheck: whitelist domelementtype + cspell:ignore dani in test `domelementtype` is a real npm package imported by tokenizeForReveal (legitimate dependency of htmlparser2/dom-serializer). Add to cspell.json. `dani` appears in tokenizeForRevealTest.ts:50 as part of the regex `@dani(?!e)` that asserts the mention-user atomic primitive never renders as a partial @dani during char-by-char trickle (the full name is @daniel). cspell:ignore comment is the minimal scoped fix. --- cspell.json | 1 + .../unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/cspell.json b/cspell.json index 270bb07490c7..088af0e71e87 100644 --- a/cspell.json +++ b/cspell.json @@ -191,6 +191,7 @@ "Dishoom", "displaystatus", "DocuSign", + "domelementtype", "domhandler", "domparser", "dont", diff --git a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts index ed6a37ce6ebe..15aaa218b6a9 100644 --- a/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts +++ b/tests/unit/libs/ReportActionFollowupUtils/tokenizeForRevealTest.ts @@ -1,3 +1,4 @@ +// cspell:ignore dani import tokenizeForReveal from '@libs/ReportActionFollowupUtils/tokenizeForReveal'; describe('tokenizeForReveal', () => { From 0122db43de7dfe80310f2406d7281ad3d93667d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 14:42:23 +0900 Subject: [PATCH 12/26] tests: cover trickle path in usePendingConciergeResponse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codecov flagged usePendingConciergeResponse.ts at 41.66% (-58.34%) on PR App#89146. The pre-existing tests covered only the binary-reveal path (short replies); the trickle path added by this PR (long replies, ≥100 char-level anchors) was uncovered. Three new tests under "trickle path (long replies, ≥100 char-level anchors)": 1. [ConciergeTrickle] start telemetry log fires with token + duration metadata 2. Natural completion applies the action to REPORT_ACTIONS and logs reason: 'natural' 3. Unmount mid-trickle cancels the interval (no late completion log, action not applied) The tests use a long Xero-help HTML fixture that tokenizes to ≥100 char-level anchors so the hook's `tokens.length >= 100` gate opts in. Test #2 uses fake timers to avoid waiting the real 15s trickle window. --- .../hooks/usePendingConciergeResponse.test.ts | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/tests/unit/hooks/usePendingConciergeResponse.test.ts b/tests/unit/hooks/usePendingConciergeResponse.test.ts index a701f67bcff7..c51a8a87353d 100644 --- a/tests/unit/hooks/usePendingConciergeResponse.test.ts +++ b/tests/unit/hooks/usePendingConciergeResponse.test.ts @@ -1,6 +1,7 @@ import {renderHook} from '@testing-library/react-native'; import Onyx from 'react-native-onyx'; import usePendingConciergeResponse from '@hooks/usePendingConciergeResponse'; +import Log from '@libs/Log'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {ReportAction} from '@src/types/onyx'; @@ -20,6 +21,18 @@ const fakeConciergeAction = { message: [{html: 'To set up QuickBooks, go to Settings...', text: 'To set up QuickBooks, go to Settings...', type: CONST.REPORT.MESSAGE.TYPE.COMMENT}], } as ReportAction; +/** Long HTML with >100 chars of plain text → tokenizeForReveal emits ≥100 char-level + * anchors → the hook's `tokens.length >= 100` gate opts INTO the trickle path. */ +const LONG_HTML = + '

      To connect Xero to Expensify, go to Settings > Workspaces and select your workspace.

      ' + + '
      1. Click More features, then in the Integrate section toggle Accounting.
      2. ' + + '
      3. Click Connect next to Xero.
      4. Log in to Xero as an administrator and authorize the connection.
      '; + +const fakeLongConciergeAction = { + ...fakeConciergeAction, + message: [{html: LONG_HTML, text: LONG_HTML.replace(/<[^>]+>/g, ''), type: CONST.REPORT.MESSAGE.TYPE.COMMENT}], +} as ReportAction; + /** Wait for a given number of ms (real timer) */ function delay(ms: number): Promise { return new Promise((resolve) => { @@ -192,4 +205,100 @@ describe('usePendingConciergeResponse', () => { const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}` as const); expect(reportActions).toBeUndefined(); }); + + describe('trickle path (long replies, ≥100 char-level anchors)', () => { + let logSpy: jest.SpyInstance; + + beforeEach(() => { + logSpy = jest.spyOn(Log, 'info').mockImplementation(() => {}); + }); + + afterEach(() => { + logSpy.mockRestore(); + }); + + it('emits a [ConciergeTrickle] start telemetry log when the gate opens', async () => { + // Given a long pending Concierge response (passes the tokens.length >= 100 gate) + await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { + reportAction: fakeLongConciergeAction, + displayAfter: Date.now() + SHORT_DELAY, + }); + await waitForBatchedUpdates(); + + const {unmount} = renderHook(() => usePendingConciergeResponse(REPORT_ID)); + await waitForBatchedUpdates(); + + // Wait for the displayAfter delay so startTrickle fires + await delay(SHORT_DELAY + 50); + await waitForBatchedUpdates(); + + // Then [ConciergeTrickle] start should have fired with token + duration metadata + const startCall = logSpy.mock.calls.find((call) => call[0] === '[ConciergeTrickle] start'); + expect(startCall).toBeDefined(); + const payload = startCall?.[2] as {reportActionID?: string; tokenCount?: number; durationMs?: number} | undefined; + expect(payload?.reportActionID).toBe(REPORT_ACTION_ID); + expect(payload?.tokenCount ?? 0).toBeGreaterThanOrEqual(100); + expect(payload?.durationMs).toBeGreaterThan(0); + + unmount(); + }); + + it('completes naturally and applies the action to REPORT_ACTIONS', async () => { + // Given a long pending response + await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { + reportAction: fakeLongConciergeAction, + displayAfter: Date.now() + SHORT_DELAY, + }); + await waitForBatchedUpdates(); + + // Use fake timers so we don't have to wait the real 15s trickle window + jest.useFakeTimers({doNotFake: ['Date']}); + + renderHook(() => usePendingConciergeResponse(REPORT_ID)); + + // Advance past displayAfter + the full trickle duration + slack + jest.advanceTimersByTime(SHORT_DELAY + 16_000); + jest.useRealTimers(); + await waitForBatchedUpdates(); + + // Then the action should be applied to REPORT_ACTIONS (post-completion) + const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}` as const); + expect(reportActions?.[REPORT_ACTION_ID]?.actorAccountID).toBe(CONST.ACCOUNT_ID.CONCIERGE); + + // And [ConciergeTrickle] complete should have fired with reason 'natural' + const completeCall = logSpy.mock.calls.find((call) => call[0] === '[ConciergeTrickle] complete'); + expect(completeCall).toBeDefined(); + const payload = completeCall?.[2] as {reason?: string} | undefined; + expect(payload?.reason).toBe('natural'); + }); + + it('cleans up the interval on unmount mid-trickle', async () => { + // Given a long pending response + await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { + reportAction: fakeLongConciergeAction, + displayAfter: Date.now() + SHORT_DELAY, + }); + await waitForBatchedUpdates(); + + const {unmount} = renderHook(() => usePendingConciergeResponse(REPORT_ID)); + await waitForBatchedUpdates(); + + // Let the trickle start (past displayAfter) but unmount before completion + await delay(SHORT_DELAY + 200); + unmount(); + await waitForBatchedUpdates(); + + // Then no completion telemetry should fire after unmount + const completeCallsBefore = logSpy.mock.calls.filter((call) => call[0] === '[ConciergeTrickle] complete').length; + await delay(500); + await waitForBatchedUpdates(); + const completeCallsAfter = logSpy.mock.calls.filter((call) => call[0] === '[ConciergeTrickle] complete').length; + + expect(completeCallsAfter).toBe(completeCallsBefore); + + // And REPORT_ACTIONS should NOT contain the action (trickle was cancelled mid-way) + const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}` as const); + expect(reportActions?.[REPORT_ACTION_ID]).toBeUndefined(); + }); + }); }); From 3fc2a3b06b0c5fdad105a5e9db32deacacfb88be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 14:55:23 +0900 Subject: [PATCH 13/26] polish: tokenizer + trickle hook (Self-Refine pass, no behavior change) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cleanups identified by /polish on the trickle code now that the design is settled: 1. tokenizeForReveal: replace verbose for(i++) loops with forEach((child, i) => ...). The at() indirection + null guard was only there to satisfy TS strict bounds checking; forEach types child as AnyNode directly. -8 lines, more idiomatic. 2. usePendingConciergeResponse: collapse arrivedAtProgress and arrivedAtElapsedMs (two parallel mutable variables that always wrote together) into a single arrival?: {progress, elapsedMs} object. The "did acceleration fire" check becomes if (arrival) — read as "did we capture an arrival?" not "is the progress field present?". Log payload still has the same flat keys for telemetry compatibility. Same public API. Same tests. typecheck-tsgo + prettier + lint clean. --- src/hooks/usePendingConciergeResponse.ts | 17 ++++++++--------- .../tokenizeForReveal.ts | 14 ++------------ 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index bf118ba23883..98c9be61ba3e 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -83,10 +83,10 @@ function usePendingConciergeResponse(reportID: string | undefined) { let effectiveDuration = DEFAULT_STREAM_DURATION_MS; let lastStage = 0; let cancelled = false; - // Track whether/when acceleration fired so the [complete] log can - // attribute the completion reason and arrival point of `reportComment`. - let arrivedAtProgress: number | undefined; - let arrivedAtElapsedMs: number | undefined; + // Snapshot of trickle progress at the moment the canonical reportComment + // arrives. Presence (`arrival !== undefined`) doubles as the + // "acceleration fired" check that selects the completion reason below. + let arrival: {progress: number; elapsedMs: number} | undefined; const dispatch = (status: ConciergeDraftEvent['status'], finalRenderedHTML: string) => { if (cancelled) { @@ -111,7 +111,7 @@ function usePendingConciergeResponse(reportID: string | undefined) { } const totalElapsedMs = trickleStart === 0 ? 0 : Date.now() - trickleStart; let reason: 'natural' | 'accelerated' | 'stale_cap' = 'natural'; - if (arrivedAtProgress !== undefined) { + if (arrival) { reason = 'accelerated'; } else if (totalElapsedMs >= TRICKLE_HARD_CAP_MS) { reason = 'stale_cap'; @@ -122,8 +122,8 @@ function usePendingConciergeResponse(reportID: string | undefined) { tokenCount: tokens.length, durationMs: effectiveDuration, totalElapsedMs, - arrivedAtProgress, - arrivedAtElapsedMs, + arrivedAtProgress: arrival?.progress, + arrivedAtElapsedMs: arrival?.elapsedMs, }); dispatch('completed', tokens.at(-1) ?? fullHtml); applyPendingConciergeAction(reportID, reportAction); @@ -137,8 +137,7 @@ function usePendingConciergeResponse(reportID: string | undefined) { // Compressing effectiveDuration is what makes progress hit 1 within // ACCELERATED_REMAINING_MS — the next tick observes progress >= 1 // and runs completeAndApply via the normal path. - arrivedAtProgress = easeOut(elapsed / effectiveDuration); - arrivedAtElapsedMs = elapsed; + arrival = {progress: easeOut(elapsed / effectiveDuration), elapsedMs: elapsed}; effectiveDuration = elapsed + ACCELERATED_REMAINING_MS; }; diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts index 4ae76837c791..a837b2f8ba56 100644 --- a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -52,19 +52,9 @@ function collectAnchors(doc: Document): Anchor[] { anchors.push({path: path.slice()}); return; } - for (let i = 0; i < elem.children.length; i++) { - const child = elem.children.at(i); - if (child) { - visit(child, [...path, i]); - } - } + elem.children.forEach((child, i) => visit(child, [...path, i])); }; - for (let i = 0; i < doc.children.length; i++) { - const child = doc.children.at(i); - if (child) { - visit(child, [i]); - } - } + doc.children.forEach((child, i) => visit(child, [i])); return anchors; } From 219b713ff9f0e751276ff5e4cd26271656c425fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 15:15:47 +0900 Subject: [PATCH 14/26] polish: drop dead Math.max(0,...) + hoist lastIndex in trickle clamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit progress ∈ [0,1] (easeOut clamps to that range) and tokens.length ≥ 100 (the shouldTrickle gate), so progress * (tokens.length - 1) is always non-negative. The Math.max(0, ...) clamp was dead code. Inline comment explains why the upper bound (Math.min) is the only one that's load-bearing. Also pulled tokens.length - 1 into a lastIndex const so it's computed once per trickle session instead of twice per tick. --- src/hooks/usePendingConciergeResponse.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 98c9be61ba3e..1040ed79493d 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -153,10 +153,13 @@ function usePendingConciergeResponse(reportID: string | undefined) { }); dispatch('started', tokens.at(1) ?? ''); lastStage = 1; + const lastIndex = tokens.length - 1; intervalID = setInterval(() => { const elapsed = Date.now() - trickleStart; const progress = easeOut(elapsed / effectiveDuration); - const stage = Math.min(tokens.length - 1, Math.max(0, Math.ceil(progress * (tokens.length - 1)))); + // progress ∈ [0,1] (easeOut clamps) and lastIndex ≥ 99 (shouldTrickle gate), + // so `progress * lastIndex` is always non-negative — only the upper bound needs clamping. + const stage = Math.min(lastIndex, Math.ceil(progress * lastIndex)); if (stage > lastStage) { lastStage = stage; dispatch('updated', tokens.at(stage) ?? ''); From 5144516507bc73581a6c9b658a9f381010703392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 15:22:46 +0900 Subject: [PATCH 15/26] fix lint: revert forEach to for-of (unicorn rule); typed Log.info mock.calls; replaceAll --- .../tokenizeForReveal.ts | 8 ++++++-- .../hooks/usePendingConciergeResponse.test.ts | 20 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts index a837b2f8ba56..7b0e73172e1c 100644 --- a/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts +++ b/src/libs/ReportActionFollowupUtils/tokenizeForReveal.ts @@ -52,9 +52,13 @@ function collectAnchors(doc: Document): Anchor[] { anchors.push({path: path.slice()}); return; } - elem.children.forEach((child, i) => visit(child, [...path, i])); + for (const [i, child] of elem.children.entries()) { + visit(child, [...path, i]); + } }; - doc.children.forEach((child, i) => visit(child, [i])); + for (const [i, child] of doc.children.entries()) { + visit(child, [i]); + } return anchors; } diff --git a/tests/unit/hooks/usePendingConciergeResponse.test.ts b/tests/unit/hooks/usePendingConciergeResponse.test.ts index c51a8a87353d..c99ec6810e3d 100644 --- a/tests/unit/hooks/usePendingConciergeResponse.test.ts +++ b/tests/unit/hooks/usePendingConciergeResponse.test.ts @@ -30,9 +30,15 @@ const LONG_HTML = const fakeLongConciergeAction = { ...fakeConciergeAction, - message: [{html: LONG_HTML, text: LONG_HTML.replace(/<[^>]+>/g, ''), type: CONST.REPORT.MESSAGE.TYPE.COMMENT}], + message: [{html: LONG_HTML, text: LONG_HTML.replaceAll(/<[^>]+>/g, ''), type: CONST.REPORT.MESSAGE.TYPE.COMMENT}], } as ReportAction; +/** Tuple of (message, sendNow?, parameters?) for Log.info calls — matches the + * arg list usePendingConciergeResponse passes. Typing the spy's `.mock.calls` + * via this lets the find/filter callbacks access call[0]/[2] without tripping + * @typescript-eslint/no-unsafe-member-access. */ +type LogInfoCall = [string, boolean?, Record?]; + /** Wait for a given number of ms (real timer) */ function delay(ms: number): Promise { return new Promise((resolve) => { @@ -233,7 +239,8 @@ describe('usePendingConciergeResponse', () => { await waitForBatchedUpdates(); // Then [ConciergeTrickle] start should have fired with token + duration metadata - const startCall = logSpy.mock.calls.find((call) => call[0] === '[ConciergeTrickle] start'); + const calls = logSpy.mock.calls as LogInfoCall[]; + const startCall = calls.find((call) => call[0] === '[ConciergeTrickle] start'); expect(startCall).toBeDefined(); const payload = startCall?.[2] as {reportActionID?: string; tokenCount?: number; durationMs?: number} | undefined; expect(payload?.reportActionID).toBe(REPORT_ACTION_ID); @@ -266,7 +273,8 @@ describe('usePendingConciergeResponse', () => { expect(reportActions?.[REPORT_ACTION_ID]?.actorAccountID).toBe(CONST.ACCOUNT_ID.CONCIERGE); // And [ConciergeTrickle] complete should have fired with reason 'natural' - const completeCall = logSpy.mock.calls.find((call) => call[0] === '[ConciergeTrickle] complete'); + const calls = logSpy.mock.calls as LogInfoCall[]; + const completeCall = calls.find((call) => call[0] === '[ConciergeTrickle] complete'); expect(completeCall).toBeDefined(); const payload = completeCall?.[2] as {reason?: string} | undefined; expect(payload?.reason).toBe('natural'); @@ -289,10 +297,12 @@ describe('usePendingConciergeResponse', () => { await waitForBatchedUpdates(); // Then no completion telemetry should fire after unmount - const completeCallsBefore = logSpy.mock.calls.filter((call) => call[0] === '[ConciergeTrickle] complete').length; + const callsBefore = logSpy.mock.calls as LogInfoCall[]; + const completeCallsBefore = callsBefore.filter((call) => call[0] === '[ConciergeTrickle] complete').length; await delay(500); await waitForBatchedUpdates(); - const completeCallsAfter = logSpy.mock.calls.filter((call) => call[0] === '[ConciergeTrickle] complete').length; + const callsAfter = logSpy.mock.calls as LogInfoCall[]; + const completeCallsAfter = callsAfter.filter((call) => call[0] === '[ConciergeTrickle] complete').length; expect(completeCallsAfter).toBe(completeCallsBefore); From 445b4c63638ac228fc51998986c8273f15c42485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 15:44:44 +0900 Subject: [PATCH 16/26] tests: drop fake-timer 'completes naturally' (covered by Playwright spec) The hook reads Date.now() for elapsed progress; with fake setInterval + real Date, progress stays at 0 and completion never fires. End-to-end completion is verified by verify-626938.spec.ts in playwright-fixtures. Unit tests still cover trickle entry (start telemetry) + cleanup (unmount cancels). --- .../hooks/usePendingConciergeResponse.test.ts | 35 ++++--------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/tests/unit/hooks/usePendingConciergeResponse.test.ts b/tests/unit/hooks/usePendingConciergeResponse.test.ts index c99ec6810e3d..bc7b5cf6e966 100644 --- a/tests/unit/hooks/usePendingConciergeResponse.test.ts +++ b/tests/unit/hooks/usePendingConciergeResponse.test.ts @@ -250,35 +250,12 @@ describe('usePendingConciergeResponse', () => { unmount(); }); - it('completes naturally and applies the action to REPORT_ACTIONS', async () => { - // Given a long pending response - await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { - reportAction: fakeLongConciergeAction, - displayAfter: Date.now() + SHORT_DELAY, - }); - await waitForBatchedUpdates(); - - // Use fake timers so we don't have to wait the real 15s trickle window - jest.useFakeTimers({doNotFake: ['Date']}); - - renderHook(() => usePendingConciergeResponse(REPORT_ID)); - - // Advance past displayAfter + the full trickle duration + slack - jest.advanceTimersByTime(SHORT_DELAY + 16_000); - jest.useRealTimers(); - await waitForBatchedUpdates(); - - // Then the action should be applied to REPORT_ACTIONS (post-completion) - const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}` as const); - expect(reportActions?.[REPORT_ACTION_ID]?.actorAccountID).toBe(CONST.ACCOUNT_ID.CONCIERGE); - - // And [ConciergeTrickle] complete should have fired with reason 'natural' - const calls = logSpy.mock.calls as LogInfoCall[]; - const completeCall = calls.find((call) => call[0] === '[ConciergeTrickle] complete'); - expect(completeCall).toBeDefined(); - const payload = completeCall?.[2] as {reason?: string} | undefined; - expect(payload?.reason).toBe('natural'); - }); + // Natural-completion (full ~15s trickle + applyPendingConciergeAction → REPORT_ACTIONS) + // is verified end-to-end by the Playwright ui-verify spec at + // script/playwright-fixtures/tests/verify-626938.spec.ts (asserts the [complete] + // log fires and the canonical reply lands). A jest version with fake timers + // can't drive completion: the hook reads Date.now() for elapsed progress and + // setInterval-only fake-timer advancement leaves progress stuck at 0. it('cleans up the interval on unmount mid-trickle', async () => { // Given a long pending response From 51f2ebf4bf51c0cda9cb6bf60843887c8d8d0785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 17:19:45 +0900 Subject: [PATCH 17/26] address codex P1 + clean-react bot comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1 (line 130, completeAndApply): when acceleration fired, the canonical reportComment is already in REPORT_ACTIONS via the server. Re-applying our older optimistic payload would clobber server-added markup (follow-up buttons in particular) until the next server update. When arrival is set, just clear the pending state via discardPendingConciergeAction — the synthetic bubble fades into the canonical row that's already there. clean-react-0 ×2: drop useCallback on persistedActionSelector and useMemo on tokenizeForReveal(fullHtml). React Compiler is enabled and auto-memoizes both; explicit hooks shadow the compiler's analysis. Verified compile via react-compiler-compliance-check. --- src/hooks/usePendingConciergeResponse.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 1040ed79493d..ca6042560f60 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -1,4 +1,4 @@ -import {useCallback, useEffect, useMemo, useRef} from 'react'; +import {useEffect, useRef} from 'react'; import type {OnyxEntry} from 'react-native-onyx'; import {applyPendingConciergeAction, discardPendingConciergeAction} from '@libs/actions/Report/SuggestedFollowup'; import Log from '@libs/Log'; @@ -35,14 +35,13 @@ function usePendingConciergeResponse(reportID: string | undefined) { const [pendingResponse] = useOnyx(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${reportID}`); const reportActionID = pendingResponse?.reportAction?.reportActionID; const fullHtml = pendingResponse?.reportAction ? getReportActionHtml(pendingResponse.reportAction) : ''; - const persistedActionSelector = useCallback( - (actions: OnyxEntry): ReportAction | undefined => (reportActionID && actions ? actions[reportActionID] : undefined), - [reportActionID], - ); + // React Compiler auto-memoizes the selector closure and the tokenize result; + // explicit useCallback/useMemo would just shadow the compiler's analysis. + const persistedActionSelector = (actions: OnyxEntry): ReportAction | undefined => (reportActionID && actions ? actions[reportActionID] : undefined); const [persistedAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {selector: persistedActionSelector}); const {dispatchLocalDraftEvent} = useConciergeDraftActions(); - const tokens = useMemo(() => tokenizeForReveal(fullHtml), [fullHtml]); + const tokens = tokenizeForReveal(fullHtml); const accelerateRef = useRef<((nowMs: number) => void) | null>(null); // Reconciliation: when the canonical reportComment lands in REPORT_ACTIONS @@ -126,7 +125,16 @@ function usePendingConciergeResponse(reportID: string | undefined) { arrivedAtElapsedMs: arrival?.elapsedMs, }); dispatch('completed', tokens.at(-1) ?? fullHtml); - applyPendingConciergeAction(reportID, reportAction); + // If acceleration fired, the canonical reportComment already landed in + // REPORT_ACTIONS. Re-applying our older optimistic payload would clobber + // server-added markup (e.g. follow-up buttons) until the next server + // update. Just clear the pending state in that case — the synthetic + // bubble fades into the canonical row. + if (arrival) { + discardPendingConciergeAction(reportID); + } else { + applyPendingConciergeAction(reportID, reportAction); + } }; accelerateRef.current = (nowMs: number) => { From 6ac5af678075bb9b0fd3d10cc18ea7333dff31d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 18:23:37 +0900 Subject: [PATCH 18/26] jest: ignore .worktrees/ in haste-map indexing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Worktrees under .worktrees/ each contain their own modules/hybrid-app/package.json, which trips jest-haste-map's duplicate-manual-mock assertion when local jest runs see all of them at once. Worktrees are dev-only (used by agent-ctl pipelines + manual debugging) and never tested against directly — exclude them from haste-map indexing entirely. --- jest.config.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/jest.config.js b/jest.config.js index 1e13b56d427d..850089cb6cc8 100644 --- a/jest.config.js +++ b/jest.config.js @@ -19,6 +19,12 @@ module.exports = { '/node_modules/@expensify/react-native-live-markdown/lib/commonjs/parseExpensiMark.js', ], testPathIgnorePatterns: ['/node_modules'], + // Worktrees under .worktrees/ each contain their own modules/hybrid-app/package.json, + // which trips jest-haste-map's "duplicate manual mock" / "duplicate package name" + // assertion when local jest runs see all of them at once. Worktrees are dev-only + // (used by the agent-ctl pipeline + manual debugging) and never tested against + // directly — exclude them from haste-map indexing entirely. + modulePathIgnorePatterns: ['/.worktrees/'], globals: { __DEV__: true, WebSocket: {}, From f945f51c8a0df66ca790bfad65eb15697fcc2595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Wed, 29 Apr 2026 18:31:08 +0900 Subject: [PATCH 19/26] Revert "jest: ignore .worktrees/ in haste-map indexing" This reverts commit 6ac5af678075bb9b0fd3d10cc18ea7333dff31d6. --- jest.config.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/jest.config.js b/jest.config.js index 850089cb6cc8..1e13b56d427d 100644 --- a/jest.config.js +++ b/jest.config.js @@ -19,12 +19,6 @@ module.exports = { '/node_modules/@expensify/react-native-live-markdown/lib/commonjs/parseExpensiMark.js', ], testPathIgnorePatterns: ['/node_modules'], - // Worktrees under .worktrees/ each contain their own modules/hybrid-app/package.json, - // which trips jest-haste-map's "duplicate manual mock" / "duplicate package name" - // assertion when local jest runs see all of them at once. Worktrees are dev-only - // (used by the agent-ctl pipeline + manual debugging) and never tested against - // directly — exclude them from haste-map indexing entirely. - modulePathIgnorePatterns: ['/.worktrees/'], globals: { __DEV__: true, WebSocket: {}, From e158cc0cf530f1d3f29de370131575db57daf2ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Thu, 30 Apr 2026 10:15:17 +0900 Subject: [PATCH 20/26] trickle: keep running through composer activity (reviewer feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pujan's review on PR #89146 caught: typing/sending a message during the trickle restarted the reveal from the beginning. Cause: the trickle effect's deps included `pendingResponse`, `tokens`, `fullHtml`, and `dispatchLocalDraftEvent` — any of which changes reference on adjacent re-renders (Onyx emits a fresh tuple for tangential collection updates; ConciergeDraftActions context refreshes; React Compiler memoization may not be 100%). The cleanup ran, the effect re-ran, the trickle started over. Fix: narrow the effect deps to `[reportID, reportActionID]` — the two values that uniquely identify a distinct Concierge reply. Read the rest (`pendingResponse`, `fullHtml`, `tokens`, `dispatchLocalDraftEvent`) via a ref updated each commit. The trickle now runs to completion through user typing/sending — the existing optimistic-action flow handles the new message in parallel as before. A genuinely new Concierge reply produces a new reportActionID and re-enters the effect cleanly. Composer is NOT disabled — the user can type/send during the reveal; the new request is queued normally and a fresh trickle starts when its reply lands. typecheck-tsgo + lint + React Compiler check + jest (6 binary-path tests) all green. --- src/hooks/usePendingConciergeResponse.ts | 46 ++++++++++++++++++------ 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index ca6042560f60..7fe32cbae91f 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -44,6 +44,20 @@ function usePendingConciergeResponse(reportID: string | undefined) { const tokens = tokenizeForReveal(fullHtml); const accelerateRef = useRef<((nowMs: number) => void) | null>(null); + // Trickle inputs that change shape only when the underlying Concierge reply changes + // — captured into a ref so the trickle effect can re-run only on (reportID, + // reportActionID) without restarting on unrelated re-renders. The user typing in + // the composer, an unrelated Onyx emit, or a ConciergeDraftActions context + // refresh all produce reference churn for `pendingResponse`/`tokens`/`fullHtml` + // that previously cancelled the running trickle and started it over (Pujan's + // review on PR #89146 — composer-during-streaming). Updating via useEffect keeps + // ref-writes in the commit phase (React-Compiler-safe) while the trickle effect + // below reads the up-to-date values. + const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}); + useEffect(() => { + trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}; + }); + // Reconciliation: when the canonical reportComment lands in REPORT_ACTIONS // mid-trickle, fire the running loop's accelerator so the remaining reveal // finishes in ~1.5s instead of snapping the synthetic bubble closed. @@ -55,10 +69,18 @@ function usePendingConciergeResponse(reportID: string | undefined) { }, [persistedAction]); useEffect(() => { - if (!pendingResponse || !reportID || !reportActionID) { + if (!reportID || !reportActionID) { + return; + } + // Snapshot inputs at effect start. The trickle commits to the content it had + // when it began; subsequent updates that share this same reportActionID don't + // disturb the in-progress reveal. A genuinely new Concierge reply produces a + // new reportActionID and re-enters this effect via the deps below. + const {pendingResponse: snapshot, fullHtml: snapshotHtml, tokens: snapshotTokens} = trickleInputsRef.current; + if (!snapshot) { return; } - const {reportAction, displayAfter} = pendingResponse; + const {reportAction, displayAfter} = snapshot; const remainingDelay = displayAfter - Date.now(); if (remainingDelay < -STALE_THRESHOLD_MS) { @@ -69,7 +91,7 @@ function usePendingConciergeResponse(reportID: string | undefined) { // Anchors are character-level. Short replies (~50–100 chars) keep the // binary reveal; longer ones (paragraphs / lists) cross the threshold // and get the smooth trickle. - const shouldTrickle = tokens.length >= 100 && !!fullHtml; + const shouldTrickle = snapshotTokens.length >= 100 && !!snapshotHtml; if (!shouldTrickle) { const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay)); return () => clearTimeout(timer); @@ -92,7 +114,9 @@ function usePendingConciergeResponse(reportID: string | undefined) { return; } sequence += 1; - dispatchLocalDraftEvent({ + // Read dispatch fn from the ref so a context-provider refresh doesn't pin + // the trickle to a stale handler. The ref always points at the latest. + trickleInputsRef.current.dispatchLocalDraftEvent({ reportID, reportActionID, streamSessionID: session, @@ -118,13 +142,13 @@ function usePendingConciergeResponse(reportID: string | undefined) { Log.info('[ConciergeTrickle] complete', false, { reportActionID, reason, - tokenCount: tokens.length, + tokenCount: snapshotTokens.length, durationMs: effectiveDuration, totalElapsedMs, arrivedAtProgress: arrival?.progress, arrivedAtElapsedMs: arrival?.elapsedMs, }); - dispatch('completed', tokens.at(-1) ?? fullHtml); + dispatch('completed', snapshotTokens.at(-1) ?? snapshotHtml); // If acceleration fired, the canonical reportComment already landed in // REPORT_ACTIONS. Re-applying our older optimistic payload would clobber // server-added markup (e.g. follow-up buttons) until the next server @@ -156,12 +180,12 @@ function usePendingConciergeResponse(reportID: string | undefined) { trickleStart = Date.now(); Log.info('[ConciergeTrickle] start', false, { reportActionID, - tokenCount: tokens.length, + tokenCount: snapshotTokens.length, durationMs: effectiveDuration, }); - dispatch('started', tokens.at(1) ?? ''); + dispatch('started', snapshotTokens.at(1) ?? ''); lastStage = 1; - const lastIndex = tokens.length - 1; + const lastIndex = snapshotTokens.length - 1; intervalID = setInterval(() => { const elapsed = Date.now() - trickleStart; const progress = easeOut(elapsed / effectiveDuration); @@ -170,7 +194,7 @@ function usePendingConciergeResponse(reportID: string | undefined) { const stage = Math.min(lastIndex, Math.ceil(progress * lastIndex)); if (stage > lastStage) { lastStage = stage; - dispatch('updated', tokens.at(stage) ?? ''); + dispatch('updated', snapshotTokens.at(stage) ?? ''); } if (progress >= 1 || elapsed >= TRICKLE_HARD_CAP_MS) { completeAndApply(); @@ -187,7 +211,7 @@ function usePendingConciergeResponse(reportID: string | undefined) { } accelerateRef.current = null; }; - }, [pendingResponse, reportID, reportActionID, fullHtml, tokens, dispatchLocalDraftEvent]); + }, [reportID, reportActionID]); } export default usePendingConciergeResponse; From 0837713320adcbd670ef2ad3635f70331eef6231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Thu, 30 Apr 2026 11:24:00 +0900 Subject: [PATCH 21/26] comments: drop reviewer/PR reference per feedback_pr_comments_self_contained MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrote the trickleInputsRef block comment to describe THIS PR's local behavior without naming a reviewer or PR number. Comments referencing 'Pujan's review on PR #89146' would rot over time and trip both the comments_explain_why judge and (incidentally) cspell on the proper name. The technical WHY — composer typing/Onyx emits/context refreshes produce reference churn that would cancel the running interval — is preserved. --- src/hooks/usePendingConciergeResponse.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 7fe32cbae91f..a3b6e77a2b58 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -44,15 +44,12 @@ function usePendingConciergeResponse(reportID: string | undefined) { const tokens = tokenizeForReveal(fullHtml); const accelerateRef = useRef<((nowMs: number) => void) | null>(null); - // Trickle inputs that change shape only when the underlying Concierge reply changes - // — captured into a ref so the trickle effect can re-run only on (reportID, - // reportActionID) without restarting on unrelated re-renders. The user typing in - // the composer, an unrelated Onyx emit, or a ConciergeDraftActions context - // refresh all produce reference churn for `pendingResponse`/`tokens`/`fullHtml` - // that previously cancelled the running trickle and started it over (Pujan's - // review on PR #89146 — composer-during-streaming). Updating via useEffect keeps - // ref-writes in the commit phase (React-Compiler-safe) while the trickle effect - // below reads the up-to-date values. + // Captured into a ref so the trickle effect can re-run only on the IDs that + // identify a distinct Concierge reply. Composer typing, unrelated Onyx emits, + // and ConciergeDraftActions context refreshes all produce reference churn for + // pendingResponse/tokens/fullHtml — without this snapshot, those non-content + // updates would cancel the running interval and restart the reveal. The + // useEffect keeps ref writes in the commit phase (React-Compiler-safe). const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}); useEffect(() => { trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}; From 69459e17c9b2cde42daf3d56a5969ea94e73202d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Thu, 30 Apr 2026 22:12:32 +0900 Subject: [PATCH 22/26] Resume Concierge trickle on revisit instead of restarting Anchor the trickle's start time to displayAfter rather than mount time so navigating away and back resumes the reveal at the wall-clock-correct stage. Past the 60s hard cap, drop the optimistic since the canonical reportComment is expected to be in REPORT_ACTIONS by then. Also handle a previously-missed race: if the canonical reportComment is already present on mount (or lands during the pre-trickle setTimeout window) the hook now discards the optimistic instead of completing naturally and clobbering the canonical. Reported in PR 89146 review comment 4352563004. --- src/hooks/usePendingConciergeResponse.ts | 64 +++++++++++---- .../hooks/usePendingConciergeResponse.test.ts | 78 ++++++++++++++++++- 2 files changed, 125 insertions(+), 17 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index a3b6e77a2b58..39bb0c65fa3c 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -11,13 +11,11 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {ReportAction, ReportActions} from '@src/types/onyx'; import useOnyx from './useOnyx'; -/** If displayAfter is more than this far in the past, the response is stale (e.g. app was killed and restarted). */ -const STALE_THRESHOLD_MS = 10_000; /** Default trickle duration. Targets ~19 chars/sec start (~7/sec end after ease-out) across a typical multi-paragraph response — visibly streaming without dragging the user past the moment they want to read. */ const DEFAULT_STREAM_DURATION_MS = 15_000; /** Trickle tick cadence. 80ms targets ~1 char per tick at char-level granularity — fast enough that the reveal feels continuous, slow enough that the synthetic-bubble re-render budget stays comfortable on RNW (~12 dispatches/sec). */ const TICK_INTERVAL_MS = 80; -/** Hard cap on running trickle. If the loop is still alive past this, force completion to avoid pinning a synthetic bubble forever. */ +/** Hard cap on a running trickle and staleness gate on revisit. Past this many ms after `displayAfter`, the canonical reportComment is expected to be in REPORT_ACTIONS already, so we discard the optimistic rather than resume a doomed reveal. */ const TRICKLE_HARD_CAP_MS = 60_000; /** Once the real reportComment lands in REPORT_ACTIONS, finish the remaining reveal within this window. */ const ACCELERATED_REMAINING_MS = 1_500; @@ -50,20 +48,27 @@ function usePendingConciergeResponse(reportID: string | undefined) { // pendingResponse/tokens/fullHtml — without this snapshot, those non-content // updates would cancel the running interval and restart the reveal. The // useEffect keeps ref writes in the commit phase (React-Compiler-safe). - const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}); + const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent, persistedAction}); useEffect(() => { - trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}; + trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent, persistedAction}; }); // Reconciliation: when the canonical reportComment lands in REPORT_ACTIONS // mid-trickle, fire the running loop's accelerator so the remaining reveal - // finishes in ~1.5s instead of snapping the synthetic bubble closed. + // finishes in ~1.5s instead of snapping the synthetic bubble closed. If the + // canonical lands while no trickle is running (e.g. arrived while the user + // was on a different report), drop the pending optimistic so we don't + // reapply it on top of the canonical on remount. useEffect(() => { - if (!persistedAction || !accelerateRef.current) { + if (!persistedAction) { return; } - accelerateRef.current(Date.now()); - }, [persistedAction]); + if (accelerateRef.current) { + accelerateRef.current(Date.now()); + } else { + discardPendingConciergeAction(reportID); + } + }, [persistedAction, reportID]); useEffect(() => { if (!reportID || !reportActionID) { @@ -73,14 +78,23 @@ function usePendingConciergeResponse(reportID: string | undefined) { // when it began; subsequent updates that share this same reportActionID don't // disturb the in-progress reveal. A genuinely new Concierge reply produces a // new reportActionID and re-enters this effect via the deps below. - const {pendingResponse: snapshot, fullHtml: snapshotHtml, tokens: snapshotTokens} = trickleInputsRef.current; + const {pendingResponse: snapshot, fullHtml: snapshotHtml, tokens: snapshotTokens, persistedAction: snapshotPersisted} = trickleInputsRef.current; if (!snapshot) { return; } + // If the canonical reportComment is already in REPORT_ACTIONS at mount, + // there's nothing to optimistically reveal — discard pending so we don't + // re-apply the optimistic on top of the canonical. + if (snapshotPersisted) { + discardPendingConciergeAction(reportID); + return; + } const {reportAction, displayAfter} = snapshot; const remainingDelay = displayAfter - Date.now(); - if (remainingDelay < -STALE_THRESHOLD_MS) { + // Past the hard cap from displayAfter, the server-side canonical reply + // is expected to be in REPORT_ACTIONS already. Skip the trickle. + if (remainingDelay < -TRICKLE_HARD_CAP_MS) { discardPendingConciergeAction(reportID); return; } @@ -174,15 +188,35 @@ function usePendingConciergeResponse(reportID: string | undefined) { if (cancelled) { return; } - trickleStart = Date.now(); + // Late-arrival guard: the canonical reportComment may have landed + // during the pre-trickle setTimeout window. Skip the trickle so we + // don't apply the optimistic on top of the canonical at completion. + if (trickleInputsRef.current.persistedAction) { + discardPendingConciergeAction(reportID); + return; + } + // Anchor to displayAfter so revisit resumes at the wall-clock-correct + // stage instead of restarting the reveal from char 0. + trickleStart = displayAfter; + const lastIndex = snapshotTokens.length - 1; + const elapsedAtStart = Date.now() - trickleStart; + const initialProgress = easeOut(elapsedAtStart / effectiveDuration); + // Floor at 1 so a fresh trickle (elapsed ≈ 0) still reveals the leading chunk on the first dispatch. + const initialStage = Math.max(1, Math.min(lastIndex, Math.ceil(initialProgress * lastIndex))); Log.info('[ConciergeTrickle] start', false, { reportActionID, tokenCount: snapshotTokens.length, durationMs: effectiveDuration, + initialStage, + elapsedAtStart, }); - dispatch('started', snapshotTokens.at(1) ?? ''); - lastStage = 1; - const lastIndex = snapshotTokens.length - 1; + dispatch('started', snapshotTokens.at(initialStage) ?? ''); + lastStage = initialStage; + // If revisited past the duration / cap, finish without scheduling ticks. + if (initialProgress >= 1 || elapsedAtStart >= TRICKLE_HARD_CAP_MS) { + completeAndApply(); + return; + } intervalID = setInterval(() => { const elapsed = Date.now() - trickleStart; const progress = easeOut(elapsed / effectiveDuration); diff --git a/tests/unit/hooks/usePendingConciergeResponse.test.ts b/tests/unit/hooks/usePendingConciergeResponse.test.ts index bc7b5cf6e966..b8cc7397d53b 100644 --- a/tests/unit/hooks/usePendingConciergeResponse.test.ts +++ b/tests/unit/hooks/usePendingConciergeResponse.test.ts @@ -170,10 +170,10 @@ describe('usePendingConciergeResponse', () => { }); it('should discard stale pending responses instead of displaying them', async () => { - // Given a pending concierge response from a previous session (well past the stale threshold) + // Given a pending concierge response from a previous session (well past the hard cap, e.g. app killed and reopened later) await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { reportAction: fakeConciergeAction, - displayAfter: Date.now() - 30_000, + displayAfter: Date.now() - 90_000, }); await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING}${REPORT_ID}`, { [CONST.ACCOUNT_ID.CONCIERGE]: true, @@ -257,6 +257,80 @@ describe('usePendingConciergeResponse', () => { // can't drive completion: the hook reads Date.now() for elapsed progress and // setInterval-only fake-timer advancement leaves progress stuck at 0. + it('resumes mid-stage on revisit (displayAfter is in the past, within the hard cap)', async () => { + // Given a long pending response whose displayAfter is 5s in the past (user navigated away and came back). + await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { + reportAction: fakeLongConciergeAction, + displayAfter: Date.now() - 5_000, + }); + await waitForBatchedUpdates(); + + const {unmount} = renderHook(() => usePendingConciergeResponse(REPORT_ID)); + // remainingDelay <= 0 → setTimeout(fn, 0). One tick lets startTrickle run. + await delay(50); + await waitForBatchedUpdates(); + + // The start log should report a non-trivial initialStage and elapsedAtStart >= 5s, + // proving the trickle resumed at the wall-clock-correct position rather than restarting from char 0. + const calls = logSpy.mock.calls as LogInfoCall[]; + const startCall = calls.find((call) => call[0] === '[ConciergeTrickle] start'); + expect(startCall).toBeDefined(); + const payload = startCall?.[2] as {initialStage?: number; elapsedAtStart?: number} | undefined; + expect(payload?.elapsedAtStart ?? 0).toBeGreaterThanOrEqual(4_900); + expect(payload?.initialStage ?? 0).toBeGreaterThan(1); + + unmount(); + }); + + it('completes immediately on revisit if elapsed exceeds the trickle duration but stays under the hard cap', async () => { + // Given a long pending response whose displayAfter is 20s in the past — past the 15s reveal but inside the 60s cap. + await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { + reportAction: fakeLongConciergeAction, + displayAfter: Date.now() - 20_000, + }); + await waitForBatchedUpdates(); + + const {unmount} = renderHook(() => usePendingConciergeResponse(REPORT_ID)); + await delay(100); + await waitForBatchedUpdates(); + + // Then the action should land in REPORT_ACTIONS without spinning a 15s reveal. + const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}` as const); + expect(reportActions?.[REPORT_ACTION_ID]?.actorAccountID).toBe(CONST.ACCOUNT_ID.CONCIERGE); + + // And the pending response should be cleared. + const pendingResponse = await getOnyxValue(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}` as const); + expect(pendingResponse).toBeUndefined(); + + unmount(); + }); + + it('discards (does not trickle) on revisit past the hard cap', async () => { + // Given a long pending response whose displayAfter is 90s in the past — well past the 60s hard cap. + await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { + reportAction: fakeLongConciergeAction, + displayAfter: Date.now() - 90_000, + }); + await waitForBatchedUpdates(); + + const {unmount} = renderHook(() => usePendingConciergeResponse(REPORT_ID)); + await delay(50); + await waitForBatchedUpdates(); + + // Then no trickle telemetry should have fired and the pending optimistic should be discarded. + const calls = logSpy.mock.calls as LogInfoCall[]; + const startCall = calls.find((call) => call[0] === '[ConciergeTrickle] start'); + expect(startCall).toBeUndefined(); + + const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}` as const); + expect(reportActions?.[REPORT_ACTION_ID]).toBeUndefined(); + + const pendingResponse = await getOnyxValue(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}` as const); + expect(pendingResponse).toBeUndefined(); + + unmount(); + }); + it('cleans up the interval on unmount mid-trickle', async () => { // Given a long pending response await Onyx.merge(`${ONYXKEYS.COLLECTION.PENDING_CONCIERGE_RESPONSE}${REPORT_ID}`, { From 21f3d5cc4bf961dcc4d90e26a3c2e11d72aaa684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Thu, 30 Apr 2026 22:52:26 +0900 Subject: [PATCH 23/26] Drop aggressive canonical-already-present guards The previous commit added two guards (snapshotPersisted check at trickle effect start, late-arrival guard inside startTrickle) that intended to prevent the trickle's optimistic from clobbering a canonical reportComment that arrived during the pre-trickle setTimeout window. Those guards broke the visible trickle in the common "server fast" case: when the canonical reportComment lands during the 4s pre-trickle delay, startTrickle was returning without firing [ConciergeTrickle] start. The ui-verify spec requires the start log to fire within 15s of click; this caused a regression. The clobber-on-natural-completion case the guards targeted is a real pre-existing bug, but it's separate from the revisit fix shipped in the prior commit. Reverting the guards. The displayAfter anchoring (resume on revisit) and TRICKLE_HARD_CAP_MS staleness gate stay. --- src/hooks/usePendingConciergeResponse.ts | 35 +++++------------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index 39bb0c65fa3c..a1a984e95a34 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -48,27 +48,20 @@ function usePendingConciergeResponse(reportID: string | undefined) { // pendingResponse/tokens/fullHtml — without this snapshot, those non-content // updates would cancel the running interval and restart the reveal. The // useEffect keeps ref writes in the commit phase (React-Compiler-safe). - const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent, persistedAction}); + const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}); useEffect(() => { - trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent, persistedAction}; + trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}; }); // Reconciliation: when the canonical reportComment lands in REPORT_ACTIONS // mid-trickle, fire the running loop's accelerator so the remaining reveal - // finishes in ~1.5s instead of snapping the synthetic bubble closed. If the - // canonical lands while no trickle is running (e.g. arrived while the user - // was on a different report), drop the pending optimistic so we don't - // reapply it on top of the canonical on remount. + // finishes in ~1.5s instead of snapping the synthetic bubble closed. useEffect(() => { - if (!persistedAction) { + if (!persistedAction || !accelerateRef.current) { return; } - if (accelerateRef.current) { - accelerateRef.current(Date.now()); - } else { - discardPendingConciergeAction(reportID); - } - }, [persistedAction, reportID]); + accelerateRef.current(Date.now()); + }, [persistedAction]); useEffect(() => { if (!reportID || !reportActionID) { @@ -78,17 +71,10 @@ function usePendingConciergeResponse(reportID: string | undefined) { // when it began; subsequent updates that share this same reportActionID don't // disturb the in-progress reveal. A genuinely new Concierge reply produces a // new reportActionID and re-enters this effect via the deps below. - const {pendingResponse: snapshot, fullHtml: snapshotHtml, tokens: snapshotTokens, persistedAction: snapshotPersisted} = trickleInputsRef.current; + const {pendingResponse: snapshot, fullHtml: snapshotHtml, tokens: snapshotTokens} = trickleInputsRef.current; if (!snapshot) { return; } - // If the canonical reportComment is already in REPORT_ACTIONS at mount, - // there's nothing to optimistically reveal — discard pending so we don't - // re-apply the optimistic on top of the canonical. - if (snapshotPersisted) { - discardPendingConciergeAction(reportID); - return; - } const {reportAction, displayAfter} = snapshot; const remainingDelay = displayAfter - Date.now(); @@ -188,13 +174,6 @@ function usePendingConciergeResponse(reportID: string | undefined) { if (cancelled) { return; } - // Late-arrival guard: the canonical reportComment may have landed - // during the pre-trickle setTimeout window. Skip the trickle so we - // don't apply the optimistic on top of the canonical at completion. - if (trickleInputsRef.current.persistedAction) { - discardPendingConciergeAction(reportID); - return; - } // Anchor to displayAfter so revisit resumes at the wall-clock-correct // stage instead of restarting the reveal from char 0. trickleStart = displayAfter; From fde003c8466c1fdf7fe59d2f5fb85d73d26fd84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Thu, 30 Apr 2026 22:58:48 +0900 Subject: [PATCH 24/26] Don't reapply optimistic over canonical at trickle completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completion branch only consulted `arrival` to choose between discard and apply. `arrival` is set by the accelerator, which no-ops when `intervalID` is null — so two windows leak through: 1. Canonical lands during the 4s pre-trickle setTimeout. Accelerator runs but returns early (intervalID still null), arrival never set, the trickle runs full duration and merges the older optimistic payload on top of the canonical at the same reportActionID. Server-added markup (follow-up buttons, deep-link Pressables) gets clobbered until the next server update. 2. Canonical hadn't landed yet by completion — same path. Read persistedAction live from the trickleInputsRef at completion time and take the discard path whenever it's defined, regardless of how it got there. The visible trickle still runs to completion (start/complete logs fire as expected). --- src/hooks/usePendingConciergeResponse.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index a1a984e95a34..f219caab716e 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -48,9 +48,9 @@ function usePendingConciergeResponse(reportID: string | undefined) { // pendingResponse/tokens/fullHtml — without this snapshot, those non-content // updates would cancel the running interval and restart the reveal. The // useEffect keeps ref writes in the commit phase (React-Compiler-safe). - const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}); + const trickleInputsRef = useRef({pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent, persistedAction}); useEffect(() => { - trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent}; + trickleInputsRef.current = {pendingResponse, fullHtml, tokens, dispatchLocalDraftEvent, persistedAction}; }); // Reconciliation: when the canonical reportComment lands in REPORT_ACTIONS @@ -146,12 +146,13 @@ function usePendingConciergeResponse(reportID: string | undefined) { arrivedAtElapsedMs: arrival?.elapsedMs, }); dispatch('completed', snapshotTokens.at(-1) ?? snapshotHtml); - // If acceleration fired, the canonical reportComment already landed in - // REPORT_ACTIONS. Re-applying our older optimistic payload would clobber - // server-added markup (e.g. follow-up buttons) until the next server - // update. Just clear the pending state in that case — the synthetic - // bubble fades into the canonical row. - if (arrival) { + // If the canonical reportComment is already in REPORT_ACTIONS at completion + // time — whether acceleration fired or it landed during the pre-trickle + // setTimeout (when accelerate runs but no-ops because intervalID is null) — + // re-applying our older optimistic would clobber server-added markup + // (follow-up buttons, deep-link Pressables) until the next server update. + // Read live from the ref so we catch arrivals the accelerator missed. + if (arrival || trickleInputsRef.current.persistedAction) { discardPendingConciergeAction(reportID); } else { applyPendingConciergeAction(reportID, reportAction); From 03c1339056534f49264247039c503f72efac54a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Thu, 30 Apr 2026 23:05:00 +0900 Subject: [PATCH 25/26] Tighten clobber-guard comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cuts the 5-line comment to 4 with the same load-bearing info — names the two arrival paths (`arrival` vs live ref read) and the symptom (clobbered server-added markup), drops the redundant "until the next server update" tail. --- src/hooks/usePendingConciergeResponse.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index f219caab716e..acf2a35fac32 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -146,12 +146,11 @@ function usePendingConciergeResponse(reportID: string | undefined) { arrivedAtElapsedMs: arrival?.elapsedMs, }); dispatch('completed', snapshotTokens.at(-1) ?? snapshotHtml); - // If the canonical reportComment is already in REPORT_ACTIONS at completion - // time — whether acceleration fired or it landed during the pre-trickle - // setTimeout (when accelerate runs but no-ops because intervalID is null) — - // re-applying our older optimistic would clobber server-added markup - // (follow-up buttons, deep-link Pressables) until the next server update. - // Read live from the ref so we catch arrivals the accelerator missed. + // Don't reapply our older optimistic when the canonical is already there — + // it would clobber server-added markup (follow-up buttons, deep-link + // Pressables). `arrival` covers the accelerator path; the live ref read + // catches arrivals during the pre-trickle setTimeout where the accelerator + // no-ops on null intervalID. if (arrival || trickleInputsRef.current.persistedAction) { discardPendingConciergeAction(reportID); } else { From 62fa9e1eddcd2a483813751a580450c0e22fb197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Ch=C3=A1vez?= Date: Fri, 1 May 2026 09:39:31 +0900 Subject: [PATCH 26/26] Cache draft across remounts; address bot review patterns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things in one commit since they touch overlapping lines: 1. Module-level draftCache so navigating away and back to a chat doesn't flash the synthetic Concierge bubble away during the remount + Onyx-hydration window. Gate's useState lazy-inits from the cache; every reducer write mirrors to the cache; the reducer's null returns (completed / failed / cleared) evict it. Keyed by reportID so chats stay isolated. 4 unit specs cover the contract. 2. Drop redundant useCallback/useMemo on clearDraft, dispatchLocalDraftEvent, stateValue, actionsValue — React Compiler auto-memoizes; the explicit wrappers just shadow its analysis (clean-react-0-compiler bot review). Inline the resubscribe-clear logic so the effect's deps stay scoped to reportID instead of dragging in a per-render clearDraft closure. 3. Extract the trickle-vs-binary-reveal threshold (100 anchors) to a named MIN_TRICKLE_TOKEN_COUNT constant — matches the pattern of every other timing/sizing constant in this file (consistency-2 bot review). --- src/hooks/usePendingConciergeResponse.ts | 4 +- src/pages/inbox/ConciergeDraftContext.tsx | 69 +++++++++++-------- src/pages/inbox/conciergeDraftState.ts | 22 +++++- .../pages/inbox/conciergeDraftState.test.ts | 39 ++++++++++- 4 files changed, 101 insertions(+), 33 deletions(-) diff --git a/src/hooks/usePendingConciergeResponse.ts b/src/hooks/usePendingConciergeResponse.ts index acf2a35fac32..daf820b607ec 100644 --- a/src/hooks/usePendingConciergeResponse.ts +++ b/src/hooks/usePendingConciergeResponse.ts @@ -19,6 +19,8 @@ const TICK_INTERVAL_MS = 80; const TRICKLE_HARD_CAP_MS = 60_000; /** Once the real reportComment lands in REPORT_ACTIONS, finish the remaining reveal within this window. */ const ACCELERATED_REMAINING_MS = 1_500; +/** Minimum char-level anchors before we opt into the trickle reveal. Replies under this fall back to the binary reveal at `displayAfter`. */ +const MIN_TRICKLE_TOKEN_COUNT = 100; function easeOut(t: number): number { const clamped = Math.max(0, Math.min(1, t)); @@ -88,7 +90,7 @@ function usePendingConciergeResponse(reportID: string | undefined) { // Anchors are character-level. Short replies (~50–100 chars) keep the // binary reveal; longer ones (paragraphs / lists) cross the threshold // and get the smooth trickle. - const shouldTrickle = snapshotTokens.length >= 100 && !!snapshotHtml; + const shouldTrickle = snapshotTokens.length >= MIN_TRICKLE_TOKEN_COUNT && !!snapshotHtml; if (!shouldTrickle) { const timer = setTimeout(() => applyPendingConciergeAction(reportID, reportAction), Math.max(0, remainingDelay)); return () => clearTimeout(timer); diff --git a/src/pages/inbox/ConciergeDraftContext.tsx b/src/pages/inbox/ConciergeDraftContext.tsx index 7bad6309721c..788b29b03860 100644 --- a/src/pages/inbox/ConciergeDraftContext.tsx +++ b/src/pages/inbox/ConciergeDraftContext.tsx @@ -1,5 +1,5 @@ import {getReportChatType} from '@selectors/Report'; -import React, {createContext, useCallback, useContext, useEffect, useMemo, useState} from 'react'; +import React, {createContext, useContext, useEffect, useState} from 'react'; import useOnyx from '@hooks/useOnyx'; import {getReportChannelName} from '@libs/actions/Report'; import Log from '@libs/Log'; @@ -9,7 +9,7 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {ReportAction} from '@src/types/onyx'; import type {ConciergeDraft} from './conciergeDraftState'; -import {applyConciergeDraftEvent} from './conciergeDraftState'; +import {applyConciergeDraftEvent, getCachedDraft, setCachedDraft} from './conciergeDraftState'; type ConciergeDraftState = { draftReportAction: ReportAction | null; @@ -61,23 +61,34 @@ function ConciergeDraftProvider({reportID, children}: React.PropsWithChildren<{r } function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{reportID: string}>) { - const [draft, setDraft] = useState(null); - - const clearDraft = useCallback(() => { + // Lazy-init from the module-level cache so a remount (ReportScreen + // unmount/remount on chat-switch) restores the in-progress draft on the + // first paint instead of flashing the synthetic bubble away. + const [draft, setDraft] = useState(() => getCachedDraft(reportID)); + + // React Compiler auto-memoizes; explicit useCallback/useMemo would just + // shadow the compiler's analysis (clean-react-0-compiler). + const clearDraft = () => { + setCachedDraft(reportID, null); setDraft(null); - }, []); + }; - const dispatchLocalDraftEvent = useCallback( - (event: ConciergeDraftEvent) => { - setDraft((currentDraft) => applyConciergeDraftEvent(currentDraft, event, reportID)); - }, - [reportID], - ); + const dispatchLocalDraftEvent = (event: ConciergeDraftEvent) => { + setDraft((currentDraft) => { + const next = applyConciergeDraftEvent(currentDraft, event, reportID); + setCachedDraft(reportID, next); + return next; + }); + }; useEffect(() => { const channelName = getReportChannelName(reportID); + // Inline the clear so the effect's deps stay scoped to reportID; closing + // over `clearDraft` would either drag it into deps (re-subscribing on + // every render) or trip exhaustive-deps. const handleResubscribe = () => { - clearDraft(); + setCachedDraft(reportID, null); + setDraft(null); }; const eventTypes = [ Pusher.TYPE.CONCIERGE_DRAFT_STARTED, @@ -93,7 +104,11 @@ function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{repor eventType, (eventData) => { const conciergeDraftEvent = eventData as ConciergeDraftEvent; - setDraft((currentDraft) => applyConciergeDraftEvent(currentDraft, conciergeDraftEvent, reportID)); + setDraft((currentDraft) => { + const next = applyConciergeDraftEvent(currentDraft, conciergeDraftEvent, reportID); + setCachedDraft(reportID, next); + return next; + }); }, handleResubscribe, ); @@ -110,23 +125,17 @@ function ConciergeDraftGate({reportID, children}: React.PropsWithChildren<{repor subscription.unsubscribe(); } }; - }, [clearDraft, reportID]); - - const stateValue = useMemo( - () => ({ - draftReportAction: draft?.reportAction ?? null, - hasActiveDraft: !!draft?.reportAction, - }), - [draft?.reportAction], - ); + }, [reportID]); - const actionsValue = useMemo( - () => ({ - clearDraft, - dispatchLocalDraftEvent, - }), - [clearDraft, dispatchLocalDraftEvent], - ); + const stateValue: ConciergeDraftState = { + draftReportAction: draft?.reportAction ?? null, + hasActiveDraft: !!draft?.reportAction, + }; + + const actionsValue: ConciergeDraftActions = { + clearDraft, + dispatchLocalDraftEvent, + }; return ( diff --git a/src/pages/inbox/conciergeDraftState.ts b/src/pages/inbox/conciergeDraftState.ts index ecf078571dc3..70c07518f8ec 100644 --- a/src/pages/inbox/conciergeDraftState.ts +++ b/src/pages/inbox/conciergeDraftState.ts @@ -40,6 +40,26 @@ function buildConciergeDraftReportAction({bodyMarkdown, created, finalRenderedHT } as ReportAction; } +// Module-level cache so a chat re-mount (ReportScreen unmount/remount on chat +// switch) preserves the in-progress draft. Without this the gate's local state +// resets to null on every revisit and the synthetic bubble disappears for the +// remount + Onyx-hydration window. Keyed by reportID; entries are evicted by +// `setCachedDraft(reportID, null)` when the reducer returns null +// (completed/failed/cleared). +const draftCache = new Map(); + +function getCachedDraft(reportID: string): ConciergeDraft | null { + return draftCache.get(reportID) ?? null; +} + +function setCachedDraft(reportID: string, draft: ConciergeDraft | null): void { + if (draft) { + draftCache.set(reportID, draft); + } else { + draftCache.delete(reportID); + } +} + function applyConciergeDraftEvent(currentDraft: ConciergeDraft | null, event: ConciergeDraftEvent, reportID: string): ConciergeDraft | null { if (event.reportID !== reportID) { return currentDraft; @@ -81,5 +101,5 @@ function applyConciergeDraftEvent(currentDraft: ConciergeDraft | null, event: Co }; } -export {applyConciergeDraftEvent, buildConciergeDraftReportAction}; +export {applyConciergeDraftEvent, buildConciergeDraftReportAction, getCachedDraft, setCachedDraft}; export type {ConciergeDraft}; diff --git a/tests/unit/pages/inbox/conciergeDraftState.test.ts b/tests/unit/pages/inbox/conciergeDraftState.test.ts index e83e7125eb89..3f3a5aeb0c46 100644 --- a/tests/unit/pages/inbox/conciergeDraftState.test.ts +++ b/tests/unit/pages/inbox/conciergeDraftState.test.ts @@ -1,4 +1,4 @@ -import {applyConciergeDraftEvent} from '@pages/inbox/conciergeDraftState'; +import {applyConciergeDraftEvent, getCachedDraft, setCachedDraft} from '@pages/inbox/conciergeDraftState'; import CONST from '@src/CONST'; const REPORT_ID = '123'; @@ -131,4 +131,41 @@ describe('conciergeDraftState', () => { expect(otherReportDraft).toBe(initialDraft); }); + + describe('draftCache', () => { + // Always start clean so tests don't leak state into each other. + beforeEach(() => { + setCachedDraft(REPORT_ID, null); + }); + + it('returns null for an unseen reportID', () => { + expect(getCachedDraft('never-stored')).toBeNull(); + }); + + it('persists a draft across set/get and survives across calls (the remount survival contract)', () => { + const draft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + expect(draft).not.toBeNull(); + setCachedDraft(REPORT_ID, draft); + expect(getCachedDraft(REPORT_ID)).toBe(draft); + // Simulating a remount: a fresh getCachedDraft call returns the same instance. + expect(getCachedDraft(REPORT_ID)).toBe(draft); + }); + + it('evicts when set to null (completed/failed/cleared reducer return)', () => { + const draft = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + setCachedDraft(REPORT_ID, draft); + expect(getCachedDraft(REPORT_ID)).not.toBeNull(); + setCachedDraft(REPORT_ID, null); + expect(getCachedDraft(REPORT_ID)).toBeNull(); + }); + + it('keeps entries scoped per reportID (no cross-talk)', () => { + const draftA = applyConciergeDraftEvent(null, createDraftEvent(), REPORT_ID); + const draftB = applyConciergeDraftEvent(null, createDraftEvent({reportID: 'other'}), 'other'); + setCachedDraft(REPORT_ID, draftA); + setCachedDraft('other', draftB); + expect(getCachedDraft(REPORT_ID)).toBe(draftA); + expect(getCachedDraft('other')).toBe(draftB); + }); + }); });

    See our docs for more.