Show Concierge thinking indicator on expense reports and threads - #97363
Show Concierge thinking indicator on expense reports and threads#97363chiragsalian wants to merge 8 commits into
Conversation
The provider only mounted for Concierge DMs, #admins rooms and custom-agent chats, so candidateAgentIDs stayed empty everywhere else and the thinking bubble never rendered — even though Auth already emits status updates for expense reports with transactions and the labels reached Onyx. Mount the gate whenever the processing-indicator NVP names an active agent, so the client stops re-deriving where agents respond. The chat-type arms stay for the eager reasoning subscription before the first NVP arrives.
The /search/r/<reportID> tree mounted neither the AgentZero status context nor the indicator itself, so mentioning Concierge on an expense report showed nothing even once the gate allows it. Wrap both feed branches in AgentZeroStatusProvider and fill the unified list's already-supported listFooterComponent slot. No draft guard is needed here: that tree has no ConciergeDraftProvider, so no Concierge reply streams into it.
Three of these fail without the gate change; the 'stays inert' case is the negative control that must keep passing so the gate doesn't start mounting for every report.
|
@ChavdaSachin 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] |
There was a problem hiding this comment.
Pull request overview
This PR enables the existing “Concierge is thinking…” / AgentZero processing indicator UI to appear on money request expense reports and their threads (e.g. /search/r/<reportID>), aligning the client’s gating logic with the server’s shouldEmitConciergeStatusUpdates by keying off the processing-indicator NVP the backend already writes.
Changes:
- Extend
AgentZeroStatusProvider’s guard logic to mount whenever the server’sagentZeroProcessingRequestIndicatornames active agent(s), not only for specific chat types. - Wrap
MoneyRequestReportView’s feed area inAgentZeroStatusProviderso the status context is available on expense report/search views. - Render
ConciergeThinkingMessageat the bottom of the non-inverted unified list vialistFooterComponentinMoneyRequestReportActionsList.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/unit/AgentZeroStatusContextTest.ts | Adds unit coverage for server-driven gating on expense reports/threads and indicator clearing behavior. |
| src/pages/inbox/AgentZeroStatusContext.tsx | Mounts status gate when server indicates processing; passes server agent IDs into the gate to avoid duplicate subscription logic. |
| src/components/MoneyRequestReportView/MoneyRequestReportView.tsx | Wraps both feed branches with AgentZeroStatusProvider so the indicator context exists on money request report views. |
| src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx | Adds ConciergeThinkingMessage as a unified list footer for the non-inverted list variant. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: a4ec38f3ec
ℹ️ 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".
chiragsalian
changed the title
Returning bare children for inert reports meant the false-to-true flip on the indicator NVP inserted the gate above an already-mounted feed, remounting MoneyRequestReportActionsList/ReportActionsList and dropping scroll position exactly as the indicator appeared. Always mount the gate and make it inert instead, so children keep a stable position. The inert gate skips the reasoning subscription and keys its report-actions read off nothing, so non-AgentZero reports stay as cheap as before.
…king-expense-reports
The indicator renders in the list footer, outside the FlashList data that scrollToBottom targets, and this list is not inverted and sets no autoscrollToBottomThreshold — so a user at the bottom got the bubble just below the viewport. The inverted ReportActionsList gets this for free, which is why only the money-request view needs it. Scroll only when already at the bottom, and only once per appearance, so a user reading history further up is never yanked back down.
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.
|
chiragsalian
changed the title
|
✋ |
|
David has a webinar tomorrow and asked me if i could get this reviewed/merged and CP'd to staging soon. Hopefully to improve the demo experience. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Review: Show Concierge thinking indicator on expense reports and threadsVerdict: Looks solid and well-reasoned. The two-blocker framing is correct, the fix is minimal, and the comments explain why rather than what. All CI is green (typecheck, ESLint, React Compiler compliance, unit tests). No blocking issues found — a few minor observations below. What it does
Strengths
Minor observations (non-blocking)
Test coverageThe context-level behavior is well covered. Not covered (and hard to unit test): the footer actually rendering in |
Regression huntVerdict: no correctness regressions found. Previously-working AgentZero chats (Concierge DMs, Biggest blast radius — verified safeThe refactor swaps the provider's early
Agent-chat parity — no regressionFor No double-provider in the search tree
Low-impact notes (not regressions)
CI is fully green (typecheck, ESLint, React Compiler compliance, unit tests), and the new tests cover the server-driven arm, inert state, NVP-clear, and mount stability. |
Explanation of Change
The backend already emits Concierge status updates for expense reports —
shouldEmitConciergeStatusUpdatesinProcessAgentZeroRequest.cppincludesisMoneyRequestReportWithTransactions, and the labels reach Onyx correctly. Nothing on the client rendered them. Two independent blockers:AgentZeroStatusProvideronly mounted for Concierge DMs, #admins rooms and custom-agent chats. It re-derived "does an agent respond here?" from chat type and had drifted from the server's rule, socandidateAgentIDsstayed empty on expense reports and threads./search/r/<reportID>tree (MoneyRequestReportView) mounted neither the status context nor the indicator at all, so that screen showed nothing even once the gate allowed it.Changes:
agentZeroProcessingRequestIndicatorNVP names an active agent, so the client stops re-deriving where agents respond. The chat-type arms stay, since they mount the reasoning Pusher subscription before the first NVP lands.MoneyRequestReportViewfeed branches inAgentZeroStatusProviderand fill the unified list's already-supportedlistFooterComponentslot. That list is not inverted, so the footer is the bottom of the message feed — the same position the indicator occupies in the invertedReportActionsList.Note: reasoning summaries ("Concierge is thinking" expandable details) were absent from my local run because the LLM returned no
reasoningSummaryentries for it, not because of a gate.emitConciergeReasoningis governed by the sameshouldEmitConciergeStatusUpdatesthat was already true. With the UI fixed, reasoning should render whenever the model produces it.Fixed Issues
$ #97378
PROPOSAL:
Tests
@concierge@expensify.com can you list how many expenses categorized advertising are there?Offline tests
None
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari