-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix: Prevent shimmer after permission deny #4057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
| function AssistantMessage(props: { message: AssistantMessage; parts: Part[]; last: boolean }) { | ||
| const local = useLocal() | ||
| const { theme } = useTheme() | ||
| const inProgress = createMemo(() => !props.message.time.completed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess this kinda works but I belive this will still be showing the wrong state sometimes during the agent loop i dont think this is the best solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't come up with a scenario where this is the wrong indicator, either empirically or analytically. Or even if there is such a scenario, IMO it should definitely be considered a bug in the Session.prompt loop.
An alternative solution could be to use the session busy/idle state. It has the added benefit of being robust to opening a session after abruptly closing OC where OC didn't get the chance to update the session/message states (e.g. after closing the terminal window where OC is running).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the more I think about it, the more I believe that the best solution is to make inProgress equal to isLastAssistantMessage && session.busy (pseudo code).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this approach is reasonable?
772b621 to
eb855e1
Compare
f1dc981 to
3e15a39
Compare
f8ee907 to
6a9856d
Compare
Fixes #3951.