-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
feat: implement live task status update in task window #329
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
||
const TIMEOUT_LONG = 1000; | ||
const TIMOUT_SHORT = 800; | ||
|
||
class AutonomousAgent { | ||
name: string; | ||
goal: string; | ||
tasks: string[] = []; | ||
tasks: Message[] = []; |
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.
@asim-shrestha update tasks
, i's now a list of Message[]
instead of a list of strings
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.
Should be fine but why not use a different type?
{
title: string;
state: "starting" | "executing" | "complete";
}
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.
status
type? Mostly incase other types of messages might have their own status
const result = await this.executeTask(currentTask.value); | ||
|
||
currentTask.status = TASK_STATUS_EXECUTING; | ||
currentTask.info = result; |
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.
@asim-shrestha, .info
will now contain execution details. .value
will always track task value regardless of message types
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.
@awtkns, because .info
and .value
are now swapped for executing tasks. Execution messages from saved agents may look a bit funny
|
||
const TIMEOUT_LONG = 1000; | ||
const TIMOUT_SHORT = 800; | ||
|
||
class AutonomousAgent { | ||
name: string; | ||
goal: string; | ||
tasks: string[] = []; | ||
tasks: Message[] = []; |
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.
Should be fine but why not use a different type?
{
title: string;
state: "starting" | "executing" | "complete";
}
src/components/AutonomousAgent.ts
Outdated
this.sendExecutionMessage(currentTask as string, result); | ||
const result = await this.executeTask(currentTask.value); | ||
|
||
currentTask.status = TASK_STATUS_EXECUTING; |
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.
Should be before the previous line
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.
gotcha! this will be fixed as part of the status/ui revamp.
src/components/utils/helpers.tsx
Outdated
case TASK_STATUS_EXECUTING: | ||
return "border-white/20 text-white"; | ||
case TASK_STATUS_COMPLETED: | ||
return "border-green-500 hover:border-green-400 hover:text-green-400 text-green-500"; |
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.
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.
nit: maybe in a different PR we should refactor the message into its own file. This probably doesn't belong in helpers
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.
hmm, I think some sort of refactoring is warranted. Right now TaskWindowMessage and ChatWindowMessage are 2 distinct components sharing some similarities.
But agreed, helper is probably not the most ideal structure
src/components/store/helpers.ts
Outdated
import type { StoreApi, UseBoundStore } from "zustand"; | ||
|
||
type WithSelectors<S> = S extends { getState: () => infer T } | ||
? S & { use: { [K in keyof T]: () => T[K] } } | ||
: never; | ||
|
||
export const createSelectors = <S extends UseBoundStore<StoreApi<object>>>( | ||
_store: S | ||
) => { | ||
const store = _store as WithSelectors<typeof _store>; | ||
store.use = {}; | ||
for (const k of Object.keys(store.getState())) { | ||
(store.use as any)[k] = () => store((s) => s[k as keyof typeof s]); | ||
} | ||
|
||
return store; | ||
}; |
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.
Can we comment what these are used for?
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.
will add comment
@Jshen123 looking good to me, one minor thing, love the green outline but would it be possible to have the text remain white? IMHO the green text can make it quite hard to read. |
@awtkns @asim-shrestha thoughts on lighter shade of green? I prefer this over white text |
@Jshen123 what if we made the task green and the output white? Just concerned for people that are colourblind too |
Agree with Adam here, liking the white more |
if (getTaskStatus(message) === TASK_STATUS_EXECUTING) { | ||
return null; | ||
} |
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.
whats this for?
Description
Other changes