refactor: sessions #248
refactor: sessions #248
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
jonathanlab
force-pushed
the
12-12-refactor_sessions
branch
from
f7606c2 to
a6e1855
Compare
joshsny
left a comment
joshsny
left a comment
There was a problem hiding this comment.
LGTM - left some nits, none of them blocking
| .filter(({ event }) => | ||
| JSON.stringify(event).toLowerCase().includes(query), | ||
| ); | ||
| }, [events, searchQuery]); |
There was a problem hiding this comment.
it's annoying we do this stuff for all events on every new event, would be nicer to do this stuff in a store somewhere, but not a big deal so lets just leave it
| ); | ||
| }, [events, searchQuery]); | ||
|
|
||
| const copyToClipboard = useCallback((text: string) => { |
There was a problem hiding this comment.
nit: does this need a useCallback
| events, | ||
| isPromptPending, | ||
| }: ConversationViewProps) { | ||
| const turns = useMemo(() => buildTurns(events), [events]); |
There was a problem hiding this comment.
nit: this too, I think all this logic should be in zustand but its not a big issue
| taskId ? state.getSessionForTask(taskId) : undefined, | ||
| ); | ||
| const { setSessionModel } = useSessionActions(); | ||
| const session = useSessionForTask(taskId ?? ""); |
There was a problem hiding this comment.
what happens if we pass "" here?
|
|
||
| export function useKeyboardShortcut( | ||
| key: string, | ||
| callback: () => void, |
There was a problem hiding this comment.
this will always need to be a useCallback to avoid the effect from registering and unregistering the event listener on every render
jonathanlab
force-pushed
the
12-12-refactor_sessions
branch
from
b2c1284 to
69fbd7a
Compare
Merge activity
|
Session store and all related types were an absolute mess. I refactored it so we now only use ACP types directly, so I can actually work on improving session rendering. Also updated our knip config.