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

fix(agent): chat with view and other dataset #1660

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

Conversation

scaliseraoul
Copy link
Contributor

@scaliseraoul scaliseraoul commented Mar 5, 2025


Important

Refactor SQL query execution in Agent class by removing redundant methods and updating tests accordingly.

  • Behavior:
    • Refactor _execute_sql_query in base.py to handle both local and virtual dataframes, removing _execute_local_sql_query.
    • Remove _parse_correct_table_name and _execute_local_sql_query from base.py.
    • Update AgentState in state.py to remove _validate_input method.
  • Tests:
    • Remove tests for _execute_local_sql_query in test_agent.py.
    • Update test_execute_sql_query_success_local and test_execute_sql_query_success_virtual_dataframe to reflect new _execute_sql_query logic.
  • Misc:
    • Add import for Source in base.py.

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

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 7b8a9e3 in 2 minutes and 16 seconds

More details
  • Looked at 123 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. pandasai/agent/base.py:75
  • Draft comment:
    Fix grammatical error: change 'are not compatibles' to 'are not compatible'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. pandasai/agent/base.py:142
  • Draft comment:
    Good consolidation of SQL execution logic by merging local and query builder SQL paths. Please ensure there are sufficient unit tests for both branches (i.e., when a DataFrame has a query_builder vs. when it doesn’t).
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
3. pandasai/agent/state.py:55
  • Draft comment:
    Removal of _validate_input in init is noted. Ensure that schema source compatibility is validated elsewhere if needed.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
4. pandasai/agent/base.py:74
  • Draft comment:
    Typo in error message: change 'are not compatibles' to 'are not compatible'.
  • Reason this comment was not posted:
    Comment was on unchanged code.
5. pandasai/agent/state.py:55
  • Draft comment:
    The _validate_input method has been removed. Ensure that schema compatibility validation is well-documented and handled solely in the Agent's constructor.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. pandasai/agent/base.py:76
  • Draft comment:
    Typographical error: The error message says 'are not compatibles'. Consider changing it to 'are not compatible' for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
7. pandasai/agent/base.py:186
  • Draft comment:
    Typographical error: In the train() method docstring, the parameter description for 'queries' reads 'user user'. It seems to be a mistake. Please clarify the intended description.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. tests/unit_tests/agent/test_agent.py:112
  • Draft comment:
    Typographical error detected: The string 'Cust om SQL prompt' appears to have a typo. It likely should be 'Custom SQL prompt'. Please correct the spelling mistake.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_PhpODEZyDEkByq1T


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.

* commit '869f048b60976c1fc5be313922368c626308a44d':
  fix(query builders): quoting identifiers by default (sinaptik-ai#1658)
  fix(agent): remove wrong double validation (sinaptik-ai#1659)
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.

1 participant