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

Add hacker news emailer example via openai assistant #1360

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lingalarahul7
Copy link

@lingalarahul7 lingalarahul7 commented Feb 22, 2025

Description

Fetch the latest posts from hacker news and send an email to a particular receiver provided as input.

Used OpenAI Assistant framework.

Created requirements.txt using pip freeze command to make it reliable.

Testing

Screenshot 2025-02-22 at 11 21 08 AM

Important

Add Hacker News emailer example using OpenAI and Composio tools with setup scripts and documentation.

  • Behavior:
    • New script main.py fetches Hacker News frontpage posts and emails them to [email protected] using OpenAI and Composio tools.
    • Hardcoded email receiver, no dynamic input handling.
  • Setup:
    • Adds setup.sh to create a virtual environment, install dependencies, and prepare .env file.
    • Adds requirements.txt for dependency management.
  • Documentation:
    • Adds readme.md with instructions for setting up and running the example.
    • Includes .env.example for environment variable setup.

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

Copy link

vercel bot commented Feb 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 Feb 22, 2025 5:59am


assistant_instruction = "You are a super intelligent assistant"

assistant = openai_client.beta.assistants.create(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code lacks proper error handling and resource cleanup. Consider adding:

  1. Try-catch blocks around API calls
  2. Proper cleanup of OpenAI resources (assistant and thread)
  3. Validation of environment variables
  4. Email address validation

Example structure:

try:
    assistant = openai_client.beta.assistants.create(...)
    thread = openai_client.beta.threads.create()
    # ... rest of the code ...
finally:
    # Cleanup resources
    if 'assistant' in locals():
        openai_client.beta.assistants.delete(assistant.id)
    if 'thread' in locals():
        openai_client.beta.threads.delete(thread.id)

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.

❌ Changes requested. Reviewed everything up to 725353a in 2 minutes and 2 seconds

More details
  • Looked at 187 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 10 drafted comments based on config settings.
1. python/examples/advanced_agents/hackernews_emailer/.env.example:2
  • Draft comment:
    Add a newline at end of file for POSIX compliance.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    While the comment is technically correct about POSIX compliance requiring a newline at end of file, this is a .env.example template file, not actual source code. The missing newline won't affect functionality. Most text editors will automatically add newlines when editing real .env files. This seems like an overly pedantic comment for an example file.
    The comment is technically accurate and follows a standard convention. Not having a newline could cause issues if someone copies this file directly.
    While technically correct, this level of perfectionism for an example template file doesn't provide enough value to justify a PR comment. The real .env file will be created by users who can format it as needed.
    Delete this comment as it's too minor of an issue for an example file to warrant a PR comment.
2. python/examples/advanced_agents/hackernews_emailer/setup.sh:26
  • Draft comment:
    Ensure the script ends with a newline for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
3. python/examples/advanced_agents/hackernews_emailer/readme.md:1
  • Draft comment:
    Title mismatch: currently 'Twitter Posting Agent Guide'; update to 'Hackernews Emailer Guide' to reflect the actual functionality.
  • Reason this comment was not posted:
    Marked as duplicate.
4. python/examples/advanced_agents/hackernews_emailer/main.py:13
  • Draft comment:
    Consider using an f-string for clarity when constructing the 'task' variable.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
5. python/examples/advanced_agents/hackernews_emailer/main.py:20
  • Draft comment:
    Verify that the model name 'gpt-4o' is correct and intended, as it may be a typo or subject to change.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify the model name, which is against the rules as it asks for confirmation of intention. It does not provide a specific suggestion or point out a clear issue.
6. python/examples/advanced_agents/hackernews_emailer/main.py:40
  • Draft comment:
    The loop breaks after printing the first message; confirm that printing only the first message is intended.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    According to the rules, we should NOT ask authors to confirm their intentions or double-check things. The comment starts with asking to "confirm" something, which is explicitly mentioned as a type of comment to avoid. The break statement is clearly intentional code - if it wasn't wanted, it wouldn't be there.
    Maybe there's a legitimate reason to question this - what if printing only one message means important information is being lost?
    Even if there might be a reason to print more messages, asking for confirmation is not the right approach. If there's a clear issue, it should be stated directly.
    The comment should be deleted as it violates the rule about not asking authors to confirm their intentions.
7. python/examples/advanced_agents/hackernews_emailer/requirements.txt:1
  • Draft comment:
    The requirements file, generated via pip freeze, includes many packages that might be unnecessary. Consider cleaning it to list only dependencies needed for the emailer example.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While the comment makes a valid point about having many potentially unnecessary transitive dependencies, this violates our review rules. We don't have strong evidence that any specific dependency is definitely unnecessary - we'd need to see the actual code to know what's really needed. Additionally, the rules say not to comment on dependency-related issues.
    The comment could be helpful for maintainability. Smaller dependency lists are easier to maintain and reduce security risks.
    Per our rules, we should not comment on dependency issues without strong evidence, and we'd need to see the actual code to prove which dependencies are unnecessary.
    Delete this comment as it violates our rule about not commenting on dependency-related issues without strong evidence of specific problems.
8. python/examples/advanced_agents/hackernews_emailer/main.py:20
  • Draft comment:
    Typographical issue: The model string 'gpt-4o' on Line 20 might be a typo. Consider verifying if this should be 'gpt-4' or another intended value.
  • Reason this comment was not posted:
    Marked as duplicate.
9. python/examples/advanced_agents/hackernews_emailer/main.py:13
  • Draft comment:
    Consider checking the casing of 'hackernews' in the task string on Line 13. For clarity and consistency, you might want to write it as 'Hacker News' if that matches the expected style.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a purely cosmetic change to a string that will be interpreted by an AI model. The casing difference between "Hackernews" and "Hacker News" is unlikely to affect functionality. The comment doesn't point out any actual code issues or necessary improvements. It falls under the category of obvious/unimportant changes.
    Maybe consistent branding is important for professional code? Maybe the casing could affect how the AI model interprets the command?
    The AI model will understand the intent regardless of casing, and this is an example file where perfect branding isn't critical. The change wouldn't impact functionality.
    Delete the comment as it suggests a purely cosmetic change that doesn't affect functionality or code quality.
10. python/examples/advanced_agents/hackernews_emailer/readme.md:1
  • Draft comment:
    The title 'Twitter Posting Agent Guide' on line 1 appears to be a typo or copy-paste error, as the guide instructions later refer to fetching Hacker News posts for an emailer. Consider updating the title to reflect that this is a Hacker News Emailer guide.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_2kC1F5jZMsE5nNZd


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.


run = openai_client.beta.threads.runs.create(thread_id=thread.id,assistant_id=assistant.id)

response_after_tool_calls = composio_toolset.wait_and_handle_assistant_tool_calls(
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for tool calls to manage potential failures.

@shreysingla11
Copy link
Collaborator

Code Review Summary

Overall Rating: 🟡 Needs Improvement

The example demonstrates a good use case for combining HackerNews and email functionality, but there are several issues that need to be addressed before it's production-ready:

Critical Issues:

  1. Invalid model name ("gpt-4o")
  2. Hardcoded email address
  3. Missing error handling and resource cleanup
  4. Incorrect documentation title

Recommendations:

  1. Fix the model name to use a valid identifier
  2. Move configuration to environment variables
  3. Add proper error handling and resource cleanup
  4. Update documentation to reflect actual functionality
  5. Add type hints and docstrings for better code maintainability

Once these issues are addressed, this will be a valuable example for the repository. The core functionality is sound, but the implementation needs to be more robust and follow better security practices.

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