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

Hydra config #289

Merged
merged 15 commits into from
Feb 22, 2024
Merged

Hydra config #289

merged 15 commits into from
Feb 22, 2024

Conversation

20001LastOrder
Copy link
Collaborator

@20001LastOrder 20001LastOrder commented Feb 8, 2024

Description

Add ability to configure any agent with hydra, includes:

  • type of agent
  • parameters for agents
  • actions for agents including configuring each action
  • validations for agents including configuring each validation

Also had a temporary fix for the GitHub readme extractor failure so now we have a clean test run.
Will create a separate PR for documentation on how to configure the agents.

Your checklist for this pull request

Thank you for submitting a pull request! To speed up the review process, please follow this checklist:

  • My Pull Request is small and focused on one topic so it can be reviewed easily
  • My code follows the style guidelines of this project (make format)
  • Commit messages are detailed
  • I have performed a self-review of my code
  • I commented hard-to-understand parts of my code
  • I updated the documentation (docstrings, /docs)
  • My changes generate no new warnings (or explain any new warnings and why they're ok)
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass when I run pytest tests (offline mode)

Additional steps for code with networking dependencies:

Copy link
Collaborator

@oshoma oshoma left a comment

Choose a reason for hiding this comment

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

Please fix the failing tests

2024-02-14-hydra-tests.log

src/sherpa_ai/output_parsers/citation_validation.py Outdated Show resolved Hide resolved
src/sherpa_ai/output_parsers/citation_validation.py Outdated Show resolved Hide resolved
src/sherpa_ai/scrape/extract_github_readme.py Outdated Show resolved Hide resolved
src/sherpa_ai/test_utils/data.py Outdated Show resolved Hide resolved
src/sherpa_ai/agents/base.py Outdated Show resolved Hide resolved
src/sherpa_ai/agents/base.py Outdated Show resolved Hide resolved
src/sherpa_ai/agents/qa_agent.py Outdated Show resolved Hide resolved
src/sherpa_ai/memory/belief.py Show resolved Hide resolved
src/sherpa_ai/memory/belief.py Outdated Show resolved Hide resolved
src/apps/slackapp/slackapp/bolt_app.py Show resolved Hide resolved
src/sherpa_ai/agents/qa_agent.py Outdated Show resolved Hide resolved
src/sherpa_ai/agents/base.py Show resolved Hide resolved
Copy link
Collaborator

@oshoma oshoma left a comment

Choose a reason for hiding this comment

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

Some small changes requested, in addition to Eyob's comments.

Also please run make format.

src/apps/slackapp/slackapp/bolt_app.py Outdated Show resolved Hide resolved
src/sherpa_ai/agents/qa_agent.py Outdated Show resolved Hide resolved
src/sherpa_ai/config/__init__.py Show resolved Hide resolved

return self.add_citations(generated, resources)

def add_citations(self, generated: str, resources: list[dict]) -> ValidationResult:
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, I agree.

@oshoma
Copy link
Collaborator

oshoma commented Feb 19, 2024

@20001LastOrder I notice the logic around team_id and user_id it is in a few functions that it shouldn't be in. For example SherpaOpenAI. I am thinking a class like SherpaOpenAI should accept a "unique user ID" as a parameter for token usage tracking, but it shouldn't know about Slack, team IDs, user IDs. wdyt? To me this feels like another issue needing some refactoring.

@oshoma oshoma mentioned this pull request Feb 19, 2024
11 tasks
@20001LastOrder
Copy link
Collaborator Author

I notice the logic around team_id and user_id it is in a few functions that it shouldn't be in. For example SherpaOpenAI. I am thinking a class like SherpaOpenAI should accept a "unique user ID" as a parameter for token usage tracking, but it shouldn't know about Slack, team IDs, user IDs. wdyt? To me this feels like another issue needing some refactoring.

OK. I'll make an issue out of it.

Eyobyb
Eyobyb previously approved these changes Feb 21, 2024
@oshoma oshoma merged commit 0cdaba3 into Aggregate-Intellect:main Feb 22, 2024
@20001LastOrder 20001LastOrder deleted the hydra_config branch April 16, 2024 15:51
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.

3 participants