i-am-bee / bee-agent-framework

The framework for building scalable agentic applications.
https://i-am-bee.github.io/bee-agent-framework/
Apache License 2.0
91 stars 22 forks source link

Tool creation validation #77

Open Tomas2D opened 6 days ago

Tomas2D commented 6 days ago

Throws an error (in the base tool constructor) when:

mhdawson commented 1 day ago

@Tomas2D I assume this would be validating in cases where the tools were not created with TypeScript as I already get an error when compiling if the name or description is empty?

midawson@midawson-virtualbox:~/funccalling/ai-tool-experimentation/bee-agent-framework$ npm run build

> experiment-with-bee-agent-framework@0.0.1 build
> tsc --p ./src

src/favoriteColorTool.ts:9:14 - error TS2515: Non-abstract class 'FavoriteColorTool' does not implement inherited abstract member description from class 'Tool<StringToolOutput, BaseToolOptions<any>, BaseToolRunOptions>'.

9 export class FavoriteColorTool extends Tool<StringToolOutput> {
               ~~~~~~~~~~~~~~~~~

Found 1 error in src/favoriteColorTool.ts:9
Tomas2D commented 1 day ago

The error you are getting is about implementing the desired interface.

The tool name is specified as a name property or passed to the constructor in DynamicTool. Please take a look at the documentation.

mhdawson commented 1 day ago

k, it was not clear that this issue was just about DynamicTool creation, that clarifies things.

Tomas2D commented 1 day ago

It is not. We also need to validate the initial values for the name property, which is being set inside the constructor (GoogleSearchTool and others). TypeScript just allows us to write the following syntax sugar.

class A {
   name = "A"
}

translates to

class A {
  constructor() {
     this.name = "A"
  }
}
mhdawson commented 1 day ago

I'm not that up to speed on TypeScript but if you mean to add validation in: https://github.com/i-am-bee/bee-agent-framework/blob/1f64012de4ffd8d0050dee116c473dd0f5802f30/src/tools/base.ts#L186 to validate the abstract name and description fields, from my reading so far that might need some changes that would affect subclasses. If I've understood correctly, the values set by the subclass won't have been set when the base contructor has been called.

This is some discussion of the topic on stack overflow - https://stackoverflow.com/questions/54273218/abstract-property-in-class-cannot-be-accessed-in-the-constructor

From experimentation, switching to defining a get/set would require changes to all of the subclasses and remove the validation that they are defined by the subclass. Similarly updating the constructor of base.ts to require passing in the parameters to be validated would require changes to all subclasses.

I'm wondering if I still don't quite understand what validation is desired or possibly there is some easy way to do the validation in the constructor in base.ts that I've not figured out yet.

Tomas2D commented 1 day ago

That is right. In that case - add this validation to DynamicTool only.

mhdawson commented 18 hours ago

@Tomas2D ok will do.