- Notifications
You must be signed in to change notification settings - Fork907
feat: add experimental Chat UI#17650
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
Conversation
9cc72d3 toe7559c3Comparea7d6a2f to54a20dbCompare6a805f9 toae9fbc0Compare| args: unknown; | ||
| result?: unknown; |
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.
review: changed tounknown to avoidany
| @@ -717,11 +708,12 @@ const FilePreview: FC<{ files: Record<string, string>; prefix?: string }> = | |||
| ); | |||
| }); | |||
| // TODO: generate these from codersdk/toolsdk.go. | |||
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.
review: ideally these would be autogenerated, but deferring this for now
| export interface ChatContext { | ||
| selectedModel: string; | ||
| modelConfig: LanguageModelConfig; | ||
| setSelectedModel: (model: string) => void; | ||
| } |
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.
review: Ideally I'd like to move this intouseAgenticChat but ran out of time.
cb80800 toe29b101CompareThere 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.
Overall, it is ok and I didn't find anything that could cause major bugs or bad user experiences. However, we are not following some FE agreements we did like using the new shadcn components, TailwindCSS for styles, or Lucide icons.
I also think the design is not the best when dealing with error handling and loading states. We probably could make a better use of design work here to be honest, but I'm not sure how urgent this release is.
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 would move this file tosites/src/modules/chat/useAgenticChat.ts.
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.
Even though it's imported inmodules/dashboard/NavBar/NavbarView?
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.
Yeap! I think it makes more sense. But no strong thoughts, you can leave as it is if it makes more sense to you.
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'm going to leave it where it is for now; I think there's more scope to refactor here.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
Not sure if this should be included in the scope of this PR, but all the new pages and components are made using shadcn/ui components, TailwindCSS styles, and Lucide icons as part of our redesign effort.
5fb99bd to5f2536fCompareBuilds on#17570Frontend portion ofhttps://github.com/coder/coder/tree/chat originallyauthored by@kylecarbsAdditional changes:- Addresses linter complaints- Brings `ChatToolInvocation` argument definitions in line with thosedefined in `codersdk/toolsdk`- Ensures chat-related features are not shown unless`ExperimentAgenticChat` is enabled.Co-authored-by: Kyle Carberry <kyle@carberry.com>
a1c03b6 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Builds on#17570
Frontend portion ofhttps://github.com/coder/coder/tree/chat originally authored by@kylecarbs
Additional changes:
ChatToolInvocationargument definitions in line with those defined incodersdk/toolsdkExperimentAgenticChatis enabled.