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

feat(agents-api): Naive speed improvement to cozo queries by adding limit=1 to all relevant queries #580

Merged
merged 5 commits into from
Oct 4, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Oct 3, 2024

Signed-off-by: Diwank Singh Tomer [email protected]


Important

Improve CozoDB query performance by adding :limit 1 to relevant queries and restructure CI/CD workflows for agents-api.

  • Performance Improvement:
    • Add :limit 1 to CozoDB queries in get_agent.py, get_cached_response.py, and prepare_chat_context.py among others to improve query performance.
  • CI/CD Workflow:
    • Rename lint-and-format.yml to lint-agents-api-pr.yml and remove typecheck and test steps.
    • Add test-agents-api-pr.yml for running tests on pull requests.
    • Add typecheck-agents-api-pr.yml for type-checking on pull requests.

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

…imit=1 to all relevant queries

Signed-off-by: Diwank Singh Tomer <[email protected]>
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.

👍 Looks good to me! Reviewed everything up to 348079e in 17 seconds

More details
  • Looked at 479 lines of code in 24 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/models/agent/get_agent.py:105
  • Draft comment:
    Ensure that adding :limit 1 does not affect the logic where multiple results might be expected. This change is applied across multiple files, so verify its appropriateness in each context.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds a :limit 1 clause to several CozoDB queries to improve performance by limiting the result set to one record. This is a valid optimization for queries where only one result is expected or needed. However, it's important to ensure that this change does not affect the logic of the application, especially in cases where multiple results might be expected under certain conditions. The change seems consistent across multiple files, and the logic of the queries appears to be maintained. The PR also includes changes to GitHub Actions workflows, splitting linting, testing, and type-checking into separate workflows, which is a good practice for modularity and clarity. Overall, the changes seem appropriate and beneficial for performance and CI/CD process improvements.

Workflow ID: wflow_2DAtnrKOcXGEboh4


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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. Incremental review on 248288f in 17 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_CUpJ89wxKz5hXqkS


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.

.github/workflows/typecheck-agents-api-pr.yml Outdated Show resolved Hide resolved
…from running after first fail

Signed-off-by: Diwank Singh Tomer <[email protected]>
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.

👍 Looks good to me! Incremental review on 4cd6bb0 in 11 seconds

More details
  • Looked at 27 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/typecheck-agents-api-pr.yml:40
  • Draft comment:
    The change in the cache key to include ${{ github.base_ref }} is appropriate for ensuring cache specificity to the base branch in pull request workflows.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The change in the cache key for pytype in the typecheck workflow is correct. It ensures that the cache is specific to the base branch, which is useful for pull request workflows. This change is appropriate and aligns with best practices for caching in CI workflows.

Workflow ID: wflow_HQXDGcgmYWom5eVa


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@creatorrr creatorrr merged commit c5ea3ba into dev Oct 4, 2024
3 of 5 checks passed
@creatorrr creatorrr deleted the f/faster-cozo-queries branch October 4, 2024 00:38
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