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.

Sign up for GitHub

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

Jump to bottom

added Agno integration #36

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

added Agno integration #36

wants to merge 15 commits into from

Conversation

wenqiglantz
Copy link

Changes included in this PR:

  • added Agno package
  • added agno_personal_finance workflow

@mdemoret-nv
Copy link
Collaborator

@dnandakumar-nv To review Monday.

@mdemoret-nv
Copy link
Collaborator

@wenqiglantz Before we can get this merged, we will need the DCO check to pass. You can find more info about this here: https://github.com/NVIDIA/AgentIQ/pull/36/checks?check_run_id=39465660296

@wenqiglantz
added Agno integration
63a3764 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz
updated llm.py in Agno package
cf4a4d9 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
dnandakumar-nv
dnandakumar-nv requested changes Mar 31, 2025
View reviewed changes
Copy link
Contributor

@dnandakumar-nv dnandakumar-nv left a comment

Choose a reason for hiding this comment

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

Thank you for your PR, @wenqiglantz ! The additions look great and I'm sure adding support for the Agno package will be valuable. I did have a few comments during the review that I'm hoping we can address to get this merged.

Additionally, I think increased unit testing in the following two areas would be beneficial:

  1. Unit tests in the agentiq_agno package for the tool and llm wrappers to ensure they build correctly in a few different cases. You can find an example of tests here and here
  2. Unit tests for the agno_callback_handler to ensure events are being captured and traced correctly. You can find examples here.

@dnandakumar-nv dnandakumar-nv added enhancement New feature or request non-breaking Non-breaking change improvement Improvement to existing functionality and removed enhancement New feature or request labels Mar 31, 2025
@wenqiglantz
fixes per review comments
d0cdbb0 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz
fixes per review comments
cb2adeb 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz
fixes per review comments
6ffe97f 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz wenqiglantz requested a review from a team as a code owner April 1, 2025 18:34
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Apr 1, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@wenqiglantz
fixes per review comments
5940cd9 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@dnandakumar-nv
Copy link
Contributor

/ok to test

dnandakumar-nv
dnandakumar-nv approved these changes Apr 2, 2025
View reviewed changes
Copy link
Contributor

@dnandakumar-nv dnandakumar-nv left a comment

Choose a reason for hiding this comment

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

Changes look good to me

@dnandakumar-nv
Copy link
Contributor

/ok to test

dnandakumar-nv
dnandakumar-nv requested changes Apr 2, 2025
View reviewed changes
Copy link
Contributor

@dnandakumar-nv dnandakumar-nv left a comment

Choose a reason for hiding this comment

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

@wenqiglantz thanks for your changes! The code looks good to me now, but we need the CI to pass before a merge into the repo. Looks like some of the CI is failing due to the lack of copyright headers:

https://github.com/NVIDIA/AgentIQ/actions/runs/14224833050/job/39861721846?pr=36

@wenqiglantz
fix pylint issue
4affa90 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz
Merge branch 'develop' of https://github.com/wenqiglantz/AgentIQ into…
e8b40b6 
… develop
@dnandakumar-nv
Copy link
Contributor

/ok to test

@wenqiglantz
fix CI check warning
54e788a 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz
fix CI check warning
2eb6ea7 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@dnandakumar-nv
Copy link
Contributor

/ok to test

@wenqiglantz
fix CI error
8573853 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@wenqiglantz
fix CI error
7db41e2 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@dnandakumar-nv
Copy link
Contributor

/ok to test

@wenqiglantz
fix CI error
aae0971 
Signed-off-by: Wenqi Glantz <wglantz@nvidia.com>
@dnandakumar-nv
Copy link
Contributor

/ok to test

dnandakumar-nv
dnandakumar-nv requested changes Apr 3, 2025
View reviewed changes
ci/scripts/github/tests.sh Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we changing CI here?

examples/agno_personal_finance/tests/test_agno_personal_finance_workflow.py

import pytest

from agno_personal_finance.agno_personal_finance_function import AgnoPersonalFinanceFunctionConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from agno_personal_finance.agno_personal_finance_function import AgnoPersonalFinanceFunctionConfig
from aiq_agno_personal_finance.agno_personal_finance_function import AgnoPersonalFinanceFunctionConfig

Copy link
Contributor

Choose a reason for hiding this comment

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

This might help you pass the test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnandakumar-nv dnandakumar-nv dnandakumar-nv requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
improvement Improvement to existing functionality non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wenqiglantz @mdemoret-nv @dnandakumar-nv