- Notifications
You must be signed in to change notification settings - Fork927
fix: initial app status and zero-based id#18622
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
code-asher wants to merge2 commits intomainChoose a base branch
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Open
+321 −226
Conversation
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
We are discarding all "working" updates from the screen watcher thatdoes not include a new user message, but it makes sense to accept thevery first message, because the agent could be working but neglected toreport that fact.
62b94b7 to7454e49CompareTwo things:1. Message IDs can be zero-based.2. We were not actually updating the last user message ID.I refactored a little to keep track of the last message in lastReport.
7454e49 to9cef0dfCompareSign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
1 participant
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.
Uh oh!
There was an error while loading.Please reload this page.
We are discarding all "working" updates from the screen watcher because we cannot tell the difference between the agent or user changing the screen, but it makes sense to accept it as the very first update, because the agent could be working but neglected to report that fact, so you would never get an initial "working" update (it would just eventually go straight to "idle").
Also messages can start at zero, so I made a fix for that as well, although I think the first message will be from the LLM and we ignore those anyway, so this probably has no actual effect, but seems more technically correct.
And it seems I forgot to actually update the last message ID, which I think also does not actually matter for user messages (since I think the SSE endpoint will not re-emit a user message it has already emitted), but seems more technically correct to check.
I still need tofixcoder/internal#713 but I will do that in a separate PR. Going to do the one second wait thing we talked about for updates from the watcher.