WIP feat: update default assistant name to [Deleted Assistant] #430
Conversation
ryanhopperlowe
requested review from
StrongMonkey,
thedadams,
njhale,
g-linville and
tylerslaton
|
This change is not self-explanatory. I don't understand how changing this constant addresses the issue you linked. |
My mistake. It appeared as though the code only used this variable when there was no script available for a given thread, so I didn't check the create/edit workflow. Will fix today |
ryanhopperlowe
force-pushed
the
90-handle-left-over-threads-from-deleted-assistants-better
branch
from
1ecd47b to
9ca8bd5
Compare
There was a problem hiding this comment.
This change does not work as I expect.
In this screen recording, the assistant associated to the "Casual Greeting" thread has been deleted. However, there is no indication of this other than the fact that there are no messages.
Screen.Recording.2024-09-06.at.09.45.24.mov
EDIT: This seems to have been an issue with my environment. My apologies for the false alarm.
ryanhopperlowe
force-pushed
the
90-handle-left-over-threads-from-deleted-assistants-better
branch
from
6108d09 to
af6c9b5
Compare
ryanhopperlowe
dismissed stale reviews from StrongMonkey and thedadams
via
7fbb2d1
ryanhopperlowe
force-pushed
the
90-handle-left-over-threads-from-deleted-assistants-better
branch
from
7fbb2d1 to
af6c9b5
Compare
| setThread(id); | ||
| setScriptContent((await getScript(scriptId))?.script || []); | ||
| setScriptId(scriptId); | ||
| setForceRun(true); |
There was a problem hiding this comment.
I think you shouldn't need to setsetForceRun, setting Thread and ShouldRestart should be sufficient. I see a double refreshing issue when switch to thread, like the thread reloads twices on screen.
There was a problem hiding this comment.
So I thought the same thing and removed it in a previous commit, but actually had to revert that change, because it caused the issue where it just reused the chat from the previous thread
There was a problem hiding this comment.
I can try removing the shouldRestart, but I'm not sure if that will work
There was a problem hiding this comment.
Ok I fixed this by removing setShouldRestart and keeping setForceRun the reason I opted for this is because it doesn't look like setShouldRestart consistently triggers the run event (which updates the messages). It would probably be more ideal to get shouldRestart to update the messages, but in the interest of time spent on this issue I opted to use the tool already available
ryanhopperlowe
dismissed stale reviews from thedadams and StrongMonkey
via
19da86c
There was a problem hiding this comment.
I tested this locally and noticed some weird behavior that seemed unrelated to your PR.
Switching between a thread and then sending a message causes me to be in a weird state forever. Are you seeing the same thing @ryanhopperlowe?
Screen.Recording.2024-09-09.at.2.51.58.PM.mov
|
Related to Tyler's comment and screencast, it seems like there is an issue here. This only happens when switching to a "Tildy" chat. It doesn't have to be from a thread with a deleted assistant. I confirmed that this is not happening on Additionally, switching to a thread with a custom assistant seems to break chat. |
ryanhopperlowe
force-pushed
the
90-handle-left-over-threads-from-deleted-assistants-better
branch
from
19da86c to
fc32008
Compare
ryanhopperlowe
changed the title
|
@StrongMonkey and I noticed that there is an issue with my most recent update that causes a double refresh when switching threads. Switching to WIP while I figure out how to prevent this |
Upon deleting an assistant, any left over chats with that assistant will be displayed in a read only state.
#90
While making this change, I discovered a bug where switching threads causes the websocket server to restart, but does not wait for the server to come back up before relaying new messages. It wasn't apparent until I started making my changes and should not cause any issues after this PR.