-
Notifications
You must be signed in to change notification settings - Fork 137
chore: rework beeai sdk #1358
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
chore: rework beeai sdk #1358
Conversation
Summary of ChangesHello @tomkis, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant rework of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a significant and beneficial refactoring of the BeeAI SDK integration. The core logic for handling agent capabilities and demands has been centralized within the beeai-sdk-ts package, which greatly simplifies the beeai-ui codebase and improves overall modularity. The new handleAgentCard and handleTaskStatusUpdate utilities are excellent additions. While the refactoring is a clear improvement, there are a few areas that require attention. I've noted some typos in exported types and function names (e.g., Fullfillment instead of Fulfillment), several TODO comments indicating incomplete work, and a potential regression in OAuth handling which appears to be missing from the new implementation. Addressing these points will help solidify this excellent refactoring.
apps/beeai-sdk-ts/src/client/a2a/extensions/handle-task-status-update.ts
Outdated
Show resolved
Hide resolved
apps/beeai-sdk-ts/src/client/a2a/extensions/handle-agent-card.ts
Outdated
Show resolved
Hide resolved
apps/beeai-sdk-ts/src/client/a2a/extensions/handle-agent-card.ts
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-demands/agent-demands-context.ts
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-demands/build-fulfillments.ts
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-secrets/AgentSecretsProvider.tsx
Show resolved
Hide resolved
e1e8a07 to
9562988
Compare
|
/gemini review |
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.
Code Review
This pull request introduces a significant and beneficial refactoring of the BeeAI SDK, particularly in how agent capabilities and extensions are handled. By centralizing logic into handleAgentCard and handleTaskStatusUpdate handlers, the client-side code in beeai-ui is greatly simplified, making it more maintainable and easier to understand. The changes are consistent across the Python example, the TypeScript SDK, and the UI application. I've identified a critical bug in the Python example and a few areas for improvement in the TypeScript code, including addressing TODOs, improving maintainability, and handling a potential silent failure case. There's also a leftover debug statement that should be removed. Overall, this is a great step forward for the SDK's architecture.
apps/beeai-sdk-ts/src/client/a2a/extensions/handle-agent-card.ts
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/variables/api/mutations/useUpdateVariables.ts
Outdated
Show resolved
Hide resolved
apps/beeai-sdk-ts/src/client/a2a/extensions/handle-agent-card.ts
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-demands/AgentDemandsProvider.tsx
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-demands/AgentDemandsProvider.tsx
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-demands/AgentDemandsProvider.tsx
Show resolved
Hide resolved
| } | ||
|
|
||
| if (formDemands) { | ||
| const formFullfilment = await fullfillments.form(formDemands); |
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.
Really suboptimal. Form acts like demand/fullfilment but in fact it's not because demand is always Record<string,Demand> which is not the case for Forms, we should unify this.
apps/beeai-ui/src/modules/variables/api/mutations/useUpdateVariables.ts
Outdated
Show resolved
Hide resolved
apps/beeai-ui/src/modules/runs/contexts/agent-demands/AgentDemandsProvider.tsx
Outdated
Show resolved
Hide resolved
fe645c9 to
baaa7ec
Compare
| if (formPart) { | ||
| parts.push(formPart); | ||
| } | ||
| return [createFormPart(form)]; |
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.
We should change this to:
parts.push(createFormPart(form));
| if (!agentClient) { | ||
| return <></>; | ||
| } | ||
|
|
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.
We should add some skeleton component or loader.
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
Signed-off-by: Tomas Weiss <[email protected]>
ecd7ea8 to
d8e805a
Compare
No description provided.