chore(compass-assistant): make compass-assistant a real store COMPASS-10571#7987
chore(compass-assistant): make compass-assistant a real store COMPASS-10571#7987
Conversation
| preferences: PreferencesAccess; | ||
| logger: Logger; | ||
| track: TrackFunction; | ||
| lastContextPromptRef: { current: string | null }; |
There was a problem hiding this comment.
erm.. wondering about this lastContextPromptRef "ref"
There was a problem hiding this comment.
I suppose this is technically not used in a react component or similar.
There was a problem hiding this comment.
Ref is just a name for an object that can keep a reference to a value without necessarily being relevant for actual state updates that need to be tracked, not necessarily a react concept (even if the name got popularised there). Can rename it to a Box or something, but we're already using ref in other places 🙂
There was a problem hiding this comment.
Pull request overview
Makes compass-assistant use a “real” Redux store during plugin activation so runtime services aren’t pulled directly from React rendering context.
Changes:
- Replaces the previous “fake store” object with a Redux store +
redux-thunkextra-args for assistant services. - Refactors assistant send/entry-point logic into thunk actions that consume services via thunk extra args.
- Adds Redux-related dependencies to
packages/compass-assistant.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/compass-assistant/src/compass-assistant-provider.tsx | Introduces Redux store activation and thunk-based assistant actions, wiring provider via react-redux. |
| packages/compass-assistant/package.json | Adds react-redux, redux, and redux-thunk dependencies needed for the new store approach. |
| package-lock.json | Updates lockfile for the new dependencies. |
| preferences: PreferencesAccess; | ||
| logger: Logger; | ||
| track: TrackFunction; | ||
| lastContextPromptRef: { current: string | null }; |
There was a problem hiding this comment.
Ref is just a name for an object that can keep a reference to a value without necessarily being relevant for actual state updates that need to be tracked, not necessarily a react concept (even if the name got popularised there). Can rename it to a Box or something, but we're already using ref in other places 🙂
| }) => { | ||
| // chat is stable — created once in activate, never changes | ||
| const [chat] = React.useState(() => thunkDispatch(getChat())); | ||
| const [chat] = React.useState(() => getChatAction()); |
There was a problem hiding this comment.
This I imagine we'd be removing in a follow-up.
| ) | ||
| ); | ||
|
|
||
| return { store, deactivate: cleanup }; |
There was a problem hiding this comment.
Do we want to addCleanup to chat.stop() the chat on deactivate? Not sure if there are any in progress requests we might want to abort or something like that. Not a blocker.
COMPASS-10571
This is just a step. There's discussion around whether or not to move messages into the store and then remove access to chat from elsewhere.