Consolidate AgentZero status into a Context#85535
Conversation
Replace three scattered modules (useAgentZeroStatusIndicator hook, ConciergeReasoningStore, and Pusher reasoning subscriptions in Report/index.ts) with a single AgentZeroStatusContext that uses a two-level gate pattern: - Outer gate (AgentZeroStatusProvider): cheap scalar check — if not Concierge chat, renders children with no hooks/Pusher/state overhead - Inner gate (AgentZeroStatusGate): owns all logic for Concierge chats: Onyx subscription, Pusher subscription with per-callback cleanup, reasoning state with deduplication, debounced label updates Consumers (ConciergeThinkingMessage, ReportActionCompose) now read from context instead of receiving props drilled through 4 levels. The prop chain ReportScreen → ReportActionsView → ReportActionsList → ConciergeThinkingMessage is eliminated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
964e83a to
bbc1155
Compare
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.
|
The new eslint-config-expensify@2.0.107 enforces context-provider-split-values:
contexts must not mix data and functions in a single provider.
Split AgentZeroStatusContext into:
- AgentZeroStatusStateContext: {isProcessing, reasoningHistory, statusLabel}
- AgentZeroStatusActionsContext: {kickoffWaitingIndicator}
Export useAgentZeroStatus() for state and useAgentZeroStatusActions()
for actions. Update ReportActionCompose to use the actions hook.
Update tests to use the appropriate hook per test case — state-only
tests use useAgentZeroStatus(), tests needing both spread inline.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…atus-context # Conflicts: # src/hooks/useAgentZeroStatusIndicator.ts # src/pages/inbox/ReportScreen.tsx
|
@DylanDylann 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c1c53a639
ℹ️ 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".
src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx
Outdated
Show resolved
Hide resolved
|
Marked as HOLD, SQ fixes that this PR needs will likely get reverted. I'll keep monitoring with @szymonzalarski98. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
PR doesn’t need product input as a performance PR. Unassigning and unsubscribing myself. |
…me state Self-derive shouldSuppressIndicators inside AgentZeroStatusGate using side-panel context and report actions, then expose it via useAgentZeroStatus(). ConciergeThinkingMessage and a new AgentZeroAwareTypingIndicator wrapper consume the flag to hide indicators until the user sends a message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@adhorodyski I’m curious about how to simulate this test case. Could you help me? |
@adhorodyski It still doesn’t work on my side. |
|
@adhorodyski Could you please add video evidence? |
|
edit: validating this now with Rodrigo. |
|
@rlinoz will be posting a video recording from his repro, this works as expected. The LLM-oriented workflows do not trigger pusher's typing indicator payloads. |
|
🚧 @rlinoz has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
I think this is working as intended, the typing shows when someone is actually typing. Screen.Recording.2026-03-24.at.12.28.33.movRegarding the RHP, it doesn't show the typing in prod either so all good I think |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-03-25.at.15.41.20.movAndroid: mWeb ChromeScreen.Recording.2026-03-25.at.15.12.34.moviOS: HybridAppScreen.Recording.2026-03-25.at.15.39.34.moviOS: mWeb SafariScreen.Recording.2026-03-25.at.15.04.32.movMacOS: Chrome / SafariScreen.Recording.2026-03-25.at.15.01.05.mov |
| } | ||
| return Object.values(actions).some((action) => !isCreatedAction(action) && action.actorAccountID === currentUserAccountID && action.created >= sessionStartTime); | ||
| }; | ||
| // eslint-disable-next-line rulesdir/no-inline-useOnyx-selector -- React Compiler handles memoization |
There was a problem hiding this comment.
Could we remove this disabled line?
There was a problem hiding this comment.
I think so, looks like an artifact from Opus. Doing this now.
There was a problem hiding this comment.
pushing a fix, thanks
|
@rlinoz I discovered a BE bug (also happens on main): Steps to reproduce: Send a message to Concierge → navigate away before the response arrives → navigate back → the old ConciergeThinkingMessage briefly appears, disappears, and then immediately appears again. RCA: When returning to Concierge, the old reportNameValuePairs?.agentZeroProcessingRequestIndicator is still present. Then the openApp API resets it to empty, and shortly after, Pusher updates it again with a new value. Screen.Recording.2026-03-25.at.14.41.03.mov |
|
@rlinoz still working on the last comment from @DylanDylann, please HOLD with a merge in case you wanted to. |
|
@adhorodyski I think this is a minor issue and can be handled quickly 😄, so I’ve approved it in advance. |
- key={reportID} resets all state on report change (PERF-7), deletes reset effect
- Render-time derivations replace effects for clearing optimistic/reasoning state
- displayedLabelRef mirrors state so label effect drops displayedLabel dep
- addReasoning in Pusher deps, compiler auto-memoizes it
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@DylanDylann updated to make everything pass the compiler check, I believe it's way more correct now at the same time. Tests fine manually as well. |
|
🚧 @rlinoz has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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/rlinoz in version: 9.3.44-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.3.47-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.3.48-2 🚀
|
@rlinoz @mountiny @TMisiukiewicz
Explanation of Change
This PR replaces three scattered modules managing Concierge/AgentZero status with a single
AgentZeroStatusContextusing a two-level gate pattern:Consumers (
ConciergeThinkingMessage,ReportActionCompose) now read from context instead of receiving props drilled through 4 levels (ReportScreen → ReportActionsView → ReportActionsList → ConciergeThinkingMessage).Deleted modules:
src/hooks/useAgentZeroStatusIndicator.ts(162 lines)src/libs/ConciergeReasoningStore.ts(122 lines)src/libs/actions/Report/index.ts(~50 lines)New:
src/pages/inbox/AgentZeroStatusContext.tsx(224 lines)src/selectors/ReportNameValuePairs.ts— newagentZeroProcessingIndicatorSelectorTests: 21 new tests replace 3 deleted test files. 15 files changed, +462/-1386 lines.
Note: This PR depends on PR #85355 (useIsInSidePanel) and PR #85356 (Pusher per-callback handles). CI will fail until those merge.
Fixed Issues
$ #84895
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
Same as tests, plus:
Known limitation: The side panel will only show reasoning entries that arrive after it mounts. Earlier entries visible in the main pane won't backfill into the side panel. This is cosmetic — both panes converge once new entries arrive or processing completes. A new user message triggers a new
agentZeroRequestIDwhich resets the reasoning chain in both panes anyway.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