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

Update supported_llm.mdx #1219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

makkor-git
Copy link

@makkor-git makkor-git commented Jan 22, 2025

Current example of Ollama usage doesn't work, needs to be updated.


Important

Update Python example for Ollama in supported_llm.mdx to fix non-working code.

  • Python Example Update:
    • Update import statement to from openai import OpenAI in supported_llm.mdx.
    • Change openai.base_url and openai.api_key to OpenAI client initialization.
    • Update tools retrieval to use Action.GITHUB_STAR_A_REPOSITORY_FOR_THE_AUTHENTICATED_USER.
    • Add result = toolset.handle_tool_calls(response) and print(result) to handle and display tool call results.

This description was created by Ellipsis for 83269f7. It will automatically update as commits are pushed.

Current example of Ollama usage doesn't work, needs to be updated.
Copy link

vercel bot commented Jan 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
composio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 22, 2025 4:30pm

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 83269f7 in 45 seconds

More details
  • Looked at 45 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. docs/faq/supported_llms/supported_llm.mdx:28
  • Draft comment:
    The model name should be consistent with the JavaScript example. Consider using model="llama3.1" for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The difference in model names could be intentional - ":8b" likely specifies a specific model size variant. Making them match could actually be incorrect if they're meant to use different variants. Without deeper knowledge of Ollama's model naming conventions and requirements, suggesting a change could be misleading.
    I might be underestimating the importance of consistency in documentation examples. Having different model names might confuse readers.
    While consistency is valuable in docs, we can't assume the difference is a mistake - it could be intentionally showing different model variants. Making them match could give incorrect information.
    Delete the comment. Without being certain about Ollama's model naming requirements, suggesting to change the model name could lead to incorrect information.

Workflow ID: wflow_pYiv0MFtiAgbEbcB


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@shreysingla11
Copy link
Collaborator

Code Review Summary

The changes in this PR improve the documentation by updating the Ollama integration example to use the latest OpenAI client pattern. Overall, the changes are well-structured and necessary.

Positive Aspects:

✅ Updates to modern OpenAI client initialization
✅ Proper configuration of base_url and api_key
✅ More specific tool configuration using actions instead of apps
✅ Consistent code style and formatting
✅ Added result handling and output display

Suggestions for Improvement:

  • Added comments for better documentation of the Ollama configuration
  • Suggested error handling for more robust example code

Overall Assessment:

This is a good quality update that improves the documentation by providing a working, modern example of Ollama integration. The changes align with current best practices and make the code more maintainable.

Rating: 8/10 - Good quality update with minor suggestions for improvement.

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.

2 participants