feat(seer): add global AskSeerButton component#113727
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7aa68ae. Configure here.
c0b2987 to
c910f73
Compare
c967200 to
b8e5805
Compare
| ), | ||
| { | ||
| ariaLabel: t('Seer Explorer Drawer'), | ||
| drawerKey: 'seer-explorer-drawer', | ||
| resizable: true, | ||
| mode: 'passive', | ||
| onOpen, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Bug: A race condition exists where openDrawer is called with a stale runId from a closure before the setRunId state update completes, causing the drawer to load with incorrect data.
Severity: HIGH
Suggested Fix
The openDrawer call should be updated to pass the new openRunId directly to the ExplorerDrawerContent component instead of relying on the stale runId from the closure. This ensures the drawer is always rendered with the correct, most up-to-date runId. Specifically, change runId={runId} to runId={openRunId} inside the openDrawer callback.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
static/app/views/seerExplorer/components/drawer/useSeerExplorerDrawer.tsx#L82-L90
Potential issue: When `openSeerExplorerDrawer` is called with a new `runId`, it
schedules an asynchronous state update via `setRunId` but immediately calls
`openDrawer`. The renderer function passed to `openDrawer` is a closure that captures
the `runId` from the current render, which is stale. The `GlobalDrawer` then renders the
`ExplorerDrawerContent` with this stale `runId` prop. Even after the parent component
re-renders with the correct `runId`, the drawer content is not updated because the
renderer function is not re-executed, leading to the drawer displaying the wrong session
or no session at all.
|
I would expect it not to remain pressed while closed and thinking. Otherwise looks great! |
Follow-up on #113727 We were previously using the `busy` state, but `busy` intentionally makes the button non-interactive which isn't going to work for this, because users need to be able to re-open the panel even when Seer is thinking https://github.com/user-attachments/assets/a3e79136-0da5-4ac1-a806-d07e830f4bf7
Follow-up to #113727 and #113748, this PR adds a new `useSeerExplorerRunId` hook that's a `useSyncExternalStore`-backed hook with a module-level listener set and `sessionStorage` as the backing store. Every hook consumer is now synced, so the loading state in the `AskSeer` button will remain aligned with the chat.
Follow-up to #113727 and subsequently #113746 which reverted the nice loading state. This PR adds a custom implementation of the Ask Seer button busy state—unlike the default `busy` prop, this implementation does not disable interaction while thinking. https://github.com/user-attachments/assets/eabf71a7-9da0-4baf-b9b3-4a9ec8f13f72

Refactors global nav's "Ask Seer" button into a stateful component, colocated with the rest of the Seer code
seer-button-state.mov