-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
experimental: add a typescript version of autogen #5677
base: main
Are you sure you want to change the base?
Conversation
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.
Copilot reviewed 42 out of 42 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
typescript/samples/GettingStarted/Checker.ts:9
- [nitpick] The name 'TerminationF' and its usage via the property 'runUntilFunc' can be misleading since it returns a termination condition instead of a continuation indicator. Consider renaming them to something like 'shouldTerminate' for improved clarity.
type TerminationF = (x: number) => boolean;
adding doc comments into my typescript branch
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5677 +/- ##
==========================================
+ Coverage 70.16% 75.60% +5.44%
==========================================
Files 262 189 -73
Lines 14712 12734 -1978
Branches 243 0 -243
==========================================
- Hits 10322 9627 -695
+ Misses 4200 3107 -1093
+ Partials 190 0 -190
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
D A M N! |
python/uv.lock
Outdated
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 this pr should affect this file
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.
you are right, thanks!
typescript/README.md
Outdated
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 you please add quick start example that is consistent with python quick start example.
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 do!
@@ -0,0 +1,104 @@ | |||
import { AgentMessage, ChatMessage } from './Messages'; |
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.
Wouldn't it be kebab-case be a more conventional way of naming those files in typescript? e.g. chat-agent.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.
I'm not familiar with typescript naming conventions - I'd have to take your word for it or do you have a reference to follow?
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 think you can see it in most of the popular javascript/typescript projects on Github.
https://github.com/freeCodeCamp/freeCodeCamp/tree/main/api-server/src
https://github.com/vercel/next.js/tree/canary/examples/active-class-name
https://github.com/langchain-ai/langchainjs/tree/main/libs
The w3schools style guide doesn't explicitly mention kebab case but does mention lower case for file names: https://www.w3schools.com/JS/js_conventions.asp
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 also think one of the main motivations would be to reorganize the project into a workspace with multiple distribution packages, similar to python. I think the langchain package structure may be a good reference for how we may want to separate core, agentchat and extetions (which would be similar to their community packages under /libs).
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.
This is why I'd suggest to only start with core, so we have more room to finalise and discuss the project structure.
Do we have any idea of the pros and cons of the package structure of python vs dotnet? I would expect typescript to have a really good support for package-like structure similar to python but I don't know what are the distrubition considerations here. I think it could also make some sense to start with only core and later add agentchat. The cognitive load would be way more manageable. |
To create an agent, extend the `BaseAgent` class and implement `IHandle<T>`: | ||
|
||
```typescript | ||
import { BaseAgent } from '@microsoft/autogen-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.
maybe autogen-js
? It will be compiled and distributed as javascript even if we use typescript to build
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.
good point!
with: | ||
node-version: '18' | ||
- name: Install dependencies | ||
run: npm install |
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 think pnpm or yarn would be more modern package managers. Both also would also better support workspaces in case we consider a similar package structure to python
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 was shooting for lowest common denominator - can we count on most folks to already have yarn?
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.
The impact is primarily on developers and the maintainability of the project.. it would be similar to uv
vs pip
, anyone would be able to install/use autogenjs because we would distribute it through npm, even if we package it using yarn or pnpm.
Why are these changes needed?
continuing to add support for key programming languages.
Related issue number
Closes #236
This is experimental.
Its got tests (at least the same ones the dotnet side has), samples/GettingStarted, GH actions, and docs.
It still needs packaging, and since I based it off of the dotnet then the agentchat layer is much closer to the dotnet version. I think we can use an agent to compare it to the python version. I also haven't yet tested how the docs site renders. It also doesn't have anything at all to do with AI.
A lot of this code was direct translation, but then in the process of trying to get the tests to work there were deviations, largely because I think my translation was flawed in a lot of ways.... There is certainly some garbage, and may be demons but I tried to cast them away with the tests and the sample. There also is excessive console logging, because it was easier to get the AI to reason over the tests that way. What it needs is probably more tests, code reading etc.
but its a start.
Checks