Implement live view tracking and progress indication in chat box - #475
Merged
Conversation
PR Summary by QodoAdd live-view link expiry handling and timed progress indicators in chat box
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Cross-tab progress not reset
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+707
to
+709
| if (message?.sender?.id && message.sender.id !== currentUser?.id) { | ||
| resetProgress(); | ||
| } |
There was a problem hiding this comment.
1. Cross-tab progress not reset 🐞 Bug ≡ Correctness
onMessageReceivedFromClient() only calls resetProgress() when message.sender.id differs from currentUser.id, so a message sent by the same user from another tab can leave progressSince/progressStep from the previous turn intact. Because startProgressClock() returns early when progressSince is already set, the next thinking bubble can show an incorrect step/elapsed time/label.
Agent Prompt
### Issue description
`onMessageReceivedFromClient()` is intended to clear progress for turns that did not go through `sendChatMessage()` (including “this user in another tab”), but the current sender-id guard skips same-user messages. This can cause stale `progressSince/progressStep/indication` to carry into the next wait.
### Issue Context
You already avoid resetting on this tab’s own echo to prevent clearing progress after the first indication arrives.
### Fix Focus Areas
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[698-709]
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[914-920]
### Suggested fix approach
Adjust the reset condition to also reset for same-user messages that did not originate from this tab’s active turn.
One pragmatic option (no server changes):
- Treat any incoming client message as a new turn if this tab is currently idle (not waiting/streaming/thinking/sending), regardless of sender id.
- Keep the existing “don’t reset while we’re mid-turn” behavior to avoid racing with the current tab’s echo.
Example logic:
- If `message.sender.id !== currentUser.id` => reset.
- Else if `message.sender.id === currentUser.id` AND `!isWaiting` (or `!isThinking && !isStreaming && !isSendingMsg`) => reset.
If you can add metadata from the backend, a more robust fix is to include an origin/session id on the message and only skip reset for the same-origin echo.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+301
to
+304
| const timer = setInterval(() => { | ||
| linkClock = Date.now(); | ||
| if (linkClock >= liveViewExpiresAt) clearInterval(timer); | ||
| }, 30_000); |
There was a problem hiding this comment.
2. Expiry polling keeps link live 🐞 Bug ☼ Reliability
The UI decides whether a live-view link is spent using linkClock, but linkClock only updates every 30 seconds; after token expiry, the note can remain clickable until the next tick. This can send users to an executor page that now refuses the expired credential.
Agent Prompt
### Issue description
`spent` is computed from `liveView.expiresAt <= linkClock`, but `linkClock` advances on a 30s interval. That leaves a window (up to ~30s) where the UI still renders the link as actionable after the token has expired.
### Issue Context
You only need a rerender at (or immediately after) `liveViewExpiresAt` to flip from link -> non-link. You don’t display a countdown, so frequent polling isn’t necessary.
### Fix Focus Areas
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[285-306]
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[2200-2242]
### Suggested fix approach
Replace (or augment) the 30s polling with a single timeout scheduled for the exact expiry moment:
- Immediately set `linkClock = Date.now()` when an expiry is present.
- If `now >= expiresAt`, do nothing further.
- Else `setTimeout(() => linkClock = expiresAt, expiresAt - now)`.
- Clean up the timeout on effect teardown.
If you still want periodic updates for safety, you can keep a coarse interval but also schedule the exact-expiry timeout so the UI flips promptly.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.