Skip to content
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

support for agentic ai providers #655

Merged
merged 5 commits into from
Aug 27, 2024
Merged

support for agentic ai providers #655

merged 5 commits into from
Aug 27, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Aug 27, 2024

  • 🧾 Updated the content of docs/guides/agentic-tools.mdx, removing and updating several links to agentic tools.
  • 🛠️ Simplified some verbose JavaScript code in chat.ts, notably by simplifying the find() method and enhancing the output handling.
  • 🧰 Improved runpromptcontext.ts by accommodating ToolCallback, AgenticToolCallback, and AgenticToolProviderCallback in defTool function.
  • 📚Updated the interfaces in prompt_template.d.ts and prompt_type.ts defining new AgenticToolProviderCallback that includes a collection of AgenticToolCallback.
  • 🗑️ Removed the agentic-calculator.genai.mts sample code.
  • ➕ Created a new sample file agentic-tools.genai.mts which illustrates the usage of multiple agentic tools including calculator and weather tools.
  • 📦 Updated package.json in the sample package to include a new developer dependency @agentic/weather.

generated by pr-describe

- [Midjourney](https://agentic.so/tools/midjourney)
- [Novu](https://agentic.so/tools/novu)
- [People data-labs](https://agentic.so/tools/people-data-labs)
- [People data labs](https://agentic.so/tools/people-data-labs)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect hyphenation in the URL, it should be 'people-data-labs' instead of 'People data labs'.

generated by pr-docs-review-commit incorrect_hyphenation

Copy link

The pull request contains changes to three TypeScript files, with alterations focusing on function definitions, function calls, and data types.

  1. In chat.ts, the changes seemed to slightly simplify the function call and enhance the output handling. Now, an additional check for output type 'number' or 'boolean' exists, and the output is converted to a string in this case.

  2. In runpromptcontext.ts, new data types are added to the function parameter's type definition. In addition to string and ToolCallback, now AgenticToolCallback and AgenticToolProviderCallback are also accepted. This makes the function more flexible.

  3. Changes to prompt_template.d.ts and prompt_type.d.ts are related to the addition of new types (AgenticToolCallback and AgenticToolProviderCallback). This adjustment could provide more flexibility when defining tools.

Concerns:

  • The logic of handling different output types (string, number, boolean, and object) in chat.ts might be more readable if refactored into separate helper functions.
  • In runpromptcontext.ts, ensure that the casting to AgenticToolCallback or AgenticToolProviderCallback is safe and that these types indeed have the properties being accessed (impl or functions).

Overall, the changes seem sound and should enhance the flexibility of the code. LGTM 🚀

generated by pr-review

typeof output === "number" ||
typeof output === "boolean"
)
toolContent = "" + output

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using "" + output for type conversion. It's better to use String(output) for clarity and performance. 🚀

generated by pr-review-commit type_conversion

@@ -218,7 +222,7 @@ ${stderr || ""}`
${fenceMD(content, " ")}
`
} else {
toolContent = (output as ToolCallContent)?.content
toolContent = YAMLStringify(output)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function YAMLStringify is imported but not defined anywhere in the changes. Please ensure it is correctly defined and imported. 🧐

generated by pr-review-commit missing_function

tool.spec.parameters as any,
tool.impl
)
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defTool function has become quite complex with multiple nested conditions and loops. Consider refactoring it into smaller, more manageable functions. This will improve readability and maintainability. 📚

generated by pr-review-commit complexity

@pelikhan pelikhan merged commit 87b2881 into main Aug 27, 2024
7 of 9 checks passed
@pelikhan pelikhan deleted the agentic-providers branch August 27, 2024 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant