[HOLD] Remove optimistic Concierge thinking indicator#88747
Draft
chiragsalian wants to merge 4 commits intomainfrom
Draft
[HOLD] Remove optimistic Concierge thinking indicator#88747chiragsalian wants to merge 4 commits intomainfrom
chiragsalian wants to merge 4 commits intomainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the client-side “Concierge is thinking…” optimistic indicator so the UI shows AgentZero processing state only when the backend provides a processing label.
Changes:
- Removed the
kickoffWaitingIndicatoroptimistic flow fromAgentZeroStatusContext(including localization dependency and timeout logic). - Stopped triggering the optimistic indicator on message send (
useComposerSubmit). - Updated unit tests to reflect server-driven-only processing state.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/AgentZeroStatusContextTest.ts | Removes tests/mocks for the optimistic indicator and updates expectations to server-label-only behavior. |
| src/pages/inbox/report/ReportActionCompose/useComposerSubmit.ts | Stops calling the removed kickoffWaitingIndicator action on submit. |
| src/pages/inbox/AgentZeroStatusContext.tsx | Deletes optimistic state/actions and derives processing/label state solely from the server-provided indicator. |
Comments suppressed due to low confidence (1)
src/pages/inbox/AgentZeroStatusContext.tsx:103
- This component updates state during render (
setPrevServerLabel/setReasoningHistoryinside anifat the top level). Even though the condition prevents an infinite loop, setting state during render is an anti-pattern in React and can produce warnings/unpredictable behavior with concurrent rendering. Move this logic into auseEffectthat runs whenserverLabelchanges (and clearreasoningHistorythere when transitioning truthy → falsy).
// Clear reasoning when processing ends (server label transitions from truthy → falsy)
const [prevServerLabel, setPrevServerLabel] = useState(serverLabel);
if (prevServerLabel !== serverLabel) {
setPrevServerLabel(serverLabel);
if (prevServerLabel && !serverLabel && reasoningHistory.length > 0) {
setReasoningHistory([]);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HOLD till,
are live.
Explanation of Change
Removes the client-side optimistic "Concierge is thinking..." indicator entirely. Backend is now the sole source of truth for whether to show the indicator.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/621231
PROPOSAL:
Tests
Setup,
Manual verification (combined with the backend PRs):
Concierge DM
Another test message to test routing, please respond with "test me two".Admins room chat
Another test message to test routing, please respond with "test me two".Offline tests
The optimistic indicator was already gated on
!isOffline; offline behavior is unchanged — messages queue normally, no indicator renders until online and server sends a label.QA Steps
Setup,
Same as the manual verification above.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari