[Suggested Follow-ups][R3] Implement instant reply + optimistic reconciliation - #80918
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@mkhutornyi are you able to jump on this one as well ? It should be open in half an hour, but as with the previous ones, time is of essence :) |
yes |
Sweet, the test steps are good, I'm recording the videos now. If you have any questions just ping me up here on on slack |
|
@hoangzinh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@codex review |
@hoangzinh sorry, @mkhutornyi will review this one |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
| const optimisticConciergeReportActionID = rand64(); | ||
|
|
||
| // Post user's comment immediately | ||
| addComment(report, notifyReportID ?? reportID, ancestors, selectedFollowup.text, timezoneParam, false, false, { |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
There is no error handling if the API call for addComment fails on line 64. When the comment submission fails but the setTimeout has already been scheduled, the optimistic Concierge response will still appear after the delay even though the users question was never posted.
This creates a confusing UX where Concierge appears to respond to a message that failed to send.
Suggested fix: The timeout should be tied to the API calls success. Consider passing a cleanup callback to addComment or checking the API response before scheduling the optimistic response:
// One approach: pass the optimistic action as part of the API call
// and let the API success handler trigger the delayed response
// Alternative: Return a promise from addComment and only schedule on success
const commentResult = await addComment(...);
if (commentResult.success) {
addOptimisticConciergeActionWithDelay(reportID, optimisticConciergeAction);
}Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
same as below : this is actually it's getting cleared in src/libs/actions/Report/index.ts#756
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb0b010bf3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@mkhutornyi how's it going ? |
| medium | ||
| success={item.isPrimary} | ||
| innerStyles={props.styles?.button} | ||
| hoverStyles={props.styles?.buttonHover} |
There was a problem hiding this comment.
Confirmed this is safe removal
There was a problem hiding this comment.
yup, confirmed with the design team. We added it in previous PR, should be all g
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { | ||
| [optimisticConciergeAction.reportAction.reportActionID]: optimisticConciergeAction.reportAction, | ||
| }); | ||
| }, CONCIERGE_RESPONSE_DELAY_MS); |
There was a problem hiding this comment.
We usually don't accept setTimeout but 1.5s is reliable delay duration?
There was a problem hiding this comment.
For this one is fine, since we don't want to show the optimistic response so it can feels a bit more natural
There was a problem hiding this comment.
This is just to mimic "real person" typing effect from concierge a visual gimmick,
|
|
||
| // Wait for the delayed Concierge response (1500ms delay in SuggestedFollowup.ts) | ||
| await new Promise((resolve) => { | ||
| setTimeout(resolve, CONCIERGE_RESPONSE_DELAY_MS + 100); |
There was a problem hiding this comment.
just to make sure the updates with delay came through
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 615e6103fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { | ||
| [optimisticConciergeAction.reportAction.reportActionID]: optimisticConciergeAction.reportAction, | ||
| }); |
There was a problem hiding this comment.
The optimistic concierge response is merged after a fixed delay regardless of whether the underlying ADD_COMMENT succeeds. If the API call fails (offline, 5xx, validation error), the failure data removes the optimistic response, but this delayed merge will re‑insert it anyway, leaving a ghost response and clearing typing state even though the question never posted. Consider canceling the timeout or only merging the optimistic response after a successful server acknowledgment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
responded to that above: it's getting cleared in src/libs/actions/Report/index.ts#756
| [CONST.ACCOUNT_ID.CONCIERGE]: true, | ||
| }); | ||
|
|
||
| setTimeout(() => { |
There was a problem hiding this comment.
If setTimeout, there should be clearTimeout
There was a problem hiding this comment.
this one is not in the component lifecycle and the danger of user navigating away during that is small. It'll be garbage collected after it fires, so I think it's ok to leave as is.
|
On native, reply is almost immediate native.mov |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 615e6103fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { | ||
| [optimisticConciergeAction.reportAction.reportActionID]: optimisticConciergeAction.reportAction, | ||
| }); |
There was a problem hiding this comment.
This merge is scheduled unconditionally after a timeout, so if the underlying ADD_COMMENT call fails (offline/500), the optimistic concierge reply still gets inserted later. The failureData path removes any optimistic response immediately, but this delayed merge re-adds it, so users can see a concierge reply even though their question never posted. Consider canceling the timeout on failure or gating this merge on request success.
Useful? React with 👍 / 👎.
| const optimisticConciergeAction = buildOptimisticAddCommentReportAction( | ||
| selectedFollowup.response, | ||
| undefined, | ||
| CONST.ACCOUNT_ID.CONCIERGE, | ||
| CONCIERGE_RESPONSE_DELAY_MS, |
There was a problem hiding this comment.
The optimistic concierge response is created with buildOptimisticAddCommentReportAction, which sets pendingAction: ADD. Because this action is created and later merged outside the normal addActions successData flow, there’s no place that clears pendingAction/isOptimisticAction for optimisticConciergeReportActionID, so the reply can remain permanently in a “sending” state even after the API succeeds. Add a success-path cleanup for this ID.
Useful? React with 👍 / 👎.
I think this is fine, we can tweak it in a follow-up after we get some feedback |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.3.11-16 🚀
|
|
@jmusial @mkhutornyi @Beamanator We have no any Welcome message in admins room now. How can we get this? bandicam.2026-02-02.11-38-20-337.mp4 |
|
@jmusial @mkhutornyi @Beamanator @mollfpr Welcome message in #admin room is not displayed now. Is this expected or needs to be reported? |
|
Aah yes sorry for the delay @jponikarchuk @m-natarajan - we put this behind a beta at the last minute - but actually applause is on all betas, right? 🤔 |
|
Aah right yes, we put it on a beta that needs to be opted in to specifically - it doesn't work for people with access to "all betas" |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.3.12-1 🚀
|
8 participants
Explanation of Change
This PR is a part of Concierge AI Suggested Follow-ups project. It updates how the follow ups look and adds instant replies via
followup-responsetag.Fixed Issues
$ #80527
$ #80801
PROPOSAL:
Tests
Pre prequisite:
Currently has to use backend from:
#adminschat<followup-responsetags are present in the chat.Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
0090.android.native.mov
Android: mWeb Chrome
0090.android.chrome.mov
iOS: Native
0090.ios.native.mov
iOS: mWeb Safari
0090.ios.safari.mov
MacOS: Chrome / Safari
0090.chrome.desktop.mov