Feedback before OS

[Tomas2D]

The following issues needs to be addressed.

1. Never use dotenv on the user's behalf! dotenv should be a dev-dependency only, and it is up to the user how he/she decides how to work with ENVs. TIP: use parseEnv from the bee-agent-framework (you should be able to import it)

This issue occurs for instance here:

• https://github.com/i-am-bee/bee-community-tools/blob/main/src/tools/imageDescription.ts#L26-L30

2. Don't use snake_case , we are not in Python, use camelCase instead.

3. Omit non-useful commits like // We have an answer

4. Why are you using top-level functions instead of putting them inside the class? If one wants to extend/override the behavior, he might be unable to do so. Image Description tool.

5. Use E2E tests instead of unit tests if possible.

6. Not sure why this file is part of the project src/helpers/io.ts.

7. Wrong dependencies in package.json

   "dependencies": {
   "@ibm-generative-ai/node-sdk": "~3.2.3",
   "@langchain/community": "~0.2.28",
   "@langchain/core": "~0.2.27",
   "@langchain/langgraph": "~0.0.34",
   "bee-agent-framework": "^0.0.17",
   "groq-sdk": "^0.7.0",
   "ollama": "^0.5.8",
   "openai": "^4.56.0",
   "openai-chat-tokens": "^0.2.8"
   },

None of them should be present. They should be part of devDependencies + bee-agent-framework should be in devDependencies and peerDependencies. I don't see any reason to even have LangChain dependency in the project.

[grahamwhiteuk]

(1) and (2) are addressed in 59ef58b

For me (3) is personal choice. I would encourage more comments rather than fewer comments and since I'm not the original author of the imageDescriptionTool I think we can leave this in place.

(4) is addressed in 335f840

(5) is a work in progress in the feat/e2e-tests branch (and shouldn't be a blocker for release)

(6) src/helpers/io.ts is used by the included agent in index.ts for access to createConsoleReader()

(7) is addressed in 7c36e24

[Tomas2D]

Personal choice is not an argument. Using camelCase style naming for variable names in JS is a de facto standard. You are doing open source, not a personal project, so you should be even more strict about these things.

This project should serve as a package that exports tools.

But now, when somebody imports something from this library, it triggers an agent run - are you even aware of what you are doing? All content of your src is getting published. If you would like to have a playground, then use the examples directory, which is intended for exactly this use case. So please remove it ASAP.

[grahamwhiteuk]

My remark about personal choice relates to your third piece of feedback about non-useful comments. All of your feedback in your second comment about camelCase has been addressed.

The intention of the project is not one that exports tools. As previously agreed, the intention is to serve as a reference implementation for an agent that brings its own tools. I'm happy to re-visit this discussion should we need to do so.

[Tomas2D]

That is interesting. So, you do not want to publish this library as a package? In either case, it does not make sense to do it, and it is way better to utilize the examples directory, as I previously provided.

[michael-desmond]

My understanding is also that bee community tools serves as a library that can be added alongside the bee framework to give access to a broader set of tools. The sample agent is convenient to have but should be excluded from the installable package as is done in the framework.

[grahamwhiteuk]

Ok, thanks guys. I'll merge the branch that addresses most of the feedback and then start a new branch to rework the repo into one that would allow tools to be more easily consumed directly from the package.

[grahamwhiteuk]

Re-opening for the re-work to re-instate the examples dir

[grahamwhiteuk]

@michael-desmond @Tomas2D PR #6 is ready to go. On reflection, I think this is a MUCH better approach so thanks for the feedback and nudge in the right direction.

@Tomas2D can you see server-side any more details on why the imageDescription unit test is failing? This works fine for me locally and looks to me like it should pass when run on GitHub as well.

[Tomas2D]

@Tomas2D Regarding the failing test case - test and other test frameworks are printing just an error message without the stack trace. I will try to find a way how to force the vitest to dump the whole thing.

[grahamwhiteuk]

Thanks, @Tomas2D.

[grahamwhiteuk]

Re-opening due to unresolved conversation in previous PR.

[Tomas2D]

I have a few notes regarding tools.

In https://github.com/i-am-bee/bee-community-tools/blob/4447c22cdb104b1a5d9daf9604f5ff017fb883cc/src/tools/imageDescription.ts#L28-L30

You are retrieving data from ENV at the very top. Do not do that. Why? Because the user could have something like this.

import dotenv from "dotenv"
import { ImageDescriptionTool } from "..."

dotenv.load() //

const tool = new ImageDescriptionTool(...)

This would not work because you initiate your ENV variables before initializing ENV.

The best approach would be something like this.

const vllmApiEndpoint = parseEnv("IMAGE_DESC_VLLM_API", z.string().url()) //This will also throw a validation error so the user will immediately know where the problem is
const vllmApiUrl = `${vllmApiEndpoint}/v1/chat/completions`;

or do it in the constructor.

Second point:

• Consider visibility from public to protected for the following methods queryVllmAPI and requestImageDescriptionForURL. If you decide to keep public (default), you should move them at least at the very bottom of your class; otherwise, IDE will list them in the type hint before the run method.