Hide read Concierge history in main DM chat#88769
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@codex review |
![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: 3b691c2e26
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
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.
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
melvin-bot
Bot
requested review from
carlosmiceli and
joekaufmanexpensify
and removed request for
a team
|
@carlosmiceli 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] |
|
Recordings are left — I’ll add them tomorrow 😮💨 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?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: 4458026f20
ℹ️ 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".
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Nice, this is a great change!
…dary logic Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a30909547
ℹ️ 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".
|
@parasharrajat please start reviewing the code changes, I'll add the recordings tomorrow morning. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
|
@dmkt9 all bugs are fixed, please check again |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b9b712d2d
ℹ️ 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".
I still sometimes see this bug on my end. It may be due to slow performance in my development environment, but I believe this is still a potential bug since we use For example, I can also reproduce it on android native: when the target report has just appeared and is loading, if I click the back button immediately, the session is still not reset upon returning. On android chrome, it is harder to reproduce using a similar method because the delay of 2026-05-27.10-46-13.mp4 |
|
2026-05-27.10-39-41.mp4 |
…d boundary Signed-off-by: krishna2323 <belivethatkg@gmail.com>
| hasOlderActions, | ||
| loadOlderChats, | ||
| mainDMSessionStartTime, | ||
| conciergeShowFullHistory: conciergeShowFullHistory || !!reportActionIDFromRoute, |
There was a problem hiding this comment.
If there is an incomplete task in Concierge, then !!reportActionIDFromRoute === true and the session always starts with full messages
There was a problem hiding this comment.
@dmkt9 When reportActionIDFromRoute is set for an incomplete task, showing full history seems expected. Hiding it behind "Show history" would prevent them from completing it. The route param is set specifically to scroll to the task, and full history ensures it's visible.
|
2026-05-27.11-01-45.mp4 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b9b712d2d
ℹ️ 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".
@dmkt9 On mobile, LHN back and Claim Offer both produce Option A (current): Only clear the session when the user visits a different report. Going to LHN and immediately back preserves the session, but navigating LHN → Other Chat → Concierge correctly resets. Trade-off: LHN → back doesn't reset. Option B: Always clear when leaving Concierge. This resets on LHN back, but also resets on Claim Offer, Settings, Search, and any other non-report page. Trade-off: session lost on every navigation away. I'd recommend Option A — preserving the session on a quick LHN peek is reasonable UX, and it avoids breaking Claim Offer and Settings flows. |
@Krishna2323 But in that case, if the user needs to reset the session, they need to open another report, then come back. I think it is quite inconvenient. Can we add a focus route check to exclude the reset from Claim Offer? |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
I can't reproduce this, can you check again? Monosnap.screencast.2026-05-27.10-09-09.mp4 |
FIxed |
…ead markers Signed-off-by: krishna2323 <belivethatkg@gmail.com>
I can't reproduce it anymore 👍 |
| // chat list (LHN). Preserve for non-report pages like Settings | ||
| // (Claim Offer → /settings/subscription). | ||
| const activeRoute = Navigation.getActiveRoute(); | ||
| const isReportOrInbox = activeRoute.startsWith('/r/') || activeRoute === `/${ROUTES.INBOX}`; |
There was a problem hiding this comment.
On mobile, since we only check the /r and /inbox routes, opening and closing the sidebar concierge on other routes, such as /workspace, will not reset the session.
There was a problem hiding this comment.
This code only manages the main Concierge DM session, not the sidebar. The clearing block is gated by prevIsConciergeMainDM !== isConciergeMainDM, which only fires when navigating away from the Concierge main DM (currentReportID === conciergeReportID). On /workspace routes, isConciergeMainDM is already false, so this block never executes. The sidebar session is managed separately by useSidePanelState. Additionally, on mobile, there is no sidebar Concierge (isSidePanelReportSupported = false).
There was a problem hiding this comment.
Even though there is no Concierge sidebar on native, we should still be consistent with the mobile browser. As you can see on native, we only have one option to open and close the Concierge DM to reset the Concierge sidebar.
There was a problem hiding this comment.
"After clicking a link in the Concierge messages and navigating to a different route, the session resets upon returning to the Inbox; I find this quite annoying."
This conflicts with your earlier feedback — If we clear the session on /workspace routes, clicking any Concierge link that navigates to a workspace page would reset the session upon return. The current whitelist (/r/ and /inbox only) was specifically designed to preserve the session for those link-navigation flows.
There was a problem hiding this comment.
Apologies for the lack of clarity. The /workspace route was just an example; I meant all routes that differ from /r and /inbox. I am reporting that we should reset the session when we close the Concierge sidebar. Because we only check /r and /inbox here, opening and closing the Concierge sidebar does not reset its own session.
2026-05-27.17-09-30.mp4
There was a problem hiding this comment.
@dmkt9 If we start resetting the session on workspace/settings routes, then anytime a user clicks a link in the Concierge chat (e.g., to change a workspace setting) and navigates back, the entire session would be reset — losing the conversation context. That's the behavior we're intentionally preserving here.
@marcaaron Could you please clarify what the expected behavior should be? Currently, we only reset the main DM session state when the user navigates to another report or the inbox (LHN). Should we also reset when navigating to workspace/settings pages, even though that would cause the session to reset when following links from Concierge?
There was a problem hiding this comment.
@Krishna2323 @marcaaron Actually, I didn't mean we have to reset the Concierge DM session when we are on the workspace/settings route.
With your previous check: const shouldClear = shouldUseNarrowLayout || (!!currentReportID && currentReportID !== conciergeReportID);, I see it works well and only has a bug with the "Claim offer" button, so I recommend using the focused route to handle only that bug.
As for native, since we don't have a real concierge sidebar, I already know and accept cases like:
We are on Concierge DM > Search icon > "type: expense" > Navigate to Spend > Tap on Concierge sidebar icon > Concierge sidebar opens and it has the same session as Concierge DM, as it is Concierge DM as well.
But if we don't have a deep Concierge DM open, we shouldn't keep the Concierge sidebar from resetting when we close it.
There was a problem hiding this comment.
@dmkt9 The two scenarios — (A) following a Concierge link to a workspace page, and (B) opening/closing the sidebar from a workspace page — produce the same state transition in the provider. We can't preserve the session for (A) while clearing it for (B) with a route check alone. Could you clarify which behavior should take priority, or suggest how to distinguish between the two?
There was a problem hiding this comment.
Can you guys open a Slack thread and describe what the issue here is? I think it would be good to get consensus and loop in design / product. Curious to get more feedback.
|
@codex review |
|
2026-05-27.15-39-34.mp4 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df00b9010f
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
fixed |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…eIndicators Signed-off-by: krishna2323 <belivethatkg@gmail.com>
5 participants
Explanation of Change
Fixed Issues
$ #88635
PROPOSAL: N/A
Tests
Test 1: Main DM - Welcome state shown when all messages are read
Test 2: Main DM - Show history reveals full chat
Test 3: Main DM - Sending a message shows session messages
Test 4: Main DM - Reopening resets to welcome state
Test 5: Side panel behavior unchanged
Test 6: Non-Concierge reports unaffected
Offline tests
QA Steps
Same as Tests
Verify that no errors appear in the JS console
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
android_hybrid.mp4
Android: mWeb Chrome
android_mWeb.mp4
iOS: Native
ios_hybrid.mp4
iOS: mWeb Safari
ios_mWeb.mp4
MacOS: Chrome / Safari
web_chrome_test_1.mp4
web_chrome_test_2.mp4