-
Notifications
You must be signed in to change notification settings - Fork 0
First version of the Report Generation Agent #13
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
base: main
Are you sure you want to change the base?
Conversation
amrit110
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments
| def execute(self, query: str) -> list[Any]: | ||
| """Execute a SQLite query. | ||
| Args: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicky here, but isn't this google docstring stryle, but then the Returns and other places in the PR looks like numpy style. I prefer numpy style but we should just pick one and be consistent.
| def convert_date(date_str: str) -> str | None: | ||
| """Convert date from 'MM/DD/YY HH:MM' to 'YYYY-MM-DD HH:MM'. | ||
| Args: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here with docstring style
| ) -> SQLiteSession: | ||
| """Get existing session or create a new one for conversation persistence. | ||
| Args: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here (docstring style)
| [tool.ruff] | ||
| include = ["*.py", "pyproject.toml", "*.ipynb"] | ||
| line-length = 88 | ||
| line-length = 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really a suggestion to change but our templates have 88 which is the PEP8/black standard, but i'm fine with 120 for this project. I'm starting to see that 120 is better for github viewing as well just less eye strain lol.
Summary
This is the first version of the report generation agent. This code was based on the Agent Bootcamp's ReACT RAG code and it brings a lot of code from there.
Future PRs will include more code to do evaluations on this agent.
Clickup Ticket(s): NA
Type of Change
Changes Made
.xlsxfile at the endTesting
Only manual tests performed.
Manual testing details:
.envfile based on the.env.examplefileChecklist